From 7ef1fa13809ea1fd018b76eceb693151bedda6cc Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Tue, 26 May 2026 06:47:14 +0530 Subject: [PATCH] agent_ui: Surface pending tool call permission when scrolled out of view (#57632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #53266 When a tool call awaiting permission is below the viewport, it’s easy to miss why the agent has stopped working. This PR adds a floating row at the bottom of the panel that mirrors the pending permission request. It reuses the same permission prompt, so you can allow or deny it and see the raw input without scrolling to the end of the thread. It offers a "Scroll to" button to jump back to the inline prompt in the thread. Once the inline prompt is in view, the floating row disappears. If multiple tool calls need permission, including a subagent’s, they’re shown one at a time so rows don’t stack on top of each other. image Release Notes: - Improved visibility of pending tool call confirmations in the Agent Panel when its scrolled out of view. --------- Co-authored-by: Danilo Leal --- crates/agent_ui/src/conversation_view.rs | 366 +++++++++++++++++- .../src/conversation_view/thread_view.rs | 134 ++++++- 2 files changed, 481 insertions(+), 19 deletions(-) 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, ))