From e65929665c7326e3497ceb8fe6c7876f31e18a7e Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Sun, 6 Sep 2026 19:16:36 +0200 Subject: [PATCH] fix(drafts): stop a settled save from swallowing the next edit, and scope its retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settling a save that changed nothing hands the cell a value it already holds, and the mirror's no-op branch defused the seed guard but not the one a discard arms — so the guard stayed up and ate the user's next keystroke, which then reached neither the server nor the unload flush. It is defused in the same place and for the same reason as the other. Two more from the same helper: a released handle is no cell at all rather than a newer edit, so a save whose drawer has closed still cleans up after itself; and the delete is only re-sent for an item that moved, since on an unchanged path the write queued behind it is the user's and resending would revert it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4 --- frontend/src/lib/userDraft.svelte.ts | 23 +++++++++++++------ .../src/lib/userDraftSettleAfterWrite.test.ts | 16 +++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index 17ae1d0d74..8c681592d7 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -285,16 +285,21 @@ export async function settleDraftAfterWrite( savedPath: string, opts?: UserDraftOptions ): Promise { - if (savedPath === fromPath && !draftValuesEqual(live, written)) return + // No cell at all is not a newer edit — the editor has been released, and its + // draft is precisely what nothing is watching any more. + const diverged = live !== undefined && !draftValuesEqual(live, written) + if (savedPath === fromPath && diverged) return // Sent before this resolves, not left on the keystroke debounce: callers report // the write and remount on it, and both read a cell this has to have finished // resolving — including through a frame with a draft store of its own. - // - // Re-sent while the key is not settled on that delete: the form stays editable - // across the request, and an edit made then parks a write behind it that would - // put a draft back — under a path the item has left, once the callers re-key. - // Bounded, because a form still being typed into can always add one more. - for (let attempt = 0; attempt < 3; attempt++) { + UserDraft.discard(itemKind, fromPath, written, opts) + if (await flushDraftDelete(itemKind, fromPath, opts)) return + // Only a moved item forces the point. The form stays editable across that + // request, so an edit made then parks a write behind it: under the path the + // item has left that write would strand a draft no editor is on, but under an + // unchanged path it is the user's, and is meant to stand and read dirty. + if (savedPath === fromPath) return + for (let attempt = 0; attempt < 2; attempt++) { UserDraft.discard(itemKind, fromPath, written, opts) if (await flushDraftDelete(itemKind, fromPath, opts)) return } @@ -958,6 +963,10 @@ function acquireEntry( // stays armed: an undefined-seeded cell's initial run lands // here, and page editors rely on it to swallow their load write.) if (entry.seedNextWrite) entry.seedNextWrite = false + // Same for a `discard`/`remove` whose fallback equals what the cell + // already holds — settling a save that changed nothing does exactly + // that — which otherwise leaves the guard armed for the next edit. + if (entry.skipNextSync) entry.skipNextSync = false return } lastSerialized = next diff --git a/frontend/src/lib/userDraftSettleAfterWrite.test.ts b/frontend/src/lib/userDraftSettleAfterWrite.test.ts index 7c3407e492..cc58f25e79 100644 --- a/frontend/src/lib/userDraftSettleAfterWrite.test.ts +++ b/frontend/src/lib/userDraftSettleAfterWrite.test.ts @@ -75,6 +75,22 @@ describe('settleDraftAfterWrite', () => { expect(discard).toHaveBeenCalledTimes(3) }) + // A released handle reads as no cell at all. That is not somebody's newer edit — + // it is a draft nothing is watching — so the cleanup still has to run. + it('cleans up when the editor has been released', async () => { + await settleDraftAfterWrite('variable', sent, undefined, 'u/me/a', 'u/me/a', OPTS) + expect(discard).toHaveBeenCalledWith('variable', 'u/me/a', sent, OPTS) + }) + + // Retrying resets the cell to what was written, so it may only chase a path the + // item has left. On an unchanged path an edit made during the delete is the + // user's, and re-sending would revert it. + it('does not retry over an edit made on an unchanged path', async () => { + settles = false + await settleDraftAfterWrite('variable', sent, { ...sent }, 'u/me/a', 'u/me/a', OPTS) + expect(discard).toHaveBeenCalledTimes(1) + }) + it('sends it once when the key settles', async () => { await settleDraftAfterWrite('variable', sent, { ...sent }, 'u/me/a', 'u/me/b', OPTS) expect(discard).toHaveBeenCalledTimes(1)