Skip to content

Commit ca81494

Browse files
committed
fix(files): own presence per client id, not one-per-socket
The shared workspace socket hosts one collaborative provider per mounted view, so the chat file preview and the standalone Files editor for the same file each bind their own Yjs client id over ONE socket. The relay owned a single client id per socket, so the later JOIN overwrote the earlier and dropped its awareness — which silently broke the single-writer agent-stream election (a peer stopped seeing the streaming provider's announcement and could self-elect, duplicating streamed text for the whole stream). Track ownership per (socket, client id): a socket owns a set of client ids; the awareness gate accepts a frame only if every id it carries is owned; cleanup drops all of a socket's ids; the roster stays one-entry-per-session. Reclaim and the same-user reconnect path evict just the reclaimed id, dropping the old socket only if it empties. Falsification-verified test added.
1 parent 8f93e66 commit ca81494

2 files changed

Lines changed: 86 additions & 49 deletions

File tree

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

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -713,23 +713,33 @@ describe('setupWorkspaceFileDocHandlers', () => {
713713
expect(b.socket.join).toHaveBeenCalledWith(ROOM_NAME)
714714
})
715715

716-
it('clears a departed caret when a socket rejoins the room with a new client id', async () => {
716+
it('a socket owns MULTIPLE client ids (co-mounted providers) and relays awareness for each', async () => {
717+
// The shared workspace socket hosts one provider per collaborative view, so the chat file preview
718+
// and the standalone Files editor for the same file each JOIN with their own Yjs client id over ONE
719+
// socket. Ownership is per client id: BOTH announcements must relay. (The old one-owner-per-socket
720+
// model let the later JOIN overwrite the earlier, dropping its awareness — which broke the
721+
// single-writer agent-stream election, letting a peer also self-elect and duplicate streamed text.)
717722
const { io, sent } = createIo()
718-
const { frame: awFrame } = awarenessFrame(500, 'A')
719723
const a = setup('socket-a', io)
720724
await a.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 500 })
721-
a.handlers[FILE_DOC_EVENTS.MESSAGE](awFrame)
722-
sent.length = 0
723-
724725
await a.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 501 })
725-
726-
// The old client (500) caret removal is broadcast to the room.
727-
const removal = sent.find(
728-
(m) =>
729-
m.event === FILE_DOC_EVENTS.MESSAGE &&
730-
(m.payload as Uint8Array)[0] === FILE_DOC_MESSAGE_TYPE.AWARENESS
731-
)
732-
expect(removal).toBeDefined()
726+
expect(joinSuccessFileId(a.socket)).toBe('file-1')
727+
728+
const relayedFor = (clientId: number) => {
729+
sent.length = 0
730+
a.handlers[FILE_DOC_EVENTS.MESSAGE](awarenessFrame(clientId, `c${clientId}`).frame)
731+
return sent.find(
732+
(m) =>
733+
m.event === FILE_DOC_EVENTS.MESSAGE &&
734+
(m.payload as Uint8Array)[0] === FILE_DOC_MESSAGE_TYPE.AWARENESS
735+
)
736+
}
737+
// The FIRST provider's client id (500) is still owned after the second joins — its awareness relays.
738+
expect(relayedFor(500)).toBeDefined()
739+
// The second provider's client id (501) relays too.
740+
expect(relayedFor(501)).toBeDefined()
741+
// A client id this socket does NOT own is still dropped (ownership is not blanket-allowed).
742+
expect(relayedFor(999)).toBeUndefined()
733743
})
734744

735745
it('preserves the existing caret when a rebind to a foreign client id is rejected', async () => {

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

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,16 @@ const MERGE_LOCK_RETRIES = Math.ceil(
9090
(MERGE_LOCK_TTL_MS + FILE_DOC_TIMEOUTS.mergeRequestMs) / MERGE_LOCK_RETRY_MS
9191
)
9292

93-
/** A socket's presence ownership within a room. */
93+
/** One presence ownership within a room: a (socket, clientID) pair. */
9494
interface FileDocOwner {
9595
/**
96-
* The awareness clientID the socket declared at join. It owns exactly this one
97-
* and may only publish/remove awareness for it, so an authenticated peer cannot
98-
* forge or clear another collaborator's presence.
96+
* An awareness clientID this socket declared at join. The socket may only publish/remove awareness
97+
* for a clientID it owns, so an authenticated peer cannot forge or clear another collaborator's
98+
* presence. A single socket can own SEVERAL clientIDs at once — the shared workspace socket hosts one
99+
* provider per mounted collaborative view, so e.g. the chat file preview and the standalone Files
100+
* editor for the same file each bind their own Yjs clientID over the one socket. The election that
101+
* picks a single agent-stream writer depends on every such provider's awareness propagating, so
102+
* ownership is tracked per clientID, not one-per-socket (which would drop the later joiner's frames).
99103
*/
100104
clientId: number
101105
/** The owning user — used to tell a reconnect (same user reusing its Yjs client
@@ -112,8 +116,9 @@ interface FileDocRoom {
112116
fileId: string
113117
doc: Y.Doc
114118
awareness: awarenessProtocol.Awareness
115-
/** socketId → its presence ownership. */
116-
owners: Map<string, FileDocOwner>
119+
/** socketId → (clientId → its presence ownership). A socket owns one entry per collaborative provider
120+
* it mounted for this file (see {@link FileDocOwner}); an empty inner map is never kept. */
121+
owners: Map<string, Map<number, FileDocOwner>>
117122
/** True once the server-side seed fetch has started, so concurrent joins don't each fetch.
118123
* Reset on a fetch FAILURE so a later join can retry (a genuinely empty file stays empty). */
119124
serverSeedStarted: boolean
@@ -365,7 +370,12 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr
365370
*/
366371
function broadcastFileDocPresence(io: Server, name: string, room: FileDocRoom) {
367372
const users: FileDocPresenceUser[] = []
368-
for (const [socketId, owner] of room.owners) {
373+
// One entry PER SOCKET (session), not per clientID: a socket's several providers are the same
374+
// authenticated user, so any of its owners carries the identity; the client dedupes per user for the
375+
// avatar stack (see the roster comment above). An empty inner map is never stored, so `owner` exists.
376+
for (const [socketId, clientMap] of room.owners) {
377+
const owner = clientMap.values().next().value
378+
if (!owner) continue
369379
users.push({
370380
socketId,
371381
userId: owner.userId,
@@ -799,8 +809,9 @@ function handleMessage(socket: AuthenticatedSocket, data: unknown) {
799809

800810
switch (messageType) {
801811
case FILE_DOC_MESSAGE_TYPE.SYNC: {
802-
// Attribute a server-side persist of the resulting edit to the actual editor (blob metadata).
803-
const editor = room.owners.get(socket.id)?.userId
812+
// Attribute a server-side persist of the resulting edit to the actual editor (blob metadata). A
813+
// socket's providers are all the same user, so any owner's userId identifies the editor.
814+
const editor = room.owners.get(socket.id)?.values().next().value?.userId
804815
if (editor) room.lastEditorUserId = editor
805816
const encoder = encoding.createEncoder()
806817
encoding.writeVarUint(encoder, FILE_DOC_MESSAGE_TYPE.SYNC)
@@ -831,11 +842,12 @@ function handleMessage(socket: AuthenticatedSocket, data: unknown) {
831842
}
832843
case FILE_DOC_MESSAGE_TYPE.AWARENESS: {
833844
const update = decoding.readVarUint8Array(decoder)
834-
// Enforce presence ownership: a socket may only publish/remove awareness
835-
// for the clientID it bound at join, so a peer cannot spoof or clear
836-
// another collaborator's caret.
837-
const owned = room.owners.get(socket.id)?.clientId
838-
if (owned === undefined || awarenessUpdateClientIds(update).some((id) => id !== owned)) {
845+
// Enforce presence ownership: a socket may only publish/remove awareness for a clientID it bound
846+
// at join, so a peer cannot spoof or clear another collaborator's caret. A socket can own SEVERAL
847+
// clientIDs (one per mounted provider), so the frame is accepted only if EVERY id it carries is
848+
// owned by this socket.
849+
const owned = room.owners.get(socket.id)
850+
if (owned === undefined || awarenessUpdateClientIds(update).some((id) => !owned.has(id))) {
839851
logger.warn('Dropping awareness frame for an unowned client id', { socketId: socket.id })
840852
return
841853
}
@@ -873,12 +885,16 @@ export function cleanupFileDocForSocket(socketId: string, io: Server, endOfLife
873885
const room = fileDocRooms.get(name)
874886
if (!room) return
875887

876-
const owner = room.owners.get(socketId)
888+
// The socket may own several clientIDs (one per provider it mounted for this file); drop them ALL.
889+
// The client only emits LEAVE / disconnects once its LAST provider for the file tears down, so a
890+
// per-socket cleanup here is correct — an earlier single-provider unmount already cleared its own
891+
// caret via its awareness removal.
892+
const clientMap = room.owners.get(socketId)
877893
room.owners.delete(socketId)
878-
if (owner !== undefined) {
879-
// Fires the awareness `update` handler with a non-socket origin → the removal
880-
// is broadcast to every remaining client, so the departed caret vanishes.
881-
awarenessProtocol.removeAwarenessStates(room.awareness, [owner.clientId], null)
894+
if (clientMap !== undefined && clientMap.size > 0) {
895+
// Fires the awareness `update` handler with a non-socket origin → the removals
896+
// are broadcast to every remaining client, so the departed carets vanish.
897+
awarenessProtocol.removeAwarenessStates(room.awareness, [...clientMap.keys()], null)
882898
// Refresh the roster for whoever remains (server-authenticated identity).
883899
broadcastFileDocPresence(io, name, room)
884900
}
@@ -980,20 +996,27 @@ export function setupWorkspaceFileDocHandlers(
980996
// rejected. This runs BEFORE any teardown of the socket's current binding below, so a
981997
// rejected rebind — even during a document switch — leaves the socket's existing document
982998
// and caret untouched.
983-
for (const [otherSid, owner] of entry.owners) {
984-
if (owner.clientId !== clientId || otherSid === socket.id) continue
999+
for (const [otherSid, clientMap] of entry.owners) {
1000+
if (otherSid === socket.id) continue
1001+
const owner = clientMap.get(clientId)
1002+
if (owner === undefined) continue
9851003
if (owner.userId !== userId) {
9861004
emitJoinError(socket, fileId, 'Client id already in use', 'CLIENT_ID_IN_USE', false)
9871005
return
9881006
}
989-
// Fully evict the stale prior socket of the same user — owner + caret AND its room
990-
// mapping + Socket.IO membership — so it can no longer send document (sync) frames:
991-
// handleMessage's SYNC path gates on socketToRoomName, not owners. Done inline rather
992-
// than via cleanupFileDocForSocket, which could destroyRoomIfIdle the room we're joining.
993-
entry.owners.delete(otherSid)
994-
awarenessProtocol.removeAwarenessStates(entry.awareness, [owner.clientId], null)
995-
socketToRoomName.delete(otherSid)
996-
io.in(otherSid).socketsLeave(name)
1007+
// Same user reclaiming its client id on a stale prior socket: evict just THAT clientID's binding
1008+
// + caret from the old socket. If that leaves the old socket with no providers, also drop its
1009+
// room mapping + Socket.IO membership so it can no longer send document (sync) frames
1010+
// (handleMessage's SYNC path gates on socketToRoomName, not owners); an old socket that still
1011+
// hosts OTHER providers keeps them. Done inline rather than via cleanupFileDocForSocket, which
1012+
// could destroyRoomIfIdle the room we're joining.
1013+
clientMap.delete(clientId)
1014+
awarenessProtocol.removeAwarenessStates(entry.awareness, [clientId], null)
1015+
if (clientMap.size === 0) {
1016+
entry.owners.delete(otherSid)
1017+
socketToRoomName.delete(otherSid)
1018+
io.in(otherSid).socketsLeave(name)
1019+
}
9971020
}
9981021

9991022
// Only now that the rebind is guaranteed to succeed, leave a previously-joined document if
@@ -1005,14 +1028,18 @@ export function setupWorkspaceFileDocHandlers(
10051028
cleanupFileDocForSocket(socket.id, io)
10061029
}
10071030

1008-
// Accepted: a same socket rebinding to a NEW client id clears its old caret
1009-
// so it doesn't linger as a ghost after the binding is overwritten.
1010-
const previous = entry.owners.get(socket.id)
1011-
if (previous !== undefined && previous.clientId !== clientId) {
1012-
awarenessProtocol.removeAwarenessStates(entry.awareness, [previous.clientId], null)
1031+
// ADD this provider's clientID to the socket's ownership set (do NOT overwrite a sibling provider
1032+
// on the same socket — that lone-owner overwrite is exactly what dropped the chat preview's
1033+
// awareness when the Files editor co-mounted). A re-JOIN of the same clientID is idempotent. A
1034+
// single provider that later unmounts clears its own caret via its awareness removal; the whole
1035+
// set is dropped on the socket's LEAVE/disconnect (client emits LEAVE only after its LAST provider
1036+
// for the file tears down).
1037+
let clientMap = entry.owners.get(socket.id)
1038+
if (clientMap === undefined) {
1039+
clientMap = new Map<number, FileDocOwner>()
1040+
entry.owners.set(socket.id, clientMap)
10131041
}
1014-
1015-
entry.owners.set(socket.id, { clientId, userId, userName, avatarUrl })
1042+
clientMap.set(clientId, { clientId, userId, userName, avatarUrl })
10161043
socketToRoomName.set(socket.id, name)
10171044
socket.join(name)
10181045

0 commit comments

Comments
 (0)