From d58ef8eb55d1e63af0885dc1d339969deb623804 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Sun, 6 Sep 2026 21:24:13 +0200 Subject: [PATCH] fix(resources): keep ResourceEditor.save() boolean, and report writes through saveWritten MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ResourceEditor` is an entry point of the packaged `windmill-components`, so its `save()` is a public contract: consumers outside this repo branch on the boolean it has always returned, and an array reads as success on every failure. The per-workspace results move to `saveWritten`, which the drawer takes. The released-values doc goes back on the map it describes, with the constant carrying its own — and the kinds it excludes stated as they are: a variable's cell is recorded, decrypted secret and all, since that is a kind whose settle reads the record back. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4 --- .../src/lib/components/ResourceEditor.svelte | 22 +++++++++++++++---- .../components/ResourceEditorDrawer.svelte | 4 ++-- .../copilot/chat/AssistantMcpSection.svelte | 2 +- frontend/src/lib/userDraft.svelte.ts | 9 +++++--- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index 38098a7e68..553cfbc997 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -337,7 +337,22 @@ /** 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 }[]> { + /** + * Whether the save completed, as this component's packaged API has always + * reported it: it is an entry point of `windmill-components`, so callers + * outside this repo branch on the boolean. A host that has to follow what each + * workspace wrote takes `saveWritten` instead. + */ + export async function save(): Promise { + return (await runSave()).ok + } + + /** Every workspace this save wrote, in order, each with the path it wrote there. */ + export async function saveWritten(): Promise<{ ws: string; path: string }[]> { + return (await runSave()).written + } + + async function runSave(): Promise<{ written: { ws: string; path: string }[]; ok: boolean }> { // 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 @@ -349,7 +364,6 @@ ini: $state.snapshot(initialStates[ws]) as ResourceState, existed: !!existedInitially[ws] })) - // 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. @@ -405,13 +419,13 @@ : `Saved resource` ) dispatch('refresh', written.find((w) => w.ws === effectiveWorkspace)?.path ?? from) - return written + return { written, ok: true } } catch (err) { sendUserToast(`Could not save resource: ${err.body ?? err.message}`, true) // 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 + return { written, ok: false } } } diff --git a/frontend/src/lib/components/ResourceEditorDrawer.svelte b/frontend/src/lib/components/ResourceEditorDrawer.svelte index 56f5c7aea4..f3079fb960 100644 --- a/frontend/src/lib/components/ResourceEditorDrawer.svelte +++ b/frontend/src/lib/components/ResourceEditorDrawer.svelte @@ -68,7 +68,7 @@ | { /** 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 }[]> + saveWritten: () => Promise<{ ws: string; path: string }[]> localDraftDeployed: () => unknown localDraftCurrent: () => unknown /** False when the discard removed the resource (it was draft-only). */ @@ -236,7 +236,7 @@ // Closed before the write is awaited, the way it always was: `save()` toasts its // own failures and never rejects, so waiting would only add visible lag to every // caller of this drawer. `onSaved` still fires after the write lands. - const saving = resourceEditor?.save() + const saving = resourceEditor?.saveWritten() drawer?.closeDrawer() // 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. diff --git a/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte b/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte index 24b707756a..30d0445e2c 100644 --- a/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte +++ b/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte @@ -235,7 +235,7 @@ switch that decides whether this chat carries its tools. // that still exists off, and turn on a path that was never created. // `save` reports one entry per workspace it wrote, and none at all when it // wrote nothing. - if (((await resourceEditor?.save()) ?? []).length === 0) return + if (!(await resourceEditor?.save())) return // Enablement is keyed by path, so a rename would leave the switch on the path // that no longer exists and the server itself off. if (editingPath && editingPath !== server.path) { diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index d7794c2b0c..e9db2f1696 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -142,13 +142,16 @@ function noteMarker(set: Set, key: string): void { set.delete(oldest) } } +// Only the kinds whose settle can read the record back. For the rest, recording one +// would retain a whole flow, script or app draft that nothing will ever ask about. +const RELEASE_RECORDED_KINDS: readonly UserDraftItemKind[] = ['resource', 'variable'] + /** * What a cell held when its last holder let go: an editor stays editable while its * save is in flight, so a tab left in that window releases a cell carrying an edit - * newer than the write. Only for the kinds whose settle reads it back — elsewhere - * it would retain a whole flow or app draft, or a variable's decrypted secret. + * newer than the write, and no handle is left to read it from. Same cap and reason + * as the marker sets above. */ -const RELEASE_RECORDED_KINDS: readonly UserDraftItemKind[] = ['resource', 'variable'] const releasedValues = new Map() /** Read once: what it describes is settled by the caller that reads it. */ function takeReleasedValue(