From 874ef7f6ec69dc664c7540b1e13cbeb008f04629 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 10 Sep 2026 11:56:25 -0600 Subject: [PATCH] fix(graph): reject a literal max_concurrency of 0 instead of clamping it Copilot review on PR #24. The validator already rejects 0 and the template path errors on it, but a literal 0 loaded with validate_before_run off was silently clamped to 1. Fail with the same error shape as the template path; only the absent-cap default clamps. --- src/graph/map.rs | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/graph/map.rs b/src/graph/map.rs index b95966c..a2ee287 100644 --- a/src/graph/map.rs +++ b/src/graph/map.rs @@ -300,8 +300,10 @@ fn wants_peer_identity(node: &Node) -> bool { /// A templated cap is resolved against the parent state right before the /// fan-out. Scripts often emit numbers as strings, so a numeric string is -/// accepted; anything else, including 0, is the author's bug and surfaces -/// as an error rather than being clamped. +/// accepted; anything else is the author's bug and surfaces as an error +/// rather than being clamped. A literal 0 is rejected the same way, so a +/// graph loaded with validation off fails here instead of silently running +/// one item at a time. Only the absent-cap default is clamped. fn resolve_max_concurrency( node: &MapNode, state: &StateManager, @@ -310,7 +312,10 @@ fn resolve_max_concurrency( ) -> Result { let template = match &node.max_concurrency { None => return Ok(default.max(1)), - Some(ConcurrencyCap::Fixed(n)) => return Ok((*n).max(1)), + Some(ConcurrencyCap::Fixed(0)) => { + bail!("map node '{node_id}': max_concurrency 0; expected a positive integer") + } + Some(ConcurrencyCap::Fixed(n)) => return Ok(*n), Some(ConcurrencyCap::Template(t)) => t, }; @@ -531,6 +536,18 @@ mod tests { assert_eq!(resolve(Some(ConcurrencyCap::Fixed(4)), &state).unwrap(), 4); } + #[test] + fn resolve_max_concurrency_fixed_zero_errors() { + let state = StateManager::new(HashMap::new()); + let msg = resolve(Some(ConcurrencyCap::Fixed(0)), &state) + .unwrap_err() + .to_string(); + assert_eq!( + msg, + "map node 'm': max_concurrency 0; expected a positive integer" + ); + } + #[test] fn resolve_max_concurrency_none_falls_back_to_step_default() { let state = StateManager::new(HashMap::new());