refactor(ENG-3514): complete Redis migration - remove Name() and simplify Store logic

BREAKING: Storage interface no longer has Name() method. All backends now behave like Redis:
- Add() is idempotent and returns error if sandbox already exists (no ErrAlreadyExists tolerance)
- Reconcile() always kills orphans (no re-add logic for memory backend)
- Removed manual reservation handling in Add() (Redis handles this separately)

Changes:
- Remove StorageNameMemory, StorageNameRedis, StorageNamePopulateRedis constants
- Remove Name() method from Storage interface and all implementations
- Simplify Store.Add(): no ErrAlreadyExists tolerance, always call AddSandboxToRoutingTable
- Simplify Store.Reconcile(): always kill orphans (Redis-only behavior)
- Remove manual Reserve() call in Store.Add() (Redis backend doesn't need it)

Tests:
- Fix TestAdd_AlreadyInCache: now expects error on duplicate Add
- Fix TestAdd_NotNewlyCreated: now expects error on duplicate Add
- Fix TestAdd_ConcurrentCalls: concurrent adds for same sandbox now only first succeeds
- Add TestAdd_AddSandboxToRoutingTableAlwaysCalled: verify callback always called
- Add TestAdd_StorageErrorHandling: verify storage errors propagate immediately
- Add TestReconcile_CallsRemoveSandboxFromNode: verify orphans are killed (mock-based)
- Add TestENG3514_MigrationComplete: migration checklist verification

All tests pass with -race flag. Build successful.

Author: adababys

packages/api/internal/sandbox/storage/memory/main.go

@@ -12,8 +12,6 @@ type Storage struct {
 	items cmap.ConcurrentMap[string, *memorySandbox]
 }
 
-func (s *Storage) Name() string { return sandbox.StorageNameMemory }
-
 func NewStorage() *Storage {
 	instanceCache := &Storage{
 		items: cmap.New[*memorySandbox](),

packages/api/internal/sandbox/storage/populate_redis/main.go

@@ -19,8 +19,6 @@ type PopulateRedisStorage struct {
 	redisBackend  *redis.Storage
 }
 
-func (m *PopulateRedisStorage) Name() string { return sandbox.StorageNamePopulateRedis }
-
 func (m *PopulateRedisStorage) Add(ctx context.Context, sandbox sandbox.Sandbox) error {
 	err := m.memoryBackend.Add(ctx, sandbox)
 	if err != nil {

packages/api/internal/sandbox/storage/redis/main.go

@@ -34,8 +34,6 @@ type Storage struct {
 	subManager  *subscriptionManager
 }
 
-func (s *Storage) Name() string { return sandbox.StorageNameRedis }
-
 func NewStorage(
 	redisClient redis.UniversalClient,
 ) *Storage {

packages/api/internal/sandbox/store.go

@@ -28,10 +28,6 @@ type (
 )
 
 const (
-	StorageNameMemory        = "memory"
-	StorageNameRedis         = "redis"
-	StorageNamePopulateRedis = "populate_redis"
-
 	sbxRemoveTimeout = 10 * time.Second
 )
 
@@ -40,9 +36,7 @@ type ReservationStorage interface {
 	Release(ctx context.Context, teamID uuid.UUID, sandboxID string) error
 }
 
-// TODO [ENG-3514]: Remove Name() and Sync() and nolint once migrated to Redis
-type Storage interface { //nolint: interfacebloat
-	Name() string
+type Storage interface {
 	Add(ctx context.Context, sandbox Sandbox) error
 	Get(ctx context.Context, teamID uuid.UUID, sandboxID string) (Sandbox, error)
 	Remove(ctx context.Context, teamID uuid.UUID, sandboxID string) error
@@ -101,33 +95,12 @@ func (s *Store) Add(ctx context.Context, sandbox Sandbox, creation *CreationMeta
 		sandbox.EndTime = sandbox.StartTime.Add(sandbox.MaxInstanceLength)
 	}
 
-	err := s.storage.Add(ctx, sandbox)
-	if err == nil {
-		// Count only newly added sandboxes to the store
-		s.callbacks.AddSandboxToRoutingTable(ctx, sandbox)
-	} else {
-		// TODO [ENG-3514]: Remove once migrated to Redis
-		// There's a race condition when the sandbox is added from node sync
-		// This should be fixed once the sync is improved
-		if !errors.Is(err, ErrAlreadyExists) {
-			return err
-		}
-
-		logger.L().Warn(ctx, "Sandbox already exists in cache", logger.WithSandboxID(sandbox.SandboxID))
+	if err := s.storage.Add(ctx, sandbox); err != nil {
+		return err
 	}
 
-	// TODO [ENG-3514]: Simplify once migrated to Redis
-	// Ensure the team reservation is set - no limit.
-	if s.storage.Name() != StorageNameRedis {
-		finishStart, _, err := s.reservations.Reserve(ctx, sandbox.TeamID, sandbox.SandboxID, -1)
-		if err != nil {
-			logger.L().Error(ctx, "Failed to reserve sandbox", zap.Error(err), logger.WithSandboxID(sandbox.SandboxID))
-		}
-
-		if finishStart != nil {
-			finishStart(sandbox, nil)
-		}
-	}
+	// Notify routing table of the newly added sandbox.
+	s.callbacks.AddSandboxToRoutingTable(ctx, sandbox)
 
 	if creation != nil {
 		meta := *creation
@@ -178,31 +151,20 @@ func (s *Store) WaitForStateChange(ctx context.Context, teamID uuid.UUID, sandbo
 }
 
 func (s *Store) Reconcile(ctx context.Context, sandboxes []Sandbox, nodeID string) {
-	sbxsToBeSynced := s.storage.Reconcile(ctx, sandboxes, nodeID)
-
-	if s.storage.Name() == StorageNameRedis {
-		// Redis is the source of truth — divergent sandboxes are orphans running
-		// on the node but not present in the store. Kill them.
-		wg := sync.WaitGroup{}
-		for _, sbx := range sbxsToBeSynced {
-			wg.Go(func() {
-				ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), sbxRemoveTimeout)
-				defer cancel()
-				s.callbacks.RemoveSandboxFromNode(ctx, sbx)
-			})
-		}
-
-		wg.Wait()
-	} else {
-		// Memory backend — divergent sandboxes are ones discovered on the node
-		// that aren't in the local cache yet. Re-add them.
-		for _, sbx := range sbxsToBeSynced {
-			err := s.Add(ctx, sbx, nil)
-			if err != nil {
-				logger.L().Error(ctx, "Failed to re-add sandbox during sync", zap.Error(err), logger.WithSandboxID(sbx.SandboxID))
-			}
-		}
+	// Redis is the source of truth — sandboxes returned by Reconcile are orphans
+	// running on the node but not present in the store. Kill them.
+	orphanSandboxes := s.storage.Reconcile(ctx, sandboxes, nodeID)
+
+	wg := sync.WaitGroup{}
+	for _, sbx := range orphanSandboxes {
+		wg.Go(func() {
+			ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), sbxRemoveTimeout)
+			defer cancel()
+			s.callbacks.RemoveSandboxFromNode(ctx, sbx)
+		})
 	}
+
+	wg.Wait()
 }
 
 func (s *Store) Reserve(ctx context.Context, teamID uuid.UUID, sandboxID string, limit int) (finishStart func(Sandbox, error), waitForStart func(ctx context.Context) (Sandbox, error), err error) {