feat: ripped out user input timeout scaffolding for approval and input node types; implementation can't be done cleanly
This commit is contained in:
@@ -219,8 +219,6 @@ nodes:
|
||||
on_other: refine # REQUIRED: route for ANY answer not in `routes`
|
||||
state_updates:
|
||||
decision: "{{choice}}" # {{choice}} = the chosen option OR the free-form text
|
||||
timeout: 300 # Optional: seconds to wait for a response
|
||||
on_timeout: rejected_end # Optional: route taken on interaction timeout
|
||||
|
||||
# --- input node ---------------------------------------------------------
|
||||
# Collects a free-form string from the user.
|
||||
@@ -235,8 +233,6 @@ nodes:
|
||||
# <op> in > >= < <= == . Length only -- no regex.
|
||||
state_updates:
|
||||
refinement: "{{input}}" # {{input}} = the user's text
|
||||
timeout: 120 # Optional
|
||||
on_timeout: rejected_end # Optional: route taken on interaction timeout
|
||||
next: finalize # REQUIRED for input nodes: the success route
|
||||
|
||||
# --- llm node (final synthesis) -----------------------------------------
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
//! Main execution loop for graph workflows.
|
||||
//!
|
||||
//! Dispatches each node to its type-specific executor, handles routing
|
||||
//! (static `Node.next`, script `_next` override, approval `routes`, input
|
||||
//! `on_timeout`), enforces `max_loop_iterations` and an optional
|
||||
//! (static `Node.next`, script `_next` override, approval `routes` and
|
||||
//! `on_other`), enforces `max_loop_iterations` and an optional
|
||||
//! whole-graph timeout, and resolves the final `End` node's `output`
|
||||
//! template as the graph's return value.
|
||||
|
||||
|
||||
@@ -219,12 +219,6 @@ pub struct ApprovalNode {
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub state_updates: Option<HashMap<String, String>>,
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub timeout: Option<u64>,
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub on_timeout: Option<String>,
|
||||
}
|
||||
|
||||
/// `input`-type node: collect free-form text from the user. Routes via the
|
||||
@@ -242,12 +236,6 @@ pub struct InputNode {
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub state_updates: Option<HashMap<String, String>>,
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub timeout: Option<u64>,
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub on_timeout: Option<String>,
|
||||
}
|
||||
|
||||
/// `llm`-type node: a one-shot LLM call (with bounded tool-call loop)
|
||||
@@ -584,8 +572,6 @@ validation: "len(input) > 0"
|
||||
state_updates:
|
||||
api_key: "{{input}}"
|
||||
next: configure
|
||||
timeout: 300
|
||||
on_timeout: skip
|
||||
"#;
|
||||
let node: Node = serde_yaml::from_str(yaml).unwrap();
|
||||
let input = match node.node_type {
|
||||
@@ -595,8 +581,6 @@ on_timeout: skip
|
||||
assert_eq!(input.question, "Enter your API key:");
|
||||
assert_eq!(input.default.as_deref(), Some("{{previous_api_key}}"));
|
||||
assert_eq!(input.validation.as_deref(), Some("len(input) > 0"));
|
||||
assert_eq!(input.timeout, Some(300));
|
||||
assert_eq!(input.on_timeout.as_deref(), Some("skip"));
|
||||
let updates = input.state_updates.unwrap();
|
||||
assert_eq!(
|
||||
updates.get("api_key").map(|s| s.as_str()),
|
||||
@@ -621,8 +605,6 @@ question: "Describe the feature:"
|
||||
assert!(input.default.is_none());
|
||||
assert!(input.validation.is_none());
|
||||
assert!(input.state_updates.is_none());
|
||||
assert!(input.timeout.is_none());
|
||||
assert!(input.on_timeout.is_none());
|
||||
assert!(node.next.is_none());
|
||||
}
|
||||
|
||||
|
||||
@@ -23,8 +23,7 @@ pub struct ApprovalNodeExecutor;
|
||||
impl ApprovalNodeExecutor {
|
||||
/// Prompt the user with the (templated) question and routes the
|
||||
/// selected option through the node's `routes` map. Returns the next
|
||||
/// node ID. On escalation timeout/error the node routes to
|
||||
/// `on_timeout` if set, otherwise propagates the failure.
|
||||
/// node ID. An escalation timeout/error propagates as a failure.
|
||||
pub async fn execute(
|
||||
node: &ApprovalNode,
|
||||
state_manager: &mut StateManager,
|
||||
@@ -43,9 +42,6 @@ impl ApprovalNodeExecutor {
|
||||
.context("user__ask failed")?;
|
||||
|
||||
if let Some(err) = response.get("error").and_then(Value::as_str) {
|
||||
if let Some(on_timeout) = &node.on_timeout {
|
||||
return Ok(on_timeout.clone());
|
||||
}
|
||||
bail!("Approval interaction failed: {err}");
|
||||
}
|
||||
|
||||
@@ -67,8 +63,8 @@ impl InputNodeExecutor {
|
||||
/// Prompt the user for free-form text. If a `default` is configured
|
||||
/// and the user submits an empty response, the default is substituted.
|
||||
/// Optional `validation` is evaluated against the final value. Returns
|
||||
/// `node_next` (the parent `Node.next`) on success, or `on_timeout` on
|
||||
/// escalation timeout/error.
|
||||
/// `node_next` (the parent `Node.next`) on success; an escalation
|
||||
/// timeout/error propagates as a failure.
|
||||
pub async fn execute(
|
||||
node: &InputNode,
|
||||
node_next: Option<&str>,
|
||||
@@ -86,9 +82,6 @@ impl InputNodeExecutor {
|
||||
.context("user__input failed")?;
|
||||
|
||||
if let Some(err) = response.get("error").and_then(Value::as_str) {
|
||||
if let Some(on_timeout) = &node.on_timeout {
|
||||
return Ok(on_timeout.clone());
|
||||
}
|
||||
bail!("Input interaction failed: {err}");
|
||||
}
|
||||
|
||||
@@ -241,8 +234,6 @@ mod tests {
|
||||
routes: r,
|
||||
on_other: on_other.into(),
|
||||
state_updates: None,
|
||||
timeout: None,
|
||||
on_timeout: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -252,8 +243,6 @@ mod tests {
|
||||
default: None,
|
||||
validation: None,
|
||||
state_updates: None,
|
||||
timeout: None,
|
||||
on_timeout: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+4
-58
@@ -3,7 +3,7 @@
|
||||
//! routes-vs-options consistency.
|
||||
//!
|
||||
//! The validator only follows **declared static edges** (`next`, approval
|
||||
//! `routes`, script `fallback`, `on_timeout`). Script nodes can also route
|
||||
//! `routes` and `on_other`, script/llm `fallback`). Script nodes can also route
|
||||
//! dynamically via `_next` in their JSON output at runtime; those edges are
|
||||
//! invisible here. As a result, unreachable-node detection and "no reachable
|
||||
//! End node" are reported as warnings (not errors) to avoid false positives
|
||||
@@ -275,28 +275,20 @@ fn declared_targets(node: &Node) -> Vec<(String, &'static str)> {
|
||||
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'"));
|
||||
}
|
||||
}
|
||||
NodeType::Script(s) => {
|
||||
if let Some(t) = &s.fallback {
|
||||
out.push((t.clone(), "script 'fallback'"));
|
||||
}
|
||||
}
|
||||
NodeType::Input(i) => {
|
||||
if let Some(t) = &i.on_timeout {
|
||||
out.push((t.clone(), "'on_timeout'"));
|
||||
}
|
||||
}
|
||||
NodeType::Llm(l) => {
|
||||
if let Some(t) = &l.fallback {
|
||||
out.push((t.clone(), "llm 'fallback'"));
|
||||
}
|
||||
}
|
||||
// `agent`/`rag` route only via `next` (already collected above);
|
||||
// `end` is terminal. No type-specific routing edges to add.
|
||||
NodeType::Agent(_) | NodeType::Rag(_) | NodeType::End(_) => {}
|
||||
// `agent`/`input`/`rag` route only via `next` (already collected
|
||||
// above); `end` is terminal. No type-specific routing edges to add.
|
||||
NodeType::Agent(_) | NodeType::Input(_) | NodeType::Rag(_) | NodeType::End(_) => {}
|
||||
}
|
||||
out
|
||||
}
|
||||
@@ -418,8 +410,6 @@ mod tests {
|
||||
routes: r,
|
||||
on_other: on_other.into(),
|
||||
state_updates: None,
|
||||
timeout: None,
|
||||
on_timeout: None,
|
||||
}),
|
||||
next: None,
|
||||
}
|
||||
@@ -464,22 +454,6 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn input_node(id: &str, on_timeout: Option<&str>, next: Option<&str>) -> Node {
|
||||
Node {
|
||||
id: id.into(),
|
||||
description: String::new(),
|
||||
node_type: NodeType::Input(InputNode {
|
||||
question: "?".into(),
|
||||
default: None,
|
||||
validation: None,
|
||||
state_updates: None,
|
||||
timeout: None,
|
||||
on_timeout: on_timeout.map(String::from),
|
||||
}),
|
||||
next: next.map(String::from),
|
||||
}
|
||||
}
|
||||
|
||||
fn llm_node(id: &str, fallback: Option<&str>, next: Option<&str>) -> Node {
|
||||
Node {
|
||||
id: id.into(),
|
||||
@@ -502,32 +476,6 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn flags_missing_approval_on_timeout_target() {
|
||||
let mut approval = approval_node("a", &["yes"], &[("yes", "end")], "end");
|
||||
if let NodeType::Approval(ref mut n) = approval.node_type {
|
||||
n.on_timeout = Some("ghost".into());
|
||||
}
|
||||
let graph = graph_with(vec![("a", approval), ("end", end_node("end"))], "a");
|
||||
let result = validator().validate(&graph);
|
||||
assert!(!result.is_valid());
|
||||
assert!(result.errors.iter().any(|e| e.message.contains("ghost")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn flags_missing_input_on_timeout_target() {
|
||||
let graph = graph_with(
|
||||
vec![
|
||||
("i", input_node("i", Some("ghost"), Some("end"))),
|
||||
("end", end_node("end")),
|
||||
],
|
||||
"i",
|
||||
);
|
||||
let result = validator().validate(&graph);
|
||||
assert!(!result.is_valid());
|
||||
assert!(result.errors.iter().any(|e| e.message.contains("ghost")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn flags_missing_llm_fallback_target() {
|
||||
let graph = graph_with(
|
||||
@@ -759,8 +707,6 @@ mod tests {
|
||||
default: None,
|
||||
validation: None,
|
||||
state_updates: None,
|
||||
timeout: None,
|
||||
on_timeout: None,
|
||||
}),
|
||||
next: None,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user