fix(syncer-updates): separate target from floors in update logic

backend/pkg/api/packages.go

@@ -382,7 +382,7 @@ func (api *API) DeletePackage(pkgID string) error {
 		var exists bool
 		var isFloor bool
 		err = api.db.QueryRow(`
-			SELECT 
+			SELECT
 				EXISTS(SELECT 1 FROM package WHERE id = $1),
 				EXISTS(SELECT 1 FROM channel_package_floors WHERE package_id = $1)
 		`, pkgID).Scan(&exists, &isFloor)

backend/pkg/api/packages_floors.go

@@ -226,13 +226,16 @@ const (
 	DefaultMaxFloorsPerResponse = 5
 )
 
-// GetRequiredChannelFloors returns floor packages between instance and target versions for a channel
-func (api *API) GetRequiredChannelFloors(channel *Channel, instanceVersion string) ([]*Package, error) {
+// GetRequiredChannelFloorsWithLimit returns floor packages between instance and target versions,
+// along with a boolean indicating if more floors remain beyond the limit.
+// This uses LIMIT+1 approach: query for limit+1 rows, if we get more than limit, there are more.
+// This is more efficient than a separate COUNT query.
+func (api *API) GetRequiredChannelFloorsWithLimit(channel *Channel, instanceVersion string) ([]*Package, bool, error) {
 	if channel == nil || channel.Package == nil {
-		return nil, ErrNoPackageFound
+		return nil, false, ErrNoPackageFound
 	}
 	if instanceVersion == "" {
-		return nil, fmt.Errorf("instance version cannot be empty")
+		return nil, false, fmt.Errorf("instance version cannot be empty")
 	}
 
 	targetVersion := channel.Package.Version
@@ -245,19 +248,20 @@ func (api *API) GetRequiredChannelFloors(channel *Channel, instanceVersion strin
 	// No blacklist check needed for floors
 	gtExpr, err := versionCompareExpr("p.version", ">", instanceVersion)
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
 	lteExpr, err := versionCompareExpr("p.version", "<=", targetVersion)
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
 	semverExpr, err := semverToIntArray("p.version")
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
+	// Query for LIMIT+1 to detect if more floors exist
 	query, _, err := goqu.From(goqu.L(`
 		package p
 		JOIN channel_package_floors cpf ON p.id = cpf.package_id
@@ -273,14 +277,26 @@ func (api *API) GetRequiredChannelFloors(channel *Channel, instanceVersion strin
 			lteExpr,
 		)).
 		Order(goqu.L(semverExpr).Asc()).
-		Limit(uint(maxFloorsPerResponse)).
+		Limit(uint(maxFloorsPerResponse + 1)).
 		ToSQL()
 
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
-	return api.getPackagesFromQuery(query)
+	floors, err := api.getPackagesFromQuery(query)
+	if err != nil {
+		return nil, false, err
+	}
+
+	// If we got more than the limit, there are more floors remaining
+	if len(floors) > maxFloorsPerResponse {
+		// Return only up to the limit, indicate more remain
+		return floors[:maxFloorsPerResponse], true, nil
+	}
+
+	// All floors returned
+	return floors, false, nil
 }
 
 // GetChannelFloorPackagesCount returns the count of floor packages for a channel

backend/pkg/api/packages_floors_test.go

@@ -55,7 +55,7 @@ func TestFloorOperations(t *testing.T) {
 	for instance, expected := range testCases {
 		ch, err := a.GetChannel(setup.Channel.ID)
 		assert.NoError(t, err)
-		floors, err := a.GetRequiredChannelFloors(ch, instance)
+		floors, _, err := a.GetRequiredChannelFloorsWithLimit(ch, instance)
 		assert.NoError(t, err)
 		assert.Len(t, floors, expected, "instance %s", instance)
 	}
@@ -84,9 +84,10 @@ func TestFloorMaxLimit(t *testing.T) {
 	// Should only get 3 floors due to limit
 	ch, err := a.GetChannel(setup.Channel.ID)
 	assert.NoError(t, err)
-	floors, err := a.GetRequiredChannelFloors(ch, "0.0.0")
+	floors, hasMore, err := a.GetRequiredChannelFloorsWithLimit(ch, "0.0.0")
 	assert.NoError(t, err)
 	assert.Len(t, floors, 3)
+	assert.True(t, hasMore, "Should indicate more floors remain beyond limit")
 }
 
 // TestFloorPagination tests paginated floor retrieval
@@ -133,7 +134,7 @@ func TestNonStandardVersions(t *testing.T) {
 	for instance, expected := range testCases {
 		ch, err := a.GetChannel(setup.Channel.ID)
 		assert.NoError(t, err)
-		floors, err := a.GetRequiredChannelFloors(ch, instance)
+		floors, _, err := a.GetRequiredChannelFloorsWithLimit(ch, instance)
 		assert.NoError(t, err)
 		assert.Len(t, floors, expected, "instance %s", instance)
 	}
@@ -232,7 +233,7 @@ func TestTargetAsFloor(t *testing.T) {
 	for instance, expected := range testCases {
 		ch, err := a.GetChannel(setup.Channel.ID)
 		assert.NoError(t, err)
-		floors, err := a.GetRequiredChannelFloors(ch, instance)
+		floors, _, err := a.GetRequiredChannelFloorsWithLimit(ch, instance)
 		assert.NoError(t, err)
 		assert.Len(t, floors, expected.expectedCount, "instance %s", instance)
 		if expected.expectedCount > 0 {

backend/pkg/api/updates.go

@@ -120,12 +120,15 @@ func (api *API) GetUpdatePackage(inst Instance, instApp InstanceApplication) (*P
 
 			// Instance hasn't reached granted version yet - return what's next to install
 			// This will be the first floor/target above current instance version
-			packages, err := api.getPackagesWithFloorsForUpdate(group, instanceVersion)
+			floors, target, err := api.getPackagesWithFloorsForUpdate(group, instanceVersion)
 			if err != nil {
 				return nil, err
 			}
-			// packages[0] should be the granted version since instance < granted
-			return packages[0], nil
+			// Return first floor if any, otherwise target
+			if len(floors) > 0 {
+				return floors[0], nil
+			}
+			return target, nil
 		}
 
 		// No granted version tracked (old instances) - safer fallback
@@ -145,15 +148,22 @@ func (api *API) GetUpdatePackage(inst Instance, instApp InstanceApplication) (*P
 		return nil, ErrNoUpdatePackageAvailable
 	}
 
-	packages, err := api.getPackagesWithFloorsForUpdate(group, instanceVersion)
+	floors, target, err := api.getPackagesWithFloorsForUpdate(group, instanceVersion)
 	if err != nil {
 		return nil, err
 	}
 
+	// Determine the next package to return (first floor or target)
+	var nextPkg *Package
+	if len(floors) > 0 {
+		nextPkg = floors[0]
+	} else {
+		nextPkg = target
+	}
+
 	// Safety check: verify the next package isn't blacklisted for this channel
 	// This should never happen (floors/targets can't be blacklisted for their own channel)
 	// but we check anyway for data consistency
-	nextPkg := packages[0]
 	if slices.Contains(nextPkg.ChannelsBlacklist, group.Channel.ID) {
 		l.Error().Str("package", nextPkg.Version).Str("channel", group.Channel.ID).
 			Msg("Package is blacklisted for its own channel - data inconsistency!")
@@ -165,7 +175,7 @@ func (api *API) GetUpdatePackage(inst Instance, instApp InstanceApplication) (*P
 	}
 
 	// Grant the update using the version we're actually returning
-	version := packages[0].Version
+	version := nextPkg.Version
 	if err := api.grantUpdate(instance, version); err != nil {
 		l.Error().Err(err).Str("version", version).Str("instance", instance.ID).Msg("GetUpdatePackage - grantUpdate error")
 		return nil, ErrUpdateGrantFailed
@@ -190,15 +200,22 @@ func (api *API) GetUpdatePackage(inst Instance, instApp InstanceApplication) (*P
 		}
 	}
 
-	return packages[0], nil
+	return nextPkg, nil
 }
 
-// GetUpdatePackagesForSyncer returns all packages (floors + target) for a syncer client
-func (api *API) GetUpdatePackagesForSyncer(inst Instance, instApp InstanceApplication) ([]*Package, error) {
+// GetUpdatePackagesForSyncer returns floor packages and target for a syncer client.
+// Returns:
+//   - floors: Required floor packages (may be empty)
+//   - target: The target package, or nil if more floors remain beyond the limit
+//   - error: Any error that occurred
+//
+// When target is nil, the syncer should request again with the highest floor version.
+// When target is not nil, all required floors have been sent and the channel can be updated.
+func (api *API) GetUpdatePackagesForSyncer(inst Instance, instApp InstanceApplication) ([]*Package, *Package, error) {
 	instance, err := api.RegisterInstance(inst, instApp)
 	if err != nil {
 		l.Error().Err(err).Msg("GetUpdatePackagesForSyncer - could not register instance")
-		return nil, ErrRegisterInstanceFailed
+		return nil, nil, ErrRegisterInstanceFailed
 	}
 
 	instanceVersion := instApp.Version
@@ -208,65 +225,78 @@ func (api *API) GetUpdatePackagesForSyncer(inst Instance, instApp InstanceApplic
 	if instance.Application.Status.Valid {
 		switch int(instance.Application.Status.Int64) {
 		case InstanceStatusDownloading, InstanceStatusDownloaded, InstanceStatusInstalled:
-			return nil, ErrUpdateInProgressOnInstance
+			return nil, nil, ErrUpdateInProgressOnInstance
 		}
 	}
 
 	group, err := api.GetGroup(groupID)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	if group.Channel == nil || group.Channel.Package == nil {
 		if err := api.newGroupActivityEntry(activityPackageNotFound, activityWarning, "0.0.0", appID, groupID); err != nil {
 			l.Error().Err(err).Msg("GetUpdatePackagesForSyncer - could not add new group activity entry")
 		}
-		return nil, ErrNoPackageFound
+		return nil, nil, ErrNoPackageFound
 	}
 
 	// Check if update is needed
 	instanceSemver, _ := semver.Make(instanceVersion)
 	packageSemver, _ := semver.Make(group.Channel.Package.Version)
 	if !instanceSemver.LT(packageSemver) {
-		return nil, ErrNoUpdatePackageAvailable
+		return nil, nil, ErrNoUpdatePackageAvailable
 	}
 
-	packages, err := api.getPackagesWithFloorsForUpdate(group, instanceVersion)
+	floors, target, err := api.getPackagesWithFloorsForUpdate(group, instanceVersion)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	// Safety check: verify no packages are blacklisted for this channel
 	// Syncers need all packages, so if any is blacklisted we can't send a valid manifest
 	// This should never happen (floors/targets can't be blacklisted for their own channel)
 	// but we check anyway for data consistency
-	for _, pkg := range packages {
+	for _, pkg := range floors {
 		if slices.Contains(pkg.ChannelsBlacklist, group.Channel.ID) {
 			l.Error().Str("package", pkg.Version).Str("channel", group.Channel.ID).
 				Msg("Package is blacklisted for its own channel - data inconsistency!")
-			return nil, ErrNoUpdatePackageAvailable
+			return nil, nil, ErrNoUpdatePackageAvailable
 		}
 	}
+	if target != nil && slices.Contains(target.ChannelsBlacklist, group.Channel.ID) {
+		l.Error().Str("package", target.Version).Str("channel", group.Channel.ID).
+			Msg("Package is blacklisted for its own channel - data inconsistency!")
+		return nil, nil, ErrNoUpdatePackageAvailable
+	}
 
 	if err := api.enforceRolloutPolicy(instance, group); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	// Grant the update using target version
-	targetVersion := packages[len(packages)-1].Version
-	if err := api.grantUpdate(instance, targetVersion); err != nil {
-		l.Error().Err(err).Str("version", targetVersion).Str("instance", instance.ID).Msg("GetUpdatePackagesForSyncer - grantUpdate error")
-		return nil, ErrUpdateGrantFailed
+	// Grant the update using the highest version we're returning
+	// (last floor if no target, or target if present)
+	var grantVersion string
+	if target != nil {
+		grantVersion = target.Version
+	} else if len(floors) > 0 {
+		grantVersion = floors[len(floors)-1].Version
+	} else {
+		return nil, nil, ErrNoUpdatePackageAvailable
+	}
+	if err := api.grantUpdate(instance, grantVersion); err != nil {
+		l.Error().Err(err).Str("version", grantVersion).Str("instance", instance.ID).Msg("GetUpdatePackagesForSyncer - grantUpdate error")
+		return nil, nil, ErrUpdateGrantFailed
 	}
 
 	// Record activity
 	if !api.hasRecentActivity(activityRolloutStarted, ActivityQueryParams{
 		Severity: activityInfo,
 		AppID:    appID,
-		Version:  targetVersion,
+		Version:  grantVersion,
 		GroupID:  groupID,
 	}) {
-		if err := api.newGroupActivityEntry(activityRolloutStarted, activityInfo, targetVersion, appID, groupID); err != nil {
+		if err := api.newGroupActivityEntry(activityRolloutStarted, activityInfo, grantVersion, appID, groupID); err != nil {
 			l.Error().Err(err).Msg("GetUpdatePackagesForSyncer - could not add new group activity entry")
 		}
 	}
@@ -278,7 +308,7 @@ func (api *API) GetUpdatePackagesForSyncer(inst Instance, instApp InstanceApplic
 		}
 	}
 
-	return packages, nil
+	return floors, target, nil
 }
 
 // enforceRolloutPolicy validates if an update should be provided to the
@@ -363,29 +393,50 @@ func inOfficeHoursNow(tz string) bool {
 	return true
 }
 
-// getPackagesWithFloorsForUpdate returns floors + target for the given group and instance version
-// This is a helper method extracted from the UpdateHandler logic
-func (api *API) getPackagesWithFloorsForUpdate(group *Group, instanceVersion string) ([]*Package, error) {
+// getPackagesWithFloorsForUpdate returns floors and target for the given group and instance version.
+// This is a helper method extracted from the UpdateHandler logic.
+//
+// Returns:
+//   - floors: Required floor packages between instance version and target (may be empty)
+//   - target: The target package, or nil if more floors remain beyond the limit
+//
+// IMPORTANT: When there are more floors remaining than NEBRASKA_MAX_FLOORS_PER_RESPONSE,
+// target will be nil. This signals that the syncer should request again with the highest
+// floor version to get remaining floors.
+func (api *API) getPackagesWithFloorsForUpdate(group *Group, instanceVersion string) (floors []*Package, target *Package, err error) {
 	if group.Channel == nil || group.Channel.Package == nil {
-		return nil, ErrNoPackageFound
+		return nil, nil, ErrNoPackageFound
 	}
 
-	// Get required floors using the channel
-	requiredFloors, err := api.GetRequiredChannelFloors(
+	// Get required floors using LIMIT+1 to detect if more floors remain
+	// This is more efficient than a separate COUNT query
+	requiredFloors, hasMoreFloors, err := api.GetRequiredChannelFloorsWithLimit(
 		group.Channel,
 		instanceVersion,
 	)
 
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	targetPkg := group.Channel.Package
+
 	// Check if target is already included (when target is also a floor)
-	if len(requiredFloors) > 0 && requiredFloors[len(requiredFloors)-1].ID == targetPkg.ID {
-		return requiredFloors, nil
+	var lastFloor *Package
+	if len(requiredFloors) > 0 {
+		lastFloor = requiredFloors[len(requiredFloors)-1]
+	}
+	targetIsLastFloor := lastFloor != nil && lastFloor.ID == targetPkg.ID
+	if targetIsLastFloor {
+		return requiredFloors, lastFloor, nil
+	}
+
+	// If more floors remain, don't include target yet
+	// Syncer will request again with highest floor version
+	if hasMoreFloors {
+		return requiredFloors, nil, nil
 	}
 
-	// Append target if not already included
-	return append(requiredFloors, targetPkg), nil
+	// All floors sent (or no floors) - include target
+	return requiredFloors, targetPkg, nil
 }