From cb14f51445c4fecbae102e7c1504586083f93629 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 19 Sep 2026 23:25:21 +0200 Subject: [PATCH] fix: give nothing up until the other version has actually loaded Co-Authored-By: Claude Opus 5 (1M context) --- .../lib/components/DraftConflictAlert.svelte | 12 ++- .../src/lib/components/ResourceEditor.svelte | 74 +++++++++------ .../src/lib/components/VariableEditor.svelte | 90 +++++++++++-------- 3 files changed, 107 insertions(+), 69 deletions(-) diff --git a/frontend/src/lib/components/DraftConflictAlert.svelte b/frontend/src/lib/components/DraftConflictAlert.svelte index 03e7bbbb42..0e0fd1fb97 100644 --- a/frontend/src/lib/components/DraftConflictAlert.svelte +++ b/frontend/src/lib/components/DraftConflictAlert.svelte @@ -6,9 +6,11 @@ onReload: () => void /** Write what is on screen over the server's draft. */ onOverwrite: () => void + /** A resolution is in flight: neither answer is offered again until it settles. */ + busy?: boolean } - let { onReload, onOverwrite }: Props = $props() + let { onReload, onOverwrite, busy = false }: Props = $props() @@ -18,8 +20,12 @@ longer being saved.
- - + +
diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index dfdcdfd686..4311d07424 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -262,6 +262,9 @@ ) const anyDirty = $derived(dirtyWorkspaces.length > 0) + /** A resolution is in flight. Both buttons go disabled: clicking the other one midway would + * race two resolutions of one conflict against each other. */ + let resolvingConflict = $state(false) /** The server refused this tab's autosave because the row moved under it: another tab, or the * AI chat, which writes these drafts too. Nothing typed here reaches the server until the user * picks a version, and the unsaved-changes banner says the opposite — that the edits are held @@ -279,35 +282,47 @@ async function resolveDraftConflict(keepMine: boolean): Promise { const ws = selected const p = initialPath - if (!ws || !p) return + if (!ws || !p || resolvingConflict) return const query = { workspace: ws, itemKind: 'resource' as const, path: p } - if (keepMine) { - // Forced, so it goes over the row that refused us, and its response reseeds - // `last_sync` so the next ordinary save is conditional again. - const mine = states[ws]?.draft - if (mine) await UserDraftDbSyncer.overwrite({ ...query, value: $state.snapshot(mine) }) - return + resolvingConflict = true + try { + if (keepMine) { + // Forced, so it goes over the row that refused us, and its response reseeds + // `last_sync` so the next ordinary save is conditional again. + const mine = states[ws]?.draft + if (mine) await UserDraftDbSyncer.overwrite({ ...query, value: $state.snapshot(mine) }) + return + } + // Read BEFORE giving anything up: until the server has answered, the refused payload is + // still the only copy of this tab's edit, and the conflict is still true. + const r = await ResourceService.getResource({ workspace: ws, path: p, getDraft: true }) + const deployedState: ResourceState = { + path: r.path, + args: (r.value ?? {}) as Record, + description: r.description ?? '', + labels: r.labels ?? undefined, + wsSpecific: r.ws_specific ?? false + } + // Dropped after the read and before the baseline below: anything an autosave queued + // while it was out belongs to the version being replaced, and leaving it parked would + // let the fresh baseline make it acceptable — sending it over the version just chosen. + UserDraftDbSyncer.dropPending(query) + UserDraftDbSyncer.clearConflict(query) + initialStates[ws] = structuredClone(deployedState) + UserDraftDbSyncer.recordRemoteSync(query, (r as any).draft_saved_at) + UserDraft.seed( + 'resource', + p, + ((r as any).draft as ResourceState | undefined) ?? deployedState, + { workspace: ws } + ) + } catch (e) { + // Nothing was given up above, so the conflict stands and the edit is still here to + // resolve again — which is the whole point of reading first. + sendUserToast(`Could not load the other version: ${e}`, true) + } finally { + resolvingConflict = false } - // Taking theirs: drop the refused payload first so no later flush can send it, then read - // what the server holds and seed that in, which is also what gives this tab a baseline. - UserDraftDbSyncer.dropPending(query) - UserDraftDbSyncer.clearConflict(query) - const r = await ResourceService.getResource({ workspace: ws, path: p, getDraft: true }) - const deployedState: ResourceState = { - path: r.path, - args: (r.value ?? {}) as Record, - description: r.description ?? '', - labels: r.labels ?? undefined, - wsSpecific: r.ws_specific ?? false - } - initialStates[ws] = structuredClone(deployedState) - UserDraftDbSyncer.recordRemoteSync(query, (r as any).draft_saved_at) - UserDraft.seed( - 'resource', - p, - ((r as any).draft as ResourceState | undefined) ?? deployedState, - { workspace: ws } - ) } // The syncer owns the list-page `*` hint; the editor only CLEARS it when a @@ -582,8 +597,9 @@
{#if draftConflict} resolveDraftConflict(false)} - onOverwrite={() => resolveDraftConflict(true)} + busy={resolvingConflict} + onReload={() => void resolveDraftConflict(false)} + onOverwrite={() => void resolveDraftConflict(true)} /> {/if} diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte index f3002803ae..17b1630327 100644 --- a/frontend/src/lib/components/VariableEditor.svelte +++ b/frontend/src/lib/components/VariableEditor.svelte @@ -125,6 +125,9 @@ Object.keys(states).filter((ws) => !draftValuesEqual(states[ws].draft, initialStates[ws])) ) + /** A resolution is in flight. Both buttons go disabled: clicking the other one midway would + * race two resolutions of one conflict against each other. */ + let resolvingConflict = $state(false) /** The server refused this tab's autosave because the row moved under it: another tab, or the * AI chat, which writes these drafts too. Nothing typed here reaches the server until the user * picks a version, and the unsaved-changes banner says the opposite — that the edits are held @@ -142,43 +145,55 @@ async function resolveDraftConflict(keepMine: boolean): Promise { const ws = selected const p = editPath - if (!ws || !p) return + if (!ws || !p || resolvingConflict) return const query = { workspace: ws, itemKind: 'variable' as const, path: p } - if (keepMine) { - // Forced, so it goes over the row that refused us, and its response reseeds - // `last_sync` so the next ordinary save is conditional again. - const mine = states[ws]?.draft - if (mine) await UserDraftDbSyncer.overwrite({ ...query, value: $state.snapshot(mine) }) - return + resolvingConflict = true + try { + if (keepMine) { + // Forced, so it goes over the row that refused us, and its response reseeds + // `last_sync` so the next ordinary save is conditional again. + const mine = states[ws]?.draft + if (mine) await UserDraftDbSyncer.overwrite({ ...query, value: $state.snapshot(mine) }) + return + } + // Read BEFORE giving anything up: until the server has answered, the refused payload is + // still the only copy of this tab's edit, and the conflict is still true. + const v = await VariableService.getVariable({ + workspace: ws, + path: p, + decryptSecret: false, + getDraft: true + }) + const deployedState: VariableState = { + path: v.path, + variable: { + value: v.value ?? '', + is_secret: v.is_secret, + description: v.description ?? '' + }, + labels: v.labels ?? undefined, + wsSpecific: v.ws_specific ?? false + } + // Dropped after the read and before the baseline below: anything an autosave queued + // while it was out belongs to the version being replaced, and leaving it parked would + // let the fresh baseline make it acceptable — sending it over the version just chosen. + UserDraftDbSyncer.dropPending(query) + UserDraftDbSyncer.clearConflict(query) + initialStates[ws] = structuredClone(deployedState) + UserDraftDbSyncer.recordRemoteSync(query, (v as any).draft_saved_at) + UserDraft.seed( + 'variable', + p, + ((v as any).draft as VariableState | undefined) ?? deployedState, + { workspace: ws } + ) + } catch (e) { + // Nothing was given up above, so the conflict stands and the edit is still here to + // resolve again — which is the whole point of reading first. + sendUserToast(`Could not load the other version: ${e}`, true) + } finally { + resolvingConflict = false } - // Taking theirs: drop the refused payload first so no later flush can send it, then read - // what the server holds and seed that in, which is also what gives this tab a baseline. - UserDraftDbSyncer.dropPending(query) - UserDraftDbSyncer.clearConflict(query) - const v = await VariableService.getVariable({ - workspace: ws, - path: p, - decryptSecret: false, - getDraft: true - }) - const deployedState: VariableState = { - path: v.path, - variable: { - value: v.value ?? '', - is_secret: v.is_secret, - description: v.description ?? '' - }, - labels: v.labels ?? undefined, - wsSpecific: v.ws_specific ?? false - } - initialStates[ws] = structuredClone(deployedState) - UserDraftDbSyncer.recordRemoteSync(query, (v as any).draft_saved_at) - UserDraft.seed( - 'variable', - p, - ((v as any).draft as VariableState | undefined) ?? deployedState, - { workspace: ws } - ) } // The list-page `*` hint is owned by UserDraftDbSyncer (set on save, cleared @@ -377,8 +392,9 @@ {#snippet banner()} {#if draftConflict} resolveDraftConflict(false)} - onOverwrite={() => resolveDraftConflict(true)} + busy={resolvingConflict} + onReload={() => void resolveDraftConflict(false)} + onOverwrite={() => void resolveDraftConflict(true)} /> {/if}