Skip to content

Commit f9f43c3

Browse files
committed
fix(files): drop late sync frames once fatal; file-doc join error suppression + retry-budget reset
Final safety-audit findings: - CRITICAL: FileDocProvider.handleMessage had no fatal guard. After the connect deadline latched fatal and the editor fell back to a read-only local seed, a late SyncStep2 (slow server / flaky network / deploy) was still applied — merging server state into the seeded doc (content duplication) and flipping synced=true, which un-gated autosave and would persist the duplicate to the real file. fatal guarded (re)join but not inbound sync. Now handleMessage returns early when fatal. +test (late SyncStep2 after the deadline is ignored, doc stays empty + gated). - file-doc join catch now suppresses the client-facing error when superseded (matches workflow/tables/workspace-files) so a retryable error for an abandoned file can't make a client re-join and cancel the newer one. - table/workspace-files room hooks reset the retry budget on (re)connect so a prior full exhaustion doesn't block retries after a reconnect. - presence-visibility: corrected a stale TTL comment. Gates: tsc (sim/realtime) 0, 209 realtime + collab/hooks suites, biome, boundaries, prune.
1 parent 2643970 commit f9f43c3

6 files changed

Lines changed: 69 additions & 8 deletions

File tree

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,9 @@ export function setupWorkspaceFileDocHandlers(
434434
let currentFileId: string | null = null
435435

436436
socket.on(FILE_DOC_EVENTS.JOIN, async ({ fileId, clientId }: JoinFileDocPayload) => {
437+
// Hoisted so the catch can tell whether this join was superseded (a switch to another file)
438+
// before surfacing a retryable error for the abandoned one.
439+
let generation: number | undefined
437440
try {
438441
const userId = socket.userId
439442
const userName = socket.userName
@@ -460,7 +463,7 @@ export function setupWorkspaceFileDocHandlers(
460463

461464
// Claim this JOIN's generation before the async authorize below, and record the file the
462465
// socket now intends to edit so a leave for it can cancel this join if it's still in-flight.
463-
const generation = (joinGeneration.get(socket.id) ?? 0) + 1
466+
generation = (joinGeneration.get(socket.id) ?? 0) + 1
464467
joinGeneration.set(socket.id, generation)
465468
currentFileId = fileId
466469

@@ -582,6 +585,15 @@ export function setupWorkspaceFileDocHandlers(
582585
if (socketToRoomName.get(socket.id) === name) cleanupFileDocForSocket(socket.id, io)
583586
destroyRoomIfIdle(name)
584587
} catch {}
588+
// Suppress the client-facing error when this join was already superseded (a switch to another
589+
// file, or a disconnect): the rollback above still ran, but a retryable error naming the
590+
// abandoned file could make a client re-join it and cancel the newer one (matches the sibling
591+
// handlers).
592+
if (
593+
socket.disconnected ||
594+
(generation !== undefined && joinGeneration.get(socket.id) !== generation)
595+
)
596+
return
585597
emitJoinError(socket, fileId, 'Failed to join file document', 'JOIN_FAILED', true)
586598
}
587599
})

apps/realtime/src/rooms/presence-visibility.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@ import type { IRoomManager, UserPresence } from '@/rooms/types'
44

55
/**
66
* How stale a not-live presence entry must be before a join-time sweep reclaims
7-
* it. Kept above the 1h socket-key TTL so a normally-idle collaborator is never
8-
* evicted; only genuinely-orphaned entries (e.g. a crashed pod that never fired
9-
* `disconnecting`) are cleared. Matches the workflow join sweep.
7+
* it. The `liveIds` gate (not this threshold) is what protects an active
8+
* collaborator; the threshold only bounds how long a genuinely-orphaned entry
9+
* (e.g. a crashed pod that never fired `disconnecting`) lingers. Matches the
10+
* workflow join sweep.
1011
*/
1112
const STALE_PRESENCE_THRESHOLD_MS = 75 * 60 * 1000
1213

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,4 +324,31 @@ describe('FileDocProvider', () => {
324324
vi.useRealTimers()
325325
}
326326
})
327+
328+
it('ignores a late SyncStep2 that arrives after the connect deadline (no merge, stays gated)', () => {
329+
vi.useFakeTimers()
330+
try {
331+
const { provider, doc, fire } = createProvider(true)
332+
fire(FILE_DOC_EVENTS.JOIN_SUCCESS, { fileId: 'file-1' })
333+
334+
// Deadline lapses with no first sync → fatal fallback (editor falls back to a read-only seed).
335+
vi.advanceTimersByTime(12_000)
336+
expect(provider.joinError).toEqual(expect.objectContaining({ code: 'CONNECT_TIMEOUT' }))
337+
338+
// A delayed SyncStep2 finally arrives. Applying it would merge server content into the
339+
// already-seeded doc (duplication) and flip synced→true (un-gating autosave), so it MUST be
340+
// dropped once fatal.
341+
const remote = new Y.Doc()
342+
remote.getText('default').insert(0, 'server content')
343+
const encoder = encoding.createEncoder()
344+
encoding.writeVarUint(encoder, FILE_DOC_MESSAGE_TYPE.SYNC)
345+
syncProtocol.writeSyncStep2(encoder, remote)
346+
fire(FILE_DOC_EVENTS.MESSAGE, encoding.toUint8Array(encoder))
347+
348+
expect(provider.synced).toBe(false)
349+
expect(doc.getText('default').toString()).toBe('')
350+
} finally {
351+
vi.useRealTimers()
352+
}
353+
})
327354
})

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,13 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
183183
}
184184

185185
private handleMessage = (data: unknown) => {
186+
// Once we've given up (a non-retryable rejection, or the connect deadline lapsed and the editor
187+
// fell back to a read-only local seed), ignore ALL inbound frames. A late SyncStep2 arriving
188+
// after the deadline would otherwise merge the server's state into the already-seeded doc —
189+
// duplicating content — and flip `synced` true, which un-gates autosave and would persist the
190+
// duplicate back to the real file. `fatal` guarding (re)join alone is not enough; it must also
191+
// stop applying sync here.
192+
if (this.fatal) return
186193
const bytes = toFileDocBytes(data)
187194
if (!bytes) return
188195

apps/sim/app/workspace/[workspaceId]/files/hooks/use-workspace-files-room.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ export function useWorkspaceFilesRoom(workspaceId: string): void {
4040

4141
const join = () => socket.emit('join-workspace-files', { workspaceId })
4242

43+
// A fresh (re)connect gets a fresh retry budget, so a prior full exhaustion doesn't leave the
44+
// socket unable to retry a failed re-join until the next success.
45+
const handleConnect = () => {
46+
retries = 0
47+
join()
48+
}
49+
4350
const handleJoinSuccess = (data: { workspaceId: string }) => {
4451
if (data.workspaceId !== workspaceId) return
4552
retries = 0
@@ -68,14 +75,14 @@ export function useWorkspaceFilesRoom(workspaceId: string): void {
6875

6976
// Join now if the socket is already connected; `connect` covers (re)connects.
7077
if (socket.connected) join()
71-
socket.on('connect', join)
78+
socket.on('connect', handleConnect)
7279
socket.on('join-workspace-files-success', handleJoinSuccess)
7380
socket.on('join-workspace-files-error', handleJoinError)
7481
socket.on('workspace-files-changed', handleChanged)
7582

7683
return () => {
7784
if (retryTimer) clearTimeout(retryTimer)
78-
socket.off('connect', join)
85+
socket.off('connect', handleConnect)
7986
socket.off('join-workspace-files-success', handleJoinSuccess)
8087
socket.off('join-workspace-files-error', handleJoinError)
8188
socket.off('workspace-files-changed', handleChanged)

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-room.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,13 @@ export function useTableRoom(tableId: string): UseTableRoomResult {
7575
socket.emit(TABLE_PRESENCE_EVENTS.JOIN, { tableId, tabSessionId: tabSessionIdRef.current })
7676
}
7777

78+
// A fresh (re)connect gets a fresh retry budget, so a prior full exhaustion doesn't leave the
79+
// socket unable to retry a failed re-join until the next success.
80+
const handleConnect = () => {
81+
retries = 0
82+
join()
83+
}
84+
7885
const handleJoinSuccess = (data: JoinTableSuccess) => {
7986
if (data.tableId !== tableId) return
8087
retries = 0
@@ -128,15 +135,15 @@ export function useTableRoom(tableId: string): UseTableRoomResult {
128135

129136
// Join now if the socket is already connected; `connect` covers (re)connects.
130137
if (socket.connected) join()
131-
socket.on('connect', join)
138+
socket.on('connect', handleConnect)
132139
socket.on(TABLE_PRESENCE_EVENTS.JOIN_SUCCESS, handleJoinSuccess)
133140
socket.on(TABLE_PRESENCE_EVENTS.JOIN_ERROR, handleJoinError)
134141
socket.on(TABLE_PRESENCE_UPDATE_EVENT, handlePresence)
135142
socket.on(TABLE_PRESENCE_EVENTS.CELL_SELECTION, handleCellSelection)
136143

137144
return () => {
138145
if (retryTimer) clearTimeout(retryTimer)
139-
socket.off('connect', join)
146+
socket.off('connect', handleConnect)
140147
socket.off(TABLE_PRESENCE_EVENTS.JOIN_SUCCESS, handleJoinSuccess)
141148
socket.off(TABLE_PRESENCE_EVENTS.JOIN_ERROR, handleJoinError)
142149
socket.off(TABLE_PRESENCE_UPDATE_EVENT, handlePresence)

0 commit comments

Comments
 (0)