feat: auto continuation is unavailable in llm nodes; continuation driven loops should be in agent nodes only
This commit is contained in:
4 files changed
+159
-12
No files matched your search
@@ -2516,7 +2516,8 @@ impl RequestContext {
|
|||||||
|
|
||||||
if let Some(ref tool_names) = role_filter {
|
if let Some(ref tool_names) = role_filter {
|
||||||
agent_functions.retain(|v| {
|
agent_functions.retain(|v| {
|
||||||
tool_names.contains(&v.name)
|
!(self.in_graph_llm_node && v.name.starts_with(TODO_FUNCTION_PREFIX))
|
||||||
|
&& (tool_names.contains(&v.name)
|
||||||
|| (!matches!(agent.skills_enabled(), Some(false))
|
|| (!matches!(agent.skills_enabled(), Some(false))
|
||||||
&& v.name.starts_with(SKILL_FUNCTION_PREFIX))
|
&& v.name.starts_with(SKILL_FUNCTION_PREFIX))
|
||||||
|| (!self.in_graph_llm_node
|
|| (!self.in_graph_llm_node
|
||||||
@@ -2525,7 +2526,7 @@ impl RequestContext {
|
|||||||
|| v.name.starts_with(AGENT_FUNCTION_PREFIX)
|
|| v.name.starts_with(AGENT_FUNCTION_PREFIX)
|
||||||
|| v.name.starts_with(MEMORY_FUNCTION_PREFIX)
|
|| v.name.starts_with(MEMORY_FUNCTION_PREFIX)
|
||||||
|| v.name.starts_with(RAG_FUNCTION_PREFIX)
|
|| v.name.starts_with(RAG_FUNCTION_PREFIX)
|
||||||
|| v.name.starts_with(JOB_FUNCTION_PREFIX)
|
|| v.name.starts_with(JOB_FUNCTION_PREFIX))
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -7153,7 +7154,7 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
#[serial]
|
#[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 _guard = TestConfigDirGuard::new();
|
||||||
let mut ctx = create_test_ctx();
|
let mut ctx = create_test_ctx();
|
||||||
let app = ctx.app.config.clone();
|
let app = ctx.app.config.clone();
|
||||||
@@ -7176,22 +7177,21 @@ mod tests {
|
|||||||
run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap();
|
run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap();
|
||||||
|
|
||||||
let mut role = Role::new("r", "p");
|
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;
|
ctx.in_graph_llm_node = true;
|
||||||
let fns = ctx.select_functions(&role).unwrap();
|
let fns = ctx.select_functions(&role).unwrap();
|
||||||
let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
|
let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
|
||||||
assert!(
|
assert!(
|
||||||
names.contains(&"todo__init"),
|
!names.iter().any(|n| n.starts_with("todo__")),
|
||||||
"an explicitly listed todo tool must be offered, got: {names:?}"
|
"todo tools must be denied inside a node even when explicitly listed, got: {names:?}"
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
!names.contains(&"todo__pause"),
|
names.contains(&"user__select"),
|
||||||
"opt-in must not drag in sibling todo tools, got: {names:?}"
|
"an explicitly listed user tool must still be offered, got: {names:?}"
|
||||||
);
|
|
||||||
assert!(
|
|
||||||
!names.contains(&"user__select"),
|
|
||||||
"a todo opt-in must not expose user tools, got: {names:?}"
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -126,6 +126,16 @@ pub fn handle_todo_tool(ctx: &mut RequestContext, cmd_name: &str, args: &Value)
|
|||||||
if !ctx.app.config.function_calling_support {
|
if !ctx.app.config.function_calling_support {
|
||||||
bail!("Cannot use todo tools: function calling is disabled.");
|
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();
|
let auto_config = ctx.auto_continue_config();
|
||||||
if !auto_config.enabled {
|
if !auto_config.enabled {
|
||||||
bail!(
|
bail!(
|
||||||
@@ -245,4 +255,28 @@ mod tests {
|
|||||||
assert!(error.contains("requires an active session"));
|
assert!(error.contains("requires an active session"));
|
||||||
assert!(ctx.auto_continue_paused.is_none());
|
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");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
@@ -12,6 +12,7 @@ use crate::function::ToolResult;
|
|||||||
use crate::function::agents::{GuardrailAction, check_pending_tasks_guardrail};
|
use crate::function::agents::{GuardrailAction, check_pending_tasks_guardrail};
|
||||||
use crate::function::jobs::reap_jobs;
|
use crate::function::jobs::reap_jobs;
|
||||||
use crate::function::skill::skill_function_declarations;
|
use crate::function::skill::skill_function_declarations;
|
||||||
|
use crate::function::todo::TODO_FUNCTION_PREFIX;
|
||||||
use crate::utils::AbortSignal;
|
use crate::utils::AbortSignal;
|
||||||
use anyhow::{Context, Error, Result, anyhow, bail};
|
use anyhow::{Context, Error, Result, anyhow, bail};
|
||||||
use log::warn;
|
use log::warn;
|
||||||
@@ -485,6 +486,14 @@ fn validate_tools_subset(
|
|||||||
.map(|d| d.name.as_str())
|
.map(|d| d.name.as_str())
|
||||||
.collect();
|
.collect();
|
||||||
for name in regular {
|
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()) {
|
if !known.contains(name.as_str()) {
|
||||||
let mut avail: Vec<&str> = known.iter().copied().collect();
|
let mut avail: Vec<&str> = known.iter().copied().collect();
|
||||||
avail.sort();
|
avail.sort();
|
||||||
@@ -794,6 +803,24 @@ mod tests {
|
|||||||
assert!(mcp.is_empty());
|
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]
|
#[test]
|
||||||
fn zero_timeout_resolves_to_no_bound() {
|
fn zero_timeout_resolves_to_no_bound() {
|
||||||
assert!(Some(0u64).and_then(wall_clock).is_none());
|
assert!(Some(0u64).and_then(wall_clock).is_none());
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ use super::types::{ConcurrencyCap, Graph, NextTargets, Node, NodeType};
|
|||||||
use crate::client::{Model, ModelType};
|
use crate::client::{Model, ModelType};
|
||||||
use crate::config;
|
use crate::config;
|
||||||
use crate::config::{Agent, AppConfig, paths};
|
use crate::config::{Agent, AppConfig, paths};
|
||||||
|
use crate::function::todo::TODO_FUNCTION_PREFIX;
|
||||||
use crate::function::user_interaction::USER_FUNCTION_PREFIX;
|
use crate::function::user_interaction::USER_FUNCTION_PREFIX;
|
||||||
use crate::rag::{GraphRagConfig, RagData};
|
use crate::rag::{GraphRagConfig, RagData};
|
||||||
use anyhow::{Result, bail};
|
use anyhow::{Result, bail};
|
||||||
@@ -181,6 +182,7 @@ impl GraphValidator {
|
|||||||
self.validate_fallback_capture(graph, &mut result);
|
self.validate_fallback_capture(graph, &mut result);
|
||||||
self.validate_output_schema_properties(graph, &mut result);
|
self.validate_output_schema_properties(graph, &mut result);
|
||||||
self.validate_structured_output_user_tools(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(graph, &mut result);
|
||||||
self.validate_max_concurrency_template(graph, &mut result);
|
self.validate_max_concurrency_template(graph, &mut result);
|
||||||
self.validate_orchestration_limits(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) {
|
fn validate_node_references(&self, graph: &Graph, result: &mut ValidationResult) {
|
||||||
for (node_id, node) in &graph.nodes {
|
for (node_id, node) in &graph.nodes {
|
||||||
for (target, label) in declared_targets(node) {
|
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]
|
#[test]
|
||||||
fn fallback_with_output_schema_and_empty_state_updates_warns() {
|
fn fallback_with_output_schema_and_empty_state_updates_warns() {
|
||||||
let mut node = llm_with_output_schema("l", &["summary"], Some("end"));
|
let mut node = llm_with_output_schema("l", &["summary"], Some("end"));
|
||||||
|
|||||||
Reference in new issue
Block a user