From 40110bc7158bc42c3d84bd4637a12b82fcd72a9a Mon Sep 17 00:00:00 2001
From: Ruben Fiszel <ruben@windmill.dev>
Date: Fri, 26 Jun 2026 18:56:39 +0200
Subject: [PATCH] fix: skipped suspend step no longer parks the flow forever
 (#9821)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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>
---
 backend/tests/suspend_resume.rs            | 49 ++++++++++++++++++++++
 backend/windmill-worker/src/worker_flow.rs | 11 ++++-
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/backend/tests/suspend_resume.rs b/backend/tests/suspend_resume.rs
index b8468f0cbd..704fa0d256 100644
--- a/backend/tests/suspend_resume.rs
+++ b/backend/tests/suspend_resume.rs
@@ -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(())
+    }
 }
diff --git a/backend/windmill-worker/src/worker_flow.rs b/backend/windmill-worker/src/worker_flow.rs
index 2d504a0dfa..10e8218a77 100644
--- a/backend/windmill-worker/src/worker_flow.rs
+++ b/backend/windmill-worker/src/worker_flow.rs
@@ -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
     }