mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 16:02:59 +00:00
fix(sessions): report a write per workspace, and leave the reporting editor alone
The same-path refresh reached the tab that did the save, remounting it over the edit its own settle had just preserved — the teardown's flush and the fresh editor's read race, and the older value can win. The reporting tab is skipped: it has settled its own state, which is what the report is telling everyone else. A save can also write several workspaces while only one event went out, labelled with the acting one, and a discard deletes the draft of whichever workspace the form is showing while claiming the acting one — so a linked workspace's tabs kept a replaced baseline and the wrong tabs were closed. Each write now reports its own workspace and the path it wrote there, which retires the acting-workspace bookkeeping that stood in for it. A tab re-pointed mid-write no longer swallows the report either: only the local remount was ever its to suppress. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4
This commit is contained in:
co-authored by
Claude Opus 5
parent
21ca77e415
commit
c7faaf1956
@@ -335,10 +335,9 @@
|
||||
current.path = npath
|
||||
}
|
||||
|
||||
/** The path the acting workspace's write landed on, or undefined if it failed —
|
||||
* it toasts its own failure, so a caller only needs this to know whether to run
|
||||
* bookkeeping of its own, and where the item ended up. */
|
||||
export async function save(): Promise<string | undefined> {
|
||||
/** What the save deployed: one entry per workspace written, with the path it
|
||||
* wrote there. Empty when nothing landed — it toasts its own failure. */
|
||||
export async function save(): Promise<{ ws: string; path: string }[]> {
|
||||
// Everything the writes send, read before the first await. The form stays
|
||||
// editable while they are in flight, so read later these would be whatever
|
||||
// the user has since typed — sent under an earlier workspace's path, and
|
||||
@@ -353,12 +352,11 @@
|
||||
// The path the ACTING workspace's write used. `WsSpecificVersions` can point
|
||||
// the form at a linked workspace, and a rename made there is that workspace's
|
||||
// alone — reporting it would move a host that is looking at this one.
|
||||
const savedPath = payloads.find((pl) => pl.ws === effectiveWorkspace)?.s.path ?? from
|
||||
// Set once the acting workspace's own write has committed. The workspaces are
|
||||
// written in sequence and a later one throwing aborts the rest, but what this
|
||||
// one wrote is already deployed — a caller told nothing would stay pointed at
|
||||
// a path it has moved off.
|
||||
let committed: string | undefined = undefined
|
||||
// Every workspace this save wrote, in order, each with the path it wrote there.
|
||||
// The workspaces go in sequence and a later one throwing aborts the rest, but
|
||||
// what an earlier one wrote is deployed — a caller told nothing about it would
|
||||
// stay pointed at a path the item has moved off.
|
||||
const written: { ws: string; path: string }[] = []
|
||||
try {
|
||||
for (const { ws, s, ini, existed } of payloads) {
|
||||
if (existed) {
|
||||
@@ -394,7 +392,7 @@
|
||||
// `remove`. See VariableEditor for the full rationale.
|
||||
initialStates[ws] = s
|
||||
existedInitially[ws] = true
|
||||
if (ws === effectiveWorkspace) committed = s.path
|
||||
written.push({ ws, path: s.path })
|
||||
// `s.path`, not the reported one: each workspace settles against the path
|
||||
// its own write used.
|
||||
settleDraftAfterWrite('resource', s, states[ws]?.draft, from, s.path, { workspace: ws })
|
||||
@@ -407,15 +405,14 @@
|
||||
? `Saved resource in ${payloads.length} workspaces`
|
||||
: `Saved resource`
|
||||
)
|
||||
dispatch('refresh', savedPath)
|
||||
return savedPath
|
||||
dispatch('refresh', written.find((w) => w.ws === effectiveWorkspace)?.path ?? from)
|
||||
return written
|
||||
} catch (err) {
|
||||
sendUserToast(`Could not save resource: ${err.body ?? err.message}`, true)
|
||||
// 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
|
||||
// The workspaces that never got their write keep their drafts at their own
|
||||
// paths, listed and editable from there — moving one onto a rename it never
|
||||
// wrote would be worse than not showing it here.
|
||||
return written
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
@@ -63,8 +63,9 @@
|
||||
|
||||
let resourceEditor:
|
||||
| {
|
||||
/** The path it wrote to, or undefined when it failed; it toasts its own error. */
|
||||
save: () => Promise<string | undefined>
|
||||
/** One entry per workspace written, with the path it wrote there; empty when
|
||||
* nothing landed. It toasts its own error. */
|
||||
save: () => Promise<{ ws: string; path: string }[]>
|
||||
localDraftDeployed: () => unknown
|
||||
localDraftCurrent: () => unknown
|
||||
/** False when the discard removed the resource (it was draft-only). */
|
||||
@@ -176,7 +177,10 @@
|
||||
getCurrent={() => resourceEditor?.localDraftCurrent()}
|
||||
onDiscard={async () => {
|
||||
const from = path
|
||||
const fromWs = effectiveWorkspace
|
||||
// The workspace the editor is showing, which `WsSpecificVersions` can point at
|
||||
// a linked one: the discard deletes that workspace's draft, so that is whose
|
||||
// tabs it is about.
|
||||
const fromWs = selected ?? effectiveWorkspace
|
||||
if ((await resourceEditor?.discardLocalDraft()) === false && from)
|
||||
onRemoved?.(from, fromWs)
|
||||
}}
|
||||
@@ -221,23 +225,24 @@
|
||||
// caller of this drawer. `onSaved` still fires after the write lands.
|
||||
const saving = resourceEditor?.save()
|
||||
drawer?.closeDrawer()
|
||||
// The path the write landed on, from the editor rather than the form: the form
|
||||
// may be showing a linked workspace's variant, whose rename is not this host's.
|
||||
// Undefined means the write failed — a rejected rename (a name collision, say)
|
||||
// — and everything below would move this host onto a path it never created.
|
||||
const submitted = await saving
|
||||
if (!submitted) return
|
||||
// An inline host re-pointed this editor while the write was in flight, so
|
||||
// `path` and the mounted editor are another resource's now. Moving either
|
||||
// onto this write's result would move that one instead.
|
||||
if (path !== from) return
|
||||
// What landed, per workspace, from the editor rather than the form: the form may
|
||||
// be showing a linked workspace's variant, whose rename is not this host's.
|
||||
const written = (await saving) ?? []
|
||||
if (written.length === 0) return
|
||||
const submitted = written.find((w) => w.ws === fromWs)?.path
|
||||
// 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.
|
||||
const savedPath = submitted
|
||||
if (savedPath) path = savedPath
|
||||
editorGeneration++
|
||||
onSaved?.(savedPath, from, fromWs)
|
||||
// or it comes back up on a path the save just moved the item off. Only while
|
||||
// this is still the resource it saved: re-pointed mid-write, `path` and the
|
||||
// mounted editor are another one's, and moving them would move that one.
|
||||
if (path === from) {
|
||||
if (submitted) path = submitted
|
||||
editorGeneration++
|
||||
}
|
||||
// One report per workspace written, each carrying its own — a linked
|
||||
// workspace's rename moves the tabs acting on it and no others. Reported
|
||||
// even when this drawer has moved on: the write is a fact about the item.
|
||||
for (const w of written) onSaved?.(w.path, from, w.ws)
|
||||
}}
|
||||
disabled={!canSave}
|
||||
>
|
||||
|
||||
@@ -295,15 +295,10 @@
|
||||
ini: $state.snapshot(initialStates[ws]) as VariableState,
|
||||
existed: !!existedInitially[ws]
|
||||
}))
|
||||
// The path the ACTING workspace's write used. `WsSpecificVersions` can point
|
||||
// the form at a linked workspace, and a rename made there is that workspace's
|
||||
// alone — reporting it would move a host that is looking at this one.
|
||||
const savedPath = payloads.find((pl) => pl.ws === fromWs)?.s.path ?? from
|
||||
// Set once the acting workspace's own write has committed. The workspaces are
|
||||
// written in sequence and a later one throwing aborts the rest, but what this
|
||||
// one wrote is already deployed — a host told nothing would stay pointed at a
|
||||
// path it has moved off.
|
||||
let committed: string | undefined = undefined
|
||||
// What this editor itself follows: `WsSpecificVersions` can point the form at a
|
||||
// linked workspace, and a rename made there is that workspace's alone.
|
||||
const actingPath = payloads.find((pl) => pl.ws === fromWs)?.s.path ?? from
|
||||
let actingCommitted = false
|
||||
try {
|
||||
for (const { ws, s, ini, existed } of payloads) {
|
||||
if (existed) {
|
||||
@@ -347,7 +342,12 @@
|
||||
// resets the handle to it via `discard` (not `remove` — blanking the cell
|
||||
// to `undefined` reads as dirty), and keeps an edit made mid-request. Each
|
||||
// workspace settles against the path its own write used, not the reported one.
|
||||
if (ws === fromWs) committed = s.path
|
||||
if (ws === fromWs) actingCommitted = true
|
||||
// Per workspace, as each write lands. Each carries its own workspace and
|
||||
// the path it wrote there, so a linked workspace's rename moves the tabs
|
||||
// acting on it and no others — and a workspace written before a later one
|
||||
// threw is still reported, because it is deployed.
|
||||
onSaved?.(s.path, from, ws)
|
||||
settleDraftAfterWrite('variable', s, states[ws]?.draft, from ?? '', s.path, {
|
||||
workspace: ws
|
||||
})
|
||||
@@ -363,22 +363,17 @@
|
||||
// `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
|
||||
// stays mounted, so follow the new path here.
|
||||
if (actingPath && actingPath !== editPath) editPath = actingPath
|
||||
drawer?.closeDrawer()
|
||||
}
|
||||
onSaved?.(savedPath, from, fromWs)
|
||||
} catch (err) {
|
||||
sendUserToast(`Could not save variable: ${err.body}`, true)
|
||||
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)
|
||||
}
|
||||
// Reopened, so the handles come from the server: re-keying them would bring
|
||||
// back the values this drawer opened on, which describe neither what the
|
||||
// committed workspace now has deployed nor the draft the failed one still
|
||||
// holds — that one stays at its own path, editable from its workspace.
|
||||
if (actingCommitted && actingPath && editPath === from) editVariable(actingPath)
|
||||
}
|
||||
}
|
||||
</script>
|
||||
@@ -391,16 +386,18 @@
|
||||
getCurrent={() => current}
|
||||
onDiscard={async () => {
|
||||
if (!selected) return
|
||||
// The workspace the editor is showing, which `WsSpecificVersions` can point at
|
||||
// a linked one: the discard deletes that workspace's draft, so that is whose
|
||||
// tabs it is about.
|
||||
const ws = selected
|
||||
const from = editPath ?? ''
|
||||
const fromWs = curWs
|
||||
UserDraft.discard('variable', from, initialStates[ws], { workspace: ws })
|
||||
// A draft-only variable has no deployed row under the draft, so discarding
|
||||
// it removed the variable: `initialStates` holds a synthesized stand-in, not
|
||||
// a baseline to fall back to. Only once the delete has landed — until then
|
||||
// the variable is still there, and a host would leave on a row it can see.
|
||||
const landed = await flushDraftDelete('variable', from, { workspace: ws })
|
||||
if (!existedInitially[ws] && landed && from && fromWs) onRemoved?.(from, fromWs)
|
||||
if (!existedInitially[ws] && landed && from) onRemoved?.(from, ws)
|
||||
}}
|
||||
disabled={!can_write}
|
||||
/>
|
||||
|
||||
@@ -65,12 +65,14 @@
|
||||
onLoad: (frame: HTMLIFrameElement) => void
|
||||
/** 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. */
|
||||
* sessions — `fromTabId` being the one that reported, which has settled its
|
||||
* own state already. */
|
||||
onEntityWritten: (ev: {
|
||||
kind: EntityEditorKind
|
||||
path: string
|
||||
workspace: string
|
||||
to?: string
|
||||
fromTabId: string
|
||||
}) => void
|
||||
} = $props()
|
||||
|
||||
@@ -196,13 +198,19 @@
|
||||
const reportRemoved = $derived(
|
||||
slot.kind === 'entity'
|
||||
? (kind: EntityEditorKind, fromPath: string, fromWs: string) =>
|
||||
onEntityWritten({ kind, path: fromPath, workspace: fromWs })
|
||||
onEntityWritten({ kind, path: fromPath, workspace: fromWs, fromTabId: tab.id })
|
||||
: undefined
|
||||
)
|
||||
const reportSaved = $derived(
|
||||
slot.kind === 'entity'
|
||||
? (kind: EntityEditorKind, newPath: string, fromPath: string, fromWs: string) =>
|
||||
onEntityWritten({ kind, path: fromPath, workspace: fromWs, to: newPath })
|
||||
onEntityWritten({
|
||||
kind,
|
||||
path: fromPath,
|
||||
workspace: fromWs,
|
||||
to: newPath,
|
||||
fromTabId: tab.id
|
||||
})
|
||||
: undefined
|
||||
)
|
||||
|
||||
|
||||
@@ -71,13 +71,11 @@
|
||||
showDraftBanner
|
||||
{onRemoved}
|
||||
onUpdate={(saved: string | undefined, from: string, fromWs: 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.
|
||||
// Remount only while this tab is still on the schedule that was saved:
|
||||
// pointed elsewhere since, it would be taken back through a load it never
|
||||
// asked for. The report goes out either way — the tabs on that schedule
|
||||
// elsewhere have to hear, and a draft-only save can land on a new path.
|
||||
if (from === path) generation++
|
||||
if (saved) onSavedTo?.(saved, from, fromWs)
|
||||
}}
|
||||
>
|
||||
|
||||
@@ -1480,8 +1480,8 @@
|
||||
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 — after
|
||||
// which this discard's outcome is no longer about what is on screen.
|
||||
// and an inline host can re-point the editor in the meantime. What it
|
||||
// removed is still that schedule, which is what the report is about.
|
||||
const from = initialPath
|
||||
const fromWs = wsId
|
||||
const wasDraftOnly = draftOnly
|
||||
@@ -1491,8 +1491,7 @@
|
||||
const landed = await flushDraftDelete('trigger_schedule', from, {
|
||||
workspace: fromWs ?? undefined
|
||||
})
|
||||
if (wasDraftOnly && landed && from && fromWs && initialPath === from)
|
||||
onRemoved?.(from, fromWs)
|
||||
if (wasDraftOnly && landed && from && fromWs) onRemoved?.(from, fromWs)
|
||||
}}
|
||||
disabled={!can_write}
|
||||
/>
|
||||
|
||||
@@ -620,6 +620,7 @@
|
||||
workspace: string
|
||||
/** The path it wrote to; absent when the write removed the item. */
|
||||
to?: string
|
||||
fromTabId: string
|
||||
}) {
|
||||
const page = entityListPage(ev.kind)?.path
|
||||
if (!page) return
|
||||
@@ -633,9 +634,11 @@
|
||||
// The tab's own location, so the list it came from keeps its filters.
|
||||
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' })
|
||||
// Same path: nothing to re-point, but every OTHER editor on it holds a
|
||||
// baseline the deploy has replaced and would offer to discard back to it.
|
||||
// The reporting one has settled its own, including an edit typed while the
|
||||
// save was in flight, which a remount would re-read from under.
|
||||
else if (tab.id !== ev.fromTabId) tabHosts[tabKey(s.id, tab.id)]?.reload({ entity: 'refresh' })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user