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
This commit is contained in:
parent
e3c39f64db
commit
5c432daba4
@ -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<Vec<Arc<dyn Tool>>>,
|
||||
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<Vec<Arc<dyn Tool>>>) -> 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::<std::collections::HashSet<_>>();
|
||||
|
||||
let sub_tools: Vec<Box<dyn Tool>> = self
|
||||
let parent_tools = self
|
||||
.parent_tools
|
||||
.lock()
|
||||
.map(|tools| tools.clone())
|
||||
.unwrap_or_default();
|
||||
|
||||
let sub_tools: Vec<Box<dyn Tool>> = 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<Arc<dyn Tool>>) -> 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<dyn Tool>],
|
||||
);
|
||||
|
||||
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
|
||||
|
||||
@ -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<Arc<dyn Tool>>) -> Vec<Box<dyn Tool>> {
|
||||
tools.into_iter().map(ArcDelegatingTool::boxed).collect()
|
||||
}
|
||||
|
||||
pub(crate) type SharedToolRegistry = Arc<Mutex<Vec<Arc<dyn Tool>>>>;
|
||||
|
||||
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<dyn Tool>],
|
||||
) {
|
||||
if let Ok(mut guard) = shared_registry.lock() {
|
||||
*guard = tools.to_vec();
|
||||
}
|
||||
}
|
||||
|
||||
/// Create the default tool registry
|
||||
pub fn default_tools(security: Arc<SecurityPolicy>) -> Vec<Box<dyn Tool>> {
|
||||
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<String, DelegateAgentConfig> = 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)
|
||||
}
|
||||
|
||||
|
||||
@ -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<String>,
|
||||
provider_runtime_options: providers::ProviderRuntimeOptions,
|
||||
registry: Arc<SubAgentRegistry>,
|
||||
parent_tools: Arc<Vec<Arc<dyn Tool>>>,
|
||||
parent_tools: SharedToolRegistry,
|
||||
multimodal_config: crate::config::MultimodalConfig,
|
||||
}
|
||||
|
||||
@ -44,7 +45,7 @@ impl SubAgentSpawnTool {
|
||||
security: Arc<SecurityPolicy>,
|
||||
provider_runtime_options: providers::ProviderRuntimeOptions,
|
||||
registry: Arc<SubAgentRegistry>,
|
||||
parent_tools: Arc<Vec<Arc<dyn Tool>>>,
|
||||
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<dyn Tool>],
|
||||
parent_tools: &SharedToolRegistry,
|
||||
multimodal_config: &crate::config::MultimodalConfig,
|
||||
) -> anyhow::Result<ToolResult> {
|
||||
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::<std::collections::HashSet<_>>();
|
||||
let parent_tools = parent_tools
|
||||
.lock()
|
||||
.map(|tools| tools.clone())
|
||||
.unwrap_or_default();
|
||||
|
||||
let sub_tools: Vec<Box<dyn Tool>> = 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<ToolResult> {
|
||||
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<String> {
|
||||
Ok("unused".to_string())
|
||||
}
|
||||
|
||||
async fn chat(
|
||||
&self,
|
||||
request: crate::providers::ChatRequest<'_>,
|
||||
_model: &str,
|
||||
_temperature: f64,
|
||||
) -> anyhow::Result<crate::providers::ChatResponse> {
|
||||
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<String>, 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<String, DelegateAgentConfig>,
|
||||
security: Arc<SecurityPolicy>,
|
||||
@ -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<dyn Tool>],
|
||||
);
|
||||
|
||||
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"));
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user