From 5373cc29842e68162076e5f9ae9e5a167b53ed74 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Sun, 6 Sep 2026 02:42:26 +0200 Subject: [PATCH] fix(sessions): stop a stale write from landing on the item the editor moved to Guarding the host was not enough: an editor's own continuation runs first, and every piece of state it touches after the await belongs to whatever the editor shows by then. The variable save now pins its payloads, its saved path and its draft key before the first write and only adopts a baseline while it is still the variable it saved; the resource save and the schedule remount stop at the same check. The host guard also matched on path alone, so a late report about a schedule reached a resource or variable that happened to share its path. It now matches the entity kind too, bound at the branch that mounts each editor. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4 --- .../components/ResourceEditorDrawer.svelte | 4 ++ .../src/lib/components/VariableEditor.svelte | 46 ++++++++++++------- .../components/sessions/PreviewTabHost.svelte | 35 +++++++------- .../sessions/ScheduleEditorView.svelte | 4 ++ .../schedules/ScheduleEditorInner.svelte | 5 +- 5 files changed, 57 insertions(+), 37 deletions(-) diff --git a/frontend/src/lib/components/ResourceEditorDrawer.svelte b/frontend/src/lib/components/ResourceEditorDrawer.svelte index 444cf55809..ec86e9e4c1 100644 --- a/frontend/src/lib/components/ResourceEditorDrawer.svelte +++ b/frontend/src/lib/components/ResourceEditorDrawer.svelte @@ -226,6 +226,10 @@ // must not run for a write that failed — a rejected rename (a name // collision, say) would point the tab at a path this save never created. if (!(await saving)) return + // An inline host re-pointed this editor while the write was in flight, so + // `path`, `livePath` and the mounted editor are another resource's now. + // Moving any of them onto this write's result would move that one instead. + if (path !== from) return // Rendered inline there is no drawer to close, so the mounted editor would // otherwise keep the pre-save baseline. Follow a rename before remounting, // or it comes back up on a path the save just moved the item off. diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte index 3d19050809..d555e725b3 100644 --- a/frontend/src/lib/components/VariableEditor.svelte +++ b/frontend/src/lib/components/VariableEditor.svelte @@ -277,16 +277,21 @@ } async function save(): Promise { - const dirty = dirtyWorkspaces - // The path this save started from. Read before the awaits: an inline host can - // re-point the editor at another variable while the write is in flight, and the - // caller needs to know which one the result belongs to. + // Everything the writes need, read before the first await. An inline host can + // re-point this editor at another variable mid-flight, and every one of these + // would then be that variable's: the writes would send its state under this + // one's path, and the baseline below would overwrite its own. const from = editPath + const savedPath = current?.path ?? from + const payloads = dirtyWorkspaces.map((ws) => ({ + ws, + s: $state.snapshot(states[ws].draft!) as VariableState, + ini: $state.snapshot(initialStates[ws]) as VariableState, + existed: !!existedInitially[ws] + })) try { - for (const ws of dirty) { - const s = states[ws].draft! - const ini = initialStates[ws] - if (existedInitially[ws]) { + for (const { ws, s, ini, existed } of payloads) { + if (existed) { await VariableService.updateVariable({ workspace: ws, path: ini.path, @@ -320,20 +325,27 @@ // handle to it via `discard` (not `remove` — blanking the cell to // `undefined` reads as dirty). The `value: null` POST also deletes // the server draft row so `is_draft` clears on refetch. - initialStates[ws] = $state.snapshot(s) as VariableState - existedInitially[ws] = true - UserDraft.discard('variable', editPath ?? '', s, { workspace: ws }) + UserDraft.discard('variable', from ?? '', s, { workspace: ws }) // Path now exists server-side — drop the autocomplete cache so // it shows up immediately instead of after the 60s TTL. invalidateWorkspacePaths(ws) } - sendUserToast(edit ? `Updated variable in ${dirty.length} workspace(s)` : `Created variable`) + sendUserToast( + edit ? `Updated variable in ${payloads.length} workspace(s)` : `Created variable` + ) dispatch('create') - // A rename moved the item; the drawer host closes over it, but an inline one - // stays mounted, so follow the new path here and tell the host about it. - const savedPath = current?.path ?? from - if (savedPath && savedPath !== editPath) editPath = savedPath - drawer?.closeDrawer() + // Only while this editor is still the one that was saved: re-pointed, the + // baseline and path below are the variable it moved to. + if (editPath === from) { + for (const { ws, s } of payloads) { + initialStates[ws] = s + existedInitially[ws] = true + } + // A rename moved the item; the drawer host closes over it, but an inline one + // stays mounted, so follow the new path here and tell the host about it. + if (savedPath && savedPath !== editPath) editPath = savedPath + drawer?.closeDrawer() + } onSaved?.(savedPath, from) } catch (err) { sendUserToast(`Could not save variable: ${err.body}`, true) diff --git a/frontend/src/lib/components/sessions/PreviewTabHost.svelte b/frontend/src/lib/components/sessions/PreviewTabHost.svelte index 5175acd86b..c101a912d5 100644 --- a/frontend/src/lib/components/sessions/PreviewTabHost.svelte +++ b/frontend/src/lib/components/sessions/PreviewTabHost.svelte @@ -20,6 +20,7 @@ parsePreviewSelectedId, showsView } from './previewRouter' + import type { EntityEditorKind } from './previewRouter' import type { EntityToolEffect } from './previewReload' import { withMenuHidden } from './sessionMode.svelte' import ArtifactViewer from '../copilot/chat/artifacts/ArtifactViewer.svelte' @@ -179,15 +180,13 @@ : undefined ) - // An editor reports a removal or a rename after the write it awaited, and the tab - // it was mounted for may have moved on in the meantime — retargeted onto another - // item, or onto a different page entirely. The report belongs to the item it - // started on, so both callbacks below act only while the tab is still showing it: - // otherwise they would re-point whatever the tab holds now, and build the URL out - // of that unrelated location. - function stillShowing(path: string): boolean { + // An editor reports a removal or a rename after the write it awaited, by which + // time the tab may hold something else entirely. The report is about the item it + // started on, so acting on anything else re-points an unrelated editor — and + // builds the destination out of its location. + function stillShowing(kind: EntityEditorKind, path: string): boolean { const now = resolvePreviewTab(tab.url) - return now.kind === 'entity' && now.path === path + return now.kind === 'entity' && now.entityKind === kind && now.path === path } // The item is gone (a draft-only one whose draft was discarded), so the tab has @@ -196,8 +195,8 @@ // another tab, which must not be the one sent to the list. const returnToList = $derived( slot.kind === 'entity' && runtime - ? (fromPath: string) => { - if (!stillShowing(fromPath)) return + ? (kind: EntityEditorKind, fromPath: string) => { + if (!stillShowing(kind, fromPath)) return runtime.previewTabs.retargetTabTo(tab.id, entityListHref(whereIs(tab))) } : undefined @@ -208,8 +207,8 @@ // path — so they have to move with it, or they name an item that no longer exists. const retargetTo = $derived( slot.kind === 'entity' && runtime - ? (newPath: string, fromPath: string) => { - if (!stillShowing(fromPath)) return + ? (kind: EntityEditorKind, newPath: string, fromPath: string) => { + if (!stillShowing(kind, fromPath)) return runtime.previewTabs.retargetTabTo(tab.id, entityEditorHref(whereIs(tab), newPath)) } : undefined @@ -403,8 +402,8 @@ path={slot.path} {workspaceId} onBack={backToList} - onRemoved={returnToList} - onRenamed={retargetTo} + onRemoved={(from) => returnToList?.('trigger_schedule', from)} + onRenamed={(to, from) => retargetTo?.('trigger_schedule', to, from)} /> {/await} {:else if slot.entityKind === 'resource'} @@ -415,8 +414,8 @@ path={slot.path} {workspaceId} onBack={backToList} - onRemoved={returnToList} - onRenamed={retargetTo} + onRemoved={(from) => returnToList?.('resource', from)} + onRenamed={(to, from) => retargetTo?.('resource', to, from)} /> {/await} {:else if slot.entityKind === 'variable'} @@ -427,8 +426,8 @@ path={slot.path} {workspaceId} onBack={backToList} - onRemoved={returnToList} - onRenamed={retargetTo} + onRemoved={(from) => returnToList?.('variable', from)} + onRenamed={(to, from) => retargetTo?.('variable', to, from)} /> {/await} {/if} diff --git a/frontend/src/lib/components/sessions/ScheduleEditorView.svelte b/frontend/src/lib/components/sessions/ScheduleEditorView.svelte index 599c8ac8cd..f4bf042ac8 100644 --- a/frontend/src/lib/components/sessions/ScheduleEditorView.svelte +++ b/frontend/src/lib/components/sessions/ScheduleEditorView.svelte @@ -73,6 +73,10 @@ showDraftBanner {onRemoved} onUpdate={(saved: string | undefined, from: string) => { + // The write landed after the tab was pointed at another schedule: the + // remount below would take that one back through a load it never asked + // for, and the rename is not its rename. + if (from !== path) return generation++ // A draft-only schedule opens with its path editable (saving CREATEs), // so the save can land somewhere other than where the tab is pointed. diff --git a/frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte b/frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte index b02b68e65c..c99275edce 100644 --- a/frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte +++ b/frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte @@ -1467,11 +1467,12 @@ getCurrent={() => draftSync.current} onDiscard={async () => { // The path the discard started from: it awaits a reload of the runnable, - // and an inline host can re-point the editor in the meantime. + // and an inline host can re-point the editor in the meantime — after + // which this discard's outcome is no longer about what is on screen. const from = initialPath const wasDraftOnly = draftOnly await draftSync.resetToDeployed(from) - if (wasDraftOnly && from) onRemoved?.(from) + if (wasDraftOnly && from && initialPath === from) onRemoved?.(from) }} disabled={!can_write} />