Skip to content

Commit 63a4681

Browse files
committed
fix(collab-doc): reject stale durable merges at the relay (cross-process ordering)
The in-process merge chain only orders merges within one apps/sim process. Two durable writes for the same file on DIFFERENT processes could reach the relay out of dispatch order; the relay recorded the version monotonically but still APPLIED the older markdown, regressing the live doc while the token stayed high (a later persist could then write the stale content back over the durable file). Enforce ordering at the relay — the single cross-process coordination point — using the existing Redis primitives: under the per-file Redis merge lock, read the cluster-wide synced version and SKIP a versioned merge that is not newer (a newer durable write already landed). Make recordVersion await setSyncedVersion so it is durable before the lock releases, so the next holder's staleness check reads a consistent value. Streaming (versionless) merges are unaffected — they carry no durable version and are ordered per-process by the caller. Adds a relay test asserting a stale/idempotent versioned merge returns 'stale' and never computes or publishes a diff.
1 parent 68acab6 commit 63a4681

2 files changed

Lines changed: 46 additions & 10 deletions

File tree

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,28 @@ describe('setupWorkspaceFileDocHandlers', () => {
543543
expect(mockFetchFileDocMerge).not.toHaveBeenCalled()
544544
})
545545

546+
it('rejects a stale versioned merge (not newer than the synced version) without regressing the doc', async () => {
547+
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Original')) // seed version 1
548+
const { io } = createIo()
549+
const { handlers } = setup('socket-1', io)
550+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
551+
await flushMicrotasks()
552+
553+
mockFetchFileDocMerge.mockResolvedValue(Y.encodeStateAsUpdate(new Y.Doc()))
554+
555+
// A newer durable version lands and is recorded as the synced version.
556+
expect(await applyMarkdownToLiveFileDoc('file-1', '# newer', 100)).toBe('applied')
557+
mockFetchFileDocMerge.mockClear()
558+
559+
// An older durable version arriving out of order (e.g. a concurrent write on another process) is
560+
// stale: skipped before any diff is computed, so the live doc never regresses to older content and
561+
// no diff is published that a later persist could write back.
562+
expect(await applyMarkdownToLiveFileDoc('file-1', '# older, stale', 50)).toBe('stale')
563+
// The same version is idempotent — also skipped.
564+
expect(await applyMarkdownToLiveFileDoc('file-1', '# same version', 100)).toBe('stale')
565+
expect(mockFetchFileDocMerge).not.toHaveBeenCalled()
566+
})
567+
546568
it('serializes concurrent merges for the same file (second waits for the first)', async () => {
547569
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Original'))
548570
const { io } = createIo()

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

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ export function applyMarkdownToLiveFileDoc(
541541
fileId: string,
542542
markdown: string,
543543
version?: number
544-
): Promise<'applied' | 'no-live-room' | 'merge-unavailable'> {
544+
): Promise<'applied' | 'no-live-room' | 'merge-unavailable' | 'stale'> {
545545
const name = roomName(fileDocRoom(fileId))
546546
const prior = fileDocMergeChains.get(name) ?? Promise.resolve()
547547
// `.catch` so a failed prior merge doesn't reject this one — each merge is independent.
@@ -562,22 +562,30 @@ async function mergeMarkdownIntoRoom(
562562
fileId: string,
563563
markdown: string,
564564
version?: number
565-
): Promise<'applied' | 'no-live-room' | 'merge-unavailable'> {
565+
): Promise<'applied' | 'no-live-room' | 'merge-unavailable' | 'stale'> {
566566
const store = getFileDocStore()
567567

568568
// The durable version this merge carries is now incorporated in the live doc — record it (cluster-wide
569569
// in Redis for multi-task, plus this task's room) so the persist If-Match guard treats this write as
570-
// synced rather than an out-of-band conflict. Set on success below.
571-
const recordVersion = () => {
570+
// synced rather than an out-of-band conflict. AWAITED so the version is durable before the merge lock
571+
// releases, so the next lock holder's staleness check (below) reads a consistent value.
572+
const recordVersion = async () => {
572573
if (version === undefined) return
573574
const room = fileDocRooms.get(name)
574-
// Never regress the token: merges/seeds/persists all write it (locally and via fire-and-forget
575-
// Redis), so a lower value arriving out of order must not shadow a higher one the doc already
576-
// incorporates (the Redis side is guarded identically by SET_VERSION_IF_NEWER_SCRIPT).
575+
// Never regress the token: merges/seeds/persists all write it, so a lower value arriving out of
576+
// order must not shadow a higher one the doc already incorporates (the Redis side is guarded
577+
// identically by SET_VERSION_IF_NEWER_SCRIPT).
577578
if (room) room.syncedVersion = Math.max(room.syncedVersion ?? 0, version)
578-
void store.setSyncedVersion(name, version)
579+
await store.setSyncedVersion(name, version)
579580
}
580581

582+
// A versioned (durable) merge that is not NEWER than the version the doc already incorporates is
583+
// stale — a newer durable write already landed (possibly on another process, out of dispatch order).
584+
// Diffing toward its older markdown would regress the live doc while the monotonic token stays high,
585+
// so a later persist could write that stale content back over the durable file. Skip it. Streaming
586+
// (versionless) merges carry no durable version and are ordered per-process by the caller.
587+
const isStale = (current: number): boolean => version !== undefined && version <= current
588+
581589
if (store.enabled) {
582590
// Serialize merges to this file ACROSS tasks — the per-file chain above only covers this process.
583591
// Two copilot edits to the same file landing on different tasks must not diff the SAME shared base
@@ -595,6 +603,11 @@ async function mergeMarkdownIntoRoom(
595603
return 'merge-unavailable'
596604
}
597605
try {
606+
// Staleness is checked under the lock against the cluster-wide synced version, so a durable merge
607+
// that lost the race to a newer one (on any process) is dropped rather than regressing the doc.
608+
const shared = await store.getSyncedVersion(name)
609+
const current = Math.max(shared ?? 0, fileDocRooms.get(name)?.syncedVersion ?? 0)
610+
if (isStale(current)) return 'stale'
598611
// Compute the diff against the committed SHARED state and PUBLISH it — every task with the doc
599612
// live (including this one, via its own tailer) applies it and fans it out to its clients, so the
600613
// merge reaches the live doc no matter which task the apply-edit call landed on. An empty stream
@@ -604,7 +617,7 @@ async function mergeMarkdownIntoRoom(
604617
if (!base) return 'no-live-room'
605618
const diff = await fetchFileDocMerge(fileId, base, markdown)
606619
await store.publishAndWait(name, diff)
607-
recordVersion()
620+
await recordVersion()
608621
return 'applied'
609622
} finally {
610623
await store.releaseMergeSlot(name, token)
@@ -614,12 +627,13 @@ async function mergeMarkdownIntoRoom(
614627
// Single-replica fallback: apply straight to the local authoritative doc.
615628
const room = fileDocRooms.get(name)
616629
if (!room || room.owners.size === 0 || !isDocSeeded(room.doc)) return 'no-live-room'
630+
if (isStale(room.syncedVersion ?? 0)) return 'stale'
617631
const update = await fetchFileDocMerge(fileId, Y.encodeStateAsUpdate(room.doc), markdown)
618632
// The room may have been dropped while the diff was being built; never touch a destroyed doc.
619633
if (fileDocRooms.get(name) !== room) return 'no-live-room'
620634
// No transaction origin → `doc.on('update')` relays to the WHOLE room (every editor sees copilot).
621635
Y.applyUpdate(room.doc, update)
622-
recordVersion()
636+
await recordVersion()
623637
return 'applied'
624638
}
625639

0 commit comments

Comments
 (0)