diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 65bd3a56791..fc266cbed42 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -357,6 +357,33 @@ impl Conversation { .collect() } + /// Returns the first pending tool call request for exactly `session_id`. + /// Unlike `pending_tool_call`, this does not use the global FIFO pending + /// request for non-subagent sessions. + pub fn pending_tool_call_for_session( + &self, + session_id: &acp::SessionId, + cx: &App, + ) -> Option { + let thread = self.threads.get(session_id)?; + let tool_call_id = self.permission_requests.get(session_id)?.iter().next()?; + let (_, tool_call) = thread.read(cx).tool_call(tool_call_id)?; + if !matches!( + tool_call.status, + ToolCallStatus::WaitingForConfirmation { .. } + ) { + return None; + } + Some(tool_call_id.clone()) + } + + pub fn pending_tool_call_count_for_session(&self, session_id: &acp::SessionId) -> usize { + self.permission_requests + .get(session_id) + .map(|tool_call_ids| tool_call_ids.len()) + .unwrap_or(0) + } + pub fn authorize_pending_tool_call( &mut self, session_id: &acp::SessionId, @@ -3366,7 +3393,7 @@ pub(crate) mod tests { use editor::MultiBufferOffset; use editor::actions::Paste; use fs::FakeFs; - use gpui::{ClipboardItem, EventEmitter, TestAppContext, VisualTestContext}; + use gpui::{ClipboardItem, EventEmitter, TestAppContext, VisualTestContext, size}; use parking_lot::Mutex; use project::Project; use serde_json::json; @@ -7981,6 +8008,343 @@ pub(crate) mod tests { }); } + /// Set up a `ConversationView` whose active thread has a single tool call + /// awaiting permission. Returns the conversation view, its active + /// `ThreadView`, and the entry index of the tool call within the thread. + async fn setup_pending_permission_thread<'a>( + tool_call_id: &str, + cx: &'a mut TestAppContext, + ) -> ( + Entity, + Entity, + usize, + &'a mut VisualTestContext, + ) { + let tool_call_id_value = acp::ToolCallId::new(tool_call_id); + let tool_call = acp::ToolCall::new(tool_call_id_value.clone(), "Run something") + .kind(acp::ToolKind::Edit); + + let connection = + StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( + tool_call_id_value.clone(), + PermissionOptions::Flat(vec![acp::PermissionOption::new( + "allow", + "Allow", + acp::PermissionOptionKind::AllowOnce, + )]), + )])); + connection.set_next_prompt_updates(vec![acp::SessionUpdate::ToolCall(tool_call)]); + + let (conversation_view, cx) = + setup_conversation_view(StubAgentServer::new(connection), cx).await; + add_to_workspace(conversation_view.clone(), cx); + + cx.update(|_window, cx| { + AgentSettings::override_global( + AgentSettings { + notify_when_agent_waiting: NotifyWhenAgentWaiting::Never, + ..AgentSettings::get_global(cx).clone() + }, + cx, + ); + }); + + let message_editor = message_editor(&conversation_view, cx); + message_editor.update_in(cx, |editor, window, cx| { + editor.set_text("Hello", window, cx); + }); + + active_thread(&conversation_view, cx) + .update_in(cx, |view, window, cx| view.send(window, cx)); + + cx.run_until_parked(); + + let thread_view = active_thread(&conversation_view, cx); + let entry_ix = thread_view.read_with(cx, |view, cx| { + view.thread + .read(cx) + .entries() + .iter() + .position(|entry| { + matches!( + entry, + acp_thread::AgentThreadEntry::ToolCall(call) + if call.id == tool_call_id_value + ) + }) + .expect("tool call entry should exist after run_until_parked") + }); + + (conversation_view, thread_view, entry_ix, cx) + } + + struct TestListView { + list_state: ListState, + } + + impl Render for TestListView { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + list(self.list_state.clone(), |_, _, _| { + div().h(px(20.0)).w_full().into_any_element() + }) + .size_full() + } + } + + fn draw_thread_list_at( + thread_view: &Entity, + scroll_top: ListOffset, + cx: &mut VisualTestContext, + ) { + let list_state = thread_view.read_with(cx, |view, _cx| view.list_state.clone()); + list_state.scroll_to(scroll_top); + cx.draw( + point(px(0.0), px(0.0)), + size(px(100.0), px(20.0)), + |_, cx| { + cx.new(|_| TestListView { + list_state: list_state.clone(), + }) + .into_any_element() + }, + ); + } + + #[gpui::test] + async fn test_permission_row_hidden_when_inline_bounds_unavailable(cx: &mut TestAppContext) { + init_test(cx); + + let (_view, thread_view, _entry_ix, cx) = + setup_pending_permission_thread("perm-no-bounds", cx).await; + + thread_view.update_in(cx, |view, window, cx| { + assert!( + view.render_main_agent_awaiting_permission(window, cx) + .is_none(), + "Floating row should stay hidden until the inline prompt has known list bounds" + ); + }); + } + + #[gpui::test] + async fn test_pending_tool_call_for_session_scopes_to_that_session(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 session_id_a = acp::SessionId::new("thread-a"); + let session_id_b = acp::SessionId::new("thread-b"); + 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) + }); + + // Pending tool calls in both threads. Unlike `pending_tool_call`, + // `pending_tool_call_for_session` must not fall back across threads. + 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); + + cx.read(|cx| { + let tool_call_id_a = conversation + .read(cx) + .pending_tool_call_for_session(&session_id_a, cx) + .expect("Expected a pending tool call in thread A"); + assert_eq!(tool_call_id_a, acp::ToolCallId::new("tc-a")); + + let tool_call_id_b = conversation + .read(cx) + .pending_tool_call_for_session(&session_id_b, cx) + .expect("Expected a pending tool call in thread B"); + assert_eq!(tool_call_id_b, acp::ToolCallId::new("tc-b")); + }); + } + + #[gpui::test] + async fn test_permission_row_scroll_to_dismisses_row(cx: &mut TestAppContext) { + init_test(cx); + + let (_view, thread_view, entry_ix, cx) = + setup_pending_permission_thread("perm-scroll", cx).await; + + // Start off-screen below the viewport — row visible because the item + // has bounds that do not intersect the viewport. + draw_thread_list_at( + &thread_view, + ListOffset { + item_ix: 0, + offset_in_item: px(0.0), + }, + cx, + ); + thread_view.read_with(cx, |view, _cx| { + assert!( + view.list_state.bounds_for_item(entry_ix).is_some(), + "The tool call entry must be measured for this test to exercise the\ + \"entry below viewport\" branch. If list overdraw stops measuring\ + offscreen items, this test needs to drive measurement another way." + ); + }); + thread_view.update_in(cx, |view, window, cx| { + assert!( + view.render_main_agent_awaiting_permission(window, cx) + .is_some() + ); + }); + + // Simulate clicking "Scroll to": the list scrolls to the entry and the + // measured item bounds intersect the viewport. + draw_thread_list_at( + &thread_view, + ListOffset { + item_ix: entry_ix, + offset_in_item: px(0.0), + }, + cx, + ); + + thread_view.update_in(cx, |view, window, cx| { + assert!( + view.render_main_agent_awaiting_permission(window, cx) + .is_none(), + "Floating row should disappear after scrolling brings the inline prompt into view" + ); + }); + } + + #[gpui::test] + async fn test_permission_row_disappears_when_authorized(cx: &mut TestAppContext) { + init_test(cx); + + let (conversation_view, thread_view, _entry_ix, cx) = + setup_pending_permission_thread("perm-allow", cx).await; + + // Park the inline prompt below the viewport so the floating row would render. + draw_thread_list_at( + &thread_view, + ListOffset { + item_ix: 0, + offset_in_item: px(0.0), + }, + cx, + ); + thread_view.update_in(cx, |view, window, cx| { + assert!( + view.render_main_agent_awaiting_permission(window, cx) + .is_some(), + "Floating row should be visible before authorizing" + ); + }); + + // Dispatch the same AuthorizeToolCall action the row's Allow button + // wires up. + conversation_view.update_in(cx, |_, window, cx| { + window.dispatch_action( + crate::AuthorizeToolCall { + tool_call_id: "perm-allow".to_string(), + option_id: "allow".to_string(), + option_kind: "AllowOnce".to_string(), + } + .boxed_clone(), + cx, + ); + }); + cx.run_until_parked(); + + conversation_view.read_with(cx, |view, cx| { + assert!( + view.pending_tool_call(cx).is_none(), + "Tool call should no longer be pending after Allow is clicked" + ); + }); + thread_view.update_in(cx, |view, window, cx| { + assert!( + view.render_main_agent_awaiting_permission(window, cx) + .is_none(), + "Floating row should disappear once the permission is granted" + ); + }); + } + + #[gpui::test] + async fn test_permission_row_ignores_subagent_requests(cx: &mut TestAppContext) { + init_test(cx); + + // Build a baseline ConversationView with no permission requests, so we + // have a real `ThreadView` to call `render_main_agent_awaiting_permission` on. + let (conversation_view, cx) = + setup_conversation_view(StubAgentServer::default_response(), cx).await; + add_to_workspace(conversation_view.clone(), cx); + + let message_editor = message_editor(&conversation_view, cx); + message_editor.update_in(cx, |editor, window, cx| { + editor.set_text("Hello", window, cx); + }); + active_thread(&conversation_view, cx) + .update_in(cx, |view, window, cx| view.send(window, cx)); + cx.run_until_parked(); + + let thread_view = active_thread(&conversation_view, cx); + let parent_session_id = + thread_view.read_with(cx, |view, cx| view.thread.read(cx).session_id().clone()); + let conversation = thread_view.read_with(cx, |view, _cx| view.conversation.clone()); + + // Attach a subagent thread with a pending tool-call permission request. + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let stub: Rc = Rc::new(StubAgentConnection::new()); + let subagent_thread = cx.update(|_window, cx| { + create_test_acp_thread( + Some(parent_session_id.clone()), + "subagent", + stub, + project, + cx, + ) + }); + conversation.update(cx, |conversation, cx| { + conversation.register_thread(subagent_thread.clone(), cx); + }); + let _subagent_task = + request_test_tool_authorization(&subagent_thread, "sub-tc", "allow-sub", cx); + cx.run_until_parked(); + + cx.read(|cx| { + assert!( + conversation + .read(cx) + .pending_tool_call_for_session(&parent_session_id, cx) + .is_none(), + "Subagent requests must not surface as pending in the parent session" + ); + assert!( + !conversation + .read(cx) + .subagents_awaiting_permission(cx) + .is_empty(), + "Subagent permission row should still see the pending request" + ); + }); + + thread_view.update_in(cx, |view, window, cx| { + assert!( + view.render_main_agent_awaiting_permission(window, cx) + .is_none(), + "Subagent permission requests should not trigger the main-agent floating row" + ); + }); + } + #[gpui::test] async fn test_move_queued_message_to_empty_main_editor(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 4f0adce1ad4..d7df0feba87 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -634,6 +634,17 @@ pub struct TurnFields { pub turn_tokens: Option, } +/// How a tool call is rendered relative to its surroundings. +/// +/// `Standalone` draws its own border/margin/location header. `Embedded` is +/// hosted by a container that provides its own framing (e.g. the subagent +/// card or the main-agent awaiting-permission row). +#[derive(Copy, Clone, PartialEq, Eq)] +enum ToolCallLayout { + Standalone, + Embedded, +} + impl ThreadView { pub(crate) fn new( root_thread_id: ThreadId, @@ -2550,13 +2561,15 @@ impl ThreadView { let plan = thread.plan(); let queue_is_empty = !self.has_queued_messages(); - let subagents_awaiting_permission = self.render_subagents_awaiting_permission(cx); - let has_subagents_awaiting = subagents_awaiting_permission.is_some(); + let awaiting_permission = self + .render_main_agent_awaiting_permission(window, cx) + .or_else(|| self.render_subagents_awaiting_permission(cx)); + let has_awaiting_permission = awaiting_permission.is_some(); if changed_buffers.is_empty() && plan.is_empty() && queue_is_empty - && !has_subagents_awaiting + && !has_awaiting_permission { return None; } @@ -2596,11 +2609,9 @@ impl ThreadView { blur_radius: px(2.), spread_radius: px(0.), }]) - .when_some(subagents_awaiting_permission, |this, element| { - this.child(element) - }) + .when_some(awaiting_permission, |this, element| this.child(element)) .when( - has_subagents_awaiting + has_awaiting_permission && (!plan.is_empty() || !changed_buffers.is_empty() || !queue_is_empty), |this| this.child(Divider::horizontal().color(DividerColor::Border)), ) @@ -2990,6 +3001,93 @@ impl ThreadView { ) } + /// Returns true when the entry has been measured and sits entirely below + /// the current viewport. + fn entry_is_below_viewport(&self, entry_ix: usize) -> bool { + let viewport_bounds = self.list_state.viewport_bounds(); + self.list_state + .bounds_for_item(entry_ix) + .is_some_and(|entry_bounds| entry_bounds.top() >= viewport_bounds.bottom()) + } + + pub(crate) fn render_main_agent_awaiting_permission( + &self, + window: &Window, + cx: &Context, + ) -> Option { + if self.is_subagent() { + return None; + } + + let active_session_id = self.thread.read(cx).session_id().clone(); + let conversation = self.conversation.read(cx); + let tool_call_id = conversation.pending_tool_call_for_session(&active_session_id, cx)?; + let pending_count = conversation.pending_tool_call_count_for_session(&active_session_id); + + let thread = self.thread.read(cx); + let (entry_ix, tool_call) = thread.tool_call(&tool_call_id)?; + + if !self.entry_is_below_viewport(entry_ix) { + return None; + } + + let focus_handle = self.focus_handle(cx); + + let card = self.render_any_tool_call( + &active_session_id, + entry_ix, + tool_call, + &focus_handle, + ToolCallLayout::Embedded, + window, + cx, + ); + + let label: SharedString = if pending_count > 1 { + format!("Awaiting Confirmation ({pending_count})").into() + } else { + "Awaiting Confirmation".into() + }; + + let header = h_flex() + .p_1p5() + .pl_2() + .w_full() + .gap_1p5() + .justify_between() + .border_b_1() + .border_color(cx.theme().colors().border) + .child( + h_flex() + .gap_1p5() + .child( + h_flex() + .w_2() + .justify_center() + .child(GeneratingSpinnerElement::new(SpinnerVariant::Sand)), + ) + .child(Label::new(label).size(LabelSize::Small).color(Color::Muted)), + ) + .child( + Button::new("main-agent-permission-scroll-to", "Scroll") + .label_size(LabelSize::Small) + .end_icon( + Icon::new(IconName::ArrowDown) + .size(IconSize::XSmall) + .color(Color::Default), + ) + .on_click(cx.listener(move |this, _, _, cx| { + this.list_state.scroll_to(ListOffset { + item_ix: entry_ix, + offset_in_item: px(0.0), + }); + cx.notify(); + })), + ); + + Some(v_flex().child(header).child(card).into_any()) + } + fn render_message_queue_summary( &self, _window: &mut Window, @@ -5131,7 +5229,7 @@ impl ThreadView { entry_ix, tool_call, &self.focus_handle(cx), - false, + ToolCallLayout::Standalone, window, cx, ); @@ -6332,7 +6430,7 @@ impl ThreadView { terminal: &Entity, tool_call: &ToolCall, focus_handle: &FocusHandle, - is_subagent: bool, + layout: ToolCallLayout, window: &Window, cx: &Context, ) -> AnyElement { @@ -6549,7 +6647,7 @@ impl ThreadView { .and_then(|entry| entry.terminal(terminal)); v_flex() - .when(!is_subagent, |this| { + .when(layout == ToolCallLayout::Standalone, |this| { this.my_1p5() .mx_5() .border_1() @@ -6634,7 +6732,7 @@ impl ThreadView { entry_ix: usize, tool_call: &ToolCall, focus_handle: &FocusHandle, - is_subagent: bool, + layout: ToolCallLayout, window: &Window, cx: &Context, ) -> Div { @@ -6664,7 +6762,7 @@ impl ThreadView { terminal, tool_call, focus_handle, - is_subagent, + layout, window, cx, ) @@ -6675,7 +6773,7 @@ impl ThreadView { entry_ix, tool_call, focus_handle, - is_subagent, + layout, window, cx, )) @@ -6689,7 +6787,7 @@ impl ThreadView { entry_ix: usize, tool_call: &ToolCall, focus_handle: &FocusHandle, - is_subagent: bool, + layout: ToolCallLayout, window: &Window, cx: &Context, ) -> Div { @@ -6937,7 +7035,7 @@ impl ThreadView { v_flex() .map(|this| { - if is_subagent { + if layout == ToolCallLayout::Embedded { this } else if use_card_layout { this.my_1p5() @@ -6951,7 +7049,7 @@ impl ThreadView { this.my_1() } }) - .when(!is_subagent, |this| { + .when(layout == ToolCallLayout::Standalone, |this| { this.map(|this| { if has_location && !use_card_layout { this.ml_4() @@ -7952,7 +8050,7 @@ impl ThreadView { terminal, tool_call, focus_handle, - false, + ToolCallLayout::Standalone, window, cx, ), @@ -8531,7 +8629,7 @@ impl ThreadView { entry_ix, tool_call, focus_handle, - true, + ToolCallLayout::Embedded, window, cx, ))