mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: skipped suspend step no longer parks the flow forever (#9821)
* fix: skipped suspend step no longer parks the flow forever A flow step that declares a `suspend` (approval) but is skipped via `skip_if` was leaving the flow stuck waiting for a resume that would never arrive. Suspend gates the *next* step: before pushing step N, `needs_resume` checks whether step N-1 declared a non-zero `suspend` and finished as `Success`. A step skipped via `skip_if` is also recorded as `FlowStatusModule::Success` (with `skipped: true`), so `needs_resume` treated a skipped approval gate as a real one and parked the flow waiting for an event that nothing ever sends — until the suspend timeout (up to 24h). The skip is most visible when the skipped suspend step is followed by a branch/subflow: the flow appears stuck on the *following* predicate node with a generic resume button, while none of the branch/subflow steps ran. Fix: honor the `skipped` flag in `needs_resume` and do not gate the next step on a suspend that was skipped. Adds regression test `skipped_suspend_step_does_not_block_next_step` (times out without the fix, completes with it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: reword regression test comment as a current invariant Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
1c4bb8af14
commit
40110bc715
@@ -628,4 +628,53 @@ mod suspend_resume {
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A step that declares a `suspend` but is skipped via `skip_if` never arms
|
||||
/// its approval, so it must not gate the following step. If it did, the flow
|
||||
/// would park forever waiting for a resume event that is never dispatched.
|
||||
#[cfg(feature = "deno_core")]
|
||||
#[sqlx::test(fixtures("base"))]
|
||||
async fn skipped_suspend_step_does_not_block_next_step(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
|
||||
let flow: FlowValue = serde_json::from_value(json!({
|
||||
"modules": [
|
||||
{
|
||||
"id": "a",
|
||||
"skip_if": { "type": "javascript", "expr": "true" },
|
||||
"suspend": { "required_events": 1, "timeout": 86400 },
|
||||
"value": {
|
||||
"type": "rawscript",
|
||||
"language": "deno",
|
||||
"content": "export async function main() { return 1 }",
|
||||
"input_transforms": {},
|
||||
},
|
||||
},
|
||||
{
|
||||
"id": "b",
|
||||
"value": {
|
||||
"type": "rawscript",
|
||||
"language": "deno",
|
||||
"content": "export async function main() { return 42 }",
|
||||
"input_transforms": {},
|
||||
},
|
||||
},
|
||||
],
|
||||
}))
|
||||
.unwrap();
|
||||
|
||||
let result =
|
||||
RunJob::from(JobPayload::RawFlow { value: flow, path: None, restarted_from: None })
|
||||
.run_until_complete(&db, false, server.addr.port())
|
||||
.await
|
||||
.json_result()
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(result, json!(42));
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6211,8 +6211,15 @@ fn needs_resume(flow: &FlowValue, status: &FlowStatus) -> Option<(Suspend, Uuid)
|
||||
return None;
|
||||
}
|
||||
|
||||
if let &FlowStatusModule::Success { job, .. } = status.modules.get(prev)? {
|
||||
Some((suspend.unwrap(), job))
|
||||
if let &FlowStatusModule::Success { job, skipped, .. } = status.modules.get(prev)? {
|
||||
// A step skipped via skip_if never ran, so its suspend/approval was never
|
||||
// armed and no resume event will ever arrive. Gating the next step on it
|
||||
// would park the flow forever.
|
||||
if skipped {
|
||||
None
|
||||
} else {
|
||||
Some((suspend.unwrap(), job))
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user