From 860b63b2ea10f9c3d7150e71e91076e392c5fd2b Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 20:23:09 +0200 Subject: [PATCH] fix: give a baseline-less payload a baseline before letting it go out Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/itemStore.svelte.ts | 10 ++++- frontend/src/lib/itemStore.test.ts | 41 ++++++++++++++++++++ frontend/src/lib/userDraftDbSyncer.svelte.ts | 10 +++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index 18c46a698c..e82f8e454d 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -95,6 +95,8 @@ export type ItemRowPort = { rowMark(key: ItemKey): number /** A row waiting for another attempt, if any: `undefined` when none, `null` for a delete. */ pending(key: ItemKey): unknown | undefined + /** Whether a row is waiting with no baseline behind it, so sending it would be unconditional. */ + unbased(key: ItemKey): boolean seedSync(key: ItemKey, draftSavedAt: string | undefined, since: number): void conflicted(key: ItemKey): boolean failure(key: ItemKey): string | undefined @@ -407,7 +409,10 @@ class Entry { if (!adapter.load) throw new Error('This item cannot be loaded') // A row queued by whoever held this key before is debounced: send it, or this read // answers with the deployed value and the draft reappears under a clean editor. - await this.ports.flush(key) + // Not one written when no row existed, though — it carries no baseline, so it + // would go out unconditional and take over a row created since. The read below + // gives it one, and `pending` keeps showing it meanwhile. + if (!this.ports.unbased(key)) await this.ports.flush(key) // Taken here, not when the command was asked for: rows this read's own flush sent // are in the response it is about to get. Only one handed over from now on, while // the read is out, leaves that response behind. @@ -1272,6 +1277,9 @@ const syncerRows: ItemRowPort = { pending(key) { return UserDraftDbSyncer.pendingValue(keyQuery(key)) }, + unbased(key) { + return UserDraftDbSyncer.hasUnbasedPending(keyQuery(key)) + }, seedSync(key, draftSavedAt, since) { UserDraftDbSyncer.recordRemoteSync(keyQuery(key), draftSavedAt, since) }, diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index 4d54fbb126..a223f335e7 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -37,6 +37,8 @@ function fakeRows() { const dropped: string[] = [] /** Every `last_sync` baseline the syncer accepted, in order. */ const seeds: { path: string; at: string | undefined }[] = [] + /** Paths the syncer holds a `last_sync` for. Without one a send is unconditional. */ + const baselines = new Set() /** Rows handed over per path, by any writer — the store's reconcile or the chat's own save. */ const sends = new Map() const handed = (path: string) => sends.set(path, (sends.get(path) ?? 0) + 1) @@ -89,10 +91,13 @@ function fakeRows() { }, rowMark: (key) => sends.get(key.path) ?? 0, pending: (key) => (parked.has(key.path) ? parked.get(key.path) : undefined), + unbased: (key) => parked.has(key.path) && !baselines.has(key.path), seedSync: (key, at, since) => { // The syncer refuses a baseline from a read a row handed over since has passed. if (since !== (sends.get(key.path) ?? 0)) return seeds.push({ path: key.path, at }) + if (at !== undefined) baselines.add(key.path) + else baselines.delete(key.path) // Back in sync with the server, as `recordRemoteSync` does. The baseline is fresh, so // a payload the server was refusing would be accepted from here on. conflicts.delete(key.path) @@ -1744,6 +1749,42 @@ describe('item store: conflicts', () => { expect(open.deployed).toEqual({ ...b, args: { a: 2 } }) }) + it('does not replay a first failed autosave over a draft created since', async () => { + const rows = fakeRows() + const store = createItemStore(rows.port) + const key: ItemKey = { workspace: 'w', kind: 'resource', path: 'u/me/r' } + // No draft exists, so nothing gives this key a baseline. + const a = adapter(async () => ({ + deployed: deployedRes, + draft: rows.sent.get('u/me/r') as Res | undefined + })) + const first = store.acquire(key, { workspace: 'w', path: 'u/me/r' }, a) + await settle() + rows.failing.add('u/me/r') + first.handle.value = { ...deployedRes, description: 'tab A, never sent' } + await rows.port.flush(key) + first.release() + + // Another tab creates the draft while tab A is away. + const theirs = { ...deployedRes, description: 'tab B' } + rows.sent.set('u/me/r', theirs) + rows.handExternally('u/me/r') + // Tab A's network recovers before it reopens. + rows.failing.delete('u/me/r') + + const second = store.acquire(key, { workspace: 'w', path: 'u/me/r' }, a) + await settle() + + // The parked payload has no `last_sync`, so replaying it before the read would go out + // unconditional and take their draft with it — and the read after would see only the + // overwrite, never a conflict. + expect(rows.sent.get('u/me/r')).toEqual(theirs) + // Tab A's edit is not lost either: it is what the reopened editor shows, and the read has + // given the key a baseline, so when that payload does go out it is checked against theirs. + expect(second.handle.value?.description).toBe('tab A, never sent') + expect(rows.seeds.at(-1)?.path).toBe('u/me/r') + }) + it('keeps an edit the server never received when the item is reopened', async () => { const rows = fakeRows() const store = createItemStore(rows.port) diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts index 337f894dc6..2d22fab063 100644 --- a/frontend/src/lib/userDraftDbSyncer.svelte.ts +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -543,6 +543,16 @@ export const UserDraftDbSyncer = { return parked ? parked.value : undefined }, + /** + * Whether a payload is parked for `query` with no `last_sync` behind it — it was written when + * no row existed, so sending it now would be unconditional and would take over a row created + * since. A reader must get a baseline before letting one of these go out. + */ + hasUnbasedPending(query: UserDraftLastSyncQuery): boolean { + const key = draftKey(query.workspace, query.itemKind, query.path) + return pendingSaveOpts.has(key) && !lastSyncMap.has(key) + }, + /** Rows handed over for `query` so far, for a reader to hand back to `recordRemoteSync`. */ sendsSoFar(query: UserDraftLastSyncQuery): number { return sends.get(draftKey(query.workspace, query.itemKind, query.path)) ?? 0