diff --git a/assets/settings/default.json b/assets/settings/default.json index a6e166ae9d3612d44dc2bfaaaf5638efec6c8444..0c24b3b7d6930869f21b28df6231ffa40f0d54b4 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -1083,6 +1083,7 @@ "save_file": true, "open": true, "grep": true, + "subagent": true, "terminal": true, "thinking": true, "web_search": true, @@ -1102,6 +1103,7 @@ "read_file": true, "open": true, "grep": true, + "subagent": true, "thinking": true, "web_search": true, }, diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index ac99bb8f06b471bef170a2a474b2136d13cfd87a..86ce2fa53f74f215a23258c3903d65ebff03f72d 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -1522,7 +1522,12 @@ impl AcpThread { .push(ToolCallContent::Terminal(update.terminal)); } ToolCallUpdate::UpdateSubagentThread(update) => { - call.content.clear(); + debug_assert!( + !call.content.iter().any(|c| { + matches!(c, ToolCallContent::SubagentThread(existing) if existing == &update.thread) + }), + "Duplicate SubagentThread update for the same AcpThread entity" + ); call.content .push(ToolCallContent::SubagentThread(update.thread)); } diff --git a/crates/action_log/Cargo.toml b/crates/action_log/Cargo.toml index 699d5485934bb55ee52639ebab867a67720bdae1..8488df691e40ea3bcfc04f4f6f74964fba7863dd 100644 --- a/crates/action_log/Cargo.toml +++ b/crates/action_log/Cargo.toml @@ -11,9 +11,13 @@ path = "src/action_log.rs" [lints] workspace = true +[features] +test-support = [] + [dependencies] anyhow.workspace = true buffer_diff.workspace = true +log.workspace = true clock.workspace = true collections.workspace = true futures.workspace = true diff --git a/crates/action_log/src/action_log.rs b/crates/action_log/src/action_log.rs index fd16d8a61433869f4b0a20892d94b159164cf3bb..73b156cec77d07828565db03e3f7a4146a9c6458 100644 --- a/crates/action_log/src/action_log.rs +++ b/crates/action_log/src/action_log.rs @@ -774,6 +774,15 @@ impl ActionLog { .collect() } + /// Returns all tracked buffers for debugging purposes + #[cfg(any(test, feature = "test-support"))] + pub fn tracked_buffers_for_debug( + &self, + _cx: &App, + ) -> impl Iterator, &TrackedBuffer)> { + self.tracked_buffers.iter() + } + /// Iterate over buffers changed since last read or edited by the model pub fn stale_buffers<'a>(&'a self, cx: &'a App) -> impl Iterator> { self.tracked_buffers @@ -973,7 +982,7 @@ enum TrackedBufferStatus { Deleted, } -struct TrackedBuffer { +pub struct TrackedBuffer { buffer: Entity, diff_base: Rope, unreviewed_edits: Patch, @@ -988,6 +997,16 @@ struct TrackedBuffer { } impl TrackedBuffer { + #[cfg(any(test, feature = "test-support"))] + pub fn diff(&self) -> &Entity { + &self.diff + } + + #[cfg(any(test, feature = "test-support"))] + pub fn diff_base_len(&self) -> usize { + self.diff_base.len() + } + fn has_edits(&self, cx: &App) -> bool { self.diff .read(cx) diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index a0bea1deb7372fc51b1e718cf6e51303e9c44239..b0bf39ba2e8d1b5a13060173837b8f68a78f7a0c 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -22,7 +22,6 @@ use gpui::{ http_client::FakeHttpClient, }; use indoc::indoc; - use language_model::{ LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelProviderName, LanguageModelRegistry, LanguageModelRequest, @@ -2992,7 +2991,10 @@ async fn test_tool_updates_to_completion(cx: &mut TestAppContext) { acp::ToolCall::new("1", "Thinking") .kind(acp::ToolKind::Think) .raw_input(json!({})) - .meta(acp_thread::meta_with_tool_name("thinking")) + .meta(acp::Meta::from_iter([( + "tool_name".into(), + "thinking".into() + )])) ); let update = expect_tool_call_update_fields(&mut events).await; assert_eq!( @@ -3968,47 +3970,6 @@ async fn test_subagent_tool_is_present_when_feature_flag_enabled(cx: &mut TestAp }); } -#[gpui::test] -async fn test_subagent_tool_is_absent_when_feature_flag_disabled(cx: &mut TestAppContext) { - init_test(cx); - - cx.update(|cx| { - cx.update_flags(false, vec![]); - }); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree(path!("/test"), json!({})).await; - let project = Project::test(fs, [path!("/test").as_ref()], cx).await; - let project_context = cx.new(|_cx| ProjectContext::default()); - let context_server_store = project.read_with(cx, |project, _| project.context_server_store()); - let context_server_registry = - cx.new(|cx| ContextServerRegistry::new(context_server_store.clone(), cx)); - let model = Arc::new(FakeLanguageModel::default()); - - let handle = Rc::new(cx.update(|cx| FakeTerminalHandle::new_never_exits(cx))); - let environment = Rc::new(FakeThreadEnvironment { handle }); - - let thread = cx.new(|cx| { - let mut thread = Thread::new( - project.clone(), - project_context, - context_server_registry, - Templates::new(), - Some(model), - cx, - ); - thread.add_default_tools(environment, cx); - thread - }); - - thread.read_with(cx, |thread, _| { - assert!( - !thread.has_registered_tool("subagent"), - "subagent tool should not be present when feature flag is disabled" - ); - }); -} - #[gpui::test] async fn test_subagent_thread_inherits_parent_model(cx: &mut TestAppContext) { init_test(cx); @@ -4042,6 +4003,7 @@ async fn test_subagent_thread_inherits_parent_model(cx: &mut TestAppContext) { Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4089,6 +4051,7 @@ async fn test_max_subagent_depth_prevents_tool_registration(cx: &mut TestAppCont Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ); thread.add_default_tools(environment, cx); @@ -4135,6 +4098,7 @@ async fn test_subagent_receives_task_prompt(cx: &mut TestAppContext) { Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4193,6 +4157,7 @@ async fn test_subagent_returns_summary_on_completion(cx: &mut TestAppContext) { Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4267,6 +4232,7 @@ async fn test_allowed_tools_restricts_subagent_capabilities(cx: &mut TestAppCont Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ); thread.add_tool(EchoTool); @@ -4348,6 +4314,7 @@ async fn test_parent_cancel_stops_subagent(cx: &mut TestAppContext) { Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4377,6 +4344,95 @@ async fn test_parent_cancel_stops_subagent(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_subagent_tool_cancellation(cx: &mut TestAppContext) { + // This test verifies that the subagent tool properly handles user cancellation + // via `event_stream.cancelled_by_user()` and stops all running subagents. + init_test(cx); + always_allow_tools(cx); + + cx.update(|cx| { + cx.update_flags(true, vec!["subagents".to_string()]); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/test"), json!({})).await; + let project = Project::test(fs, [path!("/test").as_ref()], cx).await; + let project_context = cx.new(|_cx| ProjectContext::default()); + let context_server_store = project.read_with(cx, |project, _| project.context_server_store()); + let context_server_registry = + cx.new(|cx| ContextServerRegistry::new(context_server_store.clone(), cx)); + let model = Arc::new(FakeLanguageModel::default()); + + let parent = cx.new(|cx| { + Thread::new( + project.clone(), + project_context.clone(), + context_server_registry.clone(), + Templates::new(), + Some(model.clone()), + cx, + ) + }); + + let parent_tools: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + + #[allow(clippy::arc_with_non_send_sync)] + let tool = Arc::new(SubagentTool::new( + parent.downgrade(), + project.clone(), + project_context, + context_server_registry, + Templates::new(), + 0, + parent_tools, + )); + + let (event_stream, _rx, mut cancellation_tx) = + crate::ToolCallEventStream::test_with_cancellation(); + + // Start the subagent tool + let task = cx.update(|cx| { + tool.run( + SubagentToolInput { + subagents: vec![crate::SubagentConfig { + label: "Long running task".to_string(), + task_prompt: "Do a very long task that takes forever".to_string(), + summary_prompt: "Summarize".to_string(), + context_low_prompt: "Context low".to_string(), + timeout_ms: None, + allowed_tools: None, + }], + }, + event_stream.clone(), + cx, + ) + }); + + cx.run_until_parked(); + + // Signal cancellation via the event stream + crate::ToolCallEventStream::signal_cancellation_with_sender(&mut cancellation_tx); + + // The task should complete promptly with a cancellation error + let timeout = cx.background_executor.timer(Duration::from_secs(5)); + let result = futures::select! { + result = task.fuse() => result, + _ = timeout.fuse() => { + panic!("subagent tool did not respond to cancellation within timeout"); + } + }; + + // Verify we got a cancellation error + let err = result.unwrap_err(); + assert!( + err.to_string().contains("cancelled by user"), + "expected cancellation error, got: {}", + err + ); +} + #[gpui::test] async fn test_subagent_model_error_returned_as_tool_error(cx: &mut TestAppContext) { let ThreadTest { model, thread, .. } = setup(cx, TestModel::Fake).await; @@ -4408,6 +4464,7 @@ async fn test_subagent_model_error_returned_as_tool_error(cx: &mut TestAppContex Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4466,6 +4523,7 @@ async fn test_subagent_timeout_triggers_early_summary(cx: &mut TestAppContext) { Templates::new(), model.clone(), subagent_context.clone(), + std::collections::BTreeMap::new(), cx, ) }); @@ -4547,6 +4605,7 @@ async fn test_context_low_check_returns_true_when_usage_high(cx: &mut TestAppCon Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4614,8 +4673,13 @@ async fn test_allowed_tools_rejects_unknown_tool(cx: &mut TestAppContext) { thread }); - let parent_tool_names: Vec = vec!["echo".into()]; + let mut parent_tools: std::collections::BTreeMap< + gpui::SharedString, + Arc, + > = std::collections::BTreeMap::new(); + parent_tools.insert("echo".into(), EchoTool.erase()); + #[allow(clippy::arc_with_non_send_sync)] let tool = Arc::new(SubagentTool::new( parent.downgrade(), project, @@ -4623,10 +4687,18 @@ async fn test_allowed_tools_rejects_unknown_tool(cx: &mut TestAppContext) { context_server_registry, Templates::new(), 0, - parent_tool_names, + parent_tools, )); - let result = tool.validate_allowed_tools(&Some(vec!["nonexistent_tool".to_string()])); + let subagent_configs = vec![crate::SubagentConfig { + label: "Test".to_string(), + task_prompt: "Do something".to_string(), + summary_prompt: "Summarize".to_string(), + context_low_prompt: "Context low".to_string(), + timeout_ms: None, + allowed_tools: Some(vec!["nonexistent_tool".to_string()]), + }]; + let result = tool.validate_subagents(&subagent_configs); assert!(result.is_err(), "should reject unknown tool"); let err_msg = result.unwrap_err().to_string(); assert!( @@ -4635,8 +4707,8 @@ async fn test_allowed_tools_rejects_unknown_tool(cx: &mut TestAppContext) { err_msg ); assert!( - err_msg.contains("not available"), - "error should explain the tool is not available: {}", + err_msg.contains("do not exist"), + "error should explain the tool does not exist: {}", err_msg ); } @@ -4672,6 +4744,7 @@ async fn test_subagent_empty_response_handled(cx: &mut TestAppContext) { Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4727,6 +4800,7 @@ async fn test_nested_subagent_at_depth_2_succeeds(cx: &mut TestAppContext) { Templates::new(), model.clone(), depth_1_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4752,6 +4826,7 @@ async fn test_nested_subagent_at_depth_2_succeeds(cx: &mut TestAppContext) { Templates::new(), model.clone(), depth_2_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4810,6 +4885,7 @@ async fn test_subagent_uses_tool_and_returns_result(cx: &mut TestAppContext) { Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ); thread.add_tool(EchoTool); @@ -4906,6 +4982,7 @@ async fn test_max_parallel_subagents_enforced(cx: &mut TestAppContext) { Templates::new(), model.clone(), subagent_context, + std::collections::BTreeMap::new(), cx, ) }); @@ -4924,8 +5001,10 @@ async fn test_max_parallel_subagents_enforced(cx: &mut TestAppContext) { ); }); - let parent_tool_names: Vec = vec![]; + let parent_tools: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + #[allow(clippy::arc_with_non_send_sync)] let tool = Arc::new(SubagentTool::new( parent.downgrade(), project.clone(), @@ -4933,7 +5012,7 @@ async fn test_max_parallel_subagents_enforced(cx: &mut TestAppContext) { context_server_registry, Templates::new(), 0, - parent_tool_names, + parent_tools, )); let (event_stream, _rx) = crate::ToolCallEventStream::test(); @@ -4941,12 +5020,14 @@ async fn test_max_parallel_subagents_enforced(cx: &mut TestAppContext) { let result = cx.update(|cx| { tool.run( SubagentToolInput { - label: "Test".to_string(), - task_prompt: "Do something".to_string(), - summary_prompt: "Summarize".to_string(), - context_low_prompt: "Context low".to_string(), - timeout_ms: None, - allowed_tools: None, + subagents: vec![crate::SubagentConfig { + label: "Test".to_string(), + task_prompt: "Do something".to_string(), + summary_prompt: "Summarize".to_string(), + context_low_prompt: "Context low".to_string(), + timeout_ms: None, + allowed_tools: None, + }], }, event_stream, cx, @@ -4995,8 +5076,13 @@ async fn test_subagent_tool_end_to_end(cx: &mut TestAppContext) { thread }); - let parent_tool_names: Vec = vec!["echo".into()]; + let mut parent_tools: std::collections::BTreeMap< + gpui::SharedString, + Arc, + > = std::collections::BTreeMap::new(); + parent_tools.insert("echo".into(), EchoTool.erase()); + #[allow(clippy::arc_with_non_send_sync)] let tool = Arc::new(SubagentTool::new( parent.downgrade(), project.clone(), @@ -5004,7 +5090,7 @@ async fn test_subagent_tool_end_to_end(cx: &mut TestAppContext) { context_server_registry, Templates::new(), 0, - parent_tool_names, + parent_tools, )); let (event_stream, _rx) = crate::ToolCallEventStream::test(); @@ -5012,12 +5098,14 @@ async fn test_subagent_tool_end_to_end(cx: &mut TestAppContext) { let task = cx.update(|cx| { tool.run( SubagentToolInput { - label: "Research task".to_string(), - task_prompt: "Find all TODOs in the codebase".to_string(), - summary_prompt: "Summarize what you found".to_string(), - context_low_prompt: "Context low, wrap up".to_string(), - timeout_ms: None, - allowed_tools: None, + subagents: vec![crate::SubagentConfig { + label: "Research task".to_string(), + task_prompt: "Find all TODOs in the codebase".to_string(), + summary_prompt: "Summarize what you found".to_string(), + context_low_prompt: "Context low, wrap up".to_string(), + timeout_ms: None, + allowed_tools: None, + }], }, event_stream, cx, diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index d60a47578e0257d9fe042305e8c67f7f2922a16f..01bad43fa6f09c6ebe42b2e5891f927ef59ccd67 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -720,6 +720,7 @@ impl Thread { templates: Arc, model: Arc, subagent_context: SubagentContext, + parent_tools: BTreeMap>, cx: &mut Context, ) -> Self { let profile_id = AgentSettings::get_global(cx).default_profile.clone(); @@ -739,7 +740,7 @@ impl Thread { completion_mode: AgentSettings::get_global(cx).preferred_completion_mode, running_turn: None, pending_message: None, - tools: BTreeMap::default(), + tools: parent_tools, tool_use_limit_reached: false, request_token_usage: HashMap::default(), cumulative_token_usage: TokenUsage::default(), @@ -1107,7 +1108,7 @@ impl Thread { self.add_tool(WebSearchTool); if cx.has_flag::() && self.depth() < MAX_SUBAGENT_DEPTH { - let tool_names = self.registered_tool_names(); + let parent_tools = self.tools.clone(); self.add_tool(SubagentTool::new( cx.weak_entity(), self.project.clone(), @@ -1115,7 +1116,7 @@ impl Thread { self.context_server_registry.clone(), self.templates.clone(), self.depth(), - tool_names, + parent_tools, )); } } @@ -2780,8 +2781,14 @@ pub struct ToolCallEventStream { impl ToolCallEventStream { #[cfg(any(test, feature = "test-support"))] pub fn test() -> (Self, ToolCallEventStreamReceiver) { + let (stream, receiver, _cancellation_tx) = Self::test_with_cancellation(); + (stream, receiver) + } + + #[cfg(any(test, feature = "test-support"))] + pub fn test_with_cancellation() -> (Self, ToolCallEventStreamReceiver, watch::Sender) { let (events_tx, events_rx) = mpsc::unbounded::>(); - let (_cancellation_tx, cancellation_rx) = watch::channel(false); + let (cancellation_tx, cancellation_rx) = watch::channel(false); let stream = ToolCallEventStream::new( "test_id".into(), @@ -2790,7 +2797,17 @@ impl ToolCallEventStream { cancellation_rx, ); - (stream, ToolCallEventStreamReceiver(events_rx)) + ( + stream, + ToolCallEventStreamReceiver(events_rx), + cancellation_tx, + ) + } + + /// Signal cancellation for this event stream. Only available in tests. + #[cfg(any(test, feature = "test-support"))] + pub fn signal_cancellation_with_sender(cancellation_tx: &mut watch::Sender) { + cancellation_tx.send(true).ok(); } fn new( diff --git a/crates/agent/src/tools.rs b/crates/agent/src/tools.rs index f2f33172824d007b8061beba5bec1aabe3c88644..78128554d3f86439b934ceee662362fba19bb6e9 100644 --- a/crates/agent/src/tools.rs +++ b/crates/agent/src/tools.rs @@ -20,6 +20,8 @@ mod thinking_tool; mod web_search_tool; use crate::AgentTool; +use feature_flags::{FeatureFlagAppExt, SubagentsFeatureFlag}; +use gpui::App; use language_model::{LanguageModelRequestTool, LanguageModelToolSchemaFormat}; pub use context_server_registry::*; @@ -46,8 +48,8 @@ pub use web_search_tool::*; macro_rules! tools { ($($tool:ty),* $(,)?) => { /// A list of all built-in tool names - pub fn supported_built_in_tool_names(provider: Option) -> impl Iterator { - [ + pub fn supported_built_in_tool_names(provider: Option, cx: &App) -> Vec { + let mut tools: Vec = [ $( (if let Some(provider) = provider.as_ref() { <$tool>::supports_provider(provider) @@ -59,6 +61,13 @@ macro_rules! tools { ] .into_iter() .flatten() + .collect(); + + if !cx.has_flag::() { + tools.retain(|name| name != SubagentTool::name()); + } + + tools } /// A list of all built-in tools @@ -96,6 +105,7 @@ tools! { ReadFileTool, RestoreFileFromDiskTool, SaveFileTool, + SubagentTool, TerminalTool, ThinkingTool, WebSearchTool, diff --git a/crates/agent/src/tools/subagent_tool.rs b/crates/agent/src/tools/subagent_tool.rs index e8d650f7d0d6507b976262cb8b0b8973a25658a2..08b4744baef7b6a9a05b4595bfee0df59c616c95 100644 --- a/crates/agent/src/tools/subagent_tool.rs +++ b/crates/agent/src/tools/subagent_tool.rs @@ -2,9 +2,10 @@ use acp_thread::{AcpThread, AgentConnection, UserMessageId}; use action_log::ActionLog; use agent_client_protocol as acp; use anyhow::{Result, anyhow}; -use collections::HashSet; -use futures::channel::mpsc; +use collections::{BTreeMap, HashSet}; +use futures::{FutureExt, channel::mpsc}; use gpui::{App, AppContext, AsyncApp, Entity, SharedString, Task, WeakEntity}; +use language_model::LanguageModelToolUseId; use project::Project; use prompt_store::ProjectContext; use schemars::JsonSchema; @@ -19,37 +20,47 @@ use util::ResultExt; use watch; use crate::{ - AgentTool, ContextServerRegistry, MAX_PARALLEL_SUBAGENTS, MAX_SUBAGENT_DEPTH, SubagentContext, - Templates, Thread, ThreadEvent, ToolCallAuthorization, ToolCallEventStream, + AgentTool, AnyAgentTool, ContextServerRegistry, MAX_PARALLEL_SUBAGENTS, MAX_SUBAGENT_DEPTH, + SubagentContext, Templates, Thread, ThreadEvent, ToolCallAuthorization, ToolCallEventStream, }; /// When a subagent's remaining context window falls below this fraction (25%), /// the "context running out" prompt is sent to encourage the subagent to wrap up. const CONTEXT_LOW_THRESHOLD: f32 = 0.25; -/// Spawns a subagent with its own context window to perform a delegated task. +/// Spawns one or more subagents with their own context windows to perform delegated tasks. +/// Multiple subagents run in parallel. /// -/// Use this tool when you need to: -/// - Perform research that would consume too many tokens in the main context -/// - Execute a complex subtask independently -/// - Run multiple parallel investigations +/// Use this tool when you want to do any of the following: +/// - Perform an investigation where all you need to know is the outcome, not the research that led to that outcome. +/// - Complete a self-contained task where you need to know if it succeeded or failed (and how), but none of its intermediate output. +/// - Run multiple tasks in parallel that would take significantly longer to run sequentially. /// -/// You control what the subagent does by providing: +/// You control what each subagent does by providing: /// 1. A task prompt describing what the subagent should do /// 2. A summary prompt that tells the subagent how to summarize its work when done /// 3. A "context running out" prompt for when the subagent is low on tokens /// -/// The subagent has access to the same tools you do. You can optionally restrict -/// which tools the subagent can use. +/// Each subagent has access to the same tools you do. You can optionally restrict +/// which tools each subagent can use. /// -/// IMPORTANT: -/// - Maximum 8 subagents can be spawned per turn +/// Note: +/// - Maximum 8 subagents can run in parallel /// - Subagents cannot use tools you don't have access to /// - If spawning multiple subagents that might write to the filesystem, provide -/// guidance on how to avoid conflicts (e.g., assign each to different directories) +/// guidance on how to avoid conflicts (e.g. assign each to different directories) /// - Instruct subagents to be concise in their summaries to conserve your context #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct SubagentToolInput { + /// The list of subagents to spawn. At least one is required. + /// All subagents run in parallel and their results are collected. + #[schemars(length(min = 1, max = 8))] + pub subagents: Vec, +} + +/// Configuration for a single subagent. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +pub struct SubagentConfig { /// Short label displayed in the UI while the subagent runs (e.g., "Researching alternatives") pub label: String, @@ -83,6 +94,7 @@ pub struct SubagentToolInput { pub allowed_tools: Option>, } +/// Tool that spawns subagent threads to work on tasks in parallel. pub struct SubagentTool { parent_thread: WeakEntity, project: Entity, @@ -90,7 +102,10 @@ pub struct SubagentTool { context_server_registry: Entity, templates: Arc, current_depth: u8, - parent_tool_names: HashSet, + /// The tools available to the parent thread, captured before SubagentTool was added. + /// Subagents inherit from this set (or a subset via `allowed_tools` in the config). + /// This is captured early so subagents don't get the subagent tool themselves. + parent_tools: BTreeMap>, } impl SubagentTool { @@ -101,7 +116,7 @@ impl SubagentTool { context_server_registry: Entity, templates: Arc, current_depth: u8, - parent_tool_names: Vec, + parent_tools: BTreeMap>, ) -> Self { Self { parent_thread, @@ -110,22 +125,48 @@ impl SubagentTool { context_server_registry, templates, current_depth, - parent_tool_names: parent_tool_names.into_iter().collect(), + parent_tools, } } - pub fn validate_allowed_tools(&self, allowed_tools: &Option>) -> Result<()> { - if let Some(tools) = allowed_tools { - for tool in tools { - if !self.parent_tool_names.contains(tool.as_str()) { - return Err(anyhow!( - "Tool '{}' is not available to the parent agent. Available tools: {:?}", - tool, - self.parent_tool_names.iter().collect::>() - )); + pub fn validate_subagents(&self, subagents: &[SubagentConfig]) -> Result<()> { + if subagents.is_empty() { + return Err(anyhow!("At least one subagent configuration is required")); + } + + if subagents.len() > MAX_PARALLEL_SUBAGENTS { + return Err(anyhow!( + "Maximum {} subagents can be spawned at once, but {} were requested", + MAX_PARALLEL_SUBAGENTS, + subagents.len() + )); + } + + // Collect all invalid tools across all subagents + let mut all_invalid_tools: Vec = Vec::new(); + for config in subagents { + if let Some(ref tools) = config.allowed_tools { + for tool in tools { + if !self.parent_tools.contains_key(tool.as_str()) + && !all_invalid_tools.contains(tool) + { + all_invalid_tools.push(tool.clone()); + } } } } + + if !all_invalid_tools.is_empty() { + return Err(anyhow!( + "The following tools do not exist: {}", + all_invalid_tools + .iter() + .map(|t| format!("'{}'", t)) + .collect::>() + .join(", ") + )); + } + Ok(()) } } @@ -148,8 +189,14 @@ impl AgentTool for SubagentTool { _cx: &mut App, ) -> SharedString { input - .map(|i| i.label.into()) - .unwrap_or_else(|_| "Subagent".into()) + .map(|i| { + if i.subagents.len() == 1 { + i.subagents[0].label.clone().into() + } else { + format!("{} subagents", i.subagents.len()).into() + } + }) + .unwrap_or_else(|_| "Subagents".into()) } fn run( @@ -165,7 +212,7 @@ impl AgentTool for SubagentTool { ))); } - if let Err(e) = self.validate_allowed_tools(&input.allowed_tools) { + if let Err(e) = self.validate_subagents(&input.subagents) { return Task::ready(Err(e)); } @@ -177,96 +224,182 @@ impl AgentTool for SubagentTool { }; let running_count = parent_thread.read(cx).running_subagent_count(); - if running_count >= MAX_PARALLEL_SUBAGENTS { + let available_slots = MAX_PARALLEL_SUBAGENTS.saturating_sub(running_count); + if available_slots == 0 { return Task::ready(Err(anyhow!( "Maximum parallel subagents ({}) reached. Wait for existing subagents to complete.", MAX_PARALLEL_SUBAGENTS ))); } - let parent_thread_id = parent_thread.read(cx).id().clone(); - let parent_model = parent_thread.read(cx).model().cloned(); - let tool_use_id = event_stream.tool_use_id().clone(); + if input.subagents.len() > available_slots { + return Task::ready(Err(anyhow!( + "Cannot spawn {} subagents: only {} slots available (max {} parallel)", + input.subagents.len(), + available_slots, + MAX_PARALLEL_SUBAGENTS + ))); + } + let parent_model = parent_thread.read(cx).model().cloned(); let Some(model) = parent_model else { return Task::ready(Err(anyhow!("No model configured"))); }; - let subagent_context = SubagentContext { - parent_thread_id, - tool_use_id, - depth: self.current_depth + 1, - summary_prompt: input.summary_prompt.clone(), - context_low_prompt: input.context_low_prompt.clone(), - }; - + let parent_thread_id = parent_thread.read(cx).id().clone(); let project = self.project.clone(); let project_context = self.project_context.clone(); let context_server_registry = self.context_server_registry.clone(); let templates = self.templates.clone(); - let task_prompt = input.task_prompt; - let timeout_ms = input.timeout_ms; - let allowed_tools: Option> = input - .allowed_tools - .map(|tools| tools.into_iter().map(SharedString::from).collect()); + let parent_tools = self.parent_tools.clone(); + let current_depth = self.current_depth; + let parent_thread_weak = self.parent_thread.clone(); - let parent_thread = self.parent_thread.clone(); + // Spawn all subagents in parallel + let subagent_configs = input.subagents; cx.spawn(async move |cx| { - let subagent_thread: Entity = cx.new(|cx| { - Thread::new_subagent( - project.clone(), - project_context.clone(), - context_server_registry.clone(), - templates.clone(), - model, - subagent_context, - cx, - ) - }); + // Create all subagent threads upfront so we can track them for cancellation + let mut subagent_data: Vec<( + String, // label + Entity, // subagent thread + Entity, // acp thread for display + String, // task prompt + Option, // timeout + )> = Vec::new(); + + for config in subagent_configs { + let subagent_context = SubagentContext { + parent_thread_id: parent_thread_id.clone(), + tool_use_id: LanguageModelToolUseId::from(uuid::Uuid::new_v4().to_string()), + depth: current_depth + 1, + summary_prompt: config.summary_prompt.clone(), + context_low_prompt: config.context_low_prompt.clone(), + }; + + // Determine which tools this subagent gets + let subagent_tools: BTreeMap> = + if let Some(ref allowed) = config.allowed_tools { + let allowed_set: HashSet<&str> = + allowed.iter().map(|s| s.as_str()).collect(); + parent_tools + .iter() + .filter(|(name, _)| allowed_set.contains(name.as_ref())) + .map(|(name, tool)| (name.clone(), tool.clone())) + .collect() + } else { + parent_tools.clone() + }; + + let label = config.label.clone(); + let task_prompt = config.task_prompt.clone(); + let timeout_ms = config.timeout_ms; + + let subagent_thread: Entity = cx.new(|cx| { + Thread::new_subagent( + project.clone(), + project_context.clone(), + context_server_registry.clone(), + templates.clone(), + model.clone(), + subagent_context, + subagent_tools, + cx, + ) + }); - let subagent_weak = subagent_thread.downgrade(); - - let acp_thread: Entity = cx.new(|cx| { - let session_id = subagent_thread.read(cx).id().clone(); - let action_log: Entity = cx.new(|_| ActionLog::new(project.clone())); - let connection: Rc = Rc::new(SubagentDisplayConnection); - AcpThread::new( - "Subagent", - connection, - project.clone(), - action_log, - session_id, - watch::Receiver::constant(acp::PromptCapabilities::new()), - cx, - ) - }); + let subagent_weak = subagent_thread.downgrade(); + + let acp_thread: Entity = cx.new(|cx| { + let session_id = subagent_thread.read(cx).id().clone(); + let action_log: Entity = cx.new(|_| ActionLog::new(project.clone())); + let connection: Rc = Rc::new(SubagentDisplayConnection); + AcpThread::new( + &label, + connection, + project.clone(), + action_log, + session_id, + watch::Receiver::constant(acp::PromptCapabilities::new()), + cx, + ) + }); - event_stream.update_subagent_thread(acp_thread.clone()); + event_stream.update_subagent_thread(acp_thread.clone()); - if let Some(parent) = parent_thread.upgrade() { - parent.update(cx, |thread, _cx| { - thread.register_running_subagent(subagent_weak.clone()); - }); + if let Some(parent) = parent_thread_weak.upgrade() { + parent.update(cx, |thread, _cx| { + thread.register_running_subagent(subagent_weak.clone()); + }); + } + + subagent_data.push((label, subagent_thread, acp_thread, task_prompt, timeout_ms)); } - let result = run_subagent( - &subagent_thread, - &acp_thread, - allowed_tools, - task_prompt, - timeout_ms, - cx, - ) - .await; - - if let Some(parent) = parent_thread.upgrade() { - let _ = parent.update(cx, |thread, _cx| { - thread.unregister_running_subagent(&subagent_weak); - }); + // Collect weak refs for cancellation cleanup + let subagent_threads: Vec> = subagent_data + .iter() + .map(|(_, thread, _, _, _)| thread.downgrade()) + .collect(); + + // Spawn tasks for each subagent + let tasks: Vec<_> = subagent_data + .into_iter() + .map( + |(label, subagent_thread, acp_thread, task_prompt, timeout_ms)| { + let parent_thread_weak = parent_thread_weak.clone(); + cx.spawn(async move |cx| { + let subagent_weak = subagent_thread.downgrade(); + + let result = run_subagent( + &subagent_thread, + &acp_thread, + task_prompt, + timeout_ms, + cx, + ) + .await; + + if let Some(parent) = parent_thread_weak.upgrade() { + let _ = parent.update(cx, |thread, _cx| { + thread.unregister_running_subagent(&subagent_weak); + }); + } + + (label, result) + }) + }, + ) + .collect(); + + // Wait for all subagents to complete, or cancellation + let results: Vec<(String, Result)> = futures::select! { + results = futures::future::join_all(tasks).fuse() => results, + _ = event_stream.cancelled_by_user().fuse() => { + // Cancel all running subagents + for subagent_weak in &subagent_threads { + if let Some(subagent) = subagent_weak.upgrade() { + let _ = subagent.update(cx, |thread, cx| { + thread.cancel(cx).detach(); + }); + } + } + anyhow::bail!("Subagent tool cancelled by user"); + } + }; + + // Format the combined results + let mut output = String::new(); + for (label, result) in &results { + output.push_str(&format!("## {}\n\n", label)); + match result { + Ok(summary) => output.push_str(&summary), + Err(e) => output.push_str(&format!("Error: {}", e)), + } + output.push_str("\n\n"); } - result + Ok(output.trim().to_string()) }) } } @@ -274,17 +407,10 @@ impl AgentTool for SubagentTool { async fn run_subagent( subagent_thread: &Entity, acp_thread: &Entity, - allowed_tools: Option>, task_prompt: String, timeout_ms: Option, cx: &mut AsyncApp, ) -> Result { - if let Some(ref allowed) = allowed_tools { - subagent_thread.update(cx, |thread, _cx| { - thread.restrict_tools(allowed); - }); - } - let mut events_rx = subagent_thread.update(cx, |thread, cx| thread.submit_user_message(task_prompt, cx))?; @@ -476,26 +602,52 @@ mod tests { ); let properties = schema_json.get("properties").unwrap(); - assert!(properties.get("label").is_some(), "should have label field"); assert!( - properties.get("task_prompt").is_some(), - "should have task_prompt field" + properties.get("subagents").is_some(), + "should have subagents field" + ); + + let subagents_schema = properties.get("subagents").unwrap(); + assert!( + subagents_schema.get("items").is_some(), + "subagents should have items schema" + ); + + // The items use a $ref to definitions/SubagentConfig, so we need to look up + // the actual schema in the definitions section + let definitions = schema_json + .get("definitions") + .expect("schema should have definitions"); + let subagent_config_schema = definitions + .get("SubagentConfig") + .expect("definitions should have SubagentConfig"); + let item_properties = subagent_config_schema + .get("properties") + .expect("SubagentConfig should have properties"); + + assert!( + item_properties.get("label").is_some(), + "subagent item should have label field" + ); + assert!( + item_properties.get("task_prompt").is_some(), + "subagent item should have task_prompt field" ); assert!( - properties.get("summary_prompt").is_some(), - "should have summary_prompt field" + item_properties.get("summary_prompt").is_some(), + "subagent item should have summary_prompt field" ); assert!( - properties.get("context_low_prompt").is_some(), - "should have context_low_prompt field" + item_properties.get("context_low_prompt").is_some(), + "subagent item should have context_low_prompt field" ); assert!( - properties.get("timeout_ms").is_some(), - "should have timeout_ms field" + item_properties.get("timeout_ms").is_some(), + "subagent item should have timeout_ms field" ); assert!( - properties.get("allowed_tools").is_some(), - "should have allowed_tools field" + item_properties.get("allowed_tools").is_some(), + "subagent item should have allowed_tools field" ); } diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 2cb2ea4606437264b483c5d88ef317c805da75f2..3f2da94d16b752a398a955d7df1dee52ace65f8f 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -343,6 +343,7 @@ pub struct AcpThreadView { expanded_terminal_commands: HashSet, expanded_tool_call_raw_inputs: HashSet, expanded_thinking_blocks: HashSet<(usize, usize)>, + expanded_subagents: HashSet, edits_expanded: bool, plan_expanded: bool, queue_expanded: bool, @@ -528,6 +529,7 @@ impl AcpThreadView { expanded_terminal_commands: HashSet::default(), expanded_tool_call_raw_inputs: HashSet::default(), expanded_thinking_blocks: HashSet::default(), + expanded_subagents: HashSet::default(), editing_message: None, edits_expanded: false, plan_expanded: false, @@ -2960,6 +2962,13 @@ impl AcpThreadView { let is_edit = matches!(tool_call.kind, acp::ToolKind::Edit) || tool_call.diffs().next().is_some(); + let is_subagent = tool_call.is_subagent(); + + // For subagent tool calls, render the subagent cards directly without wrapper + if is_subagent { + return self.render_subagent_tool_call(entry_ix, tool_call, window, cx); + } + let is_cancelled_edit = is_edit && matches!(tool_call.status, ToolCallStatus::Canceled); let has_revealed_diff = tool_call.diffs().next().is_some_and(|diff| { self.entry_view_state @@ -3470,14 +3479,208 @@ impl AcpThreadView { self.render_terminal_tool_call(entry_ix, terminal, tool_call, window, cx) } ToolCallContent::SubagentThread(_thread) => { - // The subagent's AcpThread entity stores the subagent's conversation - // (messages, tool calls, etc.) but we don't render it here. The entity - // is used for serialization (e.g., to_markdown) and data storage, not display. + // Subagent threads are rendered by render_subagent_tool_call, not here Empty.into_any_element() } } } + fn render_subagent_tool_call( + &self, + entry_ix: usize, + tool_call: &ToolCall, + window: &Window, + cx: &Context, + ) -> Div { + let subagent_threads: Vec<_> = tool_call + .content + .iter() + .filter_map(|c| c.subagent_thread().cloned()) + .collect(); + + let tool_call_in_progress = matches!( + tool_call.status, + ToolCallStatus::Pending | ToolCallStatus::InProgress + ); + + v_flex().ml_5().mr_5().my_1p5().gap_1().children( + subagent_threads + .into_iter() + .enumerate() + .map(|(context_ix, thread)| { + self.render_subagent_card( + entry_ix, + context_ix, + &thread, + tool_call_in_progress, + window, + cx, + ) + }), + ) + } + + fn render_subagent_card( + &self, + entry_ix: usize, + context_ix: usize, + thread: &Entity, + tool_call_in_progress: bool, + window: &Window, + cx: &Context, + ) -> AnyElement { + let thread_read = thread.read(cx); + let session_id = thread_read.session_id().clone(); + let title = thread_read.title(); + let action_log = thread_read.action_log(); + let changed_buffers = action_log.read(cx).changed_buffers(cx); + + let is_expanded = self.expanded_subagents.contains(&session_id); + let files_changed = changed_buffers.len(); + let diff_stats = DiffStats::all_files(&changed_buffers, cx); + + let is_running = tool_call_in_progress; + + let card_header_id = + SharedString::from(format!("subagent-header-{}-{}", entry_ix, context_ix)); + let card_id = SharedString::from(format!("subagent-card-{}-{}", entry_ix, context_ix)); + let disclosure_id = + SharedString::from(format!("subagent-disclosure-{}-{}", entry_ix, context_ix)); + let diff_stat_id = SharedString::from(format!("subagent-diff-{}-{}", entry_ix, context_ix)); + + v_flex() + .w_full() + .rounded_md() + .border_1() + .border_color(self.tool_card_border_color(cx)) + .bg(cx.theme().colors().editor_background) + .overflow_hidden() + .child( + h_flex() + .id(card_id) + .group(&card_header_id) + .w_full() + .p_1() + .gap_1p5() + .bg(self.tool_card_header_bg(cx)) + .child( + div() + .id(disclosure_id) + .cursor_pointer() + .on_click(cx.listener({ + move |this, _, _, cx| { + if this.expanded_subagents.contains(&session_id) { + this.expanded_subagents.remove(&session_id); + } else { + this.expanded_subagents.insert(session_id.clone()); + } + cx.notify(); + } + })) + .child(Disclosure::new( + SharedString::from(format!( + "subagent-disclosure-inner-{}-{}", + entry_ix, context_ix + )), + is_expanded, + )), + ) + .child(if is_running { + SpinnerLabel::new() + .size(LabelSize::Small) + .into_any_element() + } else { + Icon::new(IconName::Check) + .size(IconSize::Small) + .color(Color::Success) + .into_any_element() + }) + .child( + h_flex().flex_1().overflow_hidden().child( + Label::new(title.to_string()) + .size(LabelSize::Small) + .color(Color::Default), + ), + ) + .when(files_changed > 0, |this| { + this.child( + h_flex() + .gap_1() + .child(Label::new("—").size(LabelSize::Small).color(Color::Muted)) + .child( + Label::new(format!( + "{} {} changed", + files_changed, + if files_changed == 1 { "file" } else { "files" } + )) + .size(LabelSize::Small) + .color(Color::Muted), + ) + .child(DiffStat::new( + diff_stat_id.clone(), + diff_stats.lines_added as usize, + diff_stats.lines_removed as usize, + )), + ) + }), + ) + .when(is_expanded, |this| { + this.child(self.render_subagent_expanded_content( + entry_ix, context_ix, thread, is_running, window, cx, + )) + }) + .into_any_element() + } + + fn render_subagent_expanded_content( + &self, + _entry_ix: usize, + _context_ix: usize, + thread: &Entity, + is_running: bool, + window: &Window, + cx: &Context, + ) -> impl IntoElement { + let thread_read = thread.read(cx); + let entries = thread_read.entries(); + + // Find the most recent assistant message with any content (message or thought) + let last_assistant_markdown = entries.iter().rev().find_map(|entry| { + if let AgentThreadEntry::AssistantMessage(msg) = entry { + msg.chunks.iter().find_map(|chunk| match chunk { + AssistantMessageChunk::Message { block } => block.markdown().cloned(), + AssistantMessageChunk::Thought { block } => block.markdown().cloned(), + }) + } else { + None + } + }); + + let has_content = last_assistant_markdown.is_some(); + + v_flex() + .w_full() + .p_2() + .gap_2() + .border_t_1() + .border_color(self.tool_card_border_color(cx)) + .bg(cx.theme().colors().editor_background) + .when_some(last_assistant_markdown, |this, markdown| { + this.child( + div() + .when(!is_running, |d| d.max_h(px(200.)).overflow_hidden()) + .text_sm() + .child(self.render_markdown( + markdown, + default_markdown_style(false, false, window, cx), + )), + ) + }) + .when(is_running && !has_content, |this| { + this.child(SpinnerLabel::new().size(LabelSize::Small)) + }) + } + fn render_markdown_output( &self, markdown: Entity, @@ -7361,6 +7564,13 @@ impl AcpThreadView { self.expanded_tool_calls.insert(tool_call_id); cx.notify(); } + + /// Expands a subagent card so its content is visible. + /// This is primarily useful for visual testing. + pub fn expand_subagent(&mut self, session_id: acp::SessionId, cx: &mut Context) { + self.expanded_subagents.insert(session_id); + cx.notify(); + } } impl Render for AcpThreadView { diff --git a/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs b/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs index c7f395ebbd813cfd7c28f33a7e69ec32f6d90fca..c79d68f5b1243cd69eb016ab65d9f3a31da9a8c4 100644 --- a/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs +++ b/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs @@ -350,14 +350,18 @@ impl ManageProfilesModal { return; }; + //todo: This causes the web search tool to show up even it only works when using zed hosted models + let tool_names: Vec> = agent::supported_built_in_tool_names( + self.active_model.as_ref().map(|model| model.provider_id()), + cx, + ) + .into_iter() + .map(|s| Arc::from(s)) + .collect(); + let tool_picker = cx.new(|cx| { let delegate = ToolPickerDelegate::builtin_tools( - //todo: This causes the web search tool to show up even it only works when using zed hosted models - agent::supported_built_in_tool_names( - self.active_model.as_ref().map(|model| model.provider_id()), - ) - .map(|s| s.into()) - .collect::>(), + tool_names, self.fs.clone(), profile_id.clone(), profile, diff --git a/crates/feature_flags/src/flags.rs b/crates/feature_flags/src/flags.rs index 093470b64ba16700eea444fac47f4ec1a0b295ad..20a23df734e9df90a734da6826077a2b13cf997d 100644 --- a/crates/feature_flags/src/flags.rs +++ b/crates/feature_flags/src/flags.rs @@ -46,7 +46,7 @@ impl FeatureFlag for SubagentsFeatureFlag { const NAME: &'static str = "subagents"; fn enabled_for_staff() -> bool { - false + true } } diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index b8aa1deb0b6f049d9beab26f52345c9843d6ba2f..153d7983243adaf89e8d243b873c53bbbfb164da 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -41,6 +41,7 @@ visual-tests = [ "image_viewer/test-support", "clock/test-support", "acp_thread/test-support", + "action_log/test-support", "agent_ui/test-support", "db/test-support", "agent/test-support", diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index 18138ff920373d867d794820367dc2046d356f8a..8e1ac87705fbba1eee64c1806a9b169aed4767fc 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -67,6 +67,7 @@ use { sync::Arc, time::Duration, }, + watch, workspace::{AppState, Workspace}, }; @@ -403,24 +404,47 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()> } // Run Test 3: Agent Thread View tests - println!("\n--- Test 3: agent_thread_with_image (collapsed + expanded) ---"); - match run_agent_thread_view_test(app_state.clone(), &mut cx, update_baseline) { - Ok(TestResult::Passed) => { - println!("✓ agent_thread_with_image (collapsed + expanded): PASSED"); - passed += 1; - } - Ok(TestResult::BaselineUpdated(_)) => { - println!("✓ agent_thread_with_image: Baselines updated (collapsed + expanded)"); - updated += 1; + #[cfg(feature = "visual-tests")] + { + println!("\n--- Test 3: agent_thread_with_image (collapsed + expanded) ---"); + match run_agent_thread_view_test(app_state.clone(), &mut cx, update_baseline) { + Ok(TestResult::Passed) => { + println!("✓ agent_thread_with_image (collapsed + expanded): PASSED"); + passed += 1; + } + Ok(TestResult::BaselineUpdated(_)) => { + println!("✓ agent_thread_with_image: Baselines updated (collapsed + expanded)"); + updated += 1; + } + Err(e) => { + eprintln!("✗ agent_thread_with_image: FAILED - {}", e); + failed += 1; + } } - Err(e) => { - eprintln!("✗ agent_thread_with_image: FAILED - {}", e); - failed += 1; + } + + // Run Test 4: Subagent Cards visual tests + #[cfg(feature = "visual-tests")] + { + println!("\n--- Test 4: subagent_cards (running, completed, expanded) ---"); + match run_subagent_visual_tests(app_state.clone(), &mut cx, update_baseline) { + Ok(TestResult::Passed) => { + println!("✓ subagent_cards: PASSED"); + passed += 1; + } + Ok(TestResult::BaselineUpdated(_)) => { + println!("✓ subagent_cards: Baselines updated"); + updated += 1; + } + Err(e) => { + eprintln!("✗ subagent_cards: FAILED - {}", e); + failed += 1; + } } } - // Run Test 4: Breakpoint Hover visual tests - println!("\n--- Test 4: breakpoint_hover (3 variants) ---"); + // Run Test 5: Breakpoint Hover visual tests + println!("\n--- Test 5: breakpoint_hover (3 variants) ---"); match run_breakpoint_hover_visual_tests(app_state.clone(), &mut cx, update_baseline) { Ok(TestResult::Passed) => { println!("✓ breakpoint_hover: PASSED"); @@ -436,8 +460,8 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()> } } - // Run Test 5: Diff Review Button visual tests - println!("\n--- Test 5: diff_review_button (3 variants) ---"); + // Run Test 6: Diff Review Button visual tests + println!("\n--- Test 6: diff_review_button (3 variants) ---"); match run_diff_review_visual_tests(app_state.clone(), &mut cx, update_baseline) { Ok(TestResult::Passed) => { println!("✓ diff_review_button: PASSED"); @@ -1645,7 +1669,327 @@ impl AgentServer for StubAgentServer { } } -#[cfg(target_os = "macos")] +#[cfg(all(target_os = "macos", feature = "visual-tests"))] +fn run_subagent_visual_tests( + app_state: Arc, + cx: &mut VisualTestAppContext, + update_baseline: bool, +) -> Result { + use acp_thread::{ + AcpThread, SUBAGENT_TOOL_NAME, ToolCallUpdateSubagentThread, meta_with_tool_name, + }; + use agent_ui::AgentPanel; + + // Create a temporary project directory + let temp_dir = tempfile::tempdir()?; + let temp_path = temp_dir.keep(); + let canonical_temp = temp_path.canonicalize()?; + let project_path = canonical_temp.join("project"); + std::fs::create_dir_all(&project_path)?; + + // Create a project + let project = cx.update(|cx| { + project::Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + false, + cx, + ) + }); + + // Add the test directory as a worktree + let add_worktree_task = project.update(cx, |project, cx| { + project.find_or_create_worktree(&project_path, true, cx) + }); + + let _ = cx.foreground_executor.block_test(add_worktree_task); + + cx.run_until_parked(); + + // Create stub connection - we'll manually inject the subagent content + let connection = StubAgentConnection::new(); + + // Create a subagent tool call (in progress state) + let tool_call = acp::ToolCall::new("subagent-tool-1", "2 subagents") + .kind(acp::ToolKind::Other) + .meta(meta_with_tool_name(SUBAGENT_TOOL_NAME)) + .status(acp::ToolCallStatus::InProgress); + + connection.set_next_prompt_updates(vec![acp::SessionUpdate::ToolCall(tool_call)]); + + let stub_agent: Rc = Rc::new(StubAgentServer::new(connection.clone())); + + // Create a window sized for the agent panel + let window_size = size(px(600.0), px(700.0)); + let bounds = Bounds { + origin: point(px(0.0), px(0.0)), + size: window_size, + }; + + let workspace_window: WindowHandle = cx + .update(|cx| { + cx.open_window( + WindowOptions { + window_bounds: Some(WindowBounds::Windowed(bounds)), + focus: false, + show: false, + ..Default::default() + }, + |window, cx| { + cx.new(|cx| { + Workspace::new(None, project.clone(), app_state.clone(), window, cx) + }) + }, + ) + }) + .context("Failed to open agent window")?; + + cx.run_until_parked(); + + // Load the AgentPanel + let (weak_workspace, async_window_cx) = workspace_window + .update(cx, |workspace, window, cx| { + (workspace.weak_handle(), window.to_async(cx)) + }) + .context("Failed to get workspace handle")?; + + let prompt_builder = + cx.update(|cx| prompt_store::PromptBuilder::load(app_state.fs.clone(), false, cx)); + let panel = cx + .foreground_executor + .block_test(AgentPanel::load( + weak_workspace, + prompt_builder, + async_window_cx, + )) + .context("Failed to load AgentPanel")?; + + cx.update_window(workspace_window.into(), |_, _window, cx| { + workspace_window + .update(cx, |workspace, window, cx| { + workspace.add_panel(panel.clone(), window, cx); + workspace.open_panel::(window, cx); + }) + .ok(); + })?; + + cx.run_until_parked(); + + // Open the stub thread + cx.update_window(workspace_window.into(), |_, window, cx| { + panel.update(cx, |panel: &mut agent_ui::AgentPanel, cx| { + panel.open_external_thread_with_server(stub_agent.clone(), window, cx); + }); + })?; + + cx.run_until_parked(); + + // Get the thread view and send a message to trigger the subagent tool call + let thread_view = cx + .read(|cx| panel.read(cx).active_thread_view_for_tests().cloned()) + .ok_or_else(|| anyhow::anyhow!("No active thread view"))?; + + let thread = cx + .read(|cx| thread_view.read(cx).thread().cloned()) + .ok_or_else(|| anyhow::anyhow!("Thread not available"))?; + + // Send the message to trigger the subagent response + let send_future = thread.update(cx, |thread: &mut acp_thread::AcpThread, cx| { + thread.send(vec!["Run two subagents".into()], cx) + }); + + let _ = cx.foreground_executor.block_test(send_future); + + cx.run_until_parked(); + + // Get the tool call ID + let tool_call_id = cx + .read(|cx| { + thread.read(cx).entries().iter().find_map(|entry| { + if let acp_thread::AgentThreadEntry::ToolCall(tool_call) = entry { + Some(tool_call.id.clone()) + } else { + None + } + }) + }) + .ok_or_else(|| anyhow::anyhow!("Expected a ToolCall entry in thread"))?; + + // Create two subagent AcpThreads and inject them + let subagent1 = cx.update(|cx| { + let action_log = cx.new(|_| action_log::ActionLog::new(project.clone())); + let session_id = acp::SessionId::new("subagent-1"); + cx.new(|cx| { + let mut thread = AcpThread::new( + "Exploring test-repo", + Rc::new(connection.clone()), + project.clone(), + action_log, + session_id, + watch::Receiver::constant(acp::PromptCapabilities::new()), + cx, + ); + // Add some content to this subagent + thread.push_assistant_content_block( + "## Summary of test-repo\n\nThis is a test repository with:\n\n- **Files:** test.txt\n- **Purpose:** Testing".into(), + false, + cx, + ); + thread + }) + }); + + let subagent2 = cx.update(|cx| { + let action_log = cx.new(|_| action_log::ActionLog::new(project.clone())); + let session_id = acp::SessionId::new("subagent-2"); + cx.new(|cx| { + let mut thread = AcpThread::new( + "Exploring test-worktree", + Rc::new(connection.clone()), + project.clone(), + action_log, + session_id, + watch::Receiver::constant(acp::PromptCapabilities::new()), + cx, + ); + // Add some content to this subagent + thread.push_assistant_content_block( + "## Summary of test-worktree\n\nThis directory contains:\n\n- A single `config.json` file\n- Basic project setup".into(), + false, + cx, + ); + thread + }) + }); + + // Inject subagent threads into the tool call + thread.update(cx, |thread: &mut acp_thread::AcpThread, cx| { + thread + .update_tool_call( + ToolCallUpdateSubagentThread { + id: tool_call_id.clone(), + thread: subagent1, + }, + cx, + ) + .ok(); + thread + .update_tool_call( + ToolCallUpdateSubagentThread { + id: tool_call_id.clone(), + thread: subagent2, + }, + cx, + ) + .ok(); + }); + + cx.run_until_parked(); + + cx.update_window(workspace_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture subagents in RUNNING state (tool call still in progress) + let running_result = run_visual_test( + "subagent_cards_running", + workspace_window.into(), + cx, + update_baseline, + )?; + + // Now mark the tool call as completed by updating it through the thread + thread.update(cx, |thread: &mut acp_thread::AcpThread, cx| { + thread + .handle_session_update( + acp::SessionUpdate::ToolCallUpdate(acp::ToolCallUpdate::new( + tool_call_id.clone(), + acp::ToolCallUpdateFields::new().status(acp::ToolCallStatus::Completed), + )), + cx, + ) + .ok(); + }); + + cx.run_until_parked(); + + cx.update_window(workspace_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture subagents in COMPLETED state + let completed_result = run_visual_test( + "subagent_cards_completed", + workspace_window.into(), + cx, + update_baseline, + )?; + + // Expand the first subagent + thread_view.update(cx, |view: &mut agent_ui::acp::AcpThreadView, cx| { + view.expand_subagent(acp::SessionId::new("subagent-1"), cx); + }); + + cx.run_until_parked(); + + cx.update_window(workspace_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture subagent in EXPANDED state + let expanded_result = run_visual_test( + "subagent_cards_expanded", + workspace_window.into(), + cx, + update_baseline, + )?; + + // Cleanup + workspace_window + .update(cx, |workspace, _window, cx| { + let project = workspace.project().clone(); + project.update(cx, |project, cx| { + let worktree_ids: Vec<_> = + project.worktrees(cx).map(|wt| wt.read(cx).id()).collect(); + for id in worktree_ids { + project.remove_worktree(id, cx); + } + }); + }) + .ok(); + + cx.run_until_parked(); + + let _ = cx.update_window(workspace_window.into(), |_, window, _cx| { + window.remove_window(); + }); + + cx.run_until_parked(); + + for _ in 0..15 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + match (&running_result, &completed_result, &expanded_result) { + (TestResult::Passed, TestResult::Passed, TestResult::Passed) => Ok(TestResult::Passed), + (TestResult::BaselineUpdated(p), _, _) + | (_, TestResult::BaselineUpdated(p), _) + | (_, _, TestResult::BaselineUpdated(p)) => Ok(TestResult::BaselineUpdated(p.clone())), + } +} + +#[cfg(all(target_os = "macos", feature = "visual-tests"))] fn run_agent_thread_view_test( app_state: Arc, cx: &mut VisualTestAppContext,