mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-14 08:02:58 +00:00
Revert "fix: claim a run before the form mints its secrets, not after"
This reverts6c2ad4bbf6and the claim it built on in5ea8769efe, leaving the hand-off rule from93ac4f495ein place. Both surfaces of a parked call show a Run button, and pressing both starts two jobs. That is two deliberate confirmations of one proposal, and pressing Run twice on a deployed page has always started two jobs — nothing in Windmill deduplicates a run, because a run is not an idempotent operation. The window is one job-creation round trip. Covering it cost a prop on `RunForm`, which pages outside this feature use, to coordinate something that only arises in a session's preview panel. It also created a problem of its own: a losing press minted an ephemeral variable per `password` argument for a run it then refused, leaving the workspace with secrets belonging to no job. Without the claim there is no losing press, so every mint belongs to a job that ran. What remains true and intended: the second job is a normal run, listed and reported like any other, and the chat follows only the one that answered its call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
93ac4f495e
commit
ed559e78c2
@@ -91,10 +91,6 @@
|
||||
if (blockedByUnparseable) {
|
||||
return
|
||||
}
|
||||
// Last gate before the first write: everything below either mints or sends.
|
||||
if (claimRun && !claimRun()) {
|
||||
return
|
||||
}
|
||||
let processedArgs: Record<string, any>
|
||||
const { args: withDefaults, resetKeys } = enforceDisabledDefaults(args ?? {}, runnable?.schema)
|
||||
if (resetKeys.length > 0) {
|
||||
@@ -103,7 +99,6 @@
|
||||
try {
|
||||
processedArgs = await processSecretArgs(withDefaults, runnable?.schema, $operatingWorkspace)
|
||||
} catch (e) {
|
||||
releaseRun?.()
|
||||
sendUserToast('Failed to process sensitive args: ' + e, true)
|
||||
return
|
||||
}
|
||||
@@ -148,18 +143,6 @@
|
||||
* for JSON or back mounts a `password` widget, which mints from the literal it opens
|
||||
* on regardless of being disabled. */
|
||||
runRefusal?: string
|
||||
/** Take whatever this press has to own before anything is written, and return false to
|
||||
* drop it. Behind the same boundary as `runRefusal` and for the same reason: the
|
||||
* secret processing mints a variable per `password` argument, so a claim made in
|
||||
* `runAction` comes after that write. For a caller whose run can be started from a
|
||||
* second surface — the chat card holding the same tool call — where one press winning
|
||||
* has to stop the other from minting, not just from starting a job. Called once per
|
||||
* press that reaches the mint, and says itself why a press was dropped. */
|
||||
claimRun?: () => boolean
|
||||
/** Hand back what `claimRun` took, for a press that claimed and then never reached
|
||||
* `runAction` — the secret processing in between can still fail. A press that does
|
||||
* reach it releases through the caller's own start path instead. */
|
||||
releaseRun?: () => void
|
||||
runAction: (
|
||||
scheduledForStr: string | undefined,
|
||||
args: Record<string, any>,
|
||||
@@ -194,8 +177,6 @@
|
||||
runnable,
|
||||
runAction,
|
||||
runRefusal = undefined,
|
||||
claimRun = undefined,
|
||||
releaseRun = undefined,
|
||||
buttonText = 'Run',
|
||||
schedulable = true,
|
||||
detailed = true,
|
||||
|
||||
@@ -290,30 +290,6 @@
|
||||
}
|
||||
}
|
||||
|
||||
/** Held between `claimRun` and `runFlow`: the form mints its `password` arguments in
|
||||
* between, so the claim has to be taken before that write while the job is started after
|
||||
* it. Cleared as it is used, so a press that never claimed cannot inherit it. */
|
||||
let claimedCall: PendingRun | undefined = undefined
|
||||
|
||||
/** One call is one job, and the chat card's form is live alongside this page's: the first
|
||||
* of the two to claim owns the call, and the other stops here — before it mints a
|
||||
* variable per `password` argument for a run it will not start. */
|
||||
function claimRun(): boolean {
|
||||
const call = pendingRun
|
||||
if (call && !call.beginSubmit()) {
|
||||
sendUserToast('This run is already being started from the chat.', true)
|
||||
return false
|
||||
}
|
||||
claimedCall = call
|
||||
return true
|
||||
}
|
||||
|
||||
/** The form claimed and then never reached the run — its secret processing failed. */
|
||||
function releaseRun() {
|
||||
claimedCall?.endSubmit()
|
||||
claimedCall = undefined
|
||||
}
|
||||
|
||||
async function runFlow(
|
||||
scheduledForStr: string | undefined,
|
||||
args: Record<string, any>,
|
||||
@@ -323,12 +299,7 @@
|
||||
// Taken before the round trip below: the call this press answers is the one it was
|
||||
// pressed under, and a call settled meanwhile — declined, then another proposed for
|
||||
// this same flow — must not be answered with a job it never asked for.
|
||||
// The call this press claimed, not whatever is pending now: the form mints its secrets
|
||||
// between the claim and here, and a call that settled across that round trip would
|
||||
// leave this press looking like a page run of its own — starting a second job for a
|
||||
// call the chat has already answered.
|
||||
const call = claimedCall
|
||||
claimedCall = undefined
|
||||
const call = pendingRun
|
||||
loading = true
|
||||
const scheduledFor = scheduledForStr ? new Date(scheduledForStr).toISOString() : undefined
|
||||
try {
|
||||
@@ -347,9 +318,6 @@
|
||||
if (!call?.adopt(run, args, flow?.schema ?? {}))
|
||||
await onNavigate('/run/' + run + '?workspace=' + workspace)
|
||||
} catch (e) {
|
||||
// No job came of the claim, so hand it back rather than leaving the call unrunnable
|
||||
// on either surface.
|
||||
call?.endSubmit()
|
||||
throw e
|
||||
} finally {
|
||||
loading = false
|
||||
@@ -953,8 +921,6 @@
|
||||
bind:isValid
|
||||
runnable={flow}
|
||||
runAction={runFlow}
|
||||
{claimRun}
|
||||
{releaseRun}
|
||||
runRefusal={pendingRun?.planModeActive
|
||||
? PLAN_MODE_MESSAGES.runFormRefused
|
||||
: undefined}
|
||||
|
||||
@@ -379,30 +379,6 @@
|
||||
let isValid = $state(true)
|
||||
|
||||
let runLoading = $state(false)
|
||||
/** Held between `claimRun` and `runScript`: the form mints its `password` arguments in
|
||||
* between, so the claim has to be taken before that write while the job is started after
|
||||
* it. Cleared as it is used, so a press that never claimed cannot inherit it. */
|
||||
let claimedCall: PendingRun | undefined = undefined
|
||||
|
||||
/** One call is one job, and the chat card's form is live alongside this page's: the first
|
||||
* of the two to claim owns the call, and the other stops here — before it mints a
|
||||
* variable per `password` argument for a run it will not start. */
|
||||
function claimRun(): boolean {
|
||||
const call = pendingRun
|
||||
if (call && !call.beginSubmit()) {
|
||||
sendUserToast('This run is already being started from the chat.', true)
|
||||
return false
|
||||
}
|
||||
claimedCall = call
|
||||
return true
|
||||
}
|
||||
|
||||
/** The form claimed and then never reached the run — its secret processing failed. */
|
||||
function releaseRun() {
|
||||
claimedCall?.endSubmit()
|
||||
claimedCall = undefined
|
||||
}
|
||||
|
||||
async function runScript(
|
||||
scheduledForStr: string | undefined,
|
||||
args: Record<string, any>,
|
||||
@@ -412,12 +388,7 @@
|
||||
// Taken before the round trip below: the call this press answers is the one it was
|
||||
// pressed under, and a call settled meanwhile — declined, then another proposed for
|
||||
// this same script — must not be answered with a job it never asked for.
|
||||
// The call this press claimed, not whatever is pending now: the form mints its secrets
|
||||
// between the claim and here, and a call that settled across that round trip would
|
||||
// leave this press looking like a page run of its own — starting a second job for a
|
||||
// call the chat has already answered.
|
||||
const call = claimedCall
|
||||
claimedCall = undefined
|
||||
const call = pendingRun
|
||||
try {
|
||||
runLoading = true
|
||||
const scheduledFor = scheduledForStr ? new Date(scheduledForStr).toISOString() : undefined
|
||||
@@ -449,9 +420,6 @@
|
||||
if (!call?.adopt(run, args, script?.schema ?? {}))
|
||||
await onNavigate('/run/' + run + '?workspace=' + workspace)
|
||||
} catch (err) {
|
||||
// No job came of the claim, so hand it back rather than leaving the call unrunnable
|
||||
// on either surface.
|
||||
call?.endSubmit()
|
||||
sendUserToast(`Could not create job: ${err.body}`, true)
|
||||
} finally {
|
||||
// Not only on failure: a navigation that keeps this component mounted — the run
|
||||
@@ -1168,8 +1136,6 @@
|
||||
bind:isValid
|
||||
runnable={script}
|
||||
runAction={runScript}
|
||||
{claimRun}
|
||||
{releaseRun}
|
||||
runRefusal={pendingRun?.planModeActive
|
||||
? PLAN_MODE_MESSAGES.runFormRefused
|
||||
: undefined}
|
||||
|
||||
@@ -24,13 +24,4 @@ export type PendingRun = {
|
||||
* alone. */
|
||||
adopt: (jobId: string, args: Record<string, any>, schema: Record<string, any>) => boolean
|
||||
decline: () => void
|
||||
/** Claim the call's one job, before starting one. The card's form is live at the same
|
||||
* time as this page's, and the call stays pending until `adopt` — which is a round trip
|
||||
* later — so without this claim a Run on the card in between starts a second job, with
|
||||
* its own arguments and no mention in the chat. False when the other surface claimed it
|
||||
* first; send nothing and say so. */
|
||||
beginSubmit: () => boolean
|
||||
/** Hand the claim back when no job came of it, so the reader can try again. A started
|
||||
* job needs no release: adopting it settles the call. */
|
||||
endSubmit: () => void
|
||||
}
|
||||
|
||||
@@ -614,11 +614,7 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
return manager.pendingRunFormArgs(toolCallId) ?? {}
|
||||
},
|
||||
adopt: (jobId, args, schema) => manager.handleRunFormJob(toolCallId, jobId, args, schema),
|
||||
decline: () => manager.handleRunFormCancel(toolCallId),
|
||||
// The same claim the card's own form takes, so the two surfaces of one call cannot
|
||||
// both be mid-submit.
|
||||
beginSubmit: () => manager.beginRunFormSubmit(toolCallId),
|
||||
endSubmit: () => manager.endRunFormSubmit(toolCallId)
|
||||
decline: () => manager.handleRunFormCancel(toolCallId)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user