From ccec38c5b981ee5eee04ace5f28de13d38de89b0 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 10 Sep 2026 17:52:51 -0600 Subject: [PATCH] fix(graph): reject as == output_key on map nodes A map node whose item binding shares its name with output_key would have the binding collected as the chain's result whenever the chain never wrote it, while the missing-writer warning stayed silent. The collision is now a validator error attributed to the map node and, with validation off, a runtime error in run_map before any item forks. --- src/graph/map.rs | 56 +++++++++++++++++++++++++++++++++++++++ src/graph/validator.rs | 60 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/src/graph/map.rs b/src/graph/map.rs index ebf8f68..38ceb9f 100644 --- a/src/graph/map.rs +++ b/src/graph/map.rs @@ -55,6 +55,13 @@ async fn run_map( node.branch ); } + if node.as_name == node.output_key { + bail!( + "map node '{node_id}': `as` and `output_key` are both '{}'; the item binding \ + would be collected as the chain's result", + node.as_name + ); + } let subgraph = Arc::new(branch_subgraph(&step_ctx.graph, &node.branch)); let max_conc = resolve_max_concurrency(node, state, step_ctx.max_concurrency, node_id)?; @@ -810,6 +817,55 @@ nodes: } } + /// With validation off, the collision is still caught before any item + /// forks: the branch never runs. + #[tokio::test] + async fn map_as_equal_output_key_fails_at_runtime() { + if !cmd_available("bash") { + eprintln!("skipping: bash not available"); + return; + } + let ws = TestWorkspace::new(); + let sentinel = ws.dir.join("ran"); + ws.write_script( + "mark.sh", + &format!("#!/bin/bash\ntouch '{}'\necho '{{}}'\n", sentinel.display()), + ); + + let yaml = r#" +name: as_output_collision_test +start: fan_out +settings: + validate_before_run: false +initial_state: + items: [1, 2] +nodes: + fan_out: + type: map + over: "{{items}}" + as: output + branch: mark + collect_into: results + next: done + mark: + type: script + script: mark.sh + done: + type: end + output: "{{results}}" +"#; + let chain = error_chain(run_graph(yaml, &ws).await); + + assert!( + chain.contains( + "map node 'fan_out': `as` and `output_key` are both 'output'; the item binding \ + would be collected as the chain's result" + ), + "{chain}" + ); + assert!(!sentinel.exists(), "no item chain may start"); + } + #[tokio::test] async fn map_chain_two_script_steps_route_via_next() { if !cmd_available("python3") { diff --git a/src/graph/validator.rs b/src/graph/validator.rs index 99996c1..e325f86 100644 --- a/src/graph/validator.rs +++ b/src/graph/validator.rs @@ -643,6 +643,16 @@ impl GraphValidator { let NodeType::Map(m) = &node.node_type else { continue; }; + if m.as_name == m.output_key { + result.error(ValidationError::with_node( + map_id, + format!( + "map node '{map_id}': `as` and `output_key` are both '{}'; the item \ + binding would be collected as the chain's result", + m.as_name + ), + )); + } // A missing entry is reported by `validate_node_references`. let mut members: Vec = branch_subgraph(graph, &m.branch).into_iter().collect(); if members.is_empty() { @@ -3505,6 +3515,56 @@ mod tests { map } + fn map_with_as(id: &str, branch: &str, next: Option<&str>, as_name: &str) -> Node { + let mut map = map_node_basic(id, branch, next); + if let NodeType::Map(ref mut mm) = map.node_type { + mm.as_name = as_name.into(); + } + map + } + + #[test] + fn map_as_equal_output_key_errors() { + let map = map_with_as("m", "br", Some("end"), "output"); + let branch = llm_with_state_updates("br", &[("output", "{{output}}")], None); + let graph = graph_with( + vec![("m", map), ("br", branch), ("end", end_node("end"))], + "m", + ); + + let result = validator().validate(&graph); + + assert!( + result.errors.iter().any(|e| e.message + == "map node 'm': `as` and `output_key` are both 'output'; the item binding \ + would be collected as the chain's result" + && e.node_id.as_deref() == Some("m")), + "expected as/output_key collision error: {:?}", + result.errors + ); + } + + #[test] + fn map_as_distinct_from_output_key_passes() { + let map = map_with_as("m", "br", Some("end"), "item"); + let branch = llm_with_state_updates("br", &[("output", "{{output}}")], None); + let graph = graph_with( + vec![("m", map), ("br", branch), ("end", end_node("end"))], + "m", + ); + + let result = validator().validate(&graph); + + assert!( + !result + .errors + .iter() + .any(|e| e.message.contains("`as` and `output_key`")), + "distinct as/output_key must not error: {:?}", + result.errors + ); + } + #[test] fn map_max_concurrency_non_template_string_errors() { let map = map_with_cap("m", "br", Some("end"), "4");