Skip to content

Commit 6986b74

Browse files
committed
refactor(collab-doc): tidy merge-order docs + relay order object; cover multi-replica streaming stale-check
1 parent e9622e9 commit 6986b74

5 files changed

Lines changed: 146 additions & 43 deletions

File tree

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Multi-replica (store-enabled) coverage for the copilot live-merge stale-check. The main
5+
* `file-doc.test.ts` runs with the store DISABLED (single-replica fallback); this file mocks an ENABLED
6+
* store so the cross-process branch of `mergeMarkdownIntoRoom` — staleness against the SHARED synced
7+
* version under the merge lock, and `recordVersion` writing `setSyncedVersion` — is exercised directly.
8+
* The enabled merge path reads its base from the shared store (not an in-memory room), so no JOIN/seed
9+
* is needed: calling `applyMarkdownToLiveFileDoc` against the fake store drives the branch on its own.
10+
*/
11+
import { beforeEach, describe, expect, it, vi } from 'vitest'
12+
import * as Y from 'yjs'
13+
14+
const { mockFetchFileDocMerge } = vi.hoisted(() => ({
15+
mockFetchFileDocMerge: vi.fn(),
16+
}))
17+
18+
/**
19+
* A minimal ENABLED store: in-memory monotonic synced version (mirrors SET_VERSION_IF_NEWER_SCRIPT), a
20+
* non-null stream state so the merge has a base, and no-op locks/publish. Only the surface the
21+
* store-enabled merge path touches is implemented.
22+
*/
23+
const fakeStore = {
24+
enabled: true,
25+
versions: new Map<string, number>(),
26+
acquireMergeSlot: vi.fn(async () => 'token'),
27+
releaseMergeSlot: vi.fn(async () => {}),
28+
getStreamState: vi.fn(async () => new Uint8Array([1])),
29+
publishAndWait: vi.fn(async () => {}),
30+
getSyncedVersion: vi.fn(async (name: string) => fakeStore.versions.get(name) ?? null),
31+
setSyncedVersion: vi.fn(async (name: string, version: number) => {
32+
fakeStore.versions.set(name, Math.max(fakeStore.versions.get(name) ?? 0, version))
33+
}),
34+
}
35+
36+
vi.mock('@sim/platform-authz/rooms', () => ({ authorizeRoom: vi.fn() }))
37+
38+
vi.mock('@/handlers/file-doc-app', () => ({
39+
fetchFileDocSeed: vi.fn(),
40+
fetchFileDocMerge: mockFetchFileDocMerge,
41+
fetchFileDocPersist: vi.fn(),
42+
}))
43+
44+
vi.mock('@/handlers/file-doc-store', () => ({
45+
getFileDocStore: () => fakeStore,
46+
REDIS_ORIGIN: Symbol('redis'),
47+
REDIS_SNAPSHOT_ORIGIN: Symbol('redis-snapshot'),
48+
}))
49+
50+
import { applyMarkdownToLiveFileDoc } from '@/handlers/file-doc'
51+
52+
const ROOM_NAME = 'workspace-file-doc:file-1'
53+
54+
describe('applyMarkdownToLiveFileDoc — multi-replica (store-enabled) ordering', () => {
55+
beforeEach(() => {
56+
vi.clearAllMocks()
57+
fakeStore.versions.clear()
58+
fakeStore.acquireMergeSlot.mockResolvedValue('token')
59+
fakeStore.getStreamState.mockResolvedValue(new Uint8Array([1]))
60+
mockFetchFileDocMerge.mockResolvedValue(Y.encodeStateAsUpdate(new Y.Doc()))
61+
})
62+
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.
65+
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable', { version: 100 })).toBe(
66+
'applied'
67+
)
68+
expect(fakeStore.setSyncedVersion).toHaveBeenCalledWith(ROOM_NAME, 100)
69+
mockFetchFileDocMerge.mockClear()
70+
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+
)
76+
expect(mockFetchFileDocMerge).not.toHaveBeenCalled()
77+
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.
84+
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable again', { version: 150 })).toBe(
85+
'applied'
86+
)
87+
expect(fakeStore.setSyncedVersion).toHaveBeenCalledWith(ROOM_NAME, 150)
88+
// setSyncedVersion fired only for the two durable writes, never for a streaming snapshot.
89+
expect(fakeStore.setSyncedVersion).toHaveBeenCalledTimes(2)
90+
})
91+
})

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -553,15 +553,19 @@ describe('setupWorkspaceFileDocHandlers', () => {
553553
mockFetchFileDocMerge.mockResolvedValue(Y.encodeStateAsUpdate(new Y.Doc()))
554554

555555
// A newer durable version lands and is recorded as the synced version.
556-
expect(await applyMarkdownToLiveFileDoc('file-1', '# newer', 100)).toBe('applied')
556+
expect(await applyMarkdownToLiveFileDoc('file-1', '# newer', { version: 100 })).toBe('applied')
557557
mockFetchFileDocMerge.mockClear()
558558

559559
// An older durable version arriving out of order (e.g. a concurrent write on another process) is
560560
// stale: skipped before any diff is computed, so the live doc never regresses to older content and
561561
// no diff is published that a later persist could write back.
562-
expect(await applyMarkdownToLiveFileDoc('file-1', '# older, stale', 50)).toBe('stale')
562+
expect(await applyMarkdownToLiveFileDoc('file-1', '# older, stale', { version: 50 })).toBe(
563+
'stale'
564+
)
563565
// The same version is idempotent — also skipped.
564-
expect(await applyMarkdownToLiveFileDoc('file-1', '# same version', 100)).toBe('stale')
566+
expect(await applyMarkdownToLiveFileDoc('file-1', '# same version', { version: 100 })).toBe(
567+
'stale'
568+
)
565569
expect(mockFetchFileDocMerge).not.toHaveBeenCalled()
566570
})
567571

@@ -575,22 +579,25 @@ describe('setupWorkspaceFileDocHandlers', () => {
575579
mockFetchFileDocMerge.mockResolvedValue(Y.encodeStateAsUpdate(new Y.Doc()))
576580

577581
// A newer durable version lands and is recorded as the synced version.
578-
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable', 100)).toBe('applied')
582+
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable', { version: 100 })).toBe(
583+
'applied'
584+
)
579585

580586
// A delayed streaming snapshot OLDER than that durable version — e.g. a throttled copilot snapshot
581587
// 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(
588+
expect(await applyMarkdownToLiveFileDoc('file-1', '# older stream', { streamedAt: 50 })).toBe(
584589
'stale'
585590
)
586591

587592
// 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(
593+
expect(await applyMarkdownToLiveFileDoc('file-1', '# newer stream', { streamedAt: 200 })).toBe(
589594
'applied'
590595
)
591596
// ...but it is never RECORDED as the synced version: a durable write between the two (150) still
592597
// 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')
598+
expect(await applyMarkdownToLiveFileDoc('file-1', '# durable again', { version: 150 })).toBe(
599+
'applied'
600+
)
594601
})
595602

596603
it('serializes concurrent merges for the same file (second waits for the first)', async () => {

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

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,15 @@ function emptySeedUpdate(): Uint8Array {
515515
/** Serializes live merges per file so overlapping calls never race the same doc (see below). */
516516
const fileDocMergeChains = new Map<string, Promise<unknown>>()
517517

518+
/**
519+
* 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.
521+
*/
522+
interface MergeOrder {
523+
version?: number
524+
streamedAt?: number
525+
}
526+
518527
/**
519528
* Apply new markdown into a file's LIVE collaborative document (Stage C — copilot writing into an open
520529
* doc). Ships the document's current state to the app to build a minimal Yjs diff, applies it — which
@@ -540,15 +549,12 @@ const fileDocMergeChains = new Map<string, Promise<unknown>>()
540549
export function applyMarkdownToLiveFileDoc(
541550
fileId: string,
542551
markdown: string,
543-
version?: number,
544-
streamedAt?: number
552+
order: MergeOrder = {}
545553
): Promise<'applied' | 'no-live-room' | 'merge-unavailable' | 'stale'> {
546554
const name = roomName(fileDocRoom(fileId))
547555
const prior = fileDocMergeChains.get(name) ?? Promise.resolve()
548556
// `.catch` so a failed prior merge doesn't reject this one — each merge is independent.
549-
const run = prior
550-
.catch(() => {})
551-
.then(() => mergeMarkdownIntoRoom(name, fileId, markdown, version, streamedAt))
557+
const run = prior.catch(() => {}).then(() => mergeMarkdownIntoRoom(name, fileId, markdown, order))
552558
fileDocMergeChains.set(
553559
name,
554560
run.finally(() => {
@@ -562,8 +568,7 @@ async function mergeMarkdownIntoRoom(
562568
name: string,
563569
fileId: string,
564570
markdown: string,
565-
version?: number,
566-
streamedAt?: number
571+
{ version, streamedAt }: MergeOrder
567572
): Promise<'applied' | 'no-live-room' | 'merge-unavailable' | 'stale'> {
568573
const store = getFileDocStore()
569574

@@ -590,6 +595,11 @@ async function mergeMarkdownIntoRoom(
590595
// monotonic token stays high, so a later persist could write that stale content back over the durable
591596
// file. Skip it. This is what stops a delayed streaming snapshot from clobbering a newer durable merge
592597
// 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.
593603
const orderingVersion = version ?? streamedAt
594604
const isStale = (current: number): boolean =>
595605
orderingVersion !== undefined && orderingVersion <= current

apps/realtime/src/routes/http.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,12 +216,10 @@ export function createHttpHandler(roomManager: IRoomManager, logger: Logger) {
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.
218218
// `streamedAt` orders a streaming snapshot on the same version line without recording it.
219-
const result = await applyMarkdownToLiveFileDoc(
220-
fileId,
221-
markdown,
222-
typeof version === 'number' ? version : undefined,
223-
typeof streamedAt === 'number' ? streamedAt : undefined
224-
)
219+
const result = await applyMarkdownToLiveFileDoc(fileId, markdown, {
220+
version: typeof version === 'number' ? version : undefined,
221+
streamedAt: typeof streamedAt === 'number' ? streamedAt : undefined,
222+
})
225223
res.writeHead(200, { 'Content-Type': 'application/json' })
226224
res.end(JSON.stringify({ applied: result === 'applied' }))
227225
} catch (error) {

apps/sim/lib/realtime/notify.ts

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,17 @@ export async function notifyFolderResourceChanged(
109109
await FOLDER_RESOURCE_NOTIFIERS[resourceType]?.(workspaceId)
110110
}
111111

112+
/**
113+
* How a live-doc merge is positioned on the file's monotonic version line. Pass one key or the other,
114+
* never both; passing neither applies the merge without ordering it (legacy).
115+
*/
116+
export interface LiveFileDocMergeOrder {
117+
/** A durable write's `contentUpdatedAt` (epoch ms): orders the merge AND is recorded as the synced version. */
118+
version?: number
119+
/** A streaming snapshot's production time (epoch ms): orders the merge only — never recorded as a checkpoint. */
120+
streamedAt?: number
121+
}
122+
112123
/**
113124
* Best-effort: ask the realtime relay to merge a copilot edit into a file's LIVE collaborative
114125
* document, so open editors see it stream in as a CRDT merge (Stage C) rather than the file changing
@@ -125,33 +136,19 @@ export async function notifyFolderResourceChanged(
125136
* streaming caller fires and forgets it. Bounded to {@link APPLY_EDIT_TIMEOUT_MS}, so it adds latency
126137
* only when the socket pod is unreachable.
127138
*
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.
137-
*
138-
* Pass one or the other, never both. Passing neither applies the merge without ordering it (legacy).
139+
* `order` ({@link LiveFileDocMergeOrder}) positions this merge so a stale write never regresses the doc:
140+
* the relay drops any merge not NEWER than the version the doc already incorporates. A durable `version`
141+
* is recorded as the synced version (the persist If-Match guard); a streaming `streamedAt` is checked but
142+
* never recorded, so the synced version stays pinned to the last durable write — which the copilot tool's
143+
* final `edit_content` write carries, reconciling the durable file.
139144
*
140145
* Ordering is enforced at two scales. Within this process, merges for a file run on a single serialized
141146
* chain — each chained after the current tail — so a durable write applies after any in-flight streaming
142147
* 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.
148+
* chain does not apply, so the relay orders merges by that monotonic version under a cluster-wide lock.
149+
* The copilot streaming caller uses {@link isLiveDocMergeInFlight} to skip redundant snapshots while one
150+
* is in flight, so a slow relay can't backlog stale snapshots.
147151
*/
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-
155152
export async function mergeEditIntoLiveFileDoc(
156153
fileId: string,
157154
markdown: string,

0 commit comments

Comments
 (0)