From 5c432daba41a0b3d45cdcc98f00038aec758f3bf Mon Sep 17 00:00:00 2001 From: Argenis Date: Wed, 11 Mar 2026 18:40:54 -0400 Subject: [PATCH] fix(tools): sync delegate parent registry with runtime tools (#3161) * fix(tools): sync delegate parent registry with runtime tools * test(tools): cover late-bound subagent spawn registry --- src/tools/delegate.rs | 55 ++++++++++++--- src/tools/mod.rs | 25 ++++++- src/tools/subagent_spawn.rs | 135 ++++++++++++++++++++++++++++++++++-- 3 files changed, 197 insertions(+), 18 deletions(-) diff --git a/src/tools/delegate.rs b/src/tools/delegate.rs index ea26a1f0a..6b8022dc9 100644 --- a/src/tools/delegate.rs +++ b/src/tools/delegate.rs @@ -6,6 +6,7 @@ use crate::observability::traits::{Observer, ObserverEvent, ObserverMetric}; use crate::providers::{self, ChatMessage, Provider}; use crate::security::policy::ToolOperation; use crate::security::SecurityPolicy; +use crate::tools::SharedToolRegistry; use async_trait::async_trait; use serde_json::json; use std::collections::HashMap; @@ -36,7 +37,7 @@ pub struct DelegateTool { /// Depth at which this tool instance lives in the delegation chain. depth: u32, /// Parent tool registry for agentic sub-agents. - parent_tools: Arc>>, + parent_tools: SharedToolRegistry, /// Inherited multimodal handling config for sub-agent loops. multimodal_config: crate::config::MultimodalConfig, /// Optional typed coordination bus used to trace delegate lifecycle events. @@ -72,7 +73,7 @@ impl DelegateTool { fallback_credential, provider_runtime_options, depth: 0, - parent_tools: Arc::new(Vec::new()), + parent_tools: crate::tools::new_shared_tool_registry(), multimodal_config: crate::config::MultimodalConfig::default(), coordination_bus, coordination_lead_agent: DEFAULT_COORDINATION_LEAD_AGENT.to_string(), @@ -111,7 +112,7 @@ impl DelegateTool { fallback_credential, provider_runtime_options, depth, - parent_tools: Arc::new(Vec::new()), + parent_tools: crate::tools::new_shared_tool_registry(), multimodal_config: crate::config::MultimodalConfig::default(), coordination_bus, coordination_lead_agent: DEFAULT_COORDINATION_LEAD_AGENT.to_string(), @@ -119,7 +120,7 @@ impl DelegateTool { } /// Attach parent tools used to build sub-agent allowlist registries. - pub fn with_parent_tools(mut self, parent_tools: Arc>>) -> Self { + pub fn with_parent_tools(mut self, parent_tools: SharedToolRegistry) -> Self { self.parent_tools = parent_tools; self } @@ -461,9 +462,13 @@ impl DelegateTool { .map(|name| name.trim()) .filter(|name| !name.is_empty()) .collect::>(); - - let sub_tools: Vec> = self + let parent_tools = self .parent_tools + .lock() + .map(|tools| tools.clone()) + .unwrap_or_default(); + + let sub_tools: Vec> = parent_tools .iter() .filter(|tool| allowed.contains(tool.name())) .filter(|tool| tool.name() != "delegate") @@ -967,6 +972,12 @@ mod tests { } } + fn shared_parent_tools(tools: Vec>) -> SharedToolRegistry { + let shared = crate::tools::new_shared_tool_registry(); + crate::tools::sync_shared_tool_registry(&shared, &tools); + shared + } + #[test] fn name_and_schema() { let tool = DelegateTool::new(sample_agents(), None, test_security()); @@ -1278,7 +1289,7 @@ mod tests { ); let tool = DelegateTool::new(agents, None, test_security()) - .with_parent_tools(Arc::new(vec![Arc::new(EchoTool)])); + .with_parent_tools(shared_parent_tools(vec![Arc::new(EchoTool)])); let result = tool .execute(json!({"agent": "agentic", "prompt": "test"})) .await @@ -1296,7 +1307,7 @@ mod tests { async fn execute_agentic_runs_tool_call_loop_with_filtered_tools() { let config = agentic_config(vec!["echo_tool".to_string()], 10); let tool = DelegateTool::new(HashMap::new(), None, test_security()).with_parent_tools( - Arc::new(vec![ + shared_parent_tools(vec![ Arc::new(EchoTool), Arc::new(DelegateTool::new(HashMap::new(), None, test_security())), ]), @@ -1313,11 +1324,33 @@ mod tests { assert!(result.output.contains("done")); } + #[tokio::test] + async fn execute_agentic_reads_late_bound_parent_tools() { + let config = agentic_config(vec!["echo_tool".to_string()], 10); + let parent_tools = crate::tools::new_shared_tool_registry(); + let tool = DelegateTool::new(HashMap::new(), None, test_security()) + .with_parent_tools(parent_tools.clone()); + + crate::tools::sync_shared_tool_registry( + &parent_tools, + &[Arc::new(EchoTool) as Arc], + ); + + let provider = OneToolThenFinalProvider; + let result = tool + .execute_agentic("agentic", &config, &provider, "run", 0.2) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("done")); + } + #[tokio::test] async fn execute_agentic_excludes_delegate_even_if_allowlisted() { let config = agentic_config(vec!["delegate".to_string()], 10); let tool = DelegateTool::new(HashMap::new(), None, test_security()).with_parent_tools( - Arc::new(vec![Arc::new(DelegateTool::new( + shared_parent_tools(vec![Arc::new(DelegateTool::new( HashMap::new(), None, test_security(), @@ -1342,7 +1375,7 @@ mod tests { async fn execute_agentic_respects_max_iterations() { let config = agentic_config(vec!["echo_tool".to_string()], 2); let tool = DelegateTool::new(HashMap::new(), None, test_security()) - .with_parent_tools(Arc::new(vec![Arc::new(EchoTool)])); + .with_parent_tools(shared_parent_tools(vec![Arc::new(EchoTool)])); let provider = InfiniteToolCallProvider; let result = tool @@ -1362,7 +1395,7 @@ mod tests { async fn execute_agentic_propagates_provider_errors() { let config = agentic_config(vec!["echo_tool".to_string()], 10); let tool = DelegateTool::new(HashMap::new(), None, test_security()) - .with_parent_tools(Arc::new(vec![Arc::new(EchoTool)])); + .with_parent_tools(shared_parent_tools(vec![Arc::new(EchoTool)])); let provider = FailingProvider; let result = tool diff --git a/src/tools/mod.rs b/src/tools/mod.rs index fef85de3a..f5bd45635 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -126,7 +126,7 @@ use crate::runtime::{NativeRuntime, RuntimeAdapter}; use crate::security::SecurityPolicy; use async_trait::async_trait; use std::collections::HashMap; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; #[derive(Clone)] struct ArcDelegatingTool { @@ -162,6 +162,21 @@ fn boxed_registry_from_arcs(tools: Vec>) -> Vec> { tools.into_iter().map(ArcDelegatingTool::boxed).collect() } +pub(crate) type SharedToolRegistry = Arc>>>; + +pub(crate) fn new_shared_tool_registry() -> SharedToolRegistry { + Arc::new(Mutex::new(Vec::new())) +} + +pub(crate) fn sync_shared_tool_registry( + shared_registry: &SharedToolRegistry, + tools: &[Arc], +) { + if let Ok(mut guard) = shared_registry.lock() { + *guard = tools.to_vec(); + } +} + /// Create the default tool registry pub fn default_tools(security: Arc) -> Vec> { default_tools_with_runtime(security, Arc::new(NativeRuntime::new())) @@ -417,6 +432,7 @@ pub fn all_tools_with_runtime( } // Add delegation and sub-agent orchestration tools when agents are configured + let mut shared_parent_tools = None; if !agents.is_empty() { let delegate_agents: HashMap = agents .iter() @@ -442,7 +458,8 @@ pub fn all_tools_with_runtime( max_tokens_override: None, model_support_vision: root_config.model_support_vision, }; - let parent_tools = Arc::new(tool_arcs.clone()); + let parent_tools = new_shared_tool_registry(); + shared_parent_tools = Some(parent_tools.clone()); let mut delegate_tool = DelegateTool::new_with_options( delegate_agents.clone(), delegate_fallback_credential.clone(), @@ -536,6 +553,10 @@ pub fn all_tools_with_runtime( } } + if let Some(shared_registry) = shared_parent_tools.as_ref() { + sync_shared_tool_registry(shared_registry, &tool_arcs); + } + boxed_registry_from_arcs(tool_arcs) } diff --git a/src/tools/subagent_spawn.rs b/src/tools/subagent_spawn.rs index 488aa5ffe..17ba43946 100644 --- a/src/tools/subagent_spawn.rs +++ b/src/tools/subagent_spawn.rs @@ -11,6 +11,7 @@ use crate::observability::traits::{Observer, ObserverEvent, ObserverMetric}; use crate::providers::{self, ChatMessage, Provider}; use crate::security::policy::ToolOperation; use crate::security::SecurityPolicy; +use crate::tools::SharedToolRegistry; use async_trait::async_trait; use chrono::Utc; use serde_json::json; @@ -32,7 +33,7 @@ pub struct SubAgentSpawnTool { fallback_credential: Option, provider_runtime_options: providers::ProviderRuntimeOptions, registry: Arc, - parent_tools: Arc>>, + parent_tools: SharedToolRegistry, multimodal_config: crate::config::MultimodalConfig, } @@ -44,7 +45,7 @@ impl SubAgentSpawnTool { security: Arc, provider_runtime_options: providers::ProviderRuntimeOptions, registry: Arc, - parent_tools: Arc>>, + parent_tools: SharedToolRegistry, multimodal_config: crate::config::MultimodalConfig, ) -> Self { Self { @@ -395,7 +396,7 @@ async fn run_agentic_background( agent_config: &DelegateAgentConfig, provider: &dyn Provider, full_prompt: &str, - parent_tools: &[Arc], + parent_tools: &SharedToolRegistry, multimodal_config: &crate::config::MultimodalConfig, ) -> anyhow::Result { if agent_config.allowed_tools.is_empty() { @@ -414,6 +415,10 @@ async fn run_agentic_background( .map(|name| name.trim()) .filter(|name| !name.is_empty()) .collect::>(); + let parent_tools = parent_tools + .lock() + .map(|tools| tools.clone()) + .unwrap_or_default(); let sub_tools: Vec> = parent_tools .iter() @@ -530,6 +535,100 @@ mod tests { agents } + #[derive(Default)] + struct EchoTool; + + #[async_trait] + impl Tool for EchoTool { + fn name(&self) -> &str { + "echo_tool" + } + + fn description(&self) -> &str { + "Echoes the `value` argument." + } + + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "value": {"type": "string"} + }, + "required": ["value"] + }) + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + let value = args + .get("value") + .and_then(serde_json::Value::as_str) + .unwrap_or_default() + .to_string(); + Ok(ToolResult { + success: true, + output: format!("echo:{value}"), + error: None, + }) + } + } + + struct OneToolThenFinalProvider; + + #[async_trait] + impl Provider for OneToolThenFinalProvider { + async fn chat_with_system( + &self, + _system_prompt: Option<&str>, + _message: &str, + _model: &str, + _temperature: f64, + ) -> anyhow::Result { + Ok("unused".to_string()) + } + + async fn chat( + &self, + request: crate::providers::ChatRequest<'_>, + _model: &str, + _temperature: f64, + ) -> anyhow::Result { + let has_tool_message = request.messages.iter().any(|m| m.role == "tool"); + if has_tool_message { + Ok(crate::providers::ChatResponse { + text: Some("done".to_string()), + tool_calls: Vec::new(), + usage: None, + reasoning_content: None, + }) + } else { + Ok(crate::providers::ChatResponse { + text: None, + tool_calls: vec![crate::providers::ToolCall { + id: "call_1".to_string(), + name: "echo_tool".to_string(), + arguments: "{\"value\":\"ping\"}".to_string(), + }], + usage: None, + reasoning_content: None, + }) + } + } + } + + fn agentic_config(allowed_tools: Vec, max_iterations: usize) -> DelegateAgentConfig { + DelegateAgentConfig { + provider: "openrouter".to_string(), + model: "model-test".to_string(), + system_prompt: Some("You are agentic.".to_string()), + api_key: Some("delegate-test-credential".to_string()), + temperature: Some(0.2), + max_depth: 3, + agentic: true, + allowed_tools, + max_iterations, + } + } + fn make_tool( agents: HashMap, security: Arc, @@ -540,7 +639,7 @@ mod tests { security, providers::ProviderRuntimeOptions::default(), Arc::new(SubAgentRegistry::new()), - Arc::new(Vec::new()), + crate::tools::new_shared_tool_registry(), crate::config::MultimodalConfig::default(), ) } @@ -705,7 +804,7 @@ mod tests { test_security(), providers::ProviderRuntimeOptions::default(), registry, - Arc::new(Vec::new()), + crate::tools::new_shared_tool_registry(), crate::config::MultimodalConfig::default(), ); @@ -726,4 +825,30 @@ mod tests { .unwrap(); assert!(desc.contains("researcher")); } + + #[tokio::test] + async fn run_agentic_background_reads_late_bound_parent_tools() { + let config = agentic_config(vec!["echo_tool".to_string()], 10); + let parent_tools = crate::tools::new_shared_tool_registry(); + let provider = OneToolThenFinalProvider; + + crate::tools::sync_shared_tool_registry( + &parent_tools, + &[Arc::new(EchoTool) as Arc], + ); + + let result = run_agentic_background( + "agentic", + &config, + &provider, + "run", + &parent_tools, + &crate::config::MultimodalConfig::default(), + ) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("done")); + } }