Skip to content

Commit 8a7bca4

Browse files
committed
fix(collab-doc): order streaming merges by causal base version, not wall-clock
A streaming snapshot now carries baseVersion (the durable contentUpdatedAt it was built from) instead of a wall-clock streamedAt. The relay drops the snapshot when a newer durable write landed since that base, so a concurrent human save can no longer be clobbered in the live doc and then persisted over the durable file. Skew-immune: both keys are DB-monotonic contentUpdatedAt values.
1 parent 6986b74 commit 8a7bca4

11 files changed

Lines changed: 141 additions & 95 deletions

File tree

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

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,27 +60,26 @@ describe('applyMarkdownToLiveFileDoc — multi-replica (store-enabled) ordering'
6060
mockFetchFileDocMerge.mockResolvedValue(Y.encodeStateAsUpdate(new Y.Doc()))
6161
})
6262

63-
it('stale-checks against the SHARED synced version and never records a streaming merge', async () => {
64-
// A durable write records the shared synced version cluster-wide.
63+
it('drops a stale-base streaming snapshot against the SHARED synced version and never records it', async () => {
64+
// A durable write (e.g. a concurrent human save on another process) records the shared synced version.
6565
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable', { version: 100 })).toBe(
6666
'applied'
6767
)
6868
expect(fakeStore.setSyncedVersion).toHaveBeenCalledWith(ROOM_NAME, 100)
6969
mockFetchFileDocMerge.mockClear()
7070

71-
// A delayed streaming snapshot older than the SHARED version (e.g. from another process) is stale —
72-
// rejected under the lock before any diff is built, so the live doc never regresses.
73-
expect(await applyMarkdownToLiveFileDoc('file-1', '# older stream', { streamedAt: 50 })).toBe(
74-
'stale'
75-
)
71+
// A streaming snapshot built from an older base (50) than the SHARED synced version is stale —
72+
// rejected under the lock before any diff is built, so it can't clobber the durable write.
73+
expect(
74+
await applyMarkdownToLiveFileDoc('file-1', '# stale-base stream', { baseVersion: 50 })
75+
).toBe('stale')
7676
expect(mockFetchFileDocMerge).not.toHaveBeenCalled()
7777

78-
// A streaming snapshot newer than the shared version applies (advances the live view)...
79-
expect(await applyMarkdownToLiveFileDoc('file-1', '# newer stream', { streamedAt: 200 })).toBe(
80-
'applied'
81-
)
82-
// ...but it is never recorded: a durable write between the two (150) still applies. Had 200 been
83-
// recorded to the shared store, 150 would be rejected as stale.
78+
// A streaming snapshot whose base is the current shared version applies (nothing newer to clobber)...
79+
expect(
80+
await applyMarkdownToLiveFileDoc('file-1', '# current-base stream', { baseVersion: 100 })
81+
).toBe('applied')
82+
// ...but is never recorded: a later durable write at 150 still applies.
8483
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable again', { version: 150 })).toBe(
8584
'applied'
8685
)

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
569569
expect(mockFetchFileDocMerge).not.toHaveBeenCalled()
570570
})
571571

572-
it('orders a streaming merge by streamedAt but never records it (stays behind the durable version)', async () => {
572+
it('drops a streaming snapshot whose base predates a newer durable write, but never records it', async () => {
573573
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Original')) // seed version 1
574574
const { io } = createIo()
575575
const { handlers } = setup('socket-1', io)
@@ -578,23 +578,25 @@ describe('setupWorkspaceFileDocHandlers', () => {
578578

579579
mockFetchFileDocMerge.mockResolvedValue(Y.encodeStateAsUpdate(new Y.Doc()))
580580

581-
// A newer durable version lands and is recorded as the synced version.
581+
// A durable write (e.g. a concurrent human save) lands and is recorded as the synced version.
582582
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable', { version: 100 })).toBe(
583583
'applied'
584584
)
585585

586-
// A delayed streaming snapshot OLDER than that durable version — e.g. a throttled copilot snapshot
587-
// from another process arriving late — is stale, so it can't regress the doc back toward its content.
588-
expect(await applyMarkdownToLiveFileDoc('file-1', '# older stream', { streamedAt: 50 })).toBe(
589-
'stale'
590-
)
586+
// A streaming snapshot built from an OLDER base (50) — copilot loaded the file before that durable
587+
// write — is stale: applying it would diff the live doc back toward the copilot content and clobber
588+
// the durable write, which a later persist would then write over the file.
589+
expect(
590+
await applyMarkdownToLiveFileDoc('file-1', '# stale-base stream', { baseVersion: 50 })
591+
).toBe('stale')
591592

592-
// A streaming snapshot NEWER than the durable version applies (it advances the live view)...
593-
expect(await applyMarkdownToLiveFileDoc('file-1', '# newer stream', { streamedAt: 200 })).toBe(
594-
'applied'
595-
)
596-
// ...but it is never RECORDED as the synced version: a durable write between the two (150) still
597-
// applies. Had the streaming 200 been recorded, 150 would have been rejected as stale.
593+
// A streaming snapshot whose base IS the current durable version applies — nothing newer to clobber.
594+
expect(
595+
await applyMarkdownToLiveFileDoc('file-1', '# current-base stream', { baseVersion: 100 })
596+
).toBe('applied')
597+
598+
// ...and a streaming merge is never recorded as the synced version: a later durable write at 150 still
599+
// applies (only durable writes move the synced version; the final edit_content write reconciles).
598600
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable again', { version: 150 })).toBe(
599601
'applied'
600602
)

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

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -517,11 +517,12 @@ const fileDocMergeChains = new Map<string, Promise<unknown>>()
517517

518518
/**
519519
* How a merge is positioned on the file's version line — mirrors the sim-side `LiveFileDocMergeOrder`
520-
* wire fields. A durable `version` is checked AND recorded; a streaming `streamedAt` is checked only.
520+
* wire fields. A durable `version` is checked AND recorded; a streaming `baseVersion` (the durable version
521+
* the snapshot was built from) is checked only — dropped if a newer durable write has since landed.
521522
*/
522523
interface MergeOrder {
523524
version?: number
524-
streamedAt?: number
525+
baseVersion?: number
525526
}
526527

527528
/**
@@ -568,15 +569,15 @@ async function mergeMarkdownIntoRoom(
568569
name: string,
569570
fileId: string,
570571
markdown: string,
571-
{ version, streamedAt }: MergeOrder
572+
{ version, baseVersion }: MergeOrder
572573
): Promise<'applied' | 'no-live-room' | 'merge-unavailable' | 'stale'> {
573574
const store = getFileDocStore()
574575

575576
// The durable version this merge carries is now incorporated in the live doc — record it (cluster-wide
576577
// in Redis for multi-task, plus this task's room) so the persist If-Match guard treats this write as
577578
// synced rather than an out-of-band conflict. AWAITED so the version is durable before the merge lock
578579
// releases, so the next lock holder's staleness check (below) reads a consistent value. Only a durable
579-
// `version` is recorded — a streaming `streamedAt` orders the merge (below) but is never a checkpoint,
580+
// `version` is recorded — a streaming `baseVersion` orders the merge (below) but is never a checkpoint,
580581
// so the synced version stays pinned to the last durable write.
581582
const recordVersion = async () => {
582583
if (version === undefined) return
@@ -588,21 +589,24 @@ async function mergeMarkdownIntoRoom(
588589
await store.setSyncedVersion(name, version)
589590
}
590591

591-
// Position this merge on the file's monotonic version line: a durable write by its `version`, a
592-
// streaming snapshot by its `streamedAt` production time. A merge not NEWER than the version the doc
593-
// already incorporates is stale — a newer durable write already landed (possibly on another process,
594-
// out of dispatch order). Diffing toward its older markdown would regress the live doc while the
595-
// monotonic token stays high, so a later persist could write that stale content back over the durable
596-
// file. Skip it. This is what stops a delayed streaming snapshot from clobbering a newer durable merge
597-
// across processes (the per-process caller chain cannot). A merge with neither key is never stale.
598-
//
599-
// Only durable `version` (DB-monotonic `contentUpdatedAt`) is ever recorded, so the durable line is
600-
// immune to clock skew. `streamedAt` is best-effort wall-clock used ONLY to order transient streaming
601-
// snapshots; correctness never depends on it — a skewed clock at worst drops or briefly regresses the
602-
// live preview, which the next durable write reconciles.
603-
const orderingVersion = version ?? streamedAt
604-
const isStale = (current: number): boolean =>
605-
orderingVersion !== undefined && orderingVersion <= current
592+
// Order this merge on the file's version line, where `current` is the durable version the doc already
593+
// incorporates. Both keys are DB-monotonic `contentUpdatedAt` values (no wall-clock), so ordering is
594+
// immune to clock skew:
595+
// - A durable `version` is stale if it is NOT strictly newer than `current` — a newer durable write
596+
// already landed (possibly on another process, out of dispatch order); applying its older markdown
597+
// would regress the doc while the monotonic token stays high.
598+
// - A streaming `baseVersion` (the durable version the snapshot was built from) is stale if `current`
599+
// has moved PAST it — a newer durable write landed since the snapshot's base, so diffing the live
600+
// doc back toward the snapshot would clobber that write's content (which a later persist, still
601+
// holding the current If-Match token, would then write over the durable file). This is what stops a
602+
// concurrent human edit from being silently lost; the per-process caller chain cannot see it.
603+
// A merge with neither key is never stale (legacy, unordered). Only a durable `version` is recorded,
604+
// so a streaming snapshot never advances the synced version — the final `edit_content` write does.
605+
const isStale = (current: number): boolean => {
606+
if (version !== undefined) return version <= current
607+
if (baseVersion !== undefined) return current > baseVersion
608+
return false
609+
}
606610

607611
if (store.enabled) {
608612
// Serialize merges to this file ACROSS tasks — the per-file chain above only covers this process.

apps/realtime/src/routes/http.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,16 +209,16 @@ export function createHttpHandler(roomManager: IRoomManager, logger: Logger) {
209209
if (req.method === 'POST' && req.url === '/api/file-doc/apply-edit') {
210210
try {
211211
const body = await readRequestBody(req)
212-
const { fileId, markdown, version, streamedAt } = JSON.parse(body)
212+
const { fileId, markdown, version, baseVersion } = JSON.parse(body)
213213
if (!isNonEmptyString(fileId) || typeof markdown !== 'string') {
214214
return sendError(res, 'Invalid fileId or markdown', 400)
215215
}
216216
// `version` (the durable updatedAt this markdown was written with) records that the live doc now
217217
// incorporates that durable version, so the persist If-Match guard won't flag it as a conflict.
218-
// `streamedAt` orders a streaming snapshot on the same version line without recording it.
218+
// `baseVersion` is a streaming snapshot's causal base: dropped if a newer durable write landed.
219219
const result = await applyMarkdownToLiveFileDoc(fileId, markdown, {
220220
version: typeof version === 'number' ? version : undefined,
221-
streamedAt: typeof streamedAt === 'number' ? streamedAt : undefined,
221+
baseVersion: typeof baseVersion === 'number' ? baseVersion : undefined,
222222
})
223223
res.writeHead(200, { 'Content-Type': 'application/json' })
224224
res.end(JSON.stringify({ applied: result === 'applied' }))

apps/sim/lib/copilot/request/go/file-preview-adapter.test.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const { mergeEditIntoLiveFileDocMock, isLiveDocMergeInFlightMock } = vi.hoisted(
1515
(
1616
fileId: string,
1717
markdown: string,
18-
order?: { version?: number; streamedAt?: number }
18+
order?: { version?: number; baseVersion?: number }
1919
) => Promise<void>
2020
>(),
2121
isLiveDocMergeInFlightMock: vi.fn<(fileId: string) => boolean>(),
@@ -46,6 +46,8 @@ import type { ActiveFileIntent, ExecutionContext, StreamEvent } from '@/lib/copi
4646
const STREAM_ID = 'stream-1'
4747
const EDIT_TOOL_CALL_ID = 'edit-content-1'
4848
const WORKSPACE_FILE_TOOL_CALL_ID = 'workspace-file-1'
49+
/** The durable version (`contentUpdatedAt`, epoch ms) the streamed base content is at. */
50+
const BASE_VERSION_MS = 900_000
4951

5052
/** One args_delta chunk of the streamed `edit_content` JSON, as a driveable StreamEvent. */
5153
function editContentDelta(argumentsDelta: string): StreamEvent {
@@ -104,8 +106,12 @@ describe('processFilePreviewStreamEvent — live-doc streaming merge', () => {
104106
vi.clearAllMocks()
105107
mergeEditIntoLiveFileDocMock.mockResolvedValue(undefined)
106108
isLiveDocMergeInFlightMock.mockReturnValue(false)
107-
// Default: an append/patch base is available (a non-empty file), so the base-present gate passes.
108-
peekFileIntentMock.mockResolvedValue({ existingContent: 'Base.' })
109+
// Default: an append/patch base is available (a non-empty file) at durable version BASE_VERSION_MS,
110+
// so the base-present gate passes and the streaming merge carries that base version.
111+
peekFileIntentMock.mockResolvedValue({
112+
existingContent: 'Base.',
113+
fileRecord: { contentUpdatedAt: new Date(BASE_VERSION_MS) },
114+
})
109115
state = createFilePreviewAdapterState()
110116
nowMs = 1_000_000
111117
vi.spyOn(Date, 'now').mockImplementation(() => nowMs)
@@ -129,7 +135,7 @@ describe('processFilePreviewStreamEvent — live-doc streaming merge', () => {
129135
})
130136
}
131137

132-
it('merges the growing full content (no version arg) into the live doc as it streams', async () => {
138+
it('merges the growing full content (base version, no durable version) into the live doc as it streams', async () => {
133139
const intent = makeIntent({ operation: 'append', fileId: 'file-grow', fileName: 'notes.md' })
134140

135141
await drive(editContentDelta('{"content":"Hello'), intent)
@@ -143,16 +149,17 @@ describe('processFilePreviewStreamEvent — live-doc streaming merge', () => {
143149
expect(mergeEditIntoLiveFileDocMock).toHaveBeenCalledTimes(2)
144150
const [first, second] = mergeEditIntoLiveFileDocMock.mock.calls
145151
// A full-file snapshot (base + streamed), never a diff; it grows across deltas. Each streaming merge
146-
// carries `streamedAt` (its wall-clock time) to order it — never `version`, which rides the final
147-
// edit_content write; so the relay orders the snapshot without recording it as a durable checkpoint.
152+
// carries `baseVersion` (the durable version it was built from) to order it — never `version`, which
153+
// rides the final edit_content write; so the relay drops it if a newer durable write has since landed
154+
// but never records it as a durable checkpoint.
148155
expect(first[0]).toBe('file-grow')
149156
expect(first[1]).toContain('Base.')
150157
expect(first[1]).toContain('Hello')
151-
expect(typeof first[2]?.streamedAt).toBe('number')
158+
expect(first[2]?.baseVersion).toBe(BASE_VERSION_MS)
152159
expect(first[2]?.version).toBeUndefined()
153160
expect(second[1]).toContain('Hello world')
154161
expect(second[1].length).toBeGreaterThan(first[1].length)
155-
expect(typeof second[2]?.streamedAt).toBe('number')
162+
expect(second[2]?.baseVersion).toBe(BASE_VERSION_MS)
156163
})
157164

158165
it('throttles merges: two deltas within LIVE_DOC_MERGE_THROTTLE_MS yield one merge', async () => {

0 commit comments

Comments
 (0)