From 47afd85bccec44654beae5e3924f0b3de4ae75d1 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 19:01:44 +0200 Subject: [PATCH] fix: never write from a read another editor's row landed during Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/itemStore.svelte.ts | 9 ++++++++- frontend/src/lib/itemStore.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index bc2652bab1..d08e1499e1 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -283,6 +283,9 @@ class Entry { reconcile(): void { const key = this.key if (this.retired || !this.loaded || this.origin === 'new' || isTemporaryPath(key.path)) return + // Known to be behind the server: writing from here would put that back over whatever is + // actually there. A reload is what clears it. + if (this.stale) return const desired = this.dirty ? (serialize(this.value) ?? null) : null this.ports.hint(key, desired !== null) if (desired === this.row) return @@ -411,6 +414,11 @@ class Entry { // nothing can send it, so a read nobody asked for must not put the server's value in // its place, nor answer the conflict on the user's behalf. Checked here rather than // before the read, because the flush that precedes one is itself a way to find out. + // A row was handed over while this read was out, so its response predates it. The value + // is still shown, for want of a better one, but writing from it would post it back + // over that row on the baseline the row advanced: the newer write lost to an older + // read. Nothing goes out until a reload settles which is which. + this.stale = this.ports.rowMark(key) !== rowsAtRead const standOff = unasked && this.ports.conflicted(key) // Whatever landed since the read was asked for — an external write, an edit — is // newer than what it read, so the read only moves the deployed side under it. @@ -418,7 +426,6 @@ class Entry { standOff || this.edits !== at.edits || this.externals !== at.externals this.meta = res.meta this.error = undefined - this.stale = false if (isTemporaryPath(key.path)) { this.template = snapshot(res.template) this.origin = 'new' diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index dca6531139..ff67705878 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -843,6 +843,33 @@ describe('item store: origins', () => { expect(item.value?.description).toBe('typed during the deploy') }) + it('does not post a read back over a row another editor landed during it', async () => { + const rows = fakeRows() + const read = deferred>() + const store = createItemStore(rows.port) + const older = { ...deployedRes, description: 'draft A, as the GET found it' } + const { handle: item } = store.acquire( + { workspace: 'w', kind: 'resource', path: 'u/me/r' }, + { workspace: 'w', path: 'u/me/r' }, + adapter(() => read.promise) + ) + await settle() + // Another editor of the same item saves draft B through the syncer directly and it lands, + // leaving nothing pending. Only the row mark records that it happened. + rows.handExternally('u/me/r') + const writesBefore = rows.writes.length + read.resolve({ deployed: deployedRes, draft: older, draftSavedAt: 'T0' }) + await settle() + + // Posting A now would overwrite B, on the baseline B itself advanced. + expect(rows.writes.slice(writesBefore)).toEqual([]) + expect(rows.seeds).toEqual([]) + // And nothing may be written from this view until it is reloaded. + expect(item.canSave).toBe(false) + item.value = { ...deployedRes, description: 'typed on a stale view' } + expect(rows.writes.slice(writesBefore)).toEqual([]) + }) + it('tells a caller its post-deploy re-read failed, apart from never having the item', async () => { const rows = fakeRows() const store = createItemStore(rows.port)