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) -> 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.` 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,