Skip to content

Commit e9622e9

Browse files
committed
fix(collab-doc): order streaming merges by streamedAt so a late snapshot can't regress a newer durable write
1 parent b0753ad commit e9622e9

8 files changed

Lines changed: 135 additions & 50 deletions

File tree

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,34 @@ describe('setupWorkspaceFileDocHandlers', () => {
565565
expect(mockFetchFileDocMerge).not.toHaveBeenCalled()
566566
})
567567

568+
it('orders a streaming merge by streamedAt but never records it (stays behind the durable version)', async () => {
569+
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Original')) // seed version 1
570+
const { io } = createIo()
571+
const { handlers } = setup('socket-1', io)
572+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
573+
await flushMicrotasks()
574+
575+
mockFetchFileDocMerge.mockResolvedValue(Y.encodeStateAsUpdate(new Y.Doc()))
576+
577+
// A newer durable version lands and is recorded as the synced version.
578+
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable', 100)).toBe('applied')
579+
580+
// A delayed streaming snapshot OLDER than that durable version — e.g. a throttled copilot snapshot
581+
// from another process arriving late — is stale, so it can't regress the doc back toward its content.
582+
// (`version` omitted, `streamedAt` passed as the 4th arg.)
583+
expect(await applyMarkdownToLiveFileDoc('file-1', '# older stream', undefined, 50)).toBe(
584+
'stale'
585+
)
586+
587+
// A streaming snapshot NEWER than the durable version applies (it advances the live view)...
588+
expect(await applyMarkdownToLiveFileDoc('file-1', '# newer stream', undefined, 200)).toBe(
589+
'applied'
590+
)
591+
// ...but it is never RECORDED as the synced version: a durable write between the two (150) still
592+
// applies. Had the streaming 200 been recorded, 150 would have been rejected as stale.
593+
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable again', 150)).toBe('applied')
594+
})
595+
568596
it('serializes concurrent merges for the same file (second waits for the first)', async () => {
569597
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Original'))
570598
const { io } = createIo()

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

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -540,14 +540,15 @@ const fileDocMergeChains = new Map<string, Promise<unknown>>()
540540
export function applyMarkdownToLiveFileDoc(
541541
fileId: string,
542542
markdown: string,
543-
version?: number
543+
version?: number,
544+
streamedAt?: number
544545
): Promise<'applied' | 'no-live-room' | 'merge-unavailable' | 'stale'> {
545546
const name = roomName(fileDocRoom(fileId))
546547
const prior = fileDocMergeChains.get(name) ?? Promise.resolve()
547548
// `.catch` so a failed prior merge doesn't reject this one — each merge is independent.
548549
const run = prior
549550
.catch(() => {})
550-
.then(() => mergeMarkdownIntoRoom(name, fileId, markdown, version))
551+
.then(() => mergeMarkdownIntoRoom(name, fileId, markdown, version, streamedAt))
551552
fileDocMergeChains.set(
552553
name,
553554
run.finally(() => {
@@ -561,14 +562,17 @@ async function mergeMarkdownIntoRoom(
561562
name: string,
562563
fileId: string,
563564
markdown: string,
564-
version?: number
565+
version?: number,
566+
streamedAt?: number
565567
): Promise<'applied' | 'no-live-room' | 'merge-unavailable' | 'stale'> {
566568
const store = getFileDocStore()
567569

568570
// The durable version this merge carries is now incorporated in the live doc — record it (cluster-wide
569571
// in Redis for multi-task, plus this task's room) so the persist If-Match guard treats this write as
570572
// 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.
573+
// releases, so the next lock holder's staleness check (below) reads a consistent value. Only a durable
574+
// `version` is recorded — a streaming `streamedAt` orders the merge (below) but is never a checkpoint,
575+
// so the synced version stays pinned to the last durable write.
572576
const recordVersion = async () => {
573577
if (version === undefined) return
574578
const room = fileDocRooms.get(name)
@@ -579,12 +583,16 @@ async function mergeMarkdownIntoRoom(
579583
await store.setSyncedVersion(name, version)
580584
}
581585

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
586+
// Position this merge on the file's monotonic version line: a durable write by its `version`, a
587+
// streaming snapshot by its `streamedAt` production time. A merge not NEWER than the version the doc
588+
// already incorporates is stale — a newer durable write already landed (possibly on another process,
589+
// out of dispatch order). Diffing toward its older markdown would regress the live doc while the
590+
// monotonic token stays high, so a later persist could write that stale content back over the durable
591+
// file. Skip it. This is what stops a delayed streaming snapshot from clobbering a newer durable merge
592+
// across processes (the per-process caller chain cannot). A merge with neither key is never stale.
593+
const orderingVersion = version ?? streamedAt
594+
const isStale = (current: number): boolean =>
595+
orderingVersion !== undefined && orderingVersion <= current
588596

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

apps/realtime/src/routes/http.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,16 +209,18 @@ 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 } = JSON.parse(body)
212+
const { fileId, markdown, version, streamedAt } = 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.
218219
const result = await applyMarkdownToLiveFileDoc(
219220
fileId,
220221
markdown,
221-
typeof version === 'number' ? version : undefined
222+
typeof version === 'number' ? version : undefined,
223+
typeof streamedAt === 'number' ? streamedAt : undefined
222224
)
223225
res.writeHead(200, { 'Content-Type': 'application/json' })
224226
res.end(JSON.stringify({ applied: result === 'applied' }))

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,13 @@ import {
1111

1212
const { mergeEditIntoLiveFileDocMock, isLiveDocMergeInFlightMock } = vi.hoisted(() => ({
1313
mergeEditIntoLiveFileDocMock:
14-
vi.fn<(fileId: string, markdown: string, version?: number) => Promise<void>>(),
14+
vi.fn<
15+
(
16+
fileId: string,
17+
markdown: string,
18+
order?: { version?: number; streamedAt?: number }
19+
) => Promise<void>
20+
>(),
1521
isLiveDocMergeInFlightMock: vi.fn<(fileId: string) => boolean>(),
1622
}))
1723

@@ -136,15 +142,17 @@ describe('processFilePreviewStreamEvent — live-doc streaming merge', () => {
136142

137143
expect(mergeEditIntoLiveFileDocMock).toHaveBeenCalledTimes(2)
138144
const [first, second] = mergeEditIntoLiveFileDocMock.mock.calls
139-
// A full-file snapshot (base + streamed), never a diff; it grows across deltas; no version arg on
140-
// either streaming merge — the durable version rides the final edit_content write.
145+
// 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.
141148
expect(first[0]).toBe('file-grow')
142-
expect(first).toHaveLength(2)
143149
expect(first[1]).toContain('Base.')
144150
expect(first[1]).toContain('Hello')
145-
expect(second).toHaveLength(2)
151+
expect(typeof first[2]?.streamedAt).toBe('number')
152+
expect(first[2]?.version).toBeUndefined()
146153
expect(second[1]).toContain('Hello world')
147154
expect(second[1].length).toBeGreaterThan(first[1].length)
155+
expect(typeof second[2]?.streamedAt).toBe('number')
148156
})
149157

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

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -654,10 +654,12 @@ export async function processFilePreviewStreamEvent(input: {
654654
// Stream the growing content into the file's LIVE collaborative Y.Doc (when a room is open)
655655
// so collaborators watching the file see the copilot write stream in via Yjs — the AI as a
656656
// CRDT peer, applied by the relay as a minimal `updateYFragment` diff. Fire-and-forget so a
657-
// slow relay never stalls the stream; `mergeEditIntoLiveFileDoc` serializes ordering per file
658-
// and treats a versionless call as a non-durable preview merge (the final `edit_content`
659-
// write carries the real version and reconciles the durable file). No-op for `create` (never
660-
// streams here) and for a file with no open room (the relay reports `applied: false`).
657+
// slow relay never stalls the stream. Pass `streamedAt` (this snapshot's wall-clock time) so
658+
// the relay orders it on the file's version line — a delayed snapshot older than a newer
659+
// durable write, even from another process, is dropped rather than regressing the doc — but
660+
// never records it as a durable checkpoint (the final `edit_content` write carries the real
661+
// version and reconciles the durable file). No-op for `create` (never streams here) and for a
662+
// file with no open room (the relay reports `applied: false`).
661663
//
662664
// Gates: markdown only (non-markdown has no collaborative room). Only `append`/`patch` stream
663665
// — they build on the existing content, so they need the base loaded (a base-less snapshot
@@ -675,7 +677,9 @@ export async function processFilePreviewStreamEvent(input: {
675677
now - currentPreview.lastLiveMergeAt >= LIVE_DOC_MERGE_THROTTLE_MS
676678
const nextLiveMergeAt = dueForLiveMerge ? now : currentPreview.lastLiveMergeAt
677679
if (dueForLiveMerge && nextSession.fileId) {
678-
void mergeEditIntoLiveFileDoc(nextSession.fileId, nextSession.previewText)
680+
void mergeEditIntoLiveFileDoc(nextSession.fileId, nextSession.previewText, {
681+
streamedAt: now,
682+
})
679683
}
680684

681685
if (

apps/sim/lib/realtime/notify.test.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,43 @@ describe('mergeEditIntoLiveFileDoc', () => {
1717
const fetchMock = vi.fn().mockResolvedValue({ ok: true })
1818
vi.stubGlobal('fetch', fetchMock)
1919

20-
await mergeEditIntoLiveFileDoc('file-1', '# hello', 42)
20+
await mergeEditIntoLiveFileDoc('file-1', '# hello', { version: 42 })
2121

2222
expect(fetchMock).toHaveBeenCalledWith(
2323
'http://realtime/api/file-doc/apply-edit',
2424
expect.objectContaining({
2525
method: 'POST',
2626
headers: expect.objectContaining({ 'x-api-key': 'secret' }),
27+
// A durable write sends `version`; the undefined `streamedAt` is dropped by JSON.stringify.
2728
body: JSON.stringify({ fileId: 'file-1', markdown: '# hello', version: 42 }),
2829
})
2930
)
3031
})
3132

33+
it('sends streamedAt (not version) for a streaming snapshot', async () => {
34+
const fetchMock = vi.fn().mockResolvedValue({ ok: true })
35+
vi.stubGlobal('fetch', fetchMock)
36+
37+
await mergeEditIntoLiveFileDoc('file-1', '# hello', { streamedAt: 1234 })
38+
39+
// The relay orders the snapshot by streamedAt without recording it — version stays absent on the wire.
40+
expect(fetchMock.mock.calls[0][1].body).toBe(
41+
JSON.stringify({ fileId: 'file-1', markdown: '# hello', streamedAt: 1234 })
42+
)
43+
})
44+
3245
it('never throws when the realtime call fails (best-effort)', async () => {
3346
vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('socket pod down')))
34-
await expect(mergeEditIntoLiveFileDoc('file-1', '# hello', 42)).resolves.toBeUndefined()
47+
await expect(
48+
mergeEditIntoLiveFileDoc('file-1', '# hello', { version: 42 })
49+
).resolves.toBeUndefined()
3550
})
3651

3752
it('never throws on a non-2xx response', async () => {
3853
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 503 }))
39-
await expect(mergeEditIntoLiveFileDoc('file-1', '# hello', 42)).resolves.toBeUndefined()
54+
await expect(
55+
mergeEditIntoLiveFileDoc('file-1', '# hello', { version: 42 })
56+
).resolves.toBeUndefined()
4057
})
4158

4259
it('reports isLiveDocMergeInFlight while a merge runs and clears when it settles', async () => {
@@ -68,7 +85,7 @@ describe('mergeEditIntoLiveFileDoc', () => {
6885

6986
const stream = mergeEditIntoLiveFileDoc('file-durable', 'partial') // versionless, in flight
7087
await Promise.resolve()
71-
const durable = mergeEditIntoLiveFileDoc('file-durable', 'final content', 100) // versioned
88+
const durable = mergeEditIntoLiveFileDoc('file-durable', 'final content', { version: 100 }) // versioned
7289
await Promise.resolve()
7390
await Promise.resolve()
7491

@@ -107,8 +124,8 @@ describe('mergeEditIntoLiveFileDoc', () => {
107124
await flush()
108125
// Two durable writes arrive while the streaming merge is in flight — both must chain, not both
109126
// resume-and-fire concurrently.
110-
const a = mergeEditIntoLiveFileDoc('file-order', 'a', 1)
111-
const b = mergeEditIntoLiveFileDoc('file-order', 'b', 2)
127+
const a = mergeEditIntoLiveFileDoc('file-order', 'a', { version: 1 })
128+
const b = mergeEditIntoLiveFileDoc('file-order', 'b', { version: 2 })
112129
await flush()
113130
expect(applied).toEqual(['stream']) // A and B queued behind streaming
114131

apps/sim/lib/realtime/notify.ts

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -125,26 +125,40 @@ export async function notifyFolderResourceChanged(
125125
* streaming caller fires and forgets it. Bounded to {@link APPLY_EDIT_TIMEOUT_MS}, so it adds latency
126126
* only when the socket pod is unreachable.
127127
*
128-
* `version` is the durable `contentUpdatedAt` (epoch ms) this markdown was written with, for a durable
129-
* write. Omit it for a STREAMING intermediate merge (the copilot stream mid-flight): intermediate
130-
* content advances the live doc for viewers but is not a durable checkpoint, so the relay leaves its
131-
* synced version pinned to the last durable write — which is exactly the copilot tool's final
132-
* `edit_content` write, carrying the real version, that reconciles the durable file.
128+
* `order` positions this merge on the file's monotonic version line so a stale write never regresses
129+
* the doc — the relay drops any merge not NEWER than the version the doc already incorporates:
130+
* - `version` — a DURABLE write's `contentUpdatedAt` (epoch ms). Both orders the merge AND is recorded
131+
* as the doc's synced version (the persist If-Match guard), so a later persist treats this write as
132+
* synced rather than an out-of-band conflict.
133+
* - `streamedAt` — the wall-clock time (epoch ms) a STREAMING snapshot was produced (the copilot stream
134+
* mid-flight). Orders the merge so a delayed snapshot older than a newer durable write — possibly from
135+
* another app process — is dropped, but is NEVER recorded: the synced version stays pinned to the last
136+
* durable write, which is exactly the copilot tool's final `edit_content` write that reconciles the file.
133137
*
134-
* Merges for a file run on a single serialized chain: each is chained after the current tail and
135-
* applies strictly after it, so ordering can never regress the doc — a DURABLE (versioned) write
136-
* always applies after any in-flight streaming merge AND after every earlier durable write, never
137-
* concurrently. The final durable write is therefore always the last merge applied and cannot be
138-
* clobbered by a late straggler. The copilot streaming caller uses {@link isLiveDocMergeInFlight} to
139-
* skip redundant snapshots while one is in flight, so a slow relay can't backlog stale snapshots.
138+
* Pass one or the other, never both. Passing neither applies the merge without ordering it (legacy).
139+
*
140+
* Ordering is enforced at two scales. Within this process, merges for a file run on a single serialized
141+
* chain — each chained after the current tail — so a durable write applies after any in-flight streaming
142+
* merge and after every earlier durable write, never concurrently. Across processes, the per-process
143+
* chain does not apply, so the relay orders merges by the monotonic version above (durable version /
144+
* streaming `streamedAt`) under a cluster-wide lock. The copilot streaming caller uses
145+
* {@link isLiveDocMergeInFlight} to skip redundant snapshots while one is in flight, so a slow relay
146+
* can't backlog stale snapshots.
140147
*/
148+
export interface LiveFileDocMergeOrder {
149+
/** A durable write's `contentUpdatedAt` (epoch ms): orders the merge AND is recorded as the synced version. */
150+
version?: number
151+
/** A streaming snapshot's production time (epoch ms): orders the merge only — never recorded as a checkpoint. */
152+
streamedAt?: number
153+
}
154+
141155
export async function mergeEditIntoLiveFileDoc(
142156
fileId: string,
143157
markdown: string,
144-
version?: number
158+
order: LiveFileDocMergeOrder = {}
145159
): Promise<void> {
146160
const tail = liveDocMergeChain.get(fileId) ?? Promise.resolve()
147-
const run = tail.then(() => applyLiveFileDocMerge(fileId, markdown, version))
161+
const run = tail.then(() => applyLiveFileDocMerge(fileId, markdown, order))
148162
liveDocMergeChain.set(fileId, run)
149163
try {
150164
await run
@@ -171,15 +185,21 @@ export function isLiveDocMergeInFlight(fileId: string): boolean {
171185
async function applyLiveFileDocMerge(
172186
fileId: string,
173187
markdown: string,
174-
version?: number
188+
order: LiveFileDocMergeOrder
175189
): Promise<void> {
176190
try {
177191
const response = await fetch(`${getSocketServerUrl()}/api/file-doc/apply-edit`, {
178192
method: 'POST',
179193
headers: { 'Content-Type': 'application/json', 'x-api-key': env.INTERNAL_API_SECRET },
180-
// A durable `version` (the durable `updatedAt` epoch ms) records the version the live doc now
181-
// incorporates (the persist If-Match guard); omitted for a streaming intermediate merge.
182-
body: JSON.stringify({ fileId, markdown, version }),
194+
// `version` (durable `contentUpdatedAt`) records the synced version the live doc now incorporates
195+
// (the persist If-Match guard); `streamedAt` orders a streaming snapshot without recording it.
196+
// JSON.stringify drops whichever is undefined, so the wire shape is unchanged for durable writes.
197+
body: JSON.stringify({
198+
fileId,
199+
markdown,
200+
version: order.version,
201+
streamedAt: order.streamedAt,
202+
}),
183203
signal: AbortSignal.timeout(APPLY_EDIT_TIMEOUT_MS),
184204
})
185205
if (!response.ok) {

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,11 +1223,9 @@ export async function updateWorkspaceFileContent(
12231223
// incorporates this durable version — the collab persist's optimistic-concurrency guard then won't
12241224
// treat this (already-merged) write as an out-of-band conflict. Must be the SAME field the CAS
12251225
// guards on (`contentUpdatedAt`), not `updatedAt`, or the relay's token wouldn't match the CAS.
1226-
await mergeEditIntoLiveFileDoc(
1227-
fileId,
1228-
content.toString('utf-8'),
1229-
finalized.file.contentUpdatedAt.getTime()
1230-
)
1226+
await mergeEditIntoLiveFileDoc(fileId, content.toString('utf-8'), {
1227+
version: finalized.file.contentUpdatedAt.getTime(),
1228+
})
12311229
}
12321230

12331231
const pathPrefix = getServePathPrefix()

0 commit comments

Comments
 (0)