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.
This commit is contained in:
1 file changed
+20
-3
+20
-3
@@ -300,8 +300,10 @@ fn wants_peer_identity(node: &Node) -> bool {
|
|||||||
|
|
||||||
/// A templated cap is resolved against the parent state right before the
|
/// 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
|
/// 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
|
/// accepted; anything else is the author's bug and surfaces as an error
|
||||||
/// as an error rather than being clamped.
|
/// 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(
|
fn resolve_max_concurrency(
|
||||||
node: &MapNode,
|
node: &MapNode,
|
||||||
state: &StateManager,
|
state: &StateManager,
|
||||||
@@ -310,7 +312,10 @@ fn resolve_max_concurrency(
|
|||||||
) -> Result<usize> {
|
) -> Result<usize> {
|
||||||
let template = match &node.max_concurrency {
|
let template = match &node.max_concurrency {
|
||||||
None => return Ok(default.max(1)),
|
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,
|
Some(ConcurrencyCap::Template(t)) => t,
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -531,6 +536,18 @@ mod tests {
|
|||||||
assert_eq!(resolve(Some(ConcurrencyCap::Fixed(4)), &state).unwrap(), 4);
|
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]
|
#[test]
|
||||||
fn resolve_max_concurrency_none_falls_back_to_step_default() {
|
fn resolve_max_concurrency_none_falls_back_to_step_default() {
|
||||||
let state = StateManager::new(HashMap::new());
|
let state = StateManager::new(HashMap::new());
|
||||||
|
|||||||
Reference in new issue
Block a user