From 993b0c8e711166ccb03d74f68273c88a5e24e4ef Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Mon, 23 Feb 2026 16:05:15 +0100 Subject: [PATCH] Improve subagent permission UX (#49874) Before you mark this PR as ready for review, make sure that you have: - [x] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [x] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - N/A --------- Co-authored-by: Ben Brandt --- crates/acp_thread/src/acp_thread.rs | 51 +- crates/agent/src/tests/mod.rs | 31 +- crates/agent/src/tools/spawn_agent_tool.rs | 39 +- crates/agent_ui/src/acp/thread_view.rs | 486 ++++++++-- .../src/acp/thread_view/active_thread.rs | 884 +++++++----------- crates/agent_ui/src/agent_diff.rs | 3 +- 6 files changed, 805 insertions(+), 689 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index aace4c532d53f69954feadfaf3d49c3fe7b2da38..8da004b2cb967b19ec27d03ce573778bf301fcd9 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -977,7 +977,8 @@ pub enum AcpThreadEvent { TokenUsageUpdated, EntryUpdated(usize), EntriesRemoved(Range), - ToolAuthorizationRequired, + ToolAuthorizationRequested(acp::ToolCallId), + ToolAuthorizationReceived(acp::ToolCallId), Retry(RetryStatus), SubagentSpawned(acp::SessionId), Stopped, @@ -1666,7 +1667,7 @@ impl AcpThread { }) } - pub fn tool_call(&mut self, id: &acp::ToolCallId) -> Option<(usize, &ToolCall)> { + pub fn tool_call(&self, id: &acp::ToolCallId) -> Option<(usize, &ToolCall)> { self.entries .iter() .enumerate() @@ -1744,7 +1745,7 @@ impl AcpThread { tool_call: acp::ToolCallUpdate, options: PermissionOptions, cx: &mut Context, - ) -> Result> { + ) -> Result> { let (tx, rx) = oneshot::channel(); let status = ToolCallStatus::WaitingForConfirmation { @@ -1752,20 +1753,25 @@ impl AcpThread { respond_tx: tx, }; + let tool_call_id = tool_call.tool_call_id.clone(); self.upsert_tool_call_inner(tool_call, status, cx)?; - cx.emit(AcpThreadEvent::ToolAuthorizationRequired); + cx.emit(AcpThreadEvent::ToolAuthorizationRequested( + tool_call_id.clone(), + )); - let fut = async { - match rx.await { + Ok(cx.spawn(async move |this, cx| { + let outcome = match rx.await { Ok(option) => acp::RequestPermissionOutcome::Selected( acp::SelectedPermissionOutcome::new(option), ), Err(oneshot::Canceled) => acp::RequestPermissionOutcome::Cancelled, - } - } - .boxed(); - - Ok(fut) + }; + this.update(cx, |_this, cx| { + cx.emit(AcpThreadEvent::ToolAuthorizationReceived(tool_call_id)) + }) + .ok(); + outcome + })) } pub fn authorize_tool_call( @@ -1800,29 +1806,6 @@ impl AcpThread { cx.emit(AcpThreadEvent::EntryUpdated(ix)); } - pub fn first_tool_awaiting_confirmation(&self) -> Option<&ToolCall> { - let mut first_tool_call = None; - - for entry in self.entries.iter().rev() { - match &entry { - AgentThreadEntry::ToolCall(call) => { - if let ToolCallStatus::WaitingForConfirmation { .. } = call.status { - first_tool_call = Some(call); - } else { - continue; - } - } - AgentThreadEntry::UserMessage(_) | AgentThreadEntry::AssistantMessage(_) => { - // Reached the beginning of the turn. - // If we had pending permission requests in the previous turn, they have been cancelled. - break; - } - } - } - - first_tool_call - } - pub fn plan(&self) -> &Plan { &self.plan } diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index da5f5d2c8ec8b22d1412832a40a2d8f6ae4d9222..3df64794c53e7f173de9adf5f9b8893d56fa70e6 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -4294,33 +4294,26 @@ async fn test_subagent_tool_call_end_to_end(cx: &mut TestAppContext) { assert_eq!( acp_thread.read_with(cx, |thread, cx| thread.to_markdown(cx)), - format!( - indoc! {r#" - ## User + indoc! {r#" + ## User - Prompt + Prompt - ## Assistant + ## Assistant - spawning subagent + spawning subagent - **Tool Call: label** - Status: Completed + **Tool Call: label** + Status: Completed - ```json - {{ - "session_id": "{}", - "output": "subagent task response\n" - }} - ``` + subagent task response - ## Assistant - Response + ## Assistant - "#}, - subagent_session_id - ) + Response + + "#}, ); } diff --git a/crates/agent/src/tools/spawn_agent_tool.rs b/crates/agent/src/tools/spawn_agent_tool.rs index e194636bc6dff629627be290f22ecb6d148b2f2a..61446e1d438a67bd4d7e9abcf3f2a50b8ec6f47f 100644 --- a/crates/agent/src/tools/spawn_agent_tool.rs +++ b/crates/agent/src/tools/spawn_agent_tool.rs @@ -148,19 +148,26 @@ impl AgentTool for SpawnAgentTool { )]); event_stream.update_fields_with_meta(acp::ToolCallUpdateFields::new(), Some(meta)); - cx.spawn(async move |cx| { - let output = - subagent - .wait_for_output(cx) - .await - .map_err(|e| SpawnAgentToolOutput::Error { - session_id: Some(subagent_session_id.clone()), - error: e.to_string(), - })?; - Ok(SpawnAgentToolOutput::Success { - session_id: subagent_session_id, - output, - }) + cx.spawn(async move |cx| match subagent.wait_for_output(cx).await { + Ok(output) => { + event_stream.update_fields( + acp::ToolCallUpdateFields::new().content(vec![output.clone().into()]), + ); + Ok(SpawnAgentToolOutput::Success { + session_id: subagent_session_id, + output, + }) + } + Err(e) => { + let error = e.to_string(); + event_stream.update_fields( + acp::ToolCallUpdateFields::new().content(vec![error.clone().into()]), + ); + Err(SpawnAgentToolOutput::Error { + session_id: Some(subagent_session_id), + error, + }) + } }) } @@ -185,6 +192,12 @@ impl AgentTool for SpawnAgentTool { event_stream.update_fields_with_meta(acp::ToolCallUpdateFields::new(), Some(meta)); } + let content = match &output { + SpawnAgentToolOutput::Success { output, .. } => output.into(), + SpawnAgentToolOutput::Error { error, .. } => error.into(), + }; + event_stream.update_fields(acp::ToolCallUpdateFields::new().content(vec![content])); + Ok(()) } } diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 117f457ec0598fbe3a68b57515c2ed2c8578390b..cbdc3ad5d1e5d28b1597ba405846ac48dbfeb928 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -15,7 +15,7 @@ use arrayvec::ArrayVec; use audio::{Audio, Sound}; use buffer_diff::BufferDiff; use client::zed_urls; -use collections::{HashMap, HashSet}; +use collections::{HashMap, HashSet, IndexMap}; use editor::scroll::Autoscroll; use editor::{ Editor, EditorEvent, EditorMode, MultiBuffer, PathKey, SelectionEffects, SizingBehavior, @@ -159,6 +159,125 @@ impl ProfileProvider for Entity { } } +#[derive(Default)] +pub(crate) struct Conversation { + threads: HashMap>, + permission_requests: IndexMap>, + subscriptions: Vec, +} + +impl Conversation { + pub fn register_thread(&mut self, thread: Entity, cx: &mut Context) { + let session_id = thread.read(cx).session_id().clone(); + let subscription = cx.subscribe(&thread, move |this, _thread, event, _cx| match event { + AcpThreadEvent::ToolAuthorizationRequested(id) => { + this.permission_requests + .entry(session_id.clone()) + .or_default() + .push(id.clone()); + } + AcpThreadEvent::ToolAuthorizationReceived(id) => { + if let Some(tool_calls) = this.permission_requests.get_mut(&session_id) { + tool_calls.retain(|tool_call_id| tool_call_id != id); + if tool_calls.is_empty() { + this.permission_requests.shift_remove(&session_id); + } + } + } + AcpThreadEvent::NewEntry + | AcpThreadEvent::TitleUpdated + | AcpThreadEvent::TokenUsageUpdated + | AcpThreadEvent::EntryUpdated(_) + | AcpThreadEvent::EntriesRemoved(_) + | AcpThreadEvent::Retry(_) + | AcpThreadEvent::SubagentSpawned(_) + | AcpThreadEvent::Stopped + | AcpThreadEvent::Error + | AcpThreadEvent::LoadError(_) + | AcpThreadEvent::PromptCapabilitiesUpdated + | AcpThreadEvent::Refusal + | AcpThreadEvent::AvailableCommandsUpdated(_) + | AcpThreadEvent::ModeUpdated(_) + | AcpThreadEvent::ConfigOptionsUpdated(_) => {} + }); + self.subscriptions.push(subscription); + self.threads + .insert(thread.read(cx).session_id().clone(), thread); + } + + pub fn pending_tool_call<'a>( + &'a self, + session_id: &acp::SessionId, + cx: &'a App, + ) -> Option<(acp::SessionId, acp::ToolCallId, &'a PermissionOptions)> { + let thread = self.threads.get(session_id)?; + let is_subagent = thread.read(cx).parent_session_id().is_some(); + let (thread, tool_id) = if is_subagent { + let id = self.permission_requests.get(session_id)?.iter().next()?; + (thread, id) + } else { + let (id, tool_calls) = self.permission_requests.first()?; + let thread = self.threads.get(id)?; + let id = tool_calls.iter().next()?; + (thread, id) + }; + let (_, tool_call) = thread.read(cx).tool_call(tool_id)?; + + let ToolCallStatus::WaitingForConfirmation { options, .. } = &tool_call.status else { + return None; + }; + Some(( + thread.read(cx).session_id().clone(), + tool_id.clone(), + options, + )) + } + + pub fn authorize_pending_tool_call( + &mut self, + session_id: &acp::SessionId, + kind: acp::PermissionOptionKind, + cx: &mut Context, + ) -> Option<()> { + let (_, tool_call_id, options) = self.pending_tool_call(session_id, cx)?; + let option = options.first_option_of_kind(kind)?; + self.authorize_tool_call( + session_id.clone(), + tool_call_id, + option.option_id.clone(), + option.kind, + cx, + ); + Some(()) + } + + pub fn authorize_tool_call( + &mut self, + session_id: acp::SessionId, + tool_call_id: acp::ToolCallId, + option_id: acp::PermissionOptionId, + option_kind: acp::PermissionOptionKind, + cx: &mut Context, + ) { + let Some(thread) = self.threads.get(&session_id) else { + return; + }; + let agent_telemetry_id = thread.read(cx).connection().telemetry_id(); + + telemetry::event!( + "Agent Tool Call Authorized", + agent = agent_telemetry_id, + session = session_id, + option = option_kind + ); + + thread.update(cx, |thread, cx| { + thread.authorize_tool_call(tool_call_id, option_id, option_kind, cx); + }); + cx.notify(); + } +} + pub struct AcpServerView { agent: Rc, agent_server_store: Entity, @@ -184,6 +303,17 @@ impl AcpServerView { } } + pub fn pending_tool_call<'a>( + &'a self, + cx: &'a App, + ) -> Option<(acp::SessionId, acp::ToolCallId, &'a PermissionOptions)> { + let id = &self.active_thread()?.read(cx).id; + self.as_connected()? + .conversation + .read(cx) + .pending_tool_call(id, cx) + } + pub fn parent_thread(&self, cx: &App) -> Option> { match &self.server_state { ServerState::Connected(connected) => { @@ -251,6 +381,7 @@ pub struct ConnectedServerState { active_id: Option, threads: HashMap>, connection: Rc, + conversation: Entity, } enum AuthState { @@ -544,9 +675,16 @@ impl AcpServerView { this.update_in(cx, |this, window, cx| { match result { Ok(thread) => { + let conversation = cx.new(|cx| { + let mut conversation = Conversation::default(); + conversation.register_thread(thread.clone(), cx); + conversation + }); + let current = this.new_thread_view( None, thread, + conversation.clone(), resumed_without_history, resume_thread, initial_content, @@ -569,6 +707,7 @@ impl AcpServerView { auth_state: AuthState::Ok, active_id: Some(id.clone()), threads: HashMap::from_iter([(id, current)]), + conversation, }), cx, ); @@ -623,6 +762,7 @@ impl AcpServerView { &self, parent_id: Option, thread: Entity, + conversation: Entity, resumed_without_history: bool, resume_thread: Option, initial_content: Option, @@ -780,6 +920,7 @@ impl AcpServerView { AcpThreadView::new( parent_id, thread, + conversation, self.login.clone(), weak, agent_icon, @@ -879,6 +1020,7 @@ impl AcpServerView { active_id: None, threads: HashMap::default(), connection, + conversation: cx.new(|_cx| Conversation::default()), }), cx, ); @@ -1045,9 +1187,10 @@ impl AcpServerView { window, cx, ), - AcpThreadEvent::ToolAuthorizationRequired => { + AcpThreadEvent::ToolAuthorizationRequested(_) => { self.notify_with_sound("Waiting for tool confirmation", IconName::Info, window, cx); } + AcpThreadEvent::ToolAuthorizationReceived(_) => {} AcpThreadEvent::Retry(retry) => { if let Some(active) = self.thread_view(&thread_id) { active.update(cx, |active, _cx| { @@ -1513,9 +1656,19 @@ impl AcpServerView { cx.spawn_in(window, async move |this, cx| { let subagent_thread = subagent_thread_task.await?; this.update_in(cx, |this, window, cx| { + let conversation = this + .as_connected() + .map(|connected| connected.conversation.clone()); + let Some(conversation) = conversation else { + return; + }; + conversation.update(cx, |conversation, cx| { + conversation.register_thread(subagent_thread.clone(), cx); + }); let view = this.new_thread_view( Some(parent_id), subagent_thread, + conversation, false, None, None, @@ -2474,17 +2627,6 @@ impl AcpServerView { 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) { - if let Some(active) = self.active_thread() { - active.update(cx, |active, _cx| { - active.expanded_subagents.insert(session_id); - }); - cx.notify(); - } - } } impl Render for AcpServerView { @@ -5340,14 +5482,7 @@ pub(crate) mod tests { // Verify tool call is waiting for confirmation thread_view.read_with(cx, |thread_view, cx| { - let thread = thread_view - .active_thread() - .expect("Thread should exist") - .read(cx) - .thread - .clone(); - let thread = thread.read(cx); - let tool_call = thread.first_tool_awaiting_confirmation(); + let tool_call = thread_view.pending_tool_call(cx); assert!( tool_call.is_some(), "Expected a tool call waiting for confirmation" @@ -5371,14 +5506,12 @@ pub(crate) mod tests { // Verify tool call is no longer waiting for confirmation (was authorized) thread_view.read_with(cx, |thread_view, cx| { - let thread = thread_view.active_thread().expect("Thread should exist").read(cx).thread.clone(); - let thread = thread.read(cx); - let tool_call = thread.first_tool_awaiting_confirmation(); - assert!( - tool_call.is_none(), - "Tool call should no longer be waiting for confirmation after AuthorizeToolCall action" - ); - }); + let tool_call = thread_view.pending_tool_call(cx); + assert!( + tool_call.is_none(), + "Tool call should no longer be waiting for confirmation after AuthorizeToolCall action" + ); + }); } #[gpui::test] @@ -5456,14 +5589,7 @@ pub(crate) mod tests { // Verify tool call was authorized thread_view.read_with(cx, |thread_view, cx| { - let thread = thread_view - .active_thread() - .expect("Thread should exist") - .read(cx) - .thread - .clone(); - let thread = thread.read(cx); - let tool_call = thread.first_tool_awaiting_confirmation(); + let tool_call = thread_view.pending_tool_call(cx); assert!( tool_call.is_none(), "Tool call should be authorized after selecting pattern option" @@ -5639,14 +5765,7 @@ pub(crate) mod tests { // Verify tool call was authorized thread_view.read_with(cx, |thread_view, cx| { - let thread = thread_view - .active_thread() - .expect("Thread should exist") - .read(cx) - .thread - .clone(); - let thread = thread.read(cx); - let tool_call = thread.first_tool_awaiting_confirmation(); + let tool_call = thread_view.pending_tool_call(cx); assert!( tool_call.is_none(), "Tool call should be authorized after Allow with pattern granularity" @@ -5706,14 +5825,7 @@ pub(crate) mod tests { // Verify tool call was rejected (no longer waiting for confirmation) thread_view.read_with(cx, |thread_view, cx| { - let thread = thread_view - .active_thread() - .expect("Thread should exist") - .read(cx) - .thread - .clone(); - let thread = thread.read(cx); - let tool_call = thread.first_tool_awaiting_confirmation(); + let tool_call = thread_view.pending_tool_call(cx); assert!( tool_call.is_none(), "Tool call should be rejected after Deny" @@ -5871,4 +5983,272 @@ pub(crate) mod tests { } }); } + + fn create_test_acp_thread( + parent_session_id: Option, + session_id: &str, + connection: Rc, + project: Entity, + cx: &mut App, + ) -> Entity { + let action_log = cx.new(|_| ActionLog::new(project.clone())); + cx.new(|cx| { + AcpThread::new( + parent_session_id, + "Test Thread", + connection, + project, + action_log, + acp::SessionId::new(session_id), + watch::Receiver::constant(acp::PromptCapabilities::new()), + cx, + ) + }) + } + + fn request_test_tool_authorization( + thread: &Entity, + tool_call_id: &str, + option_id: &str, + cx: &mut TestAppContext, + ) -> Task { + let tool_call_id = acp::ToolCallId::new(tool_call_id); + let label = format!("Tool {tool_call_id}"); + let option_id = acp::PermissionOptionId::new(option_id); + cx.update(|cx| { + thread.update(cx, |thread, cx| { + thread + .request_tool_call_authorization( + acp::ToolCall::new(tool_call_id, label) + .kind(acp::ToolKind::Edit) + .into(), + PermissionOptions::Flat(vec![acp::PermissionOption::new( + option_id, + "Allow", + acp::PermissionOptionKind::AllowOnce, + )]), + cx, + ) + .unwrap() + }) + }) + } + + #[gpui::test] + async fn test_conversation_multiple_tool_calls_fifo_ordering(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let connection: Rc = Rc::new(StubAgentConnection::new()); + + let (thread, conversation) = cx.update(|cx| { + let thread = + create_test_acp_thread(None, "session-1", connection.clone(), project.clone(), cx); + let conversation = cx.new(|cx| { + let mut conversation = Conversation::default(); + conversation.register_thread(thread.clone(), cx); + conversation + }); + (thread, conversation) + }); + + let _task1 = request_test_tool_authorization(&thread, "tc-1", "allow-1", cx); + let _task2 = request_test_tool_authorization(&thread, "tc-2", "allow-2", cx); + + cx.read(|cx| { + let session_id = acp::SessionId::new("session-1"); + let (_, tool_call_id, _) = conversation + .read(cx) + .pending_tool_call(&session_id, cx) + .expect("Expected a pending tool call"); + assert_eq!(tool_call_id, acp::ToolCallId::new("tc-1")); + }); + + cx.update(|cx| { + conversation.update(cx, |conversation, cx| { + conversation.authorize_tool_call( + acp::SessionId::new("session-1"), + acp::ToolCallId::new("tc-1"), + acp::PermissionOptionId::new("allow-1"), + acp::PermissionOptionKind::AllowOnce, + cx, + ); + }); + }); + + cx.run_until_parked(); + + cx.read(|cx| { + let session_id = acp::SessionId::new("session-1"); + let (_, tool_call_id, _) = conversation + .read(cx) + .pending_tool_call(&session_id, cx) + .expect("Expected tc-2 to be pending after tc-1 was authorized"); + assert_eq!(tool_call_id, acp::ToolCallId::new("tc-2")); + }); + + cx.update(|cx| { + conversation.update(cx, |conversation, cx| { + conversation.authorize_tool_call( + acp::SessionId::new("session-1"), + acp::ToolCallId::new("tc-2"), + acp::PermissionOptionId::new("allow-2"), + acp::PermissionOptionKind::AllowOnce, + cx, + ); + }); + }); + + cx.run_until_parked(); + + cx.read(|cx| { + let session_id = acp::SessionId::new("session-1"); + assert!( + conversation + .read(cx) + .pending_tool_call(&session_id, cx) + .is_none(), + "Expected no pending tool calls after both were authorized" + ); + }); + } + + #[gpui::test] + async fn test_conversation_subagent_scoped_pending_tool_call(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let connection: Rc = Rc::new(StubAgentConnection::new()); + + let (parent_thread, subagent_thread, conversation) = cx.update(|cx| { + let parent_thread = + create_test_acp_thread(None, "parent", connection.clone(), project.clone(), cx); + let subagent_thread = create_test_acp_thread( + Some(acp::SessionId::new("parent")), + "subagent", + connection.clone(), + project.clone(), + cx, + ); + let conversation = cx.new(|cx| { + let mut conversation = Conversation::default(); + conversation.register_thread(parent_thread.clone(), cx); + conversation.register_thread(subagent_thread.clone(), cx); + conversation + }); + (parent_thread, subagent_thread, conversation) + }); + + let _parent_task = + request_test_tool_authorization(&parent_thread, "parent-tc", "allow-parent", cx); + let _subagent_task = + request_test_tool_authorization(&subagent_thread, "subagent-tc", "allow-subagent", cx); + + // Querying with the subagent's session ID returns only the + // subagent's own tool call (subagent path is scoped to its session) + cx.read(|cx| { + let subagent_id = acp::SessionId::new("subagent"); + let (session_id, tool_call_id, _) = conversation + .read(cx) + .pending_tool_call(&subagent_id, cx) + .expect("Expected subagent's pending tool call"); + assert_eq!(session_id, acp::SessionId::new("subagent")); + assert_eq!(tool_call_id, acp::ToolCallId::new("subagent-tc")); + }); + + // Querying with the parent's session ID returns the first pending + // request in FIFO order across all sessions + cx.read(|cx| { + let parent_id = acp::SessionId::new("parent"); + let (session_id, tool_call_id, _) = conversation + .read(cx) + .pending_tool_call(&parent_id, cx) + .expect("Expected a pending tool call from parent query"); + assert_eq!(session_id, acp::SessionId::new("parent")); + assert_eq!(tool_call_id, acp::ToolCallId::new("parent-tc")); + }); + } + + #[gpui::test] + async fn test_conversation_parent_pending_tool_call_returns_first_across_threads( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let connection: Rc = Rc::new(StubAgentConnection::new()); + + let (thread_a, thread_b, conversation) = cx.update(|cx| { + let thread_a = + create_test_acp_thread(None, "thread-a", connection.clone(), project.clone(), cx); + let thread_b = + create_test_acp_thread(None, "thread-b", connection.clone(), project.clone(), cx); + let conversation = cx.new(|cx| { + let mut conversation = Conversation::default(); + conversation.register_thread(thread_a.clone(), cx); + conversation.register_thread(thread_b.clone(), cx); + conversation + }); + (thread_a, thread_b, conversation) + }); + + let _task_a = request_test_tool_authorization(&thread_a, "tc-a", "allow-a", cx); + let _task_b = request_test_tool_authorization(&thread_b, "tc-b", "allow-b", cx); + + // Both threads are non-subagent, so pending_tool_call always returns + // the first entry from permission_requests (FIFO across all sessions) + cx.read(|cx| { + let session_a = acp::SessionId::new("thread-a"); + let (session_id, tool_call_id, _) = conversation + .read(cx) + .pending_tool_call(&session_a, cx) + .expect("Expected a pending tool call"); + assert_eq!(session_id, acp::SessionId::new("thread-a")); + assert_eq!(tool_call_id, acp::ToolCallId::new("tc-a")); + }); + + // Querying with thread-b also returns thread-a's tool call, + // because non-subagent queries always use permission_requests.first() + cx.read(|cx| { + let session_b = acp::SessionId::new("thread-b"); + let (session_id, tool_call_id, _) = conversation + .read(cx) + .pending_tool_call(&session_b, cx) + .expect("Expected a pending tool call from thread-b query"); + assert_eq!( + session_id, + acp::SessionId::new("thread-a"), + "Non-subagent queries always return the first pending request in FIFO order" + ); + assert_eq!(tool_call_id, acp::ToolCallId::new("tc-a")); + }); + + // After authorizing thread-a's tool call, thread-b's becomes first + cx.update(|cx| { + conversation.update(cx, |conversation, cx| { + conversation.authorize_tool_call( + acp::SessionId::new("thread-a"), + acp::ToolCallId::new("tc-a"), + acp::PermissionOptionId::new("allow-a"), + acp::PermissionOptionKind::AllowOnce, + cx, + ); + }); + }); + + cx.run_until_parked(); + + cx.read(|cx| { + let session_b = acp::SessionId::new("thread-b"); + let (session_id, tool_call_id, _) = conversation + .read(cx) + .pending_tool_call(&session_b, cx) + .expect("Expected thread-b's tool call after thread-a's was authorized"); + assert_eq!(session_id, acp::SessionId::new("thread-b")); + assert_eq!(tool_call_id, acp::ToolCallId::new("tc-b")); + }); + } } diff --git a/crates/agent_ui/src/acp/thread_view/active_thread.rs b/crates/agent_ui/src/acp/thread_view/active_thread.rs index 6f13caaeb25dba5c086f279920620637c42d24a5..3ef684eb941a7cae9c9ac62cc18800fc2b062cf8 100644 --- a/crates/agent_ui/src/acp/thread_view/active_thread.rs +++ b/crates/agent_ui/src/acp/thread_view/active_thread.rs @@ -191,6 +191,7 @@ pub struct AcpThreadView { pub parent_id: Option, pub login: Option, // is some <=> Active | Unauthenticated pub thread: Entity, + pub(crate) conversation: Entity, pub server_view: WeakEntity, pub agent_icon: IconName, pub agent_name: SharedString, @@ -217,7 +218,6 @@ pub struct AcpThreadView { pub expanded_tool_calls: HashSet, pub expanded_tool_call_raw_inputs: HashSet, pub expanded_thinking_blocks: HashSet<(usize, usize)>, - pub expanded_subagents: HashSet, pub subagent_scroll_handles: RefCell>, pub edits_expanded: bool, pub plan_expanded: bool, @@ -277,9 +277,10 @@ pub struct TurnFields { } impl AcpThreadView { - pub fn new( + pub(crate) fn new( parent_id: Option, thread: Entity, + conversation: Entity, login: Option, server_view: WeakEntity, agent_icon: IconName, @@ -385,6 +386,7 @@ impl AcpThreadView { parent_id, focus_handle: cx.focus_handle(), thread, + conversation, login, server_view, agent_icon, @@ -412,7 +414,6 @@ impl AcpThreadView { expanded_tool_calls: HashSet::default(), expanded_tool_call_raw_inputs: HashSet::default(), expanded_thinking_blocks: HashSet::default(), - expanded_subagents: HashSet::default(), subagent_scroll_handles: RefCell::new(HashMap::default()), edits_expanded: false, plan_expanded: false, @@ -1246,24 +1247,15 @@ impl AcpThreadView { pub fn authorize_tool_call( &mut self, + session_id: acp::SessionId, tool_call_id: acp::ToolCallId, option_id: acp::PermissionOptionId, option_kind: acp::PermissionOptionKind, window: &mut Window, cx: &mut Context, ) { - let thread = &self.thread; - let agent_telemetry_id = thread.read(cx).connection().telemetry_id(); - - telemetry::event!( - "Agent Tool Call Authorized", - agent = agent_telemetry_id, - session = thread.read(cx).session_id(), - option = option_kind - ); - - thread.update(cx, |thread, cx| { - thread.authorize_tool_call(tool_call_id, option_id, option_kind, cx); + self.conversation.update(cx, |conversation, cx| { + conversation.authorize_tool_call(session_id, tool_call_id, option_id, option_kind, cx); }); if self.should_be_following { self.workspace @@ -1293,21 +1285,17 @@ impl AcpThreadView { window: &mut Window, cx: &mut Context, ) -> Option<()> { - let thread = self.thread.read(cx); - let tool_call = thread.first_tool_awaiting_confirmation()?; - let ToolCallStatus::WaitingForConfirmation { options, .. } = &tool_call.status else { - return None; - }; - let option = options.first_option_of_kind(kind)?; - - self.authorize_tool_call( - tool_call.id.clone(), - option.option_id.clone(), - option.kind, - window, - cx, - ); - + self.conversation.update(cx, |conversation, cx| { + conversation.authorize_pending_tool_call(&self.id, kind, cx) + })?; + if self.should_be_following { + self.workspace + .update(cx, |workspace, cx| { + workspace.follow(CollaboratorId::Agent, window, cx); + }) + .ok(); + } + cx.notify(); Some(()) } @@ -1327,7 +1315,14 @@ impl AcpThreadView { _ => acp::PermissionOptionKind::AllowOnce, }; - self.authorize_tool_call(tool_call_id, option_id, option_kind, window, cx); + self.authorize_tool_call( + self.id.clone(), + tool_call_id, + option_id, + option_kind, + window, + cx, + ); } pub fn handle_select_permission_granularity( @@ -1349,13 +1344,8 @@ impl AcpThreadView { window: &mut Window, cx: &mut Context, ) -> Option<()> { - let thread = self.thread.read(cx); - let tool_call = thread.first_tool_awaiting_confirmation()?; - let ToolCallStatus::WaitingForConfirmation { options, .. } = &tool_call.status else { - return None; - }; - let tool_call_id = tool_call.id.clone(); - + let (session_id, tool_call_id, options) = + self.conversation.read(cx).pending_tool_call(&self.id, cx)?; let PermissionOptions::Dropdown(choices) = options else { let kind = if is_allow { acp::PermissionOptionKind::AllowOnce @@ -1381,6 +1371,7 @@ impl AcpThreadView { }; self.authorize_tool_call( + session_id, tool_call_id, selected_option.option_id.clone(), selected_option.kind, @@ -3411,7 +3402,7 @@ impl AcpThreadView { entry_ix: usize, total_entries: usize, entry: &AgentThreadEntry, - window: &mut Window, + window: &Window, cx: &Context, ) -> AnyElement { let is_indented = entry.is_indented(); @@ -3688,24 +3679,16 @@ impl AcpThreadView { .into_any() } } - AgentThreadEntry::ToolCall(tool_call) => { - let has_terminals = tool_call.terminals().next().is_some(); - - div() - .w_full() - .map(|this| { - if has_terminals { - this.children(tool_call.terminals().map(|terminal| { - self.render_terminal_tool_call( - entry_ix, terminal, tool_call, window, cx, - ) - })) - } else { - this.child(self.render_tool_call(entry_ix, tool_call, window, cx)) - } - }) - .into_any() - } + AgentThreadEntry::ToolCall(tool_call) => self + .render_any_tool_call( + &self.id, + entry_ix, + tool_call, + &self.focus_handle(cx), + window, + cx, + ) + .into_any(), }; let primary = if is_indented { @@ -4510,9 +4493,11 @@ impl AcpThreadView { fn render_terminal_tool_call( &self, + active_session_id: &acp::SessionId, entry_ix: usize, terminal: &Entity, tool_call: &ToolCall, + focus_handle: &FocusHandle, window: &Window, cx: &Context, ) -> AnyElement { @@ -4779,20 +4764,87 @@ impl AcpThreadView { ) }) .when_some(confirmation_options, |this, options| { + let is_first = self.is_first_tool_call(active_session_id, &tool_call.id, cx); this.child(self.render_permission_buttons( + self.id.clone(), + is_first, options, entry_ix, tool_call.id.clone(), + focus_handle, cx, )) }) .into_any() } + fn is_first_tool_call( + &self, + active_session_id: &acp::SessionId, + tool_call_id: &acp::ToolCallId, + cx: &App, + ) -> bool { + self.conversation + .read(cx) + .pending_tool_call(active_session_id, cx) + .map_or(false, |(pending_session_id, pending_tool_call_id, _)| { + self.id == pending_session_id && tool_call_id == &pending_tool_call_id + }) + } + + fn render_any_tool_call( + &self, + active_session_id: &acp::SessionId, + entry_ix: usize, + tool_call: &ToolCall, + focus_handle: &FocusHandle, + window: &Window, + cx: &Context, + ) -> Div { + let has_terminals = tool_call.terminals().next().is_some(); + + div().w_full().map(|this| { + if tool_call.is_subagent() { + this.child(self.render_subagent_tool_call( + active_session_id, + entry_ix, + tool_call, + tool_call.subagent_session_id.clone(), + focus_handle, + window, + cx, + )) + } else if has_terminals { + this.children(tool_call.terminals().map(|terminal| { + self.render_terminal_tool_call( + active_session_id, + entry_ix, + terminal, + tool_call, + focus_handle, + window, + cx, + ) + })) + } else { + this.child(self.render_tool_call( + active_session_id, + entry_ix, + tool_call, + focus_handle, + window, + cx, + )) + } + }) + } + fn render_tool_call( &self, + active_session_id: &acp::SessionId, entry_ix: usize, tool_call: &ToolCall, + focus_handle: &FocusHandle, window: &Window, cx: &Context, ) -> Div { @@ -4813,17 +4865,6 @@ impl AcpThreadView { let is_edit = matches!(tool_call.kind, acp::ToolKind::Edit) || tool_call.diffs().next().is_some(); - // For subagent tool calls, render the subagent cards directly without wrapper - if tool_call.is_subagent() { - return self.render_subagent_tool_call( - entry_ix, - tool_call, - tool_call.subagent_session_id.clone(), - 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 @@ -4863,6 +4904,7 @@ impl AcpThreadView { .map(|(content_ix, content)| { div() .child(self.render_tool_call_content( + active_session_id, entry_ix, content, content_ix, @@ -4870,6 +4912,7 @@ impl AcpThreadView { use_card_layout, has_image_content, failed_or_canceled, + focus_handle, window, cx, )) @@ -4941,9 +4984,12 @@ impl AcpThreadView { ) }) .child(self.render_permission_buttons( + self.id.clone(), + self.is_first_tool_call(active_session_id, &tool_call.id, cx), options, entry_ix, tool_call.id.clone(), + focus_handle, cx, )) .into_any(), @@ -4988,6 +5034,7 @@ impl AcpThreadView { .map(|(content_ix, content)| { div().id(("tool-call-output", entry_ix)).child( self.render_tool_call_content( + active_session_id, entry_ix, content, content_ix, @@ -4995,6 +5042,7 @@ impl AcpThreadView { use_card_layout, has_image_content, failed_or_canceled, + focus_handle, window, cx, ), @@ -5157,34 +5205,46 @@ impl AcpThreadView { fn render_permission_buttons( &self, + session_id: acp::SessionId, + is_first: bool, options: &PermissionOptions, entry_ix: usize, tool_call_id: acp::ToolCallId, + focus_handle: &FocusHandle, cx: &Context, ) -> Div { match options { - PermissionOptions::Flat(options) => { - self.render_permission_buttons_flat(options, entry_ix, tool_call_id, cx) - } - PermissionOptions::Dropdown(options) => { - self.render_permission_buttons_dropdown(options, entry_ix, tool_call_id, cx) - } + PermissionOptions::Flat(options) => self.render_permission_buttons_flat( + session_id, + is_first, + options, + entry_ix, + tool_call_id, + focus_handle, + cx, + ), + PermissionOptions::Dropdown(options) => self.render_permission_buttons_dropdown( + session_id, + is_first, + options, + entry_ix, + tool_call_id, + focus_handle, + cx, + ), } } fn render_permission_buttons_dropdown( &self, + session_id: acp::SessionId, + is_first: bool, choices: &[PermissionOptionChoice], entry_ix: usize, tool_call_id: acp::ToolCallId, + focus_handle: &FocusHandle, cx: &Context, ) -> Div { - let is_first = self - .thread - .read(cx) - .first_tool_awaiting_confirmation() - .is_some_and(|call| call.id == tool_call_id); - // Get the selected granularity index, defaulting to the last option ("Only this time") let selected_index = self .selected_permission_granularity @@ -5236,18 +5296,20 @@ impl AcpThreadView { this.key_binding( KeyBinding::for_action_in( &AllowOnce as &dyn Action, - &self.focus_handle(cx), + focus_handle, cx, ) .map(|kb| kb.size(rems_from_px(10.))), ) }) .on_click(cx.listener({ + let session_id = session_id.clone(); let tool_call_id = tool_call_id.clone(); let option_id = allow_option_id; let option_kind = allow_option_kind; move |this, _, window, cx| { this.authorize_tool_call( + session_id.clone(), tool_call_id.clone(), option_id.clone(), option_kind, @@ -5268,7 +5330,7 @@ impl AcpThreadView { this.key_binding( KeyBinding::for_action_in( &RejectOnce as &dyn Action, - &self.focus_handle(cx), + focus_handle, cx, ) .map(|kb| kb.size(rems_from_px(10.))), @@ -5280,6 +5342,7 @@ impl AcpThreadView { let option_kind = deny_option_kind; move |this, _, window, cx| { this.authorize_tool_call( + session_id.clone(), tool_call_id.clone(), option_id.clone(), option_kind, @@ -5375,16 +5438,14 @@ impl AcpThreadView { fn render_permission_buttons_flat( &self, + session_id: acp::SessionId, + is_first: bool, options: &[acp::PermissionOption], entry_ix: usize, tool_call_id: acp::ToolCallId, + focus_handle: &FocusHandle, cx: &Context, ) -> Div { - let is_first = self - .thread - .read(cx) - .first_tool_awaiting_confirmation() - .is_some_and(|call| call.id == tool_call_id); let mut seen_kinds: ArrayVec = ArrayVec::new(); div() @@ -5427,7 +5488,7 @@ impl AcpThreadView { seen_kinds.push(option.kind); this.key_binding( - KeyBinding::for_action_in(action, &self.focus_handle(cx), cx) + KeyBinding::for_action_in(action, focus_handle, cx) .map(|kb| kb.size(rems_from_px(10.))), ) }) @@ -5435,11 +5496,13 @@ impl AcpThreadView { .icon_size(IconSize::XSmall) .label_size(LabelSize::Small) .on_click(cx.listener({ + let session_id = session_id.clone(); let tool_call_id = tool_call_id.clone(); let option_id = option.option_id.clone(); let option_kind = option.kind; move |this, _, window, cx| { this.authorize_tool_call( + session_id.clone(), tool_call_id.clone(), option_id.clone(), option_kind, @@ -5705,6 +5768,7 @@ impl AcpThreadView { fn render_tool_call_content( &self, + session_id: &acp::SessionId, entry_ix: usize, content: &ToolCallContent, context_ix: usize, @@ -5712,6 +5776,7 @@ impl AcpThreadView { card_layout: bool, is_image_tool_call: bool, has_failed: bool, + focus_handle: &FocusHandle, window: &Window, cx: &Context, ) -> AnyElement { @@ -5745,9 +5810,15 @@ impl AcpThreadView { ToolCallContent::Diff(diff) => { self.render_diff_editor(entry_ix, diff, tool_call, has_failed, cx) } - ToolCallContent::Terminal(terminal) => { - self.render_terminal_tool_call(entry_ix, terminal, tool_call, window, cx) - } + ToolCallContent::Terminal(terminal) => self.render_terminal_tool_call( + session_id, + entry_ix, + terminal, + tool_call, + focus_handle, + window, + cx, + ), } } @@ -5979,14 +6050,14 @@ impl AcpThreadView { fn render_subagent_tool_call( &self, + active_session_id: &acp::SessionId, entry_ix: usize, tool_call: &ToolCall, subagent_session_id: Option, + focus_handle: &FocusHandle, window: &Window, cx: &Context, ) -> Div { - let tool_call_status = &tool_call.status; - let subagent_thread_view = subagent_session_id.and_then(|id| { self.server_view .upgrade() @@ -5995,10 +6066,11 @@ impl AcpThreadView { }); let content = self.render_subagent_card( + active_session_id, entry_ix, - 0, subagent_thread_view, - tool_call_status, + tool_call, + focus_handle, window, cx, ); @@ -6008,17 +6080,18 @@ impl AcpThreadView { fn render_subagent_card( &self, + active_session_id: &acp::SessionId, entry_ix: usize, - context_ix: usize, thread_view: Option<&Entity>, - tool_call_status: &ToolCallStatus, + tool_call: &ToolCall, + focus_handle: &FocusHandle, window: &Window, cx: &Context, ) -> AnyElement { let thread = thread_view .as_ref() .map(|view| view.read(cx).thread.clone()); - let session_id = thread + let subagent_session_id = thread .as_ref() .map(|thread| thread.read(cx).session_id().clone()); let action_log = thread.as_ref().map(|thread| thread.read(cx).action_log()); @@ -6026,20 +6099,16 @@ impl AcpThreadView { .map(|log| log.read(cx).changed_buffers(cx)) .unwrap_or_default(); - let is_expanded = if let Some(session_id) = &session_id { - self.expanded_subagents.contains(session_id) - } else { - false - }; + let is_expanded = self.expanded_tool_calls.contains(&tool_call.id); let files_changed = changed_buffers.len(); let diff_stats = DiffStats::all_files(&changed_buffers, cx); let is_running = matches!( - tool_call_status, + tool_call.status, ToolCallStatus::Pending | ToolCallStatus::InProgress ); let is_canceled_or_failed = matches!( - tool_call_status, + tool_call.status, ToolCallStatus::Canceled | ToolCallStatus::Failed | ToolCallStatus::Rejected ); @@ -6050,13 +6119,13 @@ impl AcpThreadView { if is_canceled_or_failed { "Subagent Canceled" } else { - "Spawning agent…" + "Spawning Subagent…" } .into() }); - let card_header_id = format!("subagent-header-{}-{}", entry_ix, context_ix); - let diff_stat_id = format!("subagent-diff-{}-{}", entry_ix, context_ix); + let card_header_id = format!("subagent-header-{}", entry_ix); + let diff_stat_id = format!("subagent-diff-{}", entry_ix); let icon = h_flex().w_4().justify_center().child(if is_running { SpinnerLabel::new() @@ -6074,18 +6143,9 @@ impl AcpThreadView { .into_any_element() }); - let has_expandable_content = thread.as_ref().map_or(false, |thread| { - thread.read(cx).entries().iter().rev().any(|entry| { - if let AgentThreadEntry::AssistantMessage(msg) = entry { - msg.chunks.iter().any(|chunk| match chunk { - AssistantMessageChunk::Message { block } => block.markdown().is_some(), - AssistantMessageChunk::Thought { block } => block.markdown().is_some(), - }) - } else { - false - } - }) - }); + let has_expandable_content = thread + .as_ref() + .map_or(false, |thread| !thread.read(cx).entries().is_empty()); v_flex() .w_full() @@ -6104,7 +6164,7 @@ impl AcpThreadView { .bg(self.tool_card_header_bg(cx)) .child( h_flex() - .id(format!("subagent-title-{}-{}", entry_ix, context_ix)) + .id(format!("subagent-title-{}", entry_ix)) .min_w_0() .overflow_hidden() .gap_1p5() @@ -6136,17 +6196,14 @@ impl AcpThreadView { }) .tooltip(Tooltip::text(title.to_string())), ) - .when_some(session_id, |this, session_id| { + .when_some(subagent_session_id, |this, subagent_session_id| { this.child( h_flex() .flex_shrink_0() .when(has_expandable_content, |this| { this.child( IconButton::new( - format!( - "subagent-disclosure-{}-{}", - entry_ix, context_ix - ), + format!("subagent-disclosure-{}", entry_ix), if is_expanded { IconName::ChevronUp } else { @@ -6159,14 +6216,17 @@ impl AcpThreadView { .visible_on_hover(card_header_id.clone()) .on_click( cx.listener({ - let session_id = session_id.clone(); + let tool_call_id = tool_call.id.clone(); move |this, _, _, cx| { - if this.expanded_subagents.contains(&session_id) + if this + .expanded_tool_calls + .contains(&tool_call_id) { - this.expanded_subagents.remove(&session_id); + this.expanded_tool_calls + .remove(&tool_call_id); } else { - this.expanded_subagents - .insert(session_id.clone()); + this.expanded_tool_calls + .insert(tool_call_id.clone()); } cx.notify(); } @@ -6176,7 +6236,7 @@ impl AcpThreadView { }) .child( IconButton::new( - format!("expand-subagent-{}-{}", entry_ix, context_ix), + format!("expand-subagent-{}", entry_ix), IconName::Maximize, ) .icon_color(Color::Muted) @@ -6188,7 +6248,7 @@ impl AcpThreadView { this.server_view .update(cx, |this, cx| { this.navigate_to_session( - session_id.clone(), + subagent_session_id.clone(), window, cx, ); @@ -6200,7 +6260,7 @@ impl AcpThreadView { .when(is_running, |buttons| { buttons.child( IconButton::new( - format!("stop-subagent-{}-{}", entry_ix, context_ix), + format!("stop-subagent-{}", entry_ix), IconName::Stop, ) .icon_size(IconSize::Small) @@ -6227,462 +6287,148 @@ impl AcpThreadView { ) .when_some(thread_view, |this, thread_view| { let thread = &thread_view.read(cx).thread; - this.when(is_expanded, |this| { - this.child( - self.render_subagent_expanded_content( - entry_ix, context_ix, thread, window, cx, - ), - ) - }) - .children( - thread - .read(cx) - .first_tool_awaiting_confirmation() - .and_then(|tc| { - if let ToolCallStatus::WaitingForConfirmation { options, .. } = - &tc.status - { - Some(self.render_subagent_pending_tool_call( - entry_ix, - context_ix, - thread.clone(), - tc, - options, - window, - cx, - )) - } else { - None - } - }), - ) + let pending_tool_call = self + .conversation + .read(cx) + .pending_tool_call(thread.read(cx).session_id(), cx); + + if let Some((_, subagent_tool_call_id, _)) = pending_tool_call { + if let Some((entry_ix, tool_call)) = + thread.read(cx).tool_call(&subagent_tool_call_id) + { + this.child(thread_view.read(cx).render_any_tool_call( + active_session_id, + entry_ix, + tool_call, + focus_handle, + window, + cx, + )) + } else { + this + } + } else { + this.when(is_expanded, |this| { + this.child(self.render_subagent_expanded_content( + active_session_id, + entry_ix, + thread_view, + is_running, + tool_call, + focus_handle, + window, + cx, + )) + }) + } }) .into_any_element() } fn render_subagent_expanded_content( &self, - _entry_ix: usize, - _context_ix: usize, - thread: &Entity, - window: &Window, - cx: &Context, - ) -> impl IntoElement { - let thread_read = thread.read(cx); - let session_id = thread_read.session_id().clone(); - let entries = thread_read.entries(); - - // Find the most recent agent 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 scroll_handle = self - .subagent_scroll_handles - .borrow_mut() - .entry(session_id.clone()) - .or_default() - .clone(); - - scroll_handle.scroll_to_bottom(); - let editor_bg = cx.theme().colors().editor_background; - - let gradient_overlay = { - div().absolute().inset_0().bg(linear_gradient( - 180., - linear_color_stop(editor_bg, 0.), - linear_color_stop(editor_bg.opacity(0.), 0.15), - )) - }; - - div() - .relative() - .w_full() - .max_h_56() - .p_2p5() - .text_ui(cx) - .border_t_1() - .border_color(self.tool_card_border_color(cx)) - .bg(editor_bg.opacity(0.4)) - .overflow_hidden() - .child( - div() - .id(format!("subagent-content-{}", session_id)) - .size_full() - .track_scroll(&scroll_handle) - .when_some(last_assistant_markdown, |this, markdown| { - this.child(self.render_markdown( - markdown, - MarkdownStyle::themed(MarkdownFont::Agent, window, cx), - )) - }), - ) - .child(gradient_overlay) - } - - fn render_subagent_pending_tool_call( - &self, + active_session_id: &acp::SessionId, entry_ix: usize, - context_ix: usize, - subagent_thread: Entity, + thread_view: &Entity, + is_running: bool, tool_call: &ToolCall, - options: &PermissionOptions, + focus_handle: &FocusHandle, window: &Window, cx: &Context, - ) -> Div { - let tool_call_id = tool_call.id.clone(); - let is_edit = - matches!(tool_call.kind, acp::ToolKind::Edit) || tool_call.diffs().next().is_some(); - let has_image_content = tool_call.content.iter().any(|c| c.image().is_some()); + ) -> impl IntoElement { + const MAX_PREVIEW_ENTRIES: usize = 8; - v_flex() - .w_full() - .border_t_1() - .border_color(self.tool_card_border_color(cx)) - .child( - self.render_tool_call_label( - entry_ix, tool_call, is_edit, false, // has_failed - false, // has_revealed_diff - true, // use_card_layout - window, cx, - ) - .py_1(), - ) - .children( - tool_call - .content - .iter() - .enumerate() - .map(|(content_ix, content)| { - self.render_tool_call_content( - entry_ix, - content, - content_ix, - tool_call, - true, // card_layout - has_image_content, - false, // has_failed - window, - cx, - ) - }), - ) - .child(self.render_subagent_permission_buttons( - entry_ix, - context_ix, - subagent_thread, - tool_call_id, - options, - cx, - )) - } + let subagent_view = thread_view.read(cx); + let session_id = subagent_view.thread.read(cx).session_id().clone(); - fn render_subagent_permission_buttons( - &self, - entry_ix: usize, - context_ix: usize, - subagent_thread: Entity, - tool_call_id: acp::ToolCallId, - options: &PermissionOptions, - cx: &Context, - ) -> Div { - match options { - PermissionOptions::Flat(options) => self.render_subagent_permission_buttons_flat( - entry_ix, - context_ix, - subagent_thread, - tool_call_id, - options, - cx, - ), - PermissionOptions::Dropdown(options) => self - .render_subagent_permission_buttons_dropdown( - entry_ix, - context_ix, - subagent_thread, - tool_call_id, - options, - cx, - ), - } - } + if is_running { + let entries = subagent_view.thread.read(cx).entries(); + let total_entries = entries.len(); + let start_ix = total_entries.saturating_sub(MAX_PREVIEW_ENTRIES); - fn render_subagent_permission_buttons_flat( - &self, - entry_ix: usize, - context_ix: usize, - subagent_thread: Entity, - tool_call_id: acp::ToolCallId, - options: &[acp::PermissionOption], - cx: &Context, - ) -> Div { - div() - .p_1() - .border_t_1() - .border_color(self.tool_card_border_color(cx)) - .w_full() - .v_flex() - .gap_0p5() - .children(options.iter().map(move |option| { - let option_id = SharedString::from(format!( - "subagent-{}-{}-{}", - entry_ix, context_ix, option.option_id.0 - )); - Button::new((option_id, entry_ix), option.name.clone()) - .map(|this| match option.kind { - acp::PermissionOptionKind::AllowOnce => { - this.icon(IconName::Check).icon_color(Color::Success) - } - acp::PermissionOptionKind::AllowAlways => { - this.icon(IconName::CheckDouble).icon_color(Color::Success) - } - acp::PermissionOptionKind::RejectOnce - | acp::PermissionOptionKind::RejectAlways - | _ => this.icon(IconName::Close).icon_color(Color::Error), - }) - .icon_position(IconPosition::Start) - .icon_size(IconSize::XSmall) - .label_size(LabelSize::Small) - .on_click(cx.listener({ - let subagent_thread = subagent_thread.clone(); - let tool_call_id = tool_call_id.clone(); - let option_id = option.option_id.clone(); - let option_kind = option.kind; - move |this, _, window, cx| { - this.authorize_subagent_tool_call( - subagent_thread.clone(), - tool_call_id.clone(), - option_id.clone(), - option_kind, - window, - cx, - ); - } - })) - })) - } + let scroll_handle = self + .subagent_scroll_handles + .borrow_mut() + .entry(session_id.clone()) + .or_default() + .clone(); + scroll_handle.scroll_to_bottom(); - fn authorize_subagent_tool_call( - &mut self, - subagent_thread: Entity, - tool_call_id: acp::ToolCallId, - option_id: acp::PermissionOptionId, - option_kind: acp::PermissionOptionKind, - _window: &mut Window, - cx: &mut Context, - ) { - subagent_thread.update(cx, |thread, cx| { - thread.authorize_tool_call(tool_call_id, option_id, option_kind, cx); - }); - } + let rendered_entries: Vec = entries[start_ix..] + .iter() + .enumerate() + .map(|(i, entry)| { + let actual_ix = start_ix + i; + subagent_view.render_entry(actual_ix, total_entries + 1, entry, window, cx) + }) + .collect(); - fn render_subagent_permission_buttons_dropdown( - &self, - entry_ix: usize, - context_ix: usize, - subagent_thread: Entity, - tool_call_id: acp::ToolCallId, - choices: &[PermissionOptionChoice], - cx: &Context, - ) -> Div { - let selected_index = self - .selected_permission_granularity - .get(&tool_call_id) - .copied() - .unwrap_or_else(|| choices.len().saturating_sub(1)); + let editor_bg = cx.theme().colors().editor_background; - let selected_choice = choices.get(selected_index).or(choices.last()); + let gradient_overlay = div().absolute().inset_0().bg(linear_gradient( + 180., + linear_color_stop(editor_bg, 0.), + linear_color_stop(editor_bg.opacity(0.), 0.15), + )); - let dropdown_label: SharedString = selected_choice - .map(|choice| choice.label()) - .unwrap_or_else(|| "Only this time".into()); + let interaction_blocker = div() + .absolute() + .inset_0() + .size_full() + .block_mouse_except_scroll(); - let (allow_option_id, allow_option_kind, deny_option_id, deny_option_kind) = - if let Some(choice) = selected_choice { - ( - choice.allow.option_id.clone(), - choice.allow.kind, - choice.deny.option_id.clone(), - choice.deny.kind, - ) - } else { - ( - acp::PermissionOptionId::new("allow"), - acp::PermissionOptionKind::AllowOnce, - acp::PermissionOptionId::new("deny"), - acp::PermissionOptionKind::RejectOnce, + div() + .id(format!("subagent-content-{}", session_id)) + .relative() + .w_full() + .h_56() + .border_t_1() + .border_color(self.tool_card_border_color(cx)) + .bg(editor_bg.opacity(0.4)) + .overflow_hidden() + .child( + div() + .id("entries") + .size_full() + .track_scroll(&scroll_handle) + .pb_1() + .children(rendered_entries), ) - }; - - h_flex() - .w_full() - .p_1() - .gap_2() - .justify_between() - .border_t_1() - .border_color(self.tool_card_border_color(cx)) - .child( - h_flex() - .gap_0p5() - .child( - Button::new( - ( - SharedString::from(format!( - "subagent-allow-btn-{}-{}", - entry_ix, context_ix - )), - entry_ix, - ), - "Allow", - ) - .icon(IconName::Check) - .icon_color(Color::Success) - .icon_position(IconPosition::Start) - .icon_size(IconSize::XSmall) - .label_size(LabelSize::Small) - .on_click(cx.listener({ - let subagent_thread = subagent_thread.clone(); - let tool_call_id = tool_call_id.clone(); - let option_id = allow_option_id; - let option_kind = allow_option_kind; - move |this, _, window, cx| { - this.authorize_subagent_tool_call( - subagent_thread.clone(), - tool_call_id.clone(), - option_id.clone(), - option_kind, - window, - cx, - ); - } - })), - ) - .child( - Button::new( - ( - SharedString::from(format!( - "subagent-deny-btn-{}-{}", - entry_ix, context_ix - )), - entry_ix, - ), - "Deny", - ) - .icon(IconName::Close) - .icon_color(Color::Error) - .icon_position(IconPosition::Start) - .icon_size(IconSize::XSmall) - .label_size(LabelSize::Small) - .on_click(cx.listener({ - let tool_call_id = tool_call_id.clone(); - let option_id = deny_option_id; - let option_kind = deny_option_kind; - move |this, _, window, cx| { - this.authorize_subagent_tool_call( - subagent_thread.clone(), - tool_call_id.clone(), - option_id.clone(), - option_kind, + .child(gradient_overlay) + .child(interaction_blocker) + } else { + div() + .id(format!("subagent-content-{}", session_id)) + .p_2() + .children( + tool_call + .content + .iter() + .enumerate() + .map(|(content_ix, content)| { + div().id(("tool-call-output", entry_ix)).child( + self.render_tool_call_content( + active_session_id, + entry_ix, + content, + content_ix, + tool_call, + true, + false, + matches!( + tool_call.status, + ToolCallStatus::Failed + | ToolCallStatus::Rejected + | ToolCallStatus::Canceled + ), + focus_handle, window, cx, - ); - } - })), - ), - ) - .child(self.render_subagent_permission_granularity_dropdown( - choices, - dropdown_label, - entry_ix, - context_ix, - tool_call_id, - selected_index, - cx, - )) - } - - fn render_subagent_permission_granularity_dropdown( - &self, - choices: &[PermissionOptionChoice], - current_label: SharedString, - entry_ix: usize, - context_ix: usize, - tool_call_id: acp::ToolCallId, - selected_index: usize, - _cx: &Context, - ) -> AnyElement { - let menu_options: Vec<(usize, SharedString)> = choices - .iter() - .enumerate() - .map(|(i, choice)| (i, choice.label())) - .collect(); - - let permission_dropdown_handle = self.permission_dropdown_handle.clone(); - - PopoverMenu::new(( - SharedString::from(format!( - "subagent-permission-granularity-{}-{}", - entry_ix, context_ix - )), - entry_ix, - )) - .with_handle(permission_dropdown_handle) - .trigger( - Button::new( - ( - SharedString::from(format!( - "subagent-granularity-trigger-{}-{}", - entry_ix, context_ix - )), - entry_ix, - ), - current_label, - ) - .icon(IconName::ChevronDown) - .icon_size(IconSize::XSmall) - .icon_color(Color::Muted) - .label_size(LabelSize::Small), - ) - .menu(move |window, cx| { - let tool_call_id = tool_call_id.clone(); - let options = menu_options.clone(); - - Some(ContextMenu::build(window, cx, move |mut menu, _, _| { - for (index, display_name) in options.iter() { - let display_name = display_name.clone(); - let index = *index; - let tool_call_id_for_entry = tool_call_id.clone(); - let is_selected = index == selected_index; - - menu = menu.toggleable_entry( - display_name, - is_selected, - IconPosition::End, - None, - move |window, cx| { - window.dispatch_action( - SelectPermissionGranularity { - tool_call_id: tool_call_id_for_entry.0.to_string(), - index, - } - .boxed_clone(), - cx, - ); - }, - ); - } - - menu - })) - }) - .into_any_element() + ), + ) + }), + ) + } } fn render_rules_item(&self, cx: &Context) -> Option { diff --git a/crates/agent_ui/src/agent_diff.rs b/crates/agent_ui/src/agent_diff.rs index 7a7220e5497946ccb705febfe4fcc127055225a7..b02af97881cff92714641b7f4e3fd10601e0685f 100644 --- a/crates/agent_ui/src/agent_diff.rs +++ b/crates/agent_ui/src/agent_diff.rs @@ -1413,7 +1413,8 @@ impl AgentDiff { | AcpThreadEvent::TokenUsageUpdated | AcpThreadEvent::SubagentSpawned(_) | AcpThreadEvent::EntriesRemoved(_) - | AcpThreadEvent::ToolAuthorizationRequired + | AcpThreadEvent::ToolAuthorizationRequested(_) + | AcpThreadEvent::ToolAuthorizationReceived(_) | AcpThreadEvent::PromptCapabilitiesUpdated | AcpThreadEvent::AvailableCommandsUpdated(_) | AcpThreadEvent::Retry(_)