Skip to content

Commit cf58ead

Browse files
committed
fix(file-editor): recover collaboration after reconnect
1 parent 8804d5d commit cf58ead

10 files changed

Lines changed: 647 additions & 217 deletions

File tree

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

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,10 @@ describe('setupWorkspaceFileDocHandlers', () => {
545545

546546
expect(socket.join).toHaveBeenCalledWith(ROOM_NAME)
547547
expect(joinSuccessFileId(socket)).toBe('file-1')
548+
expect(socket.emit).toHaveBeenCalledWith(
549+
FILE_DOC_EVENTS.JOIN_SUCCESS,
550+
expect.objectContaining({ fileId: 'file-1', clientId: 1 })
551+
)
548552

549553
// A binary sync-step-1 message (type tag 0) is sent to kick off the handshake.
550554
const syncMessage = socket.emit.mock.calls.find(
@@ -843,7 +847,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
843847
expect(mockFetchFileDocMerge).toHaveBeenCalledTimes(2)
844848
})
845849

846-
it('relays a document update to the rest of the room, excluding the sender', async () => {
850+
it('relays a document update to every provider, including siblings on the sender socket', async () => {
847851
const { io, sent } = createIo()
848852
const a = setup('socket-a', io)
849853
const b = setup('socket-b', io)
@@ -860,7 +864,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
860864

861865
const relayed = sent.find((m) => m.event === FILE_DOC_EVENTS.MESSAGE)
862866
expect(relayed?.target).toBe(ROOM_NAME)
863-
expect(relayed?.except).toBe('socket-a')
867+
expect(relayed?.except).toBeUndefined()
864868
expect((relayed?.payload as Uint8Array)[0]).toBe(FILE_DOC_MESSAGE_TYPE.SYNC)
865869
})
866870

@@ -959,6 +963,35 @@ describe('setupWorkspaceFileDocHandlers', () => {
959963
expect(relayedFor(999)).toBeUndefined()
960964
})
961965

966+
it('accepts concurrent joins from co-mounted providers for the same file', async () => {
967+
let resolveFirstAuth: (value: unknown) => void = () => {}
968+
mockAuthorizeRoom.mockReturnValueOnce(
969+
new Promise((resolve) => {
970+
resolveFirstAuth = resolve
971+
})
972+
)
973+
const { io } = createIo()
974+
const { socket, handlers } = setup('socket-a', io)
975+
976+
const first = handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 500 })
977+
await Promise.resolve()
978+
const second = handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 501 })
979+
await second
980+
981+
resolveFirstAuth({
982+
allowed: true,
983+
status: 200,
984+
workspaceId: 'ws-1',
985+
workspacePermission: 'write',
986+
})
987+
await first
988+
989+
const acceptedClientIds = socket.emit.mock.calls
990+
.filter(([event]) => event === FILE_DOC_EVENTS.JOIN_SUCCESS)
991+
.map(([, payload]) => (payload as { clientId: number }).clientId)
992+
expect(acceptedClientIds).toEqual(expect.arrayContaining([500, 501]))
993+
})
994+
962995
it('preserves the existing caret when a rebind to a foreign client id is rejected', async () => {
963996
const { io, sent } = createIo()
964997
const { frame: awFrame } = awarenessFrame(10, 'A')

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

Lines changed: 56 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ async function mergeMarkdownIntoRoom(
783783
/**
784784
* Get (or lazily create) the authoritative document for a room, wiring the two
785785
* relay handlers exactly once: document updates and awareness changes are
786-
* broadcast to the room, excluding the origin socket (it already applied them).
786+
* broadcast to the room.
787787
*/
788788
function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom {
789789
const name = roomName(ref)
@@ -821,18 +821,12 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom {
821821
const encoder = encoding.createEncoder()
822822
encoding.writeVarUint(encoder, FILE_DOC_MESSAGE_TYPE.SYNC)
823823
syncProtocol.writeUpdate(encoder, update)
824-
// Fan out to THIS task's clients only (excluding the origin socket if local — a user edit OR an
825-
// agent-streamed frame). Cross-task delivery rides the shared stream — every task's tailer applies +
826-
// runs its own local fan-out.
827-
// A client edit excludes its own sender socket (echo suppression). An agent frame broadcasts to the
828-
// WHOLE room — no socket excluded — so a same-socket sibling provider (chat preview + Files editor)
829-
// stays live mid-stream; the emitting provider no-ops on its own echo.
830-
broadcastLocal(
831-
io,
832-
name,
833-
encoding.toUint8Array(encoder),
834-
origin === AGENT_SYNC_ORIGIN ? null : originSocketId(origin)
835-
)
824+
// Fan out to every client on THIS task, including the origin socket. One shared Socket.IO connection
825+
// can host multiple providers for this file; excluding the whole socket would strand the sibling
826+
// provider's distinct Y.Doc. Yjs updates are idempotent, and the originating provider applies its
827+
// echo with the provider as transaction origin, so it does not send the update again. Cross-task
828+
// delivery rides the shared stream, where every task's tailer runs its own local fan-out.
829+
broadcastLocal(io, name, encoding.toUint8Array(encoder), null)
836830
// Share every locally-originated update to the stream so peers converge. Skip updates that already
837831
// came FROM the stream (REDIS_ORIGIN / REDIS_SNAPSHOT_ORIGIN / REDIS_AGENT_ORIGIN) and SEED_ORIGIN —
838832
// the seed is published EXPLICITLY and AWAITED under the seed lock (so it lands before the lock
@@ -898,12 +892,18 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom {
898892
function emitJoinError(
899893
socket: AuthenticatedSocket,
900894
fileId: unknown,
895+
clientId: unknown,
901896
error: string,
902897
code: string,
903898
retryable: boolean
904899
) {
900+
const normalizedClientId =
901+
typeof clientId === 'number' && Number.isInteger(clientId) && clientId >= 0
902+
? clientId
903+
: undefined
905904
socket.emit(FILE_DOC_EVENTS.JOIN_ERROR, {
906905
fileId: typeof fileId === 'string' ? fileId : '',
906+
clientId: normalizedClientId,
907907
error,
908908
code,
909909
retryable,
@@ -1113,11 +1113,25 @@ export function setupWorkspaceFileDocHandlers(
11131113
const userName = socket.userName
11141114

11151115
if (!userId || !userName) {
1116-
emitJoinError(socket, fileId, 'Authentication required', 'AUTHENTICATION_REQUIRED', false)
1116+
emitJoinError(
1117+
socket,
1118+
fileId,
1119+
clientId,
1120+
'Authentication required',
1121+
'AUTHENTICATION_REQUIRED',
1122+
false
1123+
)
11171124
return
11181125
}
11191126
if (!roomManager.isReady()) {
1120-
emitJoinError(socket, fileId, 'Realtime unavailable', 'ROOM_MANAGER_UNAVAILABLE', true)
1127+
emitJoinError(
1128+
socket,
1129+
fileId,
1130+
clientId,
1131+
'Realtime unavailable',
1132+
'ROOM_MANAGER_UNAVAILABLE',
1133+
true
1134+
)
11211135
return
11221136
}
11231137
if (
@@ -1128,15 +1142,20 @@ export function setupWorkspaceFileDocHandlers(
11281142
!Number.isInteger(clientId) ||
11291143
clientId < 0
11301144
) {
1131-
emitJoinError(socket, fileId, 'Invalid join payload', 'INVALID_PAYLOAD', false)
1145+
emitJoinError(socket, fileId, clientId, 'Invalid join payload', 'INVALID_PAYLOAD', false)
11321146
return
11331147
}
11341148

1135-
// Claim this JOIN's generation before the async authorize below, and record the file the
1136-
// socket now intends to edit so a leave for it can cancel this join if it's still in-flight.
1137-
generation = (joinGeneration.get(socket.id) ?? 0) + 1
1138-
joinGeneration.set(socket.id, generation)
1139-
currentFileId = fileId
1149+
// A generation represents the socket's intended FILE, not an individual provider. Co-mounted
1150+
// providers for the same file must be allowed to join concurrently; switching files advances the
1151+
// generation so every in-flight join for the old file is cancelled together.
1152+
if (currentFileId !== fileId) {
1153+
generation = (joinGeneration.get(socket.id) ?? 0) + 1
1154+
joinGeneration.set(socket.id, generation)
1155+
currentFileId = fileId
1156+
} else {
1157+
generation = joinGeneration.get(socket.id) ?? 0
1158+
}
11401159

11411160
const room = fileDocRoom(fileId)
11421161
const name = roomName(room)
@@ -1153,7 +1172,7 @@ export function setupWorkspaceFileDocHandlers(
11531172
accessDenied: 'Access denied to file',
11541173
},
11551174
emitError: ({ error, code, retryable }) =>
1156-
emitJoinError(socket, fileId, error, code, retryable),
1175+
emitJoinError(socket, fileId, clientId, error, code, retryable),
11571176
})
11581177
if (!authorized) return
11591178

@@ -1190,7 +1209,7 @@ export function setupWorkspaceFileDocHandlers(
11901209
logger.warn(
11911210
`User ${userId} lost write access to file ${fileId} before the join completed`
11921211
)
1193-
emitJoinError(socket, fileId, 'Access denied to file', 'ACCESS_DENIED', false)
1212+
emitJoinError(socket, fileId, clientId, 'Access denied to file', 'ACCESS_DENIED', false)
11941213
return
11951214
}
11961215

@@ -1219,7 +1238,14 @@ export function setupWorkspaceFileDocHandlers(
12191238
const owner = clientMap.get(clientId)
12201239
if (owner === undefined) continue
12211240
if (owner.userId !== userId) {
1222-
emitJoinError(socket, fileId, 'Client id already in use', 'CLIENT_ID_IN_USE', false)
1241+
emitJoinError(
1242+
socket,
1243+
fileId,
1244+
clientId,
1245+
'Client id already in use',
1246+
'CLIENT_ID_IN_USE',
1247+
false
1248+
)
12231249
return
12241250
}
12251251
// Same user reclaiming its client id on a stale prior socket: evict just THAT clientID's
@@ -1268,7 +1294,11 @@ export function setupWorkspaceFileDocHandlers(
12681294
// Name the document this room holds, so a client that still carries a DIFFERENT one (its room
12691295
// outlived by a document rebuilt in its place) can refuse to merge instead of unioning two
12701296
// documents into the file twice over. Read after readiness — before it, the room has no doc yet.
1271-
socket.emit(FILE_DOC_EVENTS.JOIN_SUCCESS, { fileId, docId: docIdOf(entry.doc) })
1297+
socket.emit(FILE_DOC_EVENTS.JOIN_SUCCESS, {
1298+
fileId,
1299+
clientId,
1300+
docId: docIdOf(entry.doc),
1301+
})
12721302
// Server-authenticated roster → everyone in the room, including this joiner.
12731303
broadcastFileDocPresence(io, name, entry)
12741304

@@ -1321,7 +1351,7 @@ export function setupWorkspaceFileDocHandlers(
13211351
(generation !== undefined && joinGeneration.get(socket.id) !== generation)
13221352
)
13231353
return
1324-
emitJoinError(socket, fileId, 'Failed to join file document', 'JOIN_FAILED', true)
1354+
emitJoinError(socket, fileId, clientId, 'Failed to join file document', 'JOIN_FAILED', true)
13251355
}
13261356
})
13271357

0 commit comments

Comments
 (0)