Skip to content

Commit b528509

Browse files
committed
fix(collab-doc): only persist a doc a user actually edited
Cursor review: a copilot durable write landing while a doc is being seeded could have the stale seed projected back over it on last-disconnect, clobbering the copilot edit even with no user changes. Gate server-side persistence on a genuine user edit (socket-origin update): a seed-only or merge-only doc is never projected back to the file (copilot writes the file durably itself), so it can't clobber a concurrent external write.
1 parent 161b031 commit b528509

2 files changed

Lines changed: 53 additions & 4 deletions

File tree

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,42 @@ describe('setupWorkspaceFileDocHandlers', () => {
260260
)
261261
})
262262

263+
it('does NOT persist a seeded-but-unedited doc on last disconnect (no clobber of a concurrent write)', async () => {
264+
mockFetchFileDocSeed.mockResolvedValue(encodedSeedUpdate('# From server'))
265+
const { io } = createIo()
266+
const { handlers } = setup('socket-1', io)
267+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
268+
await flushMicrotasks() // let the seed apply
269+
270+
// Last collaborator leaves without ever editing — projecting this seed back over the file could
271+
// clobber a concurrent copilot write, so the final flush must NOT persist.
272+
cleanupFileDocForSocket('socket-1', io, true)
273+
await flushMicrotasks()
274+
expect(mockFetchFileDocPersist).not.toHaveBeenCalled()
275+
})
276+
277+
it('persists on last disconnect once a genuine user edit has landed', async () => {
278+
mockFetchFileDocSeed.mockResolvedValue(encodedSeedUpdate('# From server'))
279+
const { io } = createIo()
280+
const { handlers } = setup('socket-1', io)
281+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
282+
await flushMicrotasks()
283+
284+
// A real user edit (socket-origin sync update) marks the doc dirty.
285+
const edit = new Y.Doc()
286+
edit.getText(FILE_DOC_FIELD).insert(0, 'user typed this')
287+
handlers[FILE_DOC_EVENTS.MESSAGE](
288+
frame(FILE_DOC_MESSAGE_TYPE.SYNC, (e) =>
289+
syncProtocol.writeUpdate(e, Y.encodeStateAsUpdate(edit))
290+
)
291+
)
292+
await flushMicrotasks()
293+
294+
cleanupFileDocForSocket('socket-1', io, true)
295+
await flushMicrotasks()
296+
expect(mockFetchFileDocPersist).toHaveBeenCalled()
297+
})
298+
263299
it('joins the room, sends sync step 1, and seeds the document from the server', async () => {
264300
mockFetchFileDocSeed.mockResolvedValue(encodedSeedUpdate('# From server'))
265301
const { io } = createIo()

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ interface FileDocRoom {
105105
workspaceId: string | null
106106
/** The last collaborator to edit here, for persist attribution (blob metadata) only. */
107107
lastEditorUserId: string | null
108+
/**
109+
* True once a genuine USER edit has been applied here. Persistence is gated on it so a doc that was
110+
* only seeded (or only received a copilot merge) is NEVER projected back over the file: copilot writes
111+
* the file durably itself, and a seed captured from possibly-stale markdown must not clobber a
112+
* concurrent external write.
113+
*/
114+
edited: boolean
108115
/** The pending debounced persist timer, if any. */
109116
persistTimer: ReturnType<typeof setTimeout> | null
110117
}
@@ -200,7 +207,8 @@ function schedulePersist(name: string, room: FileDocRoom): void {
200207
* caller destroys `room.doc` never encodes a destroyed doc, and the disabled path stays authoritative.
201208
*/
202209
async function flushPersist(name: string, room: FileDocRoom, final: boolean): Promise<void> {
203-
if (!room.workspaceId || !room.lastEditorUserId) return
210+
// Never project a doc no user actually edited back over the file (see {@link FileDocRoom.edited}).
211+
if (!room.edited || !room.workspaceId || !room.lastEditorUserId) return
204212
const store = getFileDocStore()
205213
// Synchronous fallback capture — before any await, since the caller may destroy `room.doc` the moment
206214
// this yields. Only meaningful once seeded; used only when the authoritative stream state is absent.
@@ -456,6 +464,7 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom {
456464
serverSeedStarted: false,
457465
workspaceId: null,
458466
lastEditorUserId: null,
467+
edited: false,
459468
persistTimer: null,
460469
}
461470
// Register synchronously BEFORE the async catch-up so a concurrent join sees this room, not a second.
@@ -472,9 +481,13 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom {
472481
// in the stream) and SEED_ORIGIN — the seed is published EXPLICITLY and AWAITED under the seed lock
473482
// (so it lands before the lock releases), which a fire-and-forget publish here couldn't guarantee.
474483
if (origin !== REDIS_ORIGIN && origin !== SEED_ORIGIN) getFileDocStore().publish(name, update)
475-
// Persist real edits (user edits + copilot merges) back to markdown, debounced. Skip the seed (it
476-
// is the file's current content) and stream-relayed updates (their originating task persists them).
477-
if (origin !== REDIS_ORIGIN && origin !== SEED_ORIGIN) schedulePersist(name, room)
484+
// Persist ONLY genuine USER edits (socket origin), debounced. A seed or a bare copilot merge must
485+
// NOT project back over the file — copilot writes the file durably itself, so persisting a
486+
// seeded-but-unedited doc (built from possibly-stale markdown) could clobber that concurrent write.
487+
if (originSocketId(origin)) {
488+
room.edited = true
489+
schedulePersist(name, room)
490+
}
478491
})
479492

480493
awareness.on('update', ({ added, updated, removed }: AwarenessChange, origin: unknown) => {

0 commit comments

Comments
 (0)