From 56e21bce832182528562688acd13ec416e01ebfc Mon Sep 17 00:00:00 2001
From: Davide Modolo <36373601+davidemodolo@users.noreply.github.com>
Date: Mon, 14 Sep 2026 19:05:45 +0200
Subject: [PATCH] fix(flows): stop re-evaluating skip_if once a loop is in
 progress (#11008)

* fix(flows): stop re-evaluating skip_if once a loop is in progress

skip_if is a one-time entry gate, but the flow stays at the same step
for a loop's whole lifetime, so it gets re-evaluated on every
iteration. previous_id stays pinned to the module preceding the loop,
but once the loop is InProgress the last completed job is an inner
iteration, and the results proxy in windmill-jseval aliases
results.<previous_id> to that job's result. skip_if then reads the
wrong value and can flip the loop's module to skipped after one
iteration.

Skip the check once status_module is already InProgress.

* fix(flows): match skip_if gate to sibling entry-state allowlists

Rewrite the skip_if gate as a positive allowlist (WaitingForPriorSteps
| WaitingForEvents | WaitingForExecutor), matching the shape already
used by the BranchOne/BranchAll predicate gates, instead of a negative
filter on InProgress. Restart-at-iteration also enters as InProgress;
document it as a separate case rather than folding it into the
aliasing reason, which does not apply there.

Add a regression test pinning skip_if to run once at while-loop entry.
---
 backend/tests/worker.rs                    | 77 ++++++++++++++++++++++
 backend/windmill-worker/src/worker_flow.rs | 13 +++-
 2 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/backend/tests/worker.rs b/backend/tests/worker.rs
index 3e2bae5092..b8bd6b30a7 100644
--- a/backend/tests/worker.rs
+++ b/backend/tests/worker.rs
@@ -5649,6 +5649,83 @@ async fn test_whileloop_propagates_inner_iterator_eval_failure(
     Ok(())
 }
 
+#[cfg(all(feature = "quickjs", feature = "python"))]
+#[sqlx::test(fixtures("base"))]
+async fn test_whileloop_skip_if_evaluated_once_at_entry(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+
+    // Regression test for #11007: `skip_if` on a while-loop module must be
+    // evaluated once, at loop entry, using the preceding step's result.
+    // Re-evaluating it on every iteration aliases `results.first` to the
+    // previous iteration's own result instead, which here lacks `.ok` and
+    // makes `skip_if` incorrectly turn true after the first iteration.
+    let port = 123;
+    let flow: FlowValue = serde_json::from_value(serde_json::json!({
+        "modules": [
+            {
+                "id": "first",
+                "value": {
+                    "type": "rawscript",
+                    "language": "python3",
+                    "content": "def main(): return {\"ok\": True}",
+                },
+            },
+            {
+                "id": "outer",
+                "value": {
+                    "type": "whileloopflow",
+                    "skip_failures": false,
+                    "modules": [
+                        {
+                            "id": "inner",
+                            "value": {
+                                "input_transforms": {
+                                    "i": {
+                                        "type": "javascript",
+                                        "expr": "flow_input.iter.index",
+                                    },
+                                },
+                                "type": "rawscript",
+                                "language": "python3",
+                                "content": "def main(i): return i",
+                            },
+                        },
+                    ],
+                },
+                "skip_if": { "expr": "!results.first.ok" },
+                "stop_after_if": {
+                    "expr": "result >= 2",
+                    "skip_if_stopped": false,
+                },
+            },
+        ],
+    }))
+    .unwrap();
+    let job = JobPayload::RawFlow { value: flow, path: None, restarted_from: None };
+
+    let cjob = RunJob::from(job).run_until_complete(&db, false, port).await;
+
+    assert!(cjob.success, "flow should succeed");
+
+    let outer_module = get_module(&cjob, "outer").expect("outer module status");
+    match outer_module {
+        windmill_common::flow_status::FlowStatusModule::Success { skipped, flow_jobs, .. } => {
+            assert!(
+                !skipped,
+                "while-loop must not be skipped: skip_if should only run once, at entry"
+            );
+            assert_eq!(
+                flow_jobs.map(|v| v.len()),
+                Some(3),
+                "while-loop should run 3 iterations before stop_after_if halts it"
+            );
+        }
+        other => panic!("expected outer module to be Success, got {other:?}"),
+    }
+
+    Ok(())
+}
+
 #[cfg(all(feature = "quickjs", feature = "python"))]
 #[sqlx::test(fixtures("base"))]
 async fn test_stop_after_all_iters_if_bad_expr_parallel_branchall(
diff --git a/backend/windmill-worker/src/worker_flow.rs b/backend/windmill-worker/src/worker_flow.rs
index 31dddcaf5f..247e618dab 100644
--- a/backend/windmill-worker/src/worker_flow.rs
+++ b/backend/windmill-worker/src/worker_flow.rs
@@ -3891,7 +3891,18 @@ async fn push_next_flow_job(
 
     drop(resume_messages);
 
-    let is_skipped = if let Some(skip_if) = &module.skip_if {
+    // `skip_if` is a one-time entry gate, so only first-entry statuses evaluate it.
+    // Once the module is looping, the last completed job is an inner iteration, not
+    // `previous_id`'s, and re-evaluating would alias `results.<previous_id>` to it.
+    // A restart-at-iteration also enters as `InProgress`: it resumes without re-gating.
+    let is_skipped = if let Some(skip_if) = module.skip_if.as_ref().filter(|_| {
+        matches!(
+            status_module,
+            FlowStatusModule::WaitingForPriorSteps { .. }
+                | FlowStatusModule::WaitingForEvents { .. }
+                | FlowStatusModule::WaitingForExecutor { .. }
+        )
+    }) {
         let idcontext = get_transform_context(&flow_job, previous_id.as_str(), &status);
         let skip_if_res = compute_bool_from_expr(
             &skip_if.expr,