From 9e2f3c0f12b1e1e305ae46e36ca9e7e13a45559a Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Tue, 15 Sep 2026 11:42:39 -0600 Subject: [PATCH] feat: auto continuation is unavailable in llm nodes; continuation driven loops should be in agent nodes only --- src/config/request_context.rs | 40 ++++++++-------- src/function/todo.rs | 34 ++++++++++++++ src/graph/llm.rs | 27 +++++++++++ src/graph/validator.rs | 86 +++++++++++++++++++++++++++++++++++ 4 files changed, 167 insertions(+), 20 deletions(-) diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 4a56409..af42090 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -2516,16 +2516,17 @@ impl RequestContext { if let Some(ref tool_names) = role_filter { agent_functions.retain(|v| { - tool_names.contains(&v.name) - || (!matches!(agent.skills_enabled(), Some(false)) - && v.name.starts_with(SKILL_FUNCTION_PREFIX)) - || (!self.in_graph_llm_node - && (v.name.starts_with(USER_FUNCTION_PREFIX) - || v.name.starts_with(TODO_FUNCTION_PREFIX))) - || v.name.starts_with(AGENT_FUNCTION_PREFIX) - || v.name.starts_with(MEMORY_FUNCTION_PREFIX) - || v.name.starts_with(RAG_FUNCTION_PREFIX) - || v.name.starts_with(JOB_FUNCTION_PREFIX) + !(self.in_graph_llm_node && v.name.starts_with(TODO_FUNCTION_PREFIX)) + && (tool_names.contains(&v.name) + || (!matches!(agent.skills_enabled(), Some(false)) + && v.name.starts_with(SKILL_FUNCTION_PREFIX)) + || (!self.in_graph_llm_node + && (v.name.starts_with(USER_FUNCTION_PREFIX) + || v.name.starts_with(TODO_FUNCTION_PREFIX))) + || v.name.starts_with(AGENT_FUNCTION_PREFIX) + || v.name.starts_with(MEMORY_FUNCTION_PREFIX) + || v.name.starts_with(RAG_FUNCTION_PREFIX) + || v.name.starts_with(JOB_FUNCTION_PREFIX)) }); } @@ -7153,7 +7154,7 @@ mod tests { #[test] #[serial] - fn select_functions_graph_llm_node_todo_tool_opt_in_is_exact() { + fn select_functions_graph_llm_node_todo_tools_cannot_be_opted_in() { let _guard = TestConfigDirGuard::new(); let mut ctx = create_test_ctx(); let app = ctx.app.config.clone(); @@ -7176,22 +7177,21 @@ mod tests { run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap(); let mut role = Role::new("r", "p"); - role.set_enabled_tools(Some(vec!["todo__init".to_string()])); + role.set_enabled_tools(Some(vec![ + "todo__init".to_string(), + "user__select".to_string(), + ])); ctx.in_graph_llm_node = true; let fns = ctx.select_functions(&role).unwrap(); let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect(); assert!( - names.contains(&"todo__init"), - "an explicitly listed todo tool must be offered, got: {names:?}" + !names.iter().any(|n| n.starts_with("todo__")), + "todo tools must be denied inside a node even when explicitly listed, got: {names:?}" ); assert!( - !names.contains(&"todo__pause"), - "opt-in must not drag in sibling todo tools, got: {names:?}" - ); - assert!( - !names.contains(&"user__select"), - "a todo opt-in must not expose user tools, got: {names:?}" + names.contains(&"user__select"), + "an explicitly listed user tool must still be offered, got: {names:?}" ); } diff --git a/src/function/todo.rs b/src/function/todo.rs index dfb089c..69b78b8 100644 --- a/src/function/todo.rs +++ b/src/function/todo.rs @@ -126,6 +126,16 @@ pub fn handle_todo_tool(ctx: &mut RequestContext, cmd_name: &str, args: &Value) if !ctx.app.config.function_calling_support { bail!("Cannot use todo tools: function calling is disabled."); } + if ctx.in_graph_llm_node { + return Ok(json!({ + "error": "todo tools are unavailable inside graph llm nodes: they drive chat turn \ + loops (auto_continue), and llm nodes have no turn loop. Node \ + continuation is owned by the node loop, max_iterations, and graph \ + edges. For driven multi-step work, use an agent node whose agent sets \ + auto_continue: true." + })); + } + let auto_config = ctx.auto_continue_config(); if !auto_config.enabled { bail!( @@ -245,4 +255,28 @@ mod tests { assert!(error.contains("requires an active session")); assert!(ctx.auto_continue_paused.is_none()); } + + #[test] + fn graph_llm_node_returns_teaching_error() { + let mut app_state = AppState::test_default(); + app_state.config = Arc::new(AppConfig { + auto_continue: true, + ..AppConfig::default() + }); + let mut ctx = RequestContext::new(Arc::new(app_state), WorkingMode::Cmd); + ctx.in_graph_llm_node = true; + + let result = handle_todo_tool(&mut ctx, "todo__init", &json!({"goal": "g"})).unwrap(); + + let error = result["error"].as_str().unwrap(); + assert!(error.contains("graph llm nodes")); + assert!( + ctx.todo_list.is_default(), + "the guard must fire before any todo state mutation (goal included)" + ); + + ctx.in_graph_llm_node = false; + let result = handle_todo_tool(&mut ctx, "todo__init", &json!({"goal": "g"})).unwrap(); + assert_eq!(result["status"], "ok"); + } } diff --git a/src/graph/llm.rs b/src/graph/llm.rs index 00ded58..47cbdd0 100644 --- a/src/graph/llm.rs +++ b/src/graph/llm.rs @@ -12,6 +12,7 @@ use crate::function::ToolResult; use crate::function::agents::{GuardrailAction, check_pending_tasks_guardrail}; use crate::function::jobs::reap_jobs; use crate::function::skill::skill_function_declarations; +use crate::function::todo::TODO_FUNCTION_PREFIX; use crate::utils::AbortSignal; use anyhow::{Context, Error, Result, anyhow, bail}; use log::warn; @@ -485,6 +486,14 @@ fn validate_tools_subset( .map(|d| d.name.as_str()) .collect(); for name in regular { + if name.starts_with(TODO_FUNCTION_PREFIX) { + bail!( + "llm node cannot enable '{name}': todo tools drive chat turn loops \ + (auto_continue), which llm nodes don't have. Node continuation is \ + owned by the node loop, max_iterations, and graph edges. For driven \ + multi-step work, use an agent node whose agent sets auto_continue: true." + ); + } if !known.contains(name.as_str()) { let mut avail: Vec<&str> = known.iter().copied().collect(); avail.sort(); @@ -794,6 +803,24 @@ mod tests { assert!(mcp.is_empty()); } + #[test] + fn validate_tools_subset_rejects_todo_tools() { + let mut ctx = plain_ctx(); + let mut agent = Agent::test_new(AgentConfig { + auto_continue: true, + ..AgentConfig::default() + }); + agent.functions_mut().append_todo_functions(); + ctx.agent = Some(agent); + + let err = validate_tools_subset(&["todo__init".to_string()], &[], &ctx).unwrap_err(); + + assert!( + err.to_string().contains("todo tools drive chat turn loops"), + "expected the todo teaching error, got: {err}" + ); + } + #[test] fn zero_timeout_resolves_to_no_bound() { assert!(Some(0u64).and_then(wall_clock).is_none()); diff --git a/src/graph/validator.rs b/src/graph/validator.rs index 0e941d6..b82e983 100644 --- a/src/graph/validator.rs +++ b/src/graph/validator.rs @@ -3,6 +3,7 @@ use super::types::{ConcurrencyCap, Graph, NextTargets, Node, NodeType}; use crate::client::{Model, ModelType}; use crate::config; use crate::config::{Agent, AppConfig, paths}; +use crate::function::todo::TODO_FUNCTION_PREFIX; use crate::function::user_interaction::USER_FUNCTION_PREFIX; use crate::rag::{GraphRagConfig, RagData}; use anyhow::{Result, bail}; @@ -181,6 +182,7 @@ impl GraphValidator { self.validate_fallback_capture(graph, &mut result); self.validate_output_schema_properties(graph, &mut result); self.validate_structured_output_user_tools(graph, &mut result); + self.validate_llm_node_todo_tools(graph, &mut result); self.validate_max_concurrency(graph, &mut result); self.validate_max_concurrency_template(graph, &mut result); self.validate_orchestration_limits(graph, &mut result); @@ -483,6 +485,28 @@ impl GraphValidator { } } + fn validate_llm_node_todo_tools(&self, graph: &Graph, result: &mut ValidationResult) { + for (node_id, node) in &graph.nodes { + let NodeType::Llm(llm) = &node.node_type else { + continue; + }; + let exposes_todo_tools = llm.tools.as_ref().is_some_and(|tools| { + tools + .iter() + .any(|t| t.trim().starts_with(TODO_FUNCTION_PREFIX)) + }); + if exposes_todo_tools { + result.error(ValidationError::with_node( + node_id, + "llm node exposes todo tools (`todo__*`): todo tools drive chat turn \ + loops (auto_continue), which llm nodes don't have. Node continuation \ + is owned by the node loop, max_iterations, and graph edges. For driven \ + multi-step work, use an agent node whose agent sets `auto_continue: true`", + )); + } + } + } + fn validate_node_references(&self, graph: &Graph, result: &mut ValidationResult) { for (node_id, node) in &graph.nodes { for (target, label) in declared_targets(node) { @@ -4793,6 +4817,68 @@ mod tests { ); } + #[test] + fn structured_output_node_with_todo_tools_errors() { + let mut node = llm_with_output_schema("l", &["verdict"], Some("end")); + if let NodeType::Llm(ref mut n) = node.node_type { + n.tools = Some(vec!["todo__init".into()]); + } + let graph = graph_with(vec![("l", node), ("end", end_node("end"))], "l"); + + let result = validator().validate(&graph); + + assert!( + result + .errors + .iter() + .any(|e| e.node_id.as_deref() == Some("l") + && e.message.contains("todo tools drive chat turn loops")), + "expected todo-tool error: {:?}", + result.errors + ); + } + + #[test] + fn non_structured_node_with_todo_tools_errors() { + let mut node = llm_node("l", None, Some("end")); + if let NodeType::Llm(ref mut n) = node.node_type { + n.tools = Some(vec!["todo__pause".into()]); + } + let graph = graph_with(vec![("l", node), ("end", end_node("end"))], "l"); + + let result = validator().validate(&graph); + + assert!( + result + .errors + .iter() + .any(|e| e.node_id.as_deref() == Some("l") + && e.message.contains("todo tools drive chat turn loops")), + "expected todo-tool error: {:?}", + result.errors + ); + } + + #[test] + fn llm_node_without_todo_tools_does_not_error() { + let mut node = llm_node("l", None, Some("end")); + if let NodeType::Llm(ref mut n) = node.node_type { + n.tools = Some(vec!["user__select".into()]); + } + let graph = graph_with(vec![("l", node), ("end", end_node("end"))], "l"); + + let result = validator().validate(&graph); + + assert!( + !result + .errors + .iter() + .any(|e| e.message.contains("todo tools drive chat turn loops")), + "unexpected todo-tool error: {:?}", + result.errors + ); + } + #[test] fn fallback_with_output_schema_and_empty_state_updates_warns() { let mut node = llm_with_output_schema("l", &["summary"], Some("end"));