starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-10-29 08:12:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(sessions): seed the run form as it is built, not after it mounts

Browse Source 
The form takes what it opens holding as a prop, and the page renders it under
the call's id. A request arriving on a tab that is already open builds a new
form already filled, so there is no mounted instance to reach for and nothing to
wait on — which is all the seeding effect was doing.

That also settles where the hand-back reads its call: with no instance to track,
`onDestroy` reads the prop, which is still there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem Lemouel
2026-10-07 07:33:14 -07:00
co-authored by Claude Opus 5
parent 5369802585
commit 8fb8d2a329
3 changed files with 78 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/lib/components/RunForm.svelte
+16
View File
@@ -146,6 +146,12 @@
* if the press then ends before `runAction`, so a guard that counts presses can stop
* counting this one. */
claimRun?: { claim: () => boolean; release: () => void }
/** What the form opens holding, applied as it is created rather than pushed in after.
* For a caller whose arguments arrive with the form itself — render it under `{#key}`
* and a new set makes a new form, already filled, with no mounted instance to reach
* for. Ignored on later renders of the same instance: from then on the arguments are
* the reader's. */
initialArgs?: Record<string, any>
/** Take no writes from the reader: no edits, and no dynamic-select helper. Both write
* before Run is ever pressed — a `password` field mints an ephemeral variable as it is
* typed, and the helper runs the `dynselect-` entrypoint when the field mounts — so
@@ -179,6 +185,7 @@
let {
runnable,
runAction,
initialArgs = undefined,
claimRun = undefined,
argsReadonly = false,
buttonText = 'Run',
@@ -199,6 +206,15 @@
actions = undefined
}: Props = $props()
// Once, as this instance is created, so the form renders already holding them — the
// `reloadArgs`/`syncJsonEditor` that `setArgs` needs are for re-seeding a form that is
// already on screen, which a fresh instance is not.
if (initialArgs !== undefined) {
const { scriptArgs, commonParams } = extractPsCommonParams(initialArgs)
args = scriptArgs
psCommonParams = commonParams
}
let showPsCommonParams = $derived(
runnable?.language === 'powershell' && runnable?.schema?.['x-windmill-ps-cmd-binding'] === true
)
frontend/src/lib/components/details/FlowDetail.svelte
+31 -44
View File
@@ -388,31 +388,12 @@
}
})
// Seeded once per call rather than once per mount: a tab already showing this item is
// reused for the next request, so a latch on "seeded" would leave the previous call's
// arguments on screen.
//
// An effect because neither end of this is an event: the form does not exist when the
// reader presses Preview — the page fetches the deployed item first — so there is nothing
// to seed until `runForm` lands, and a call arriving on an already-open tab has to seed a
// form that is already mounted. `onMount` answers neither.
let seededCallId: string | undefined = undefined
let seededCall: PendingRun | undefined = undefined
$effect(() => {
if (!pendingRun || !runForm) return
if (seededCallId === pendingRun.toolCallId) return
seededCallId = pendingRun.toolCallId
seededCall = pendingRun
runForm.setArgs(pendingRun.args ?? {})
})
// The form here is a copy, so what the reader typed into it would be lost when the call
// goes back to the card — the card would then offer, and run, the arguments this page
// opened with. Written on the way out rather than per keystroke.
//
// The call this page was holding, not whichever one it holds now: by the time this runs
// the tab has usually already stopped naming it, which is what ended the ownership.
onDestroy(() => seededCall?.handBack(args ?? {}))
// opened with. Written on the way out rather than per keystroke: every way the page stops
// carrying a call and still holds edits destroys it, since the tab either closes or
// changes url, and `loadKey` remounts on that.
onDestroy(() => pendingRun?.handBack(args ?? {}))
// The dev workspace's editor is not one the session panel can host, so from a preview tab
// it opens in a new browser tab, as the session editors' own entry does.
@@ -937,27 +918,33 @@
/>
{/if}
<RunForm
bind:scheduledForStr
bind:invisible_to_owner
bind:overrideTag
{overrideTagNote}
syncArgsToUrl={!embedded}
viewKeybinding
{loading}
autofocus
detailed={false}
bind:isValid
runnable={flow}
{runAction}
claimRun={pendingRun}
argsReadonly={pendingRun?.planModeActive}
bind:args
schedulable={!pendingRun}
bind:this={runForm}
{jsonView}
actions={promptForAi ? undefined : aiAssistant}
/>
<!-- Keyed on the call, so a request arriving on a tab that is already open
builds a new form already holding what it proposed. Re-seeding one
that is already on screen would have to wait for it to exist. -->
{#key pendingRun?.toolCallId}
<RunForm
initialArgs={pendingRun?.args}
bind:scheduledForStr
bind:invisible_to_owner
bind:overrideTag
{overrideTagNote}
syncArgsToUrl={!embedded}
viewKeybinding
{loading}
autofocus
detailed={false}
bind:isValid
runnable={flow}
{runAction}
claimRun={pendingRun}
argsReadonly={pendingRun?.planModeActive}
bind:args
schedulable={!pendingRun}
bind:this={runForm}
{jsonView}
actions={promptForAi ? undefined : aiAssistant}
/>
{/key}
</div>
<div class="pt-4 flex flex-col gap-1 w-full items-end">
frontend/src/lib/components/details/ScriptDetail.svelte
+31 -44
View File
@@ -437,31 +437,12 @@
}
})
// Seeded once per call rather than once per mount: a tab already showing this item is
// reused for the next request, so a latch on "seeded" would leave the previous call's
// arguments on screen.
//
// An effect because neither end of this is an event: the form does not exist when the
// reader presses Preview — the page fetches the deployed item first — so there is nothing
// to seed until `runForm` lands, and a call arriving on an already-open tab has to seed a
// form that is already mounted. `onMount` answers neither.
let seededCallId: string | undefined = undefined
let seededCall: PendingRun | undefined = undefined
$effect(() => {
if (!pendingRun || !runForm) return
if (seededCallId === pendingRun.toolCallId) return
seededCallId = pendingRun.toolCallId
seededCall = pendingRun
runForm.setArgs(pendingRun.args ?? {})
})
// The form here is a copy, so what the reader typed into it would be lost when the call
// goes back to the card — the card would then offer, and run, the arguments this page
// opened with. Written on the way out rather than per keystroke.
//
// The call this page was holding, not whichever one it holds now: by the time this runs
// the tab has usually already stopped naming it, which is what ended the ownership.
onDestroy(() => seededCall?.handBack(args ?? {}))
// opened with. Written on the way out rather than per keystroke: every way the page stops
// carrying a call and still holds edits destroys it, since the tab either closes or
// changes url, and `loadKey` remounts on that.
onDestroy(() => pendingRun?.handBack(args ?? {}))
// Read once on purpose: these args seed the form, so tracking the fragment would
// overwrite what the user has typed whenever it changes.
@@ -1145,27 +1126,33 @@
onReject={() => pendingRun.decline()}
/>
{/if}
<RunForm
bind:scheduledForStr
bind:invisible_to_owner
bind:overrideTag
{overrideTagNote}
syncArgsToUrl={!embedded}
viewKeybinding
loading={runLoading}
autofocus
detailed={false}
bind:isValid
runnable={script}
{runAction}
claimRun={pendingRun}
argsReadonly={pendingRun?.planModeActive}
bind:args
schedulable={!pendingRun}
bind:this={runForm}
{jsonView}
actions={promptForAi ? undefined : aiAssistant}
/>
<!-- Keyed on the call, so a request arriving on a tab that is already open
builds a new form already holding what it proposed. Re-seeding one
that is already on screen would have to wait for it to exist. -->
{#key pendingRun?.toolCallId}
<RunForm
initialArgs={pendingRun?.args}
bind:scheduledForStr
bind:invisible_to_owner
bind:overrideTag
{overrideTagNote}
syncArgsToUrl={!embedded}
viewKeybinding
loading={runLoading}
autofocus
detailed={false}
bind:isValid
runnable={script}
{runAction}
claimRun={pendingRun}
argsReadonly={pendingRun?.planModeActive}
bind:args
schedulable={!pendingRun}
bind:this={runForm}
{jsonView}
actions={promptForAi ? undefined : aiAssistant}
/>
{/key}
</div>
<div class="pt-4 flex flex-row gap-1 w-full justify-end items-center">