fix(graph): validate the whole max_concurrency template string

A string max_concurrency passed the load-time check whenever it merely
contained a {{...}} pair, so forms like "n={{k}}" or "{{ key }}" that
can never resolve at runtime were only caught after the map's inputs had
been computed. The check now requires the trimmed string to be exactly
one template reference, matching what interpolate_raw resolves as a pure
reference; padded forms like " {{budget}} " stay valid.

Also: the bash tool build-failure context names the source file instead
of the destination path, and the map-subgraph validator no longer
shadows its member id list with the resolved member nodes.
This commit is contained in:
Dark-Alex-17 committed 2026-09-11 10:56:28 -06:00
1 parent ccec38c5b9
commit 727cab11ff
3 files changed
+97 -13

No files matched your search

+42
View File
@@ -320,6 +320,16 @@ pub(super) fn template_root_keys(template: &str) -> Vec<String> {
.collect()
}
// Whether `s` is exactly one `{{key}}` reference and nothing else, after the
// same trim `interpolate_raw` applies, so this accepts exactly the strings the
// runtime resolves as a pure reference.
pub(super) fn is_lone_template(s: &str) -> bool {
let s = s.trim();
TEMPLATE_VAR_RE
.find(s)
.is_ok_and(|m| m.is_some_and(|m| m.start() == 0 && m.end() == s.len()))
}
fn value_to_string(value: &Value) -> String {
match value {
Value::String(s) => s.clone(),
@@ -931,6 +941,38 @@ mod tests {
assert_eq!(result, json!("v"));
}
#[test]
fn is_lone_template_cases() {
for accepted in [
"{{budget}}",
"{{cfg.limits[0]}}",
" {{budget}} ",
"\n{{k}}\t",
] {
assert!(
is_lone_template(accepted),
"{accepted:?} should be a lone template"
);
}
for rejected in [
"",
"4",
"{{}}",
"{{ key }}",
"n={{k}}",
"{{k}}x",
"{{a}} {{b",
"{{a}}{{b}}",
"{{a-b}}",
"{{k",
] {
assert!(
!is_lone_template(rejected),
"{rejected:?} must not be a lone template"
);
}
}
#[test]
fn interpolate_raw_inner_spaces_treated_as_mixed() {
let manager = manager_with(&[("k", json!("v"))]);
+54 -12
View File
@@ -1,4 +1,4 @@
use super::state::template_root_keys;
use super::state::{is_lone_template, template_root_keys};
use super::types::{ConcurrencyCap, Graph, NextTargets, Node, NodeType};
use crate::client::{Model, ModelType};
use crate::config;
@@ -557,12 +557,12 @@ impl GraphValidator {
for (node_id, node) in &graph.nodes {
if let NodeType::Map(m) = &node.node_type
&& let Some(ConcurrencyCap::Template(t)) = &m.max_concurrency
&& !contains_template(t)
&& !is_lone_template(t)
{
result.error(ValidationError::with_node(
node_id,
"map node's `max_concurrency` is a string but not a template; write \
an integer or a `{{key}}` template",
an integer or exactly one `{{key}}` with nothing around it",
));
}
}
@@ -654,12 +654,13 @@ impl GraphValidator {
));
}
// A missing entry is reported by `validate_node_references`.
let mut members: Vec<String> = branch_subgraph(graph, &m.branch).into_iter().collect();
if members.is_empty() {
let mut member_ids: Vec<String> =
branch_subgraph(graph, &m.branch).into_iter().collect();
if member_ids.is_empty() {
continue;
}
members.sort();
let members: Vec<(&str, &Node)> = members
member_ids.sort();
let members: Vec<(&str, &Node)> = member_ids
.iter()
.filter_map(|id| graph.get_node(id).map(|n| (id.as_str(), n)))
.collect();
@@ -1156,11 +1157,6 @@ fn primary_templated_fields(node: &Node) -> Vec<String> {
}
}
fn contains_template(s: &str) -> bool {
s.find("{{")
.is_some_and(|open| s[open + 2..].contains("}}"))
}
fn node_state_updates_map(node: &Node) -> Option<&std::collections::HashMap<String, String>> {
match &node.node_type {
NodeType::Llm(n) => n.state_updates.as_ref(),
@@ -3628,6 +3624,52 @@ mod tests {
);
}
#[test]
fn map_max_concurrency_malformed_templates_error() {
for cap in ["{{}}", "{{ key }}", "n={{k}}", "{{a}} {{b", "{{a-b}}"] {
let map = map_with_cap("m", "br", Some("end"), cap);
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's `max_concurrency` is a string but not a template; write \
an integer or exactly one `{{key}}` with nothing around it"
&& e.node_id.as_deref() == Some("m")),
"cap {cap:?} should fail the whole-string template check: {:?}",
result.errors
);
}
}
#[test]
fn map_max_concurrency_lone_template_forms_pass() {
for cap in ["{{budget}}", "{{cfg.limits[0]}}", " {{budget}} "] {
let map = map_with_cap("m", "br", Some("end"), cap);
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("is a string but not a template")),
"cap {cap:?} is a lone template and must pass: {:?}",
result.errors
);
}
}
#[test]
fn map_max_concurrency_template_reading_sibling_write_errors() {
let mut start = end_node("start");