fix(realtime): file-doc rebind must not drop the current doc or leave a writable ghost

Two Cursor findings on the file-doc client-id ownership rebind:
- On a document switch, the prior room was left BEFORE the ownership check, so a
  CLIENT_ID_IN_USE rejection dropped the socket from the old doc without joining
  the new one (contradicting its own comment). Run the ownership check first, and
  leave the previous doc only once the rebind is guaranteed to succeed.
- Reclaiming a client id removed the stale prior socket from owners + awareness
  only; its socketToRoomName + Socket.IO membership remained, and handleMessage's
  SYNC path gates on socketToRoomName (not owners), so it stayed able to write
  document frames until disconnect. Fully evict the reclaimed socket. + 2 tests.

Author: waleedlatif1

apps/realtime/src/handlers/file-doc.test.ts

@@ -33,14 +33,21 @@ interface SentMessage {
 /** An `io` mock that records every server-originated emit with its target/except. */
 function createIo() {
   const sent: SentMessage[] = []
+  /** Records `io.in(socketId).socketsLeave(room)` — a socket forced out of a room from outside. */
+  const left: { socketId: string; room: string }[] = []
   const to = vi.fn((target: string) => ({
     except: (exclude: string) => ({
       emit: (event: string, payload: unknown) =>
         sent.push({ target, except: exclude, event, payload }),
     }),
     emit: (event: string, payload: unknown) => sent.push({ target, event, payload }),
   }))
-  return { io: { to } as unknown as IRoomManager['io'], sent }
+  const inFn = vi.fn((socketId: string) => ({
+    socketsLeave: (room: string) => {
+      left.push({ socketId, room })
+    },
+  }))
+  return { io: { to, in: inFn } as unknown as IRoomManager['io'], sent, left }
 }
 
 /** Every socket id a test created, so `afterEach` can drop their rooms without a
@@ -583,6 +590,49 @@ describe('setupWorkspaceFileDocHandlers', () => {
     expect(sent.find((m) => m.event === FILE_DOC_EVENTS.SEED_REQUEST)?.target).toBe('socket-b')
   })
 
+  it('fully evicts a reclaimed prior socket so it can no longer write to the doc', async () => {
+    const { io, sent, left } = createIo()
+    const a = setup('socket-a', io)
+    await a.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 7 })
+    const b = setup('socket-b', io) // same default user-1
+    await b.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 7 }) // reclaims client id 7
+
+    // The stale prior socket is forced out of the Socket.IO room...
+    expect(left).toContainEqual({ socketId: 'socket-a', room: ROOM_NAME })
+
+    // ...and its room mapping is cleared, so a later document (SYNC) frame from it is dropped
+    // (handleMessage's SYNC path gates on socketToRoomName): nothing is applied or relayed.
+    sent.length = 0
+    const doc = new Y.Doc()
+    doc.getText('t').insert(0, 'x')
+    const updateFrame = frame(FILE_DOC_MESSAGE_TYPE.SYNC, (e) =>
+      syncProtocol.writeUpdate(e, Y.encodeStateAsUpdate(doc))
+    )
+    a.handlers[FILE_DOC_EVENTS.MESSAGE](updateFrame)
+    expect(sent.some((m) => m.event === FILE_DOC_EVENTS.MESSAGE)).toBe(false)
+  })
+
+  it('does not drop the current document when a switch is rejected for a foreign client id', async () => {
+    const { io } = createIo()
+    const a = setup('socket-a', io) // user-1
+    const other = setup('socket-c', io, { userId: 'user-b' })
+    await a.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 10 }) // a owns 10 in file-1
+    await other.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-2', clientId: 99 }) // user-b owns 99 in file-2
+    a.socket.leave.mockClear()
+    a.socket.join.mockClear()
+
+    // a tries to switch to file-2 but requests client id 99, owned by a DIFFERENT user → reject.
+    await a.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-2', clientId: 99 })
+
+    expect(a.socket.emit).toHaveBeenCalledWith(
+      FILE_DOC_EVENTS.JOIN_ERROR,
+      expect.objectContaining({ code: 'CLIENT_ID_IN_USE' })
+    )
+    // The rejected switch must leave file-1 intact — a is not torn out of its current document.
+    expect(a.socket.leave).not.toHaveBeenCalledWith('workspace-file-doc:file-1')
+    expect(a.socket.join).not.toHaveBeenCalledWith('workspace-file-doc:file-2')
+  })
+
   it('re-elects a new seeder when the elected one misses the seed deadline', async () => {
     const { io, sent } = createIo()
     const a = setup('socket-a', io)

apps/realtime/src/handlers/file-doc.ts

@@ -457,33 +457,39 @@ export function setupWorkspaceFileDocHandlers(
       // here would leak a dead socket's room or bind the socket to the wrong document.
       if (socket.disconnected || joinGeneration.get(socket.id) !== generation) return
 
-      // Switched documents on the same socket — leave the previous one first (a
-      // socket edits at most one document). A duplicate join of the SAME room
-      // falls through and simply re-runs the sync handshake, idempotently.
-      const currentName = socketToRoomName.get(socket.id)
-      if (currentName && currentName !== name) {
-        socket.leave(currentName)
-        cleanupFileDocForSocket(socket.id, io)
-      }
-
       const entry = getOrCreateRoom(io, room)
 
-      // A client id must be owned by at most one user, or a peer could bind an
-      // active collaborator's id and pass the per-frame ownership check to
-      // spoof/clear its caret. Distinguish a reconnect from a spoof by the owning
-      // user: the same user reclaiming its own client id (a dropped socket
-      // reconnecting reuses the Yjs client id, and its prior socket may not be
-      // cleaned up yet) takes over the stale binding; a DIFFERENT user is
-      // rejected. This runs BEFORE any state mutation below, so a rejected rebind
-      // leaves the socket's existing binding and caret untouched.
+      // A client id must be owned by at most one user, or a peer could bind an active
+      // collaborator's id and pass the per-frame ownership check to spoof/clear its caret.
+      // Distinguish a reconnect from a spoof by the owning user: the same user reclaiming its
+      // own client id (a dropped socket reconnecting reuses the Yjs client id, and its prior
+      // socket may not be cleaned up yet) takes over the stale binding; a DIFFERENT user is
+      // rejected. This runs BEFORE any teardown of the socket's current binding below, so a
+      // rejected rebind — even during a document switch — leaves the socket's existing document
+      // and caret untouched.
       for (const [otherSid, owner] of entry.owners) {
         if (owner.clientId !== clientId || otherSid === socket.id) continue
         if (owner.userId !== userId) {
           emitJoinError(socket, fileId, 'Client id already in use', 'CLIENT_ID_IN_USE', false)
           return
         }
+        // Fully evict the stale prior socket of the same user — owner + caret AND its room
+        // mapping + Socket.IO membership — so it can no longer send document (sync) frames:
+        // handleMessage's SYNC path gates on socketToRoomName, not owners. Done inline rather
+        // than via cleanupFileDocForSocket, which could destroyRoomIfIdle the room we're joining.
         entry.owners.delete(otherSid)
         awarenessProtocol.removeAwarenessStates(entry.awareness, [owner.clientId], null)
+        socketToRoomName.delete(otherSid)
+        io.in(otherSid).socketsLeave(name)
+      }
+
+      // Only now that the rebind is guaranteed to succeed, leave a previously-joined document if
+      // switching (a socket edits at most one). A duplicate join of the SAME room falls through
+      // and simply re-runs the sync handshake, idempotently.
+      const currentName = socketToRoomName.get(socket.id)
+      if (currentName && currentName !== name) {
+        socket.leave(currentName)
+        cleanupFileDocForSocket(socket.id, io)
       }
 
       // Accepted: a same socket rebinding to a NEW client id clears its old caret