Merge remote-tracking branch 'origin/pr/1500' into work/v0.8.34

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<Tool> {
+    fn tools_for_model(&self, agent_type: &SubAgentType) -> Vec<Tool> {
+        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::<Vec<_>>(),
+        };
+        if disallowed.is_empty() {
+            filtered
+        } else {
+            filtered
+                .into_iter()
+                .filter(|tool| !disallowed.contains(&tool.name.as_str()))
+                .collect()
         }
     }
 

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)));