From 9d7b06ecc47a5174ec53251b360cf5e3104acad7 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Sun, 6 Sep 2026 05:39:17 +0200 Subject: [PATCH] fix(drafts): stop the landed-delete fact from outliving the server state it described MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fact is sticky by design, so a delete that landed earlier for a key kept answering after the server moved out from under it: a discard that conflicted left it standing, and the conflict itself is tracked apart from the state the verdict reads, so both said the draft was gone. It is dropped now wherever the server stops being ours to describe — on a conflict, and on a remote resync. Also corrects what the partial-save comments claim. Following the committed rename re-keys every workspace's handle, so a workspace whose write never happened does lose sight of its draft in this editor; the draft is still on the server under the path that workspace holds the item at, which is where it stays reachable. Saying it stayed reachable here was wrong. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4 --- .../src/lib/components/ResourceEditor.svelte | 7 ++-- .../src/lib/components/VariableEditor.svelte | 7 ++-- frontend/src/lib/userDraftDbSyncer.svelte.ts | 13 +++++-- frontend/src/lib/userDraftLastLanded.test.ts | 37 +++++++++++++++++++ 4 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 frontend/src/lib/userDraftLastLanded.test.ts diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index 123e78591c..b077cc412a 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -411,9 +411,10 @@ return savedPath } catch (err) { sendUserToast(`Could not save resource: ${err.body ?? err.message}`, true) - // The workspaces that did not get this far keep their drafts under the path - // they still hold the item at; re-keying them onto this rename would move a - // path they never wrote. + // Following the rename remounts on the new path, so a workspace whose write + // never happened loses sight of its draft here — it stays on the server under + // the path that workspace still holds the item at, listed and editable from + // there. Re-keying it instead would move a path it never wrote. return committed } } diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte index edb0e4e8cc..ca313ea632 100644 --- a/frontend/src/lib/components/VariableEditor.svelte +++ b/frontend/src/lib/components/VariableEditor.svelte @@ -373,9 +373,10 @@ reportSaved(savedPath, true) } catch (err) { sendUserToast(`Could not save variable: ${err.body}`, true) - // The workspaces that did not get this far keep their drafts under the path - // they still hold the item at; re-keying them onto this rename would move a - // path they never wrote. They stay listed, and retryable from here. + // Following the rename re-keys every workspace's handle, so a workspace whose + // write never happened loses sight of its draft here — it stays on the server + // under the path that workspace still holds the item at, listed and editable + // from there. Re-keying it instead would move a path it never wrote. if (committed) reportSaved(committed, false) } } diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts index 1718700f07..2a7dd6db50 100644 --- a/frontend/src/lib/userDraftDbSyncer.svelte.ts +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -313,6 +313,7 @@ async function postSave(opts: UserDraftDbSyncerSaveOpts): Promise { serverTimestamp: resp.current_timestamp, localLastSync: lastSync ?? null }) + lastLanded.delete(key) return } // resp.status === 'saved' — advance lastSync (or drop on delete). @@ -546,9 +547,11 @@ export const UserDraftDbSyncer = { } else { clearLastSync(query.workspace, query.itemKind, query.path) } - // Back in sync with the server: clear any conflict / failure. + // Back in sync with the server: clear any conflict / failure. `lastLanded` + // goes with them — whatever we last wrote no longer describes what is there. conflicts.delete(key) failures.delete(key) + lastLanded.delete(key) }, /** @@ -604,16 +607,18 @@ export const UserDraftDbSyncer = { } }, - /** Reactive conflict snapshot (if any) for a draft. */ /** * Whether the last save that landed for this key was the draft's deletion. - * False while none has landed at all — a failed or conflicted delete never - * reaches the response handler, so it never claims to have landed. + * False while none has landed — a delete that failed or conflicted never + * reaches the response handler, so it never claims to have landed, and one + * that landed earlier is dropped as soon as the server's state moves outside + * our writes. */ lastLandedWasDelete(query: UserDraftLastSyncQuery): boolean { return lastLanded.get(draftKey(query.workspace, query.itemKind, query.path)) === 'delete' }, + /** Reactive conflict snapshot (if any) for a draft. */ getConflict(query: UserDraftLastSyncQuery): { readonly conflict: DraftConflictInfo | undefined } { diff --git a/frontend/src/lib/userDraftLastLanded.test.ts b/frontend/src/lib/userDraftLastLanded.test.ts new file mode 100644 index 0000000000..fd030d79b0 --- /dev/null +++ b/frontend/src/lib/userDraftLastLanded.test.ts @@ -0,0 +1,37 @@ +import { describe, it, expect, vi } from 'vitest' + +let response: { status: 'saved' | 'conflict'; current_timestamp: string } = { + status: 'saved', + current_timestamp: '2020-01-01T00:00:00Z' +} +const updateDraft = vi.fn(async (..._args: any[]) => response) +vi.mock('./gen', () => ({ + DraftService: { updateDraft: (...a: unknown[]) => updateDraft(...(a as [])) } +})) +vi.mock('./gen/core/OpenAPI', () => ({ OpenAPI: { BASE: '' } })) +vi.mock('./localDraftHints.svelte', () => ({ setLocalDraftHint: vi.fn() })) + +import { UserDraftDbSyncer } from './userDraftDbSyncer.svelte' + +const query = { workspace: 'ws', itemKind: 'variable' as const, path: 'u/me/v' } +const send = (value: unknown) => UserDraftDbSyncer.save({ ...query, value, immediate: true }) + +/** + * `flushDraftDelete` leaves an editor on this fact, and it is deliberately + * sticky — so it has to stop describing the server the moment the server stops + * being ours to describe. + */ +describe('lastLandedWasDelete', () => { + it('follows what the response handler saw land, and drops it on a conflict', async () => { + await send({ path: 'u/me/v' }) + expect(UserDraftDbSyncer.lastLandedWasDelete(query)).toBe(false) + + await send(null) + expect(UserDraftDbSyncer.lastLandedWasDelete(query)).toBe(true) + + // The server moved under us: the earlier delete no longer answers for this one. + response = { status: 'conflict', current_timestamp: '2020-01-02T00:00:00Z' } + await send(null) + expect(UserDraftDbSyncer.lastLandedWasDelete(query)).toBe(false) + }) +})