fix(realtime): keep handler-initiated table eviction retryable

Evicting leaves the Socket.IO room synchronously, which is also how the sweep
discovers work — so a presence removal failing in the per-frame path could never
be retried and left a ghost collaborator until disconnect. Failed (or
unconfirmed) removals now hand off to the sweep's existing cleanup lane instead
of a second retry loop.

Author: waleedlatif1

apps/realtime/src/access-revalidation.ts

@@ -9,7 +9,11 @@ import {
   roomName,
 } from '@sim/realtime-protocol/rooms'
 import { sleep } from '@sim/utils/helpers'
-import { evictSocketFromRoom, runRoomEvictionHandler } from '@/handlers/room-eviction'
+import {
+  evictSocketFromRoom,
+  runRoomEvictionHandler,
+  setEvictionCleanupSink,
+} from '@/handlers/room-eviction'
 import type { AuthenticatedSocket } from '@/middleware/auth'
 import { ROLE_REVALIDATION_TTL_MS, resolveCurrentRoomPermission } from '@/middleware/permissions'
 import type { IRoomManager } from '@/rooms'
@@ -255,6 +259,14 @@ export function startAccessRevalidationSweep(roomManager: IRoomManager): AccessR
       })
   }
 
+  // Handler-initiated evictions (the per-frame gates) hand failed presence cleanups
+  // here: they have already left the Socket.IO room, so the scan can no longer
+  // rediscover them, and this lane is the only thing that retries.
+  setEvictionCleanupSink((socketId, room) => {
+    pendingCleanups.set(`${socketId}:${roomName(room)}`, { socketId, room })
+    launchCleanups()
+  })
+
   function revokeSocket(socket: AuthenticatedSocket, room: RoomRef, name: string): void {
     // Security-critical, pod-local, and synchronous: stop this socket receiving
     // room broadcasts immediately, and drop the handler-local state that would
@@ -380,7 +392,10 @@ export function startAccessRevalidationSweep(roomManager: IRoomManager): AccessR
   )
 
   return {
-    stop: () => clearInterval(timer),
+    stop: () => {
+      clearInterval(timer)
+      setEvictionCleanupSink(null)
+    },
     runOnce,
   }
 }

apps/realtime/src/handlers/room-eviction.ts

@@ -46,6 +46,37 @@ export function runRoomEvictionHandler(socketId: string, room: RoomRef, io: Serv
   }
 }
 
+/** Enqueues a presence cleanup owed for an evicted socket. */
+type EvictionCleanupSink = (socketId: string, room: RoomRef) => void
+
+let evictionCleanupSink: EvictionCleanupSink | null = null
+
+/**
+ * Registers the retrying presence-cleanup lane (owned by the access-revalidation
+ * sweep) that handler-initiated evictions hand failed cleanups to.
+ *
+ * An eviction removes the socket from the Socket.IO room synchronously, which is
+ * also how the sweep discovers work — so once a handler evicts, the sweep can no
+ * longer find that (socket, room) pair to retry a Redis removal that failed. This
+ * sink is the handoff that keeps the sweep's retry semantics (re-join guard,
+ * moved-room guard, no infinite retry) as the single implementation instead of a
+ * second, subtly different retry loop per handler. Unset outside a running sweep,
+ * where {@link requestEvictionCleanup} is a no-op.
+ */
+export function setEvictionCleanupSink(sink: EvictionCleanupSink | null): void {
+  evictionCleanupSink = sink
+}
+
+/**
+ * Asks the sweep's cleanup lane to (re)try presence removal for a socket already
+ * evicted from `room`. Call only when the immediate best-effort removal failed —
+ * the happy path stays immediate so peers see the departure without waiting for a
+ * sweep tick.
+ */
+export function requestEvictionCleanup(socketId: string, room: RoomRef): void {
+  evictionCleanupSink?.(socketId, room)
+}
+
 /**
  * Evicts one socket from one non-workflow room after a confirmed loss of access:
  * tell the client, stop it receiving room broadcasts, and drop the handler-local

apps/realtime/src/handlers/tables.test.ts

@@ -19,6 +19,7 @@ vi.mock('@sim/platform-authz/rooms', () => ({
   authorizeRoom: mockAuthorizeRoom,
 }))
 
+import { setEvictionCleanupSink } from '@/handlers/room-eviction'
 import { setupTablesHandlers } from '@/handlers/tables'
 import { beginRoomPermissionRead, commitRoomPermission } from '@/middleware/permissions'
 
@@ -251,6 +252,50 @@ describe('setupTablesHandlers', () => {
     }
   })
 
+  it('hands a failed presence removal to the sweep so the eviction stays retryable', async () => {
+    // The eviction already left the Socket.IO room, so the sweep's scan can no longer
+    // rediscover this socket — a failed removal here would strand a ghost collaborator
+    // until disconnect unless it is handed to the retrying cleanup lane.
+    vi.useFakeTimers()
+    const owed: Array<{ socketId: string; roomId: string }> = []
+    setEvictionCleanupSink((socketId, room) => owed.push({ socketId, roomId: room.id }))
+    try {
+      const room = { type: ROOM_TYPES.TABLE, id: 'table-defer' }
+      const { socket, handlers } = createSocket({ id: 'socket-defer', userId: 'user-defer' })
+      const roomManager = createRoomManager({
+        getRoomForSocket: vi.fn().mockResolvedValue(room),
+        removeUserFromRoom: vi.fn().mockRejectedValue(new Error('redis down')),
+      })
+      setupTablesHandlers(socket as unknown as SetupArg, roomManager)
+
+      await handlers[TABLE_PRESENCE_EVENTS.JOIN]({ tableId: 'table-defer' })
+      await vi.advanceTimersByTimeAsync(0)
+
+      mockAuthorizeRoom.mockResolvedValue({
+        allowed: false,
+        status: 403,
+        workspaceId: 'ws-1',
+        workspacePermission: null,
+      })
+      await vi.advanceTimersByTimeAsync(31_000)
+
+      const cell = {
+        anchor: { rowId: 'row-1', columnId: 'col-a' },
+        focus: { rowId: 'row-1', columnId: 'col-a' },
+      }
+      await handlers[TABLE_PRESENCE_EVENTS.CELL_SELECTION]({ cell })
+      await vi.advanceTimersByTimeAsync(0)
+      await handlers[TABLE_PRESENCE_EVENTS.CELL_SELECTION]({ cell })
+      await vi.advanceTimersByTimeAsync(0)
+
+      expect(socket.leave).toHaveBeenCalledWith('table:table-defer')
+      expect(owed).toEqual([{ socketId: 'socket-defer', roomId: 'table-defer' }])
+    } finally {
+      setEvictionCleanupSink(null)
+      vi.useRealTimers()
+    }
+  })
+
   it('drops a malformed cell selection without storing or relaying it', async () => {
     const { socket, handlers, toEmit } = createSocket()
     const roomManager = createRoomManager({

apps/realtime/src/handlers/tables.ts

@@ -8,7 +8,7 @@ import {
   type TableCellSelection,
 } from '@sim/realtime-protocol/table-presence'
 import { resolveAvatarUrl } from '@/handlers/avatar'
-import { evictSocketFromRoom } from '@/handlers/room-eviction'
+import { evictSocketFromRoom, requestEvictionCleanup } from '@/handlers/room-eviction'
 import { resolveRoomJoinAuth } from '@/handlers/room-join-auth'
 import type { AuthenticatedSocket } from '@/middleware/auth'
 import { peekRoomPermission, resolveCurrentRoomPermission } from '@/middleware/permissions'
@@ -90,9 +90,14 @@ function normalizeCellSelection(cell: unknown): TableCellSelection | undefined {
 
 /**
  * Evicts a socket from a table room after a confirmed loss of access, then drops
- * its presence so peers stop seeing its selection. The presence half is
- * best-effort — the socket has already left the room, and the sweep's cleanup lane
- * retries any removal that fails here.
+ * its presence so peers stop seeing its selection.
+ *
+ * The eviction leaves the Socket.IO room immediately, which is also how the sweep
+ * discovers work — so a presence removal that fails here would be unretryable and
+ * strand a ghost collaborator until disconnect. The removal is therefore attempted
+ * inline (peers see the departure at once) and handed to the sweep's retrying
+ * cleanup lane if it fails or reports nothing removed, which is the same signal
+ * the sweep treats as a deferrable failure.
  */
 async function evictFromTable(
   socket: AuthenticatedSocket,
@@ -101,10 +106,21 @@ async function evictFromTable(
 ): Promise<void> {
   evictSocketFromRoom(socket, room, 'Your access to this table has been revoked', roomManager.io)
   try {
-    await roomManager.removeUserFromRoom(room, socket.id)
+    // `false` conflates "already gone" with a transport error the manager swallowed;
+    // deferring on it is harmless (the lane drops a cleanup it finds already clean)
+    // and is the only signal a swallowed failure gives us.
+    const removed = await roomManager.removeUserFromRoom(room, socket.id)
+    if (!removed) {
+      requestEvictionCleanup(socket.id, room)
+      return
+    }
     await roomManager.broadcastPresenceUpdate(room, socket.id)
   } catch (error) {
-    logger.warn(`Presence cleanup failed for evicted table socket ${socket.id}`, error)
+    logger.warn(
+      `Presence cleanup failed for evicted table socket ${socket.id}; deferring to the sweep`,
+      error
+    )
+    requestEvictionCleanup(socket.id, room)
   }
 }