From 21ca77e415b1d139d04d8246e6f752f0661ac10c Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Sun, 6 Sep 2026 06:48:17 +0200 Subject: [PATCH] fix(sessions): tell every tab on an item about any save, not only one that moved it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Editors on the same item share its draft cell but each keeps its own deployed baseline, so a save made in one left the others offering to discard back to the value it replaced — over a deployment. A save now reports the path it wrote to whether or not that moved the item, and the tabs on it that stayed put are re-read instead of re-pointed. The variable drawer reopens on the committed path after a partial multi-workspace failure rather than re-keying: re-keying brought its handles back from the values the drawer opened on, which describe neither what the committed workspace has deployed nor the draft the failed one still holds. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4 --- .../src/lib/components/VariableEditor.svelte | 35 +++++++-------- .../components/sessions/PreviewTabHost.svelte | 43 ++++++++----------- .../sessions/ResourceEditorView.svelte | 14 +++--- .../sessions/ScheduleEditorView.svelte | 18 ++++---- .../sessions/VariableEditorView.svelte | 14 +++--- .../(root)/(logged)/sessions/+page.svelte | 20 +++++---- 6 files changed, 71 insertions(+), 73 deletions(-) diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte index ca313ea632..0dea6612e5 100644 --- a/frontend/src/lib/components/VariableEditor.svelte +++ b/frontend/src/lib/components/VariableEditor.svelte @@ -304,17 +304,6 @@ // one wrote is already deployed — a host told nothing would stay pointed at a // path it has moved off. let committed: string | undefined = undefined - // Follow the rename locally and tell the host, for whatever committed. Guarded - // on this editor still being the one that was saved: re-pointed, `editPath` is - // the variable it moved to. `close` only when every workspace is done — after a - // partial failure the drawer is where the one that still needs saving is retried. - const reportSaved = (saved: string | undefined, close: boolean) => { - if (editPath === from) { - if (saved && saved !== editPath) editPath = saved - if (close) drawer?.closeDrawer() - } - onSaved?.(saved, from, fromWs) - } try { for (const { ws, s, ini, existed } of payloads) { if (existed) { @@ -370,14 +359,26 @@ edit ? `Updated variable in ${payloads.length} workspace(s)` : `Created variable` ) dispatch('create') - reportSaved(savedPath, true) + // Only while this editor is still the one that was saved: re-pointed, + // `editPath` is the variable it moved to. + if (editPath === from) { + // 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, fromWs) } catch (err) { sendUserToast(`Could not save variable: ${err.body}`, true) - // 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) + if (committed) { + onSaved?.(committed, from, fromWs) + // Reopened, not re-keyed: re-keying would bring the handles back from the + // values this drawer opened on, which describe neither what the committed + // workspace now has deployed nor the draft the failed one still holds at + // its own path. That draft stays there, listed and editable from its + // workspace — moving it onto a rename it never wrote would be worse. + if (editPath === from) editVariable(committed) + } } } diff --git a/frontend/src/lib/components/sessions/PreviewTabHost.svelte b/frontend/src/lib/components/sessions/PreviewTabHost.svelte index 9216baeff6..a59cb4041e 100644 --- a/frontend/src/lib/components/sessions/PreviewTabHost.svelte +++ b/frontend/src/lib/components/sessions/PreviewTabHost.svelte @@ -37,7 +37,7 @@ fullscreen = false, onNavigate, onLoad, - onEntityMoved + onEntityWritten }: { tab: SessionPreviewTab session: Session | undefined @@ -63,9 +63,10 @@ onNavigate: (item: WorkspaceItem) => void /** Iframe finished loading — the page reads back its observed location. */ onLoad: (frame: HTMLIFrameElement) => void - /** A hosted entity editor moved its item: renamed it (`to`) or removed it. - * The page re-points every tab on that item, across warm sessions. */ - onEntityMoved: (ev: { + /** A hosted entity editor wrote its item: saved it (`to`, the path it wrote + * to) or removed it. The page reaches every tab on that item, across warm + * sessions. */ + onEntityWritten: (ev: { kind: EntityEditorKind path: string workspace: string @@ -188,26 +189,20 @@ : undefined ) - // An editor reports a removal or a rename about the item it started on, after - // the write it awaited — by which time this tab may hold something else, and - // other warm sessions may hold that same item. Both are the page's to resolve: - // it re-points every tab still on the item, in the workspace it was written in, - // which is also the only way the reporting tab is spared a report gone stale. - // A removal has no destination path; a rename carries the one it moved to. - const returnToList = $derived( + // What an editor reports is about an item, not about this tab: other warm + // sessions can hold the same one, and by the time the write it awaited returns + // this tab may hold something else. The page resolves both by matching on the + // item, so a report gone stale simply finds nothing. + const reportRemoved = $derived( slot.kind === 'entity' ? (kind: EntityEditorKind, fromPath: string, fromWs: string) => - onEntityMoved({ kind, path: fromPath, workspace: fromWs }) + onEntityWritten({ kind, path: fromPath, workspace: fromWs }) : undefined ) - - // The tab, its label, the chat's ACTIVE PREVIEW and the draft key all address - // the item by path, so they have to move with it or they name an item that no - // longer exists. - const retargetTo = $derived( + const reportSaved = $derived( slot.kind === 'entity' ? (kind: EntityEditorKind, newPath: string, fromPath: string, fromWs: string) => - onEntityMoved({ kind, path: fromPath, workspace: fromWs, to: newPath }) + onEntityWritten({ kind, path: fromPath, workspace: fromWs, to: newPath }) : undefined ) @@ -399,8 +394,8 @@ path={slot.path} {workspaceId} onBack={backToList} - onRemoved={(from, ws) => returnToList?.('trigger_schedule', from, ws)} - onRenamed={(to, from, ws) => retargetTo?.('trigger_schedule', to, from, ws)} + onRemoved={(from, ws) => reportRemoved?.('trigger_schedule', from, ws)} + onSavedTo={(to, from, ws) => reportSaved?.('trigger_schedule', to, from, ws)} /> {/await} {:else if slot.entityKind === 'resource'} @@ -411,8 +406,8 @@ path={slot.path} {workspaceId} onBack={backToList} - onRemoved={(from, ws) => returnToList?.('resource', from, ws)} - onRenamed={(to, from, ws) => retargetTo?.('resource', to, from, ws)} + onRemoved={(from, ws) => reportRemoved?.('resource', from, ws)} + onSavedTo={(to, from, ws) => reportSaved?.('resource', to, from, ws)} /> {/await} {:else if slot.entityKind === 'variable'} @@ -423,8 +418,8 @@ path={slot.path} {workspaceId} onBack={backToList} - onRemoved={(from, ws) => returnToList?.('variable', from, ws)} - onRenamed={(to, from, ws) => retargetTo?.('variable', to, from, ws)} + onRemoved={(from, ws) => reportRemoved?.('variable', from, ws)} + onSavedTo={(to, from, ws) => reportSaved?.('variable', to, from, ws)} /> {/await} {/if} diff --git a/frontend/src/lib/components/sessions/ResourceEditorView.svelte b/frontend/src/lib/components/sessions/ResourceEditorView.svelte index 8c7e68015a..cb9b98976a 100644 --- a/frontend/src/lib/components/sessions/ResourceEditorView.svelte +++ b/frontend/src/lib/components/sessions/ResourceEditorView.svelte @@ -7,7 +7,7 @@ workspaceId, onBack, onRemoved, - onRenamed + onSavedTo }: { /** The resource this tab edits (the row its location deep-links). */ path: string @@ -20,11 +20,11 @@ * Distinct from `onBack`, which moves whichever tab is active: only this tab is * the one to send back, and only while it still shows that item. */ onRemoved?: (fromPath: string, fromWorkspace: string) => void - /** The item at `fromPath` was saved under a different path. The tab addresses - * it by path — as do its label, the chat's ACTIVE PREVIEW and the draft key — - * so the tab has to follow, or all four keep naming an item that no longer - * exists. */ - onRenamed?: (newPath: string, fromPath: string, fromWorkspace: string) => void + /** The item at `fromPath` was saved, to `newPath`. Every tab on it has to hear: + * a rename moves the ones addressing it by path — their label, the chat's + * ACTIVE PREVIEW and the draft key all do — and a plain save moves none but + * leaves the others holding a baseline the deploy has replaced. */ + onSavedTo?: (newPath: string, fromPath: string, fromWorkspace: string) => void } = $props() let editor = $state() @@ -46,6 +46,6 @@ {onBack} {onRemoved} onSaved={(saved, from, fromWs) => { - if (saved && from && fromWs && saved !== from) onRenamed?.(saved, from, fromWs) + if (saved && from && fromWs) onSavedTo?.(saved, from, fromWs) }} /> diff --git a/frontend/src/lib/components/sessions/ScheduleEditorView.svelte b/frontend/src/lib/components/sessions/ScheduleEditorView.svelte index 43450dd9d0..77eafab1ad 100644 --- a/frontend/src/lib/components/sessions/ScheduleEditorView.svelte +++ b/frontend/src/lib/components/sessions/ScheduleEditorView.svelte @@ -10,7 +10,7 @@ workspaceId, onBack, onRemoved, - onRenamed + onSavedTo }: { /** The schedule this tab edits (the row its location deep-links). */ path: string @@ -24,11 +24,11 @@ * awaits a reload of the runnable, by which time the user may be looking at * another tab, or have pointed this one somewhere else. */ onRemoved?: (fromPath: string, fromWorkspace: string) => void - /** The item at `fromPath` was saved under a different path. The tab addresses - * it by path — as do its label, the chat's ACTIVE PREVIEW and the draft key — - * so the tab has to follow, or all four keep naming an item that no longer - * exists. */ - onRenamed?: (newPath: string, fromPath: string, fromWorkspace: string) => void + /** The item at `fromPath` was saved, to `newPath`. Every tab on it has to hear: + * a rename moves the ones addressing it by path — their label, the chat's + * ACTIVE PREVIEW and the draft key all do — and a plain save moves none but + * leaves the others holding a baseline the deploy has replaced. */ + onSavedTo?: (newPath: string, fromPath: string, fromWorkspace: string) => void } = $props() // Captured at init, so it must read the current prop rather than close over it. @@ -76,9 +76,9 @@ // 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. - if (saved && saved !== from) onRenamed?.(saved, from, fromWs) + // A draft-only schedule opens with its path editable (saving CREATEs), so + // the save can land somewhere other than where the tab is pointed. + if (saved) onSavedTo?.(saved, from, fromWs) }} > {#snippet customLabel()} diff --git a/frontend/src/lib/components/sessions/VariableEditorView.svelte b/frontend/src/lib/components/sessions/VariableEditorView.svelte index 07f074653b..33f8762282 100644 --- a/frontend/src/lib/components/sessions/VariableEditorView.svelte +++ b/frontend/src/lib/components/sessions/VariableEditorView.svelte @@ -7,7 +7,7 @@ workspaceId, onBack, onRemoved, - onRenamed + onSavedTo }: { /** The variable this tab edits (the row its location deep-links). */ path: string @@ -20,11 +20,11 @@ * Distinct from `onBack`, which moves whichever tab is active: only this tab is * the one to send back, and only while it still shows that item. */ onRemoved?: (fromPath: string, fromWorkspace: string) => void - /** The item at `fromPath` was saved under a different path. The tab addresses - * it by path — as do its label, the chat's ACTIVE PREVIEW and the draft key — - * so the tab has to follow, or all four keep naming an item that no longer - * exists. */ - onRenamed?: (newPath: string, fromPath: string, fromWorkspace: string) => void + /** The item at `fromPath` was saved, to `newPath`. Every tab on it has to hear: + * a rename moves the ones addressing it by path — their label, the chat's + * ACTIVE PREVIEW and the draft key all do — and a plain save moves none but + * leaves the others holding a baseline the deploy has replaced. */ + onSavedTo?: (newPath: string, fromPath: string, fromWorkspace: string) => void } = $props() let editor = $state() @@ -50,7 +50,7 @@ {onBack} {onRemoved} onSaved={(saved, from, fromWs) => { - if (saved && from && fromWs && saved !== from) onRenamed?.(saved, from, fromWs) + if (saved && from && fromWs) onSavedTo?.(saved, from, fromWs) }} /> {/key} diff --git a/frontend/src/routes/(root)/(logged)/sessions/+page.svelte b/frontend/src/routes/(root)/(logged)/sessions/+page.svelte index 92dbefb2cf..20d69429c2 100644 --- a/frontend/src/routes/(root)/(logged)/sessions/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/sessions/+page.svelte @@ -611,16 +611,14 @@ } } } - // A hosted entity editor moved its item — renamed it, or removed it by - // discarding the draft that was all of it. Every tab on that item follows, - // across warm sessions: they share the one draft cell and the one server row, - // so a tab left behind edits a path that is gone. Matching on the item is also - // what keeps a report that landed late from moving a tab that has since been - // re-pointed — there is nothing to match. - function moveEntityTabs(ev: { + // A hosted entity editor wrote its item. Every tab on it hears, across warm + // sessions: they share the one draft cell and the one server row, so one left + // behind edits a path that is gone or discards over a value already deployed. + function entityWritten(ev: { kind: EntityEditorKind path: string workspace: string + /** The path it wrote to; absent when the write removed the item. */ to?: string }) { const page = entityListPage(ev.kind)?.path @@ -633,7 +631,11 @@ const entity = parseEntityEditorRoute(loc) if (!entity || entity.path !== ev.path || stripBase(loc) !== page) continue // The tab's own location, so the list it came from keeps its filters. - owner.retargetTabTo(tab.id, ev.to ? entityEditorHref(loc, ev.to) : entityListHref(loc)) + if (!ev.to) owner.retargetTabTo(tab.id, entityListHref(loc)) + else if (ev.to !== ev.path) owner.retargetTabTo(tab.id, entityEditorHref(loc, ev.to)) + // Same path: nothing to re-point, but every editor on it holds a baseline + // the deploy has replaced, and would offer to discard back to it. + else tabHosts[tabKey(s.id, tab.id)]?.reload({ entity: 'refresh' }) } } } @@ -1189,7 +1191,7 @@ {fullscreen} onNavigate={navigateEditorTo} onLoad={(frame) => tabs && onTabLoad(tabs, tab, frame)} - onEntityMoved={moveEntityTabs} + onEntityWritten={entityWritten} /> {/each} {/each}