diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 2b56d339..fda1926f 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -3326,7 +3326,7 @@ async fn run_subagent( unavailable_tools.join(", ") )); } - let tools = tool_registry.tools_for_model(); + let tools = tool_registry.tools_for_model(&agent_type); if let Some(mb) = runtime.mailbox.as_ref() { let _ = mb.send(MailboxMessage::started(&agent_id, agent_type.clone())); } @@ -4350,14 +4350,27 @@ impl SubAgentToolRegistry { } } - fn tools_for_model(&self) -> Vec { + fn tools_for_model(&self, agent_type: &SubAgentType) -> Vec { + let disallowed = match agent_type { + // Review agents should not spawn sub-agents (#1489). + SubAgentType::Review => &["agent_spawn"][..], + _ => &[][..], + }; let api_tools = self.registry.to_api_tools(); - match &self.allowed_tools { + let filtered = match &self.allowed_tools { None => api_tools, Some(list) => api_tools .into_iter() .filter(|tool| list.contains(&tool.name)) - .collect(), + .collect::>(), + }; + if disallowed.is_empty() { + filtered + } else { + filtered + .into_iter() + .filter(|tool| !disallowed.contains(&tool.name.as_str())) + .collect() } } diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 164ba00f..b094190f 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -691,6 +691,26 @@ fn test_subagent_tool_registry_reports_unavailable_tools() { ); } +#[test] +fn test_review_agent_tools_exclude_agent_spawn() { + let tmp = tempdir().expect("tempdir"); + let mut runtime = stub_runtime(); + runtime.context = ToolContext::new(tmp.path().to_path_buf()); + // None = full parent tool inheritance (the default for builtin types). + let registry = SubAgentToolRegistry::new( + runtime, + None, + Arc::new(Mutex::new(TodoList::new())), + Arc::new(Mutex::new(PlanState::default())), + ); + let tools = registry.tools_for_model(&SubAgentType::Review); + let names: Vec<_> = tools.iter().map(|t| t.name.as_str()).collect(); + assert!( + !names.contains(&"agent_spawn"), + "Review agent must not have agent_spawn; tools: {names:?}" + ); +} + #[tokio::test] async fn test_wait_for_result_reports_timeout_when_still_running() { let manager = Arc::new(RwLock::new(SubAgentManager::new(PathBuf::from("."), 2)));