From 3f4fd91b3f54432321f6a385f746ee6a8f49161a Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 14 May 2026 16:35:08 -0600 Subject: [PATCH] fix: added on_other field for approval nodes so users can specify an alternative free-text target when none of the options match what they want --- src/graph/parser.rs | 1 + src/graph/types.rs | 9 ++++++++ src/graph/user_interaction.rs | 39 ++++++++++++++++++++--------------- src/graph/validator.rs | 30 +++++++++++++++++++++++---- 4 files changed, 58 insertions(+), 21 deletions(-) diff --git a/src/graph/parser.rs b/src/graph/parser.rs index eab730d..e2fd513 100644 --- a/src/graph/parser.rs +++ b/src/graph/parser.rs @@ -291,6 +291,7 @@ nodes: routes: "Yes": deploy "No": cancel + on_other: cancel deploy: type: end cancel: diff --git a/src/graph/types.rs b/src/graph/types.rs index c54130c..827fc14 100644 --- a/src/graph/types.rs +++ b/src/graph/types.rs @@ -174,6 +174,13 @@ pub struct ApprovalNode { pub routes: HashMap, + /// REQUIRED. The user_ask tool always permits a free-form "type your + /// own answer" response in addition to the listed `options`. When the + /// user supplies any answer that does NOT match a key in `routes`, + /// execution routes to this node. The free-form text is available to + /// downstream nodes via `state_updates` (e.g. `clarification: "{{choice}}"`). + pub on_other: String, + #[serde(default, skip_serializing_if = "Option::is_none")] pub state_updates: Option>, @@ -427,6 +434,7 @@ nodes: routes: yes: i no: e + on_other: e i: id: i type: input @@ -559,6 +567,7 @@ routes: approve: apply reject: end_reject edit: edit_loop +on_other: edit_loop "#; let node: Node = serde_yaml::from_str(yaml).unwrap(); let approval = match node.node_type { diff --git a/src/graph/user_interaction.rs b/src/graph/user_interaction.rs index ea20713..fb3b4bf 100644 --- a/src/graph/user_interaction.rs +++ b/src/graph/user_interaction.rs @@ -139,14 +139,10 @@ fn build_input_question(node: &InputNode, state_manager: &StateManager) -> Resul } fn resolve_approval_route(node: &ApprovalNode, choice: &str) -> Result { - node.routes.get(choice).cloned().ok_or_else(|| { - let mut available: Vec<&str> = node.routes.keys().map(String::as_str).collect(); - available.sort(); - anyhow!( - "No route defined for choice '{choice}'. Available routes: {}", - available.join(", ") - ) - }) + if let Some(target) = node.routes.get(choice) { + return Ok(target.clone()); + } + Ok(node.on_other.clone()) } fn apply_state_updates_with_var( @@ -234,7 +230,7 @@ mod tests { StateManager::new(map) } - fn approval(options: &[&str], routes: &[(&str, &str)]) -> ApprovalNode { + fn approval(options: &[&str], routes: &[(&str, &str)], on_other: &str) -> ApprovalNode { let mut r = HashMap::new(); for (k, v) in routes { r.insert((*k).into(), (*v).into()); @@ -243,6 +239,7 @@ mod tests { question: "?".into(), options: options.iter().map(|s| (*s).into()).collect(), routes: r, + on_other: on_other.into(), state_updates: None, timeout: None, on_timeout: None, @@ -289,19 +286,27 @@ mod tests { #[test] fn approval_route_lookup_returns_target_on_match() { - let node = approval(&["yes", "no"], &[("yes", "deploy"), ("no", "cancel")]); + let node = approval( + &["yes", "no"], + &[("yes", "deploy"), ("no", "cancel")], + "clarify", + ); assert_eq!(resolve_approval_route(&node, "yes").unwrap(), "deploy"); assert_eq!(resolve_approval_route(&node, "no").unwrap(), "cancel"); } #[test] - fn approval_route_lookup_errors_on_unknown_choice() { - let node = approval(&["yes", "no"], &[("yes", "deploy"), ("no", "cancel")]); - let err = resolve_approval_route(&node, "maybe") - .unwrap_err() - .to_string(); - assert!(err.contains("'maybe'"), "got: {err}"); - assert!(err.contains("yes") && err.contains("no"), "got: {err}"); + fn approval_route_lookup_falls_back_to_on_other_for_unknown_choice() { + let node = approval( + &["yes", "no"], + &[("yes", "deploy"), ("no", "cancel")], + "clarify", + ); + assert_eq!(resolve_approval_route(&node, "maybe").unwrap(), "clarify"); + assert_eq!( + resolve_approval_route(&node, "free-form text").unwrap(), + "clarify" + ); } #[test] diff --git a/src/graph/validator.rs b/src/graph/validator.rs index 6b4be8f..59d1afa 100644 --- a/src/graph/validator.rs +++ b/src/graph/validator.rs @@ -248,6 +248,7 @@ fn declared_targets(node: &Node) -> Vec<(String, &'static str)> { for v in a.routes.values() { out.push((v.clone(), "approval 'routes'")); } + out.push((a.on_other.clone(), "approval 'on_other'")); if let Some(t) = &a.on_timeout { out.push((t.clone(), "'on_timeout'")); } @@ -369,7 +370,7 @@ mod tests { } } - fn approval_node(id: &str, options: &[&str], routes: &[(&str, &str)]) -> Node { + fn approval_node(id: &str, options: &[&str], routes: &[(&str, &str)], on_other: &str) -> Node { let mut r: HashMap = HashMap::new(); for (k, v) in routes { r.insert((*k).into(), (*v).into()); @@ -381,6 +382,7 @@ mod tests { question: "?".into(), options: options.iter().map(|s| (*s).into()).collect(), routes: r, + on_other: on_other.into(), state_updates: None, timeout: None, on_timeout: None, @@ -449,7 +451,12 @@ mod tests { #[test] fn flags_missing_approval_route_target() { - let approval = approval_node("ap", &["yes", "no"], &[("yes", "end"), ("no", "missing")]); + let approval = approval_node( + "ap", + &["yes", "no"], + &[("yes", "end"), ("no", "missing")], + "end", + ); let graph = graph_with(vec![("ap", approval), ("end", end_node("end"))], "ap"); let result = validator().validate(&graph); assert!(!result.is_valid()); @@ -461,6 +468,21 @@ mod tests { ); } + #[test] + fn flags_missing_approval_on_other_target() { + let approval = approval_node("ap", &["yes"], &[("yes", "end")], "missing"); + let graph = graph_with(vec![("ap", approval), ("end", end_node("end"))], "ap"); + let result = validator().validate(&graph); + assert!(!result.is_valid()); + assert!( + result + .errors + .iter() + .any(|e| e.message.contains("non-existent node 'missing'") + && e.message.contains("on_other")) + ); + } + #[test] fn flags_missing_script_fallback_target() { let scr = script_node("s", "does-not-exist.py", Some("nowhere")); @@ -597,7 +619,7 @@ mod tests { #[test] fn errors_when_approval_option_has_no_route() { - let approval = approval_node("ap", &["yes", "no"], &[("yes", "end")]); + let approval = approval_node("ap", &["yes", "no"], &[("yes", "end")], "end"); let graph = graph_with(vec![("ap", approval), ("end", end_node("end"))], "ap"); let result = validator().validate(&graph); assert!( @@ -610,7 +632,7 @@ mod tests { #[test] fn warns_when_approval_has_extra_route() { - let approval = approval_node("ap", &["yes"], &[("yes", "end"), ("maybe", "end")]); + let approval = approval_node("ap", &["yes"], &[("yes", "end"), ("maybe", "end")], "end"); let graph = graph_with(vec![("ap", approval), ("end", end_node("end"))], "ap"); let result = validator().validate(&graph); assert!(result.warnings.iter().any(|w| {