From d8ebbc06f5c56dad2570115700d640105d856c57 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Mon, 21 Sep 2026 16:39:49 +0200 Subject: [PATCH] fix: tell the user when a draft conflict has stopped their edits saving (#11241) * fix: tell the user when a draft conflict has stopped their edits saving Co-Authored-By: Claude Opus 5 (1M context) * fix: give nothing up until the other version has actually loaded Co-Authored-By: Claude Opus 5 (1M context) * fix: ignore a reload the editor has moved on from, and settle saves before resolving Co-Authored-By: Claude Opus 5 (1M context) * fix: settle the key before a forced resolution, and cover quiesce with tests Co-Authored-By: Claude Opus 5 (1M context) * fix: let a resolution tell it belongs to an editor that is gone Co-Authored-By: Claude Opus 5 (1M context) * fix: end a conflict resolution when the editing session does, not the component Co-Authored-By: Claude Opus 5 (1M context) * fix: show the resource conflict in the fixed banner, and scope busy to its session Co-Authored-By: Claude Opus 5 (1M context) * fix: keep the conflict alert in editors no host wraps Co-Authored-By: Claude Opus 5 (1M context) * fix: free a reopened editor from a resolution the closed one left in flight Co-Authored-By: Claude Opus 5 (1M context) * fix: make switching workspace-specific versions a new conflict session Co-Authored-By: Claude Opus 5 (1M context) * fix: keep the local draft when a conflict resolution is abandoned Co-Authored-By: Claude Opus 5 (1M context) * fix: let a newer session outrank a resolution that outlived its editor Co-Authored-By: Claude Opus 5 (1M context) * fix: end a conflict resolution with the session that started it Co-Authored-By: Claude Opus 5 (1M context) * fix: reopen a conflicted editor on the version the server refused Co-Authored-By: Claude Opus 5 (1M context) * fix: keep a refused draft deletion a deletion Co-Authored-By: Claude Opus 5 (1M context) * fix: ungate the editor when a resolution loads the other draft Co-Authored-By: Claude Opus 5 (1M context) * fix: settle the draft key before reading the version to load Co-Authored-By: Claude Opus 5 (1M context) * fix: settle the key again after reading, for edits made under the read Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- .../lib/components/DraftConflictAlert.svelte | 31 +++ .../src/lib/components/ResourceEditor.svelte | 179 +++++++++++++++++- .../components/ResourceEditorDrawer.svelte | 25 ++- .../src/lib/components/VariableEditor.svelte | 170 ++++++++++++++++- .../src/lib/draftConflictSession.svelte.ts | 65 +++++++ frontend/src/lib/draftConflictSession.test.ts | 31 +++ frontend/src/lib/userDraftDbSyncer.svelte.ts | 30 +++ frontend/src/lib/userDraftQuiesce.test.ts | 126 ++++++++++++ 8 files changed, 646 insertions(+), 11 deletions(-) create mode 100644 frontend/src/lib/components/DraftConflictAlert.svelte create mode 100644 frontend/src/lib/draftConflictSession.svelte.ts create mode 100644 frontend/src/lib/draftConflictSession.test.ts create mode 100644 frontend/src/lib/userDraftQuiesce.test.ts diff --git a/frontend/src/lib/components/DraftConflictAlert.svelte b/frontend/src/lib/components/DraftConflictAlert.svelte new file mode 100644 index 0000000000..0e0fd1fb97 --- /dev/null +++ b/frontend/src/lib/components/DraftConflictAlert.svelte @@ -0,0 +1,31 @@ + + + +
+
+ It was saved from another tab or session since this one read it, so your changes here are no + longer being saved. +
+
+ + +
+
+ diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index d33b31b332..9847454aca 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -19,6 +19,8 @@ import { useActingUser } from '$lib/actingUser.svelte' import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte' import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' + import { useDraftConflictSession } from '$lib/draftConflictSession.svelte' + import DraftConflictAlert from './DraftConflictAlert.svelte' import { setLocalDraftHint } from '$lib/localDraftHints.svelte' import { onUserInput } from '$lib/userDraftEditGate' import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' @@ -48,6 +50,9 @@ * so it can hide the banner's Discard button in read-only mode (matches * the trigger editors' `disabled={!can_write}` wiring). */ onCanWriteChange?: (canWrite: boolean) => void + /** The drawer renders the conflict alert: inside this scrollable form it can land above the + * viewport on a long resource, leaving the fixed banner claiming the edits are saved. */ + onDraftConflictChange?: (state: { conflicted: boolean; busy: boolean }) => void } let { @@ -61,7 +66,8 @@ selected: selectedProp = $bindable(), viewJsonSchema = $bindable(), onDraftStateChange, - onCanWriteChange + onCanWriteChange, + onDraftConflictChange }: Props = $props() type ResourceState = { @@ -263,6 +269,132 @@ ) const anyDirty = $derived(dirtyWorkspaces.length > 0) + /** Scoped by `selected` so switching workspace-specific versions is a new session. Ending is + * exported on top of that because the drawer, not this component, knows when its own session + * is over: this component outlives it. */ + const conflictSession = useDraftConflictSession(() => selected) + export function endEditingSession(): void { + conflictSession.end() + } + onDestroy(endEditingSession) + const resolvingConflict = $derived(conflictSession.busy) + /** 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 + * as a draft — so without this they are told their work is safe while it is being dropped. */ + const draftConflict = $derived( + selected && initialPath + ? UserDraftDbSyncer.getConflict({ + workspace: selected, + itemKind: 'resource', + path: initialPath + }).conflict + : undefined + ) + + async function resolveDraftConflict(keepMine: boolean): Promise { + const ws = selected + const p = initialPath + if (!ws || !p || resolvingConflict) return + const query = { workspace: ws, itemKind: 'resource' as const, path: p } + const token = conflictSession.start() + const stillOurs = () => conflictSession.holds(token) && selected === ws + try { + if (keepMine) { + // Settle the key first: an ordinary autosave still queued would displace the forced + // write below, and being conditional it would be refused — so "Keep mine" would + // finish without keeping anything and leave the alert standing. + await UserDraftDbSyncer.quiesce(query) + // A resolution belongs to the session that started it. One that outlives its editor + // stops here rather than writing on: whatever replaced it — another session on the + // same draft, or its own resolution — owns the key now, and the edit this one was + // keeping is still parked for a later flush either way. + if (!stillOurs()) return + // A parked `null` is this tab's "no draft any more" — a discard, or an edit that + // landed back on the deployed value. Keeping that means removing the row, not + // writing the baseline back as a draft with no dirty banner to discard it through. + const parked = UserDraftDbSyncer.peekPending(query) + const mine = parked?.value === null ? null : $state.snapshot(states[ws]?.draft) + // Forced, so it goes over the row that refused us, and its response reseeds + // `last_sync` so the next ordinary save is conditional again. + if (mine !== undefined) await UserDraftDbSyncer.overwrite({ ...query, value: mine }) + // Say so rather than leave the alert up with no explanation: a write displaced by + // something typed meanwhile can still lose the race. + if (UserDraftDbSyncer.getConflict(query).conflict) { + sendUserToast('Could not keep your version — try again', true) + } + return + } + // Settle the key BEFORE reading, so what comes back is the version the server is left + // holding: a write this tab started can still be in flight — a forced one it walked + // away from included — and a response fetched past it describes a version about to be + // replaced, which would then be seeded along with its already-stale `last_sync`. + // Nothing is given up by waiting; `quiesce` only stops the pipeline. + await UserDraftDbSyncer.quiesce(query) + if (!stillOurs()) 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 + } + // Everything below writes shared editor state, so first make sure it is still this + // resource's: the drawer stays closable while the read is out, and another resource + // opened meanwhile would otherwise get this one's baseline — and with it this one's + // path as its save target. + if (!stillOurs()) return + // Again, because the form stayed editable while the read was out: `quiesce` settles what + // is running when it is called, not the key for the rest of the resolution, so a + // keystroke since can have started a save of its own. Left running, its rejection lands + // after the baseline below and raises the conflict this just resolved. + await UserDraftDbSyncer.quiesce(query) + if (!stillOurs()) return + // Now, and not in `quiesce`: the refused payload belongs to the version being replaced, + // but until this point it was still the only copy of the edit, and a resolution that + // gave up before here has to leave it behind. + UserDraftDbSyncer.dropPending(query) + UserDraftDbSyncer.clearConflict(query) + initialStates[ws] = structuredClone(deployedState) + // Everything else the load path takes from this same response. The item can have been + // deleted, recreated under another type, or had its permissions changed while the + // conflict stood, and the fields below decide create-vs-update, the schema and write + // access — so refreshing only what is displayed would leave those deciding on the + // version the user just replaced. + fetchedResources[ws] = r + existedInitially[ws] = !(r as any).no_deployed + if (ws === effectiveWorkspace) resource_type = r.resource_type + UserDraftDbSyncer.recordRemoteSync(query, (r as any).draft_saved_at) + const loadedDraft = (r as any).draft as ResourceState | undefined + // Loading a draft makes this workspace one that opened with a draft, whatever it opened + // with before — a refused deletion opens gated, and gated the settling absorber would + // fold the version just loaded into the deployed baseline, leaving it silently clean + // with Save disabled. Ungate here rather than leaving it to the effect, so no write + // between the two is absorbed. + openedOnDraft[ws] = !!loadedDraft + if (loadedDraft) { + setGated(ws, false) + } else { + // Accepting "there is no draft" has to shut the gate, the way discarding one does. + // The edit that raised the conflict set `userEdited`, and left open, the form's + // settling writes — schema defaults materializing over the loaded value — read as + // the user's and recreate the draft just accepted away. + userEdited[ws] = false + setGated(ws, true) + } + UserDraft.seed('resource', p, loadedDraft ?? 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 { + conflictSession.finish(token) + } + } + // The syncer owns the list-page `*` hint; the editor only CLEARS it when a // workspace is at the deployed baseline (so a draft discarded elsewhere // vanishes on reopen). Never SET here. See VariableEditor for the full note. @@ -338,15 +470,33 @@ labels: r.labels ?? undefined, wsSpecific: r.ws_specific ?? false } - // Open with the saved draft if present, else the deployed. - const s: ResourceState = savedDraftState ?? deployedState - openedOnDraft[ws] = !!savedDraftState + // A refused save leaves this tab's own version parked. `.draft` is the + // version that refused it, so opening on that would quietly drop the edit + // the alert is about and leave "Keep mine" offering to keep the other one. + const conflictQuery = { + workspace: ws, + itemKind: 'resource' as const, + path: initialPath + } + const refused = UserDraftDbSyncer.getConflict(conflictQuery).conflict + ? UserDraftDbSyncer.peekPending(conflictQuery) + : undefined + // A parked `null` is this tab's "no draft any more", which on screen is the + // deployed value — so only a payload with content counts as a local draft. + const refusedDraft = (refused?.value ?? undefined) as ResourceState | undefined + const hasLocalDraft = refused ? refused.value !== null : !!savedDraftState + // Open with this tab's refused version if there is one, else the saved + // draft, else the deployed. + const s: ResourceState = refused + ? (refusedDraft ?? deployedState) + : (savedDraftState ?? deployedState) + openedOnDraft[ws] = hasLocalDraft // Gate BEFORE the handle is acquired: `stopSync` queues on a // not-yet-live entry, and the form can settle before the effect // above gets a chance to run. Only worth doing when no draft exists // yet — where one does, there is no phantom to prevent and // suspending could only drop a write. - if (!savedDraftState) setGated(ws, true) + if (!hasLocalDraft) setGated(ws, true) ensureHandle(ws, s) initialStates[ws] = structuredClone(deployedState) // Draft-only paths (`no_deployed`) have no row — saving must @@ -426,6 +576,13 @@ $effect(() => { onCanWriteChange?.(can_write === true) }) + $effect(() => { + onDraftConflictChange?.({ conflicted: !!draftConflict, busy: resolvingConflict }) + }) + + export function resolveDraftConflictFromBanner(keepMine: boolean): void { + void resolveDraftConflict(keepMine) + } export function localDraftDeployed(): ResourceState | undefined { return selected ? initialStates[selected] : undefined @@ -539,6 +696,18 @@
+ + {#if draftConflict && !onDraftConflictChange} + void resolveDraftConflict(false)} + onOverwrite={() => void resolveDraftConflict(true)} + /> + {/if} + {#if otherDirty.length > 0} You are going to edit the value in: {otherDirty.join(', ')} diff --git a/frontend/src/lib/components/ResourceEditorDrawer.svelte b/frontend/src/lib/components/ResourceEditorDrawer.svelte index 7259fff633..ce4d1eea5a 100644 --- a/frontend/src/lib/components/ResourceEditorDrawer.svelte +++ b/frontend/src/lib/components/ResourceEditorDrawer.svelte @@ -8,6 +8,7 @@ import { isOwner } from '$lib/utils' import { useActingUser } from '$lib/actingUser.svelte' import LocalDraftBanner from './LocalDraftBanner.svelte' + import DraftConflictAlert from './DraftConflictAlert.svelte' import OpenInSessionButton from './sessions/OpenInSessionButton.svelte' import { clearPageDrawerAnchor, @@ -62,9 +63,12 @@ localDraftDeployed: () => unknown localDraftCurrent: () => unknown discardLocalDraft: () => void + endEditingSession: () => void + resolveDraftConflictFromBanner: (keepMine: boolean) => void } | undefined = $state(undefined) let hasLocalDraft = $state(false) + let draftConflict = $state({ conflicted: false, busy: false }) let canWriteSelected = $state(true) let path: string | undefined = $state(undefined) @@ -151,6 +155,9 @@ size="50rem" {disableChatOffset} on:close={() => { + // The editor outlives this drawer, so tell it the session is over: a conflict resolution + // still in flight must not land on whatever the next opening shows. + resourceEditor?.endEditingSession?.() if (keepAnchorOnClose) { keepAnchorOnClose = false return @@ -168,7 +175,15 @@ bannerReserved={mode == 'edit'} hideClose={inline && !onClose} fullScreen={!inline} - on:close={() => (inline ? onClose?.() : drawer?.closeDrawer())} + on:close={() => { + // Inline has no drawer to emit a close, so the session ends here instead. + if (inline) { + resourceEditor?.endEditingSession?.() + onClose?.() + } else { + drawer?.closeDrawer() + } + }} > {#snippet titleExtra()} {#if mode == 'new' && resource_type} @@ -189,10 +204,18 @@ bind:selected bind:viewJsonSchema onDraftStateChange={(v) => (hasLocalDraft = v)} + onDraftConflictChange={(v) => (draftConflict = v)} onCanWriteChange={(v) => (canWriteSelected = v)} /> {/await} {#snippet banner()} + {#if draftConflict.conflicted} + resourceEditor?.resolveDraftConflictFromBanner?.(false)} + onOverwrite={() => resourceEditor?.resolveDraftConflictFromBanner?.(true)} + /> + {/if} import { VariableService, WorkspaceService } from '$lib/gen' - import { createEventDispatcher, untrack } from 'svelte' + import { createEventDispatcher, onDestroy, untrack } from 'svelte' import { Button } from './common' import Drawer from './common/drawer/Drawer.svelte' import DrawerContent from './common/drawer/DrawerContent.svelte' @@ -23,6 +23,9 @@ import { useActingUser } from '$lib/actingUser.svelte' import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte' import LocalDraftBanner from './LocalDraftBanner.svelte' + import DraftConflictAlert from './DraftConflictAlert.svelte' + import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' + import { useDraftConflictSession } from '$lib/draftConflictSession.svelte' import { isEncryptedDraftValue } from '$lib/encryptedDraft' import { setLocalDraftHint } from '$lib/localDraftHints.svelte' import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' @@ -141,6 +144,126 @@ Object.keys(states).filter((ws) => !draftValuesEqual(states[ws].draft, initialStates[ws])) ) + /** Scoped by `selected` so switching workspace-specific versions is a new session. Ended on top + * of that by every entry point and by the teardown, since reopening the same variable reuses + * this component. */ + const conflictSession = useDraftConflictSession(() => selected) + function endEditingSession(): void { + conflictSession.end() + } + onDestroy(endEditingSession) + const resolvingConflict = $derived(conflictSession.busy) + /** 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 + * as a draft — so without this they are told their work is safe while it is being dropped. */ + const draftConflict = $derived( + edit && selected && editPath + ? UserDraftDbSyncer.getConflict({ + workspace: selected, + itemKind: 'variable', + path: editPath + }).conflict + : undefined + ) + + async function resolveDraftConflict(keepMine: boolean): Promise { + const ws = selected + const p = editPath + if (!ws || !p || resolvingConflict) return + const query = { workspace: ws, itemKind: 'variable' as const, path: p } + const token = conflictSession.start() + const stillOurs = () => conflictSession.holds(token) && selected === ws + try { + if (keepMine) { + // Settle the key first: an ordinary autosave still queued would displace the forced + // write below, and being conditional it would be refused — so "Keep mine" would + // finish without keeping anything and leave the alert standing. + await UserDraftDbSyncer.quiesce(query) + // A resolution belongs to the session that started it. One that outlives its editor + // stops here rather than writing on: whatever replaced it — another session on the + // same draft, or its own resolution — owns the key now, and the edit this one was + // keeping is still parked for a later flush either way. + if (!stillOurs()) return + // A parked `null` is this tab's "no draft any more" — a discard, or an edit that + // landed back on the deployed value. Keeping that means removing the row, not + // writing the baseline back as a draft with no dirty banner to discard it through. + const parked = UserDraftDbSyncer.peekPending(query) + const mine = parked?.value === null ? null : $state.snapshot(states[ws]?.draft) + // Forced, so it goes over the row that refused us, and its response reseeds + // `last_sync` so the next ordinary save is conditional again. + if (mine !== undefined) await UserDraftDbSyncer.overwrite({ ...query, value: mine }) + // Say so rather than leave the alert up with no explanation: a write displaced by + // something typed meanwhile can still lose the race. + if (UserDraftDbSyncer.getConflict(query).conflict) { + sendUserToast('Could not keep your version — try again', true) + } + return + } + // Settle the key BEFORE reading, so what comes back is the version the server is left + // holding: a write this tab started can still be in flight — a forced one it walked + // away from included — and a response fetched past it describes a version about to be + // replaced, which would then be seeded along with its already-stale `last_sync`. + // Nothing is given up by waiting; `quiesce` only stops the pipeline. + await UserDraftDbSyncer.quiesce(query) + if (!stillOurs()) 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 + } + // Everything below writes shared editor state, so first make sure it is still this + // variable's: the drawer stays closable while the read is out, and another variable + // opened meanwhile would otherwise get this one's baseline — and with it this one's + // path as its save target. + if (!stillOurs()) return + // Again, because the form stayed editable while the read was out: `quiesce` settles what + // is running when it is called, not the key for the rest of the resolution, so a + // keystroke since can have started a save of its own. Left running, its rejection lands + // after the baseline below and raises the conflict this just resolved. + await UserDraftDbSyncer.quiesce(query) + if (!stillOurs()) return + // Now, and not in `quiesce`: the refused payload belongs to the version being replaced, + // but until this point it was still the only copy of the edit, and a resolution that + // gave up before here has to leave it behind. + UserDraftDbSyncer.dropPending(query) + UserDraftDbSyncer.clearConflict(query) + initialStates[ws] = structuredClone(deployedState) + // Everything else the load path takes from this same response. The variable can have + // been deleted, recreated, or had its permissions changed while the conflict stood, + // and these decide create-vs-update and write access — so refreshing only what is + // displayed would leave those deciding on the version the user just replaced. + existedInitially[ws] = !(v as any).no_deployed + extraPerms[ws] = v.extra_perms ?? {} + 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 { + conflictSession.finish(token) + } + } + // The list-page `*` hint is owned by UserDraftDbSyncer (set on save, cleared // on delete). The editor only CLEARS it — a workspace at the deployed // baseline has no draft, so drop any stale hint (this is how a draft @@ -210,8 +333,21 @@ labels: v.labels ?? undefined, wsSpecific: v.ws_specific ?? false } - // Open with the saved draft if present, else the deployed. - const s: VariableState = savedDraftState ?? deployedState + // A refused save leaves this tab's own version parked. `.draft` is the version + // that refused it, so opening on that would quietly drop the edit the alert is + // about and leave "Keep mine" offering to keep the other one. + const conflictQuery = { workspace: ws, itemKind: 'variable' as const, path: p } + const refused = UserDraftDbSyncer.getConflict(conflictQuery).conflict + ? UserDraftDbSyncer.peekPending(conflictQuery) + : undefined + // A parked `null` is this tab's "no draft any more", which on screen is the + // deployed value. + const refusedDraft = (refused?.value ?? undefined) as VariableState | undefined + // Open with this tab's refused version if there is one, else the saved draft, + // else the deployed. + const s: VariableState = refused + ? (refusedDraft ?? deployedState) + : (savedDraftState ?? deployedState) ensureHandle(ws, s) initialStates[ws] = structuredClone(deployedState) // Draft-only paths (`no_deployed`) have no row — saving must @@ -223,6 +359,8 @@ }) function reset() { + // A new session starts here, so anything still running for the last one is spent. + endEditingSession() // Clearing workspaceSpecs triggers useMany's reconcile to release // every acquired entry. The $derived `states` then collapses to {}. workspaceSpecs = [] @@ -334,7 +472,14 @@ {#if inline} {@render content()} {:else} - clearPageDrawerAnchor(VARIABLES_PATH)}> + { + endEditingSession() + clearPageDrawerAnchor(VARIABLES_PATH) + }} + > {@render content()} {/if} @@ -345,9 +490,24 @@ bannerReserved={edit} hideClose={inline && !onClose} fullScreen={!inline} - on:close={() => (inline ? onClose?.() : drawer?.closeDrawer())} + on:close={() => { + // Inline has no drawer to emit a close, so the session ends here instead. + if (inline) { + endEditingSession() + onClose?.() + } else { + drawer?.closeDrawer() + } + }} > {#snippet banner()} + {#if draftConflict} + void resolveDraftConflict(false)} + onOverwrite={() => void resolveDraftConflict(true)} + /> + {/if} unknown) { + const session = createDraftConflictSession() + + let lastScope = untrack(scopeOf) + $effect(() => { + const next = scopeOf() + untrack(() => { + if (next === lastScope) return + lastScope = next + session.end() + }) + }) + + return session +} diff --git a/frontend/src/lib/draftConflictSession.test.ts b/frontend/src/lib/draftConflictSession.test.ts new file mode 100644 index 0000000000..88e62f44de --- /dev/null +++ b/frontend/src/lib/draftConflictSession.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from 'vitest' + +import { createDraftConflictSession } from './draftConflictSession.svelte' + +/** + * A resolution the user has walked away from must neither speak for what replaced it nor hold its + * buttons disabled until it settles. + */ +describe('createDraftConflictSession', () => { + it('frees the next session from a resolution the last one left in flight', () => { + const session = createDraftConflictSession() + + const stale = session.start() + expect(session.busy).toBe(true) + + session.end() + expect(session.busy).toBe(false) + expect(session.holds(stale)).toBe(false) + + const fresh = session.start() + expect(session.busy).toBe(true) + expect(session.holds(fresh)).toBe(true) + + // The stale request settles last: it must not release the live session's claim. + session.finish(stale) + expect(session.busy).toBe(true) + + session.finish(fresh) + expect(session.busy).toBe(false) + }) +}) diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts index 25125a74e4..e383213cfe 100644 --- a/frontend/src/lib/userDraftDbSyncer.svelte.ts +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -723,5 +723,35 @@ export const UserDraftDbSyncer = { const key = draftKey(query.workspace, query.itemKind, query.path) pendingSaveOpts.delete(key) debouncer.cancel(key) + }, + + /** + * The value parked for this key, wrapped so a parked delete (`null`) stays distinguishable + * from nothing parked at all. While a conflict stands this is the version the server refused, + * which is to say this tab's own: an editor opening on the key takes it over the server's + * draft, which is the version that did the refusing. + */ + peekPending(query: UserDraftLastSyncQuery): { value: unknown } | undefined { + const parked = pendingSaveOpts.get(draftKey(query.workspace, query.itemKind, query.path)) + return parked ? { value: parked.value } : undefined + }, + + /** + * Stop scheduling saves for this key and wait until nothing for it is still in flight. + * Cancelling alone cannot stop a POST the runner already started, and such a POST settles + * *after* the caller has moved on — a rejected one re-raising the conflict it was told to + * resolve. Await this before installing a baseline that would make a stale payload acceptable. + * + * What is parked is deliberately left alone: a refused save keeps its payload here, and while + * the conflict stands that is the only copy of the edit outside the editor's own memory. A + * caller that gives up half way must leave it recoverable, so dropping it is the committing + * caller's job, via `dropPending`, once it has something to replace it with. + */ + async quiesce(query: UserDraftLastSyncQuery): Promise { + const key = draftKey(query.workspace, query.itemKind, query.path) + debouncer.cancel(key) + await runner.settled(key) + // A save that landed while we waited can have scheduled the next one. + debouncer.cancel(key) } } diff --git a/frontend/src/lib/userDraftQuiesce.test.ts b/frontend/src/lib/userDraftQuiesce.test.ts new file mode 100644 index 0000000000..b6c38740da --- /dev/null +++ b/frontend/src/lib/userDraftQuiesce.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, afterEach, vi } from 'vitest' + +// Mocked so a test can hold a POST in flight: that window is the whole point of `quiesce`, +// which exists to wait one out rather than return while it is still going. +const updateDraft = vi.fn(async (..._args: any[]) => ({ + status: 'saved' as const, + current_timestamp: '2020-01-01T00:00:00Z' +})) + +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' + +function deferred() { + let resolve!: (v: T) => void + const promise = new Promise((res) => (resolve = res)) + return { promise, resolve } +} + +afterEach(() => { + vi.clearAllMocks() + updateDraft.mockResolvedValue({ status: 'saved', current_timestamp: '2020-01-01T00:00:00Z' }) +}) + +/** + * Resolving a draft conflict installs a fresh baseline. Anything of the old version still in the + * air when that lands would be made acceptable by it, and a rejected one re-raises the conflict + * just resolved — so the resolution waits for the key to go quiet first. What it must NOT do is + * discard the payload while waiting: a refused save keeps it here, and until the resolution + * commits it is the only copy of the edit outside the editor's own memory. + */ +describe('UserDraftDbSyncer.quiesce', () => { + it('waits for a save already in flight', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_a' } + const inFlight = deferred() + let settledBeforeQuiesceReturned = false + updateDraft.mockImplementationOnce(async () => { + await inFlight.promise + settledBeforeQuiesceReturned = true + return { status: 'saved', current_timestamp: '2020-01-01T00:00:00Z' } + }) + + // In flight, not awaited: the POST is held open by the mock above. + const saving = UserDraftDbSyncer.save({ ...q, value: { v: 'in flight' }, immediate: true }) + + let quiesced = false + const quiescing = UserDraftDbSyncer.quiesce(q).then(() => (quiesced = true)) + // A full macrotask, not a microtask: anything that merely yields would have resolved by + // now, so this is what tells "waited for the POST" apart from "waited for nothing". + await new Promise((r) => setTimeout(r, 0)) + expect(quiesced).toBe(false) + + inFlight.resolve() + await quiescing + await saving + expect(settledBeforeQuiesceReturned).toBe(true) + }) + + it('keeps a refused payload recoverable, until the resolution that replaces it says otherwise', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_b' } + // Refused, so the payload stays parked — the state a conflict resolution starts from. + updateDraft.mockResolvedValueOnce({ + status: 'conflict', + current_timestamp: '2020-01-02T00:00:00Z' + }) + await UserDraftDbSyncer.save({ ...q, value: { v: 'refused' }, immediate: true }) + expect(UserDraftDbSyncer.getConflict(q).conflict).toBeTruthy() + + await UserDraftDbSyncer.quiesce(q) + + // A resolution abandoned here (the editor closed while quiescing) must leave the edit + // somewhere it can still be sent. Flushing is how that payload is observed, since the + // parked opts themselves are private to the syncer. + updateDraft.mockClear() + await UserDraftDbSyncer.flush(q) + expect(updateDraft).toHaveBeenCalledTimes(1) + expect(updateDraft.mock.calls[0][0]).toMatchObject({ requestBody: { value: { v: 'refused' } } }) + + // Only the caller that has something to put in its place drops it. + UserDraftDbSyncer.dropPending(q) + updateDraft.mockClear() + await UserDraftDbSyncer.flush(q) + expect(updateDraft).not.toHaveBeenCalled() + }) + + it('keeps a parked delete distinguishable from nothing parked', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_delete' } + + expect(UserDraftDbSyncer.peekPending(q)).toBeUndefined() + + // Reverting to the deployed value, or discarding, parks a delete. Refused, it stays + // parked — and a resolution reading it has to keep meaning "remove the row", not fall + // back to whatever the server holds. + updateDraft.mockResolvedValueOnce({ + status: 'conflict', + current_timestamp: '2020-01-02T00:00:00Z' + }) + await UserDraftDbSyncer.save({ ...q, value: null, immediate: true }) + + expect(UserDraftDbSyncer.peekPending(q)).toEqual({ value: null }) + }) + + it('cancels a debounced autosave so it cannot displace the write that follows', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_c' } + vi.useFakeTimers() + try { + // Debounced rather than immediate: this is the autosave a resolution has to call off, or + // it fires mid-resolution and, being conditional, is refused. + void UserDraftDbSyncer.save({ ...q, value: { v: 'queued' } }) + + await UserDraftDbSyncer.quiesce(q) + updateDraft.mockClear() + + // Past the debouncer's 10s ceiling, so a schedule that survived has certainly fired. + // Real time would not reach it: the test would pass with the cancelling removed. + await vi.advanceTimersByTimeAsync(11000) + expect(updateDraft).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) +})