From 3cf535aa67ea29a1df4a0bded0428bddac49761e Mon Sep 17 00:00:00 2001 From: lucascouts Date: Sun, 27 Sep 2026 14:13:58 -0300 Subject: [PATCH] sidebar: show a thread waiting for an answer to the agent's question The sidebar had a status for an agent waiting on a *permission* -- the yellow warning triangle -- and none for an agent waiting on an *answer*. When the agent asks a question (an `AskUserQuestion` form, which reaches Zed as a session-scoped elicitation), the turn stays open while it waits, so the row showed the grey running spinner: it read as "still working" while the thread was in fact blocked on the user. The pieces were already here. `Conversation` has kept an `elicitation_requests` map, fed by `ElicitationRequested` and `ElicitationResponded`, since upstream added session-scoped elicitations -- and nothing read it. The sidebar's `WaitingForConfirmation` is computed from `permission_requests` alone (`pending_tool_call`), which is why a question never reached it. What this adds: - `AgentThreadStatus::WaitingForAnswer`, drawn with IconName::Chat tinted Accent and a "Waiting for Your Answer" tooltip. A separate status rather than the warning triangle, because the two ask different things of the user: a permission takes one click, a question takes reading and writing. - `Conversation::has_pending_elicitation`, read through `ConversationView::root_thread_has_pending_elicitation`. Like `pending_tool_call` it spans the root thread and its subagents, so a subagent's question marks its parent's row. - Ranking: a permission outranks a question (the one-click unblock is the cheaper thing to show first), and a question outranks error, background and running, as the permission already did. - `register_thread` now seeds pending questions the way it already seeded pending permissions. A thread that was asked a question before the conversation registered it never emits `ElicitationRequested` again, and without the seed it would read as running until answered. - "Stop" and the archive guard treat it like the other waiting state: the turn is still open, so stopping it is meaningful and archiving it is not. What it deliberately does not do: the collapsed project header's badge still counts permissions only ("N threads are waiting for confirmation"); a question is not a confirmation, and folding it into that text would make it wrong. Pure Zed: elicitations are generic ACP, so any agent that asks a form-mode question gets the status. Rides the claude-agent-acp-plus flag BY CHOICE, like 0010/0012/0014/0026/0027, next to 0029, whose status list it extends. (0.2.3_p20261009): the sidebar row's status is `thread_row_status`, a pure function with a truth table -- a permission prompt outranks a question, both outrank an error, background work marks only an idle thread. A subagent's open question keeps the conversation waiting, and answering it clears that. On Zed 483b746b (bump to zeo-0.9.0_p20261109): upstream folded its pending-tool-call test into `root_thread_is_waiting_for_input`, which also counts a pending elicitation, and its new `test_elicitation_marks_sidebar_thread_waiting_until_responded` asserts WaitingForConfirmation for one. This patch keeps the narrower test, now defined here as `ConversationView::root_thread_has_pending_tool_call`, and that test asserts WaitingForAnswer instead: a question waits for an answer, not for a permission. --- diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 9f5b14b..1b66364 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -309,6 +309,22 @@ impl Conversation { for request in thread_state.pending_permission_requests() { self.add_permission_request(&session_id, &request.id); } + // A thread loaded with a question still open never emits + // `ElicitationRequested` again, so seed it here as permissions are. + for entry in thread_state.entries() { + if let AgentThreadEntry::Elicitation(elicitation_id) = entry + && thread_state + .elicitation(elicitation_id) + .is_some_and(|(_, elicitation)| { + matches!(elicitation.status, ElicitationStatus::Pending { .. }) + }) + { + self.elicitation_requests + .entry(session_id.clone()) + .or_default() + .push(elicitation_id.clone()); + } + } let subscription = cx.subscribe(&thread, { let session_id = session_id.clone(); @@ -428,6 +444,12 @@ impl Conversation { cx.notify(); } + /// Whether any thread in this conversation -- the root or one of its subagents -- has a + /// question from the agent that is still waiting for an answer. + pub fn has_pending_elicitation(&self) -> bool { + !self.elicitation_requests.is_empty() + } + fn add_permission_request( &mut self, session_id: &acp_v2::SessionId, @@ -892,6 +914,30 @@ impl ConversationView { }) } + /// The root thread waits on a tool call's permission, and only that. Upstream folded this + /// into [`Self::root_thread_is_waiting_for_input`] (zed 483b746b), which also counts a + /// pending question; the sidebar tells the two apart, so Zeo keeps the narrower test. + pub fn root_thread_has_pending_tool_call(&self, cx: &App) -> bool { + let Some(root_thread) = self.root_thread_view() else { + return false; + }; + let root_session_id = root_thread.read(cx).thread.read(cx).session_id().clone(); + self.as_connected().is_some_and(|connected| { + connected + .conversation + .read(cx) + .pending_tool_call(&root_session_id, cx) + .is_some() + }) + } + + /// Like [`Self::root_thread_has_pending_tool_call`], for a question rather than a + /// permission: an `AskUserQuestion` form, from the root thread or a subagent's. + pub fn root_thread_has_pending_elicitation(&self, cx: &App) -> bool { + self.as_connected() + .is_some_and(|connected| connected.conversation.read(cx).has_pending_elicitation()) + } + pub(crate) fn root_thread(&self, cx: &App) -> Option> { self.root_thread_view() .map(|view| view.read(cx).thread.clone()) @@ -7428,6 +7474,41 @@ pub(crate) mod tests { }); } + #[gpui::test] + async fn test_preloaded_question_counts_as_pending(cx: &mut TestAppContext) { + init_test(cx); + cx.update(|cx| { + cx.update_flags(true, vec![AcpBetaFeatureFlag::NAME.to_string()]); + }); + + // The question is asked before the conversation registers the thread, so it never + // sees `ElicitationRequested` -- the case `register_thread` seeds for. + let connection = PreloadedElicitationConnection::default(); + let elicitation_id = connection.elicitation_id.clone(); + let (conversation_view, cx) = + setup_conversation_view(StubAgentServer::new(connection), cx).await; + let elicitation_id = elicitation_id + .lock() + .clone() + .expect("connection should preload an elicitation"); + + conversation_view.read_with(cx, |view, cx| { + assert!(view.root_thread_has_pending_elicitation(cx)); + }); + + let thread = conversation_view + .read_with(cx, |view, cx| view.root_thread(cx)) + .expect("the conversation should have a root thread"); + thread.update(cx, |thread, cx| { + thread.cancel_elicitation(&elicitation_id, cx) + }); + cx.run_until_parked(); + + conversation_view.read_with(cx, |view, cx| { + assert!(!view.root_thread_has_pending_elicitation(cx)); + }); + } + #[gpui::test] async fn test_resume_without_history_adds_notice(cx: &mut TestAppContext) { init_test(cx); @@ -18461,6 +18542,76 @@ pub(crate) mod tests { }); } + // 0030: a question a subagent asks keeps the whole conversation waiting -- the parent + // row in the sidebar reads "Waiting for Your Answer" -- and answering it clears that. + #[gpui::test] + async fn test_conversation_subagent_question_marks_the_conversation_pending( + 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 (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_v2::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 + }); + (subagent_thread, conversation) + }); + cx.read(|cx| assert!(!conversation.read(cx).has_pending_elicitation())); + + let _response = subagent_thread.update(cx, |thread, cx| { + thread + .request_elicitation( + acp_v2::CreateElicitationRequest::new( + acp_v2::ElicitationFormMode::new( + acp_v2::ElicitationSessionScope::new(acp_v2::SessionId::new( + "subagent", + )), + acp_v2::ElicitationSchema::new().string("name", true), + ), + "Provide a name", + ), + cx, + ) + .expect("the subagent's question should be accepted") + }); + cx.run_until_parked(); + cx.read(|cx| { + assert!( + conversation.read(cx).has_pending_elicitation(), + "a subagent's open question keeps the conversation waiting" + ) + }); + + let question = subagent_thread.read_with(cx, |thread, _| { + thread + .entries() + .iter() + .find_map(|entry| match entry { + AgentThreadEntry::Elicitation(id) => Some(id.clone()), + _ => None, + }) + .expect("the question is an entry") + }); + subagent_thread.update(cx, |thread, cx| thread.cancel_elicitation(&question, cx)); + cx.run_until_parked(); + cx.read(|cx| assert!(!conversation.read(cx).has_pending_elicitation())); + } + #[gpui::test] async fn test_conversation_subagent_scoped_pending_tool_call(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 9c4bf4f..361a42d 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -6178,7 +6178,8 @@ impl Sidebar { AgentThreadStatus::Running | AgentThreadStatus::WaitingForConfirmation | AgentThreadStatus::Unconfirmed - | AgentThreadStatus::Background => { + | AgentThreadStatus::Background + | AgentThreadStatus::WaitingForAnswer => { return; } AgentThreadStatus::Completed | AgentThreadStatus::Error => {} @@ -6719,6 +6720,7 @@ impl Sidebar { AgentThreadStatus::Running | AgentThreadStatus::WaitingForConfirmation | AgentThreadStatus::Unconfirmed + | AgentThreadStatus::WaitingForAnswer ); let is_renaming = self.rename_target == Some(RenameTarget::Thread(thread.metadata.thread_id)); @@ -8703,9 +8705,12 @@ fn all_thread_infos_for_workspace( .conversation_views() .into_iter() .filter_map(|conversation_view| { - let is_waiting_for_input = conversation_view + let has_pending_tool_call = conversation_view .read(cx) - .root_thread_is_waiting_for_input(cx); + .root_thread_has_pending_tool_call(cx); + let has_pending_elicitation = conversation_view + .read(cx) + .root_thread_has_pending_elicitation(cx); let conversation_thread_id = conversation_view.read(cx).parent_id(); let thread_view = conversation_view.read(cx).root_thread_view()?; let thread_view_ref = thread_view.read(cx); @@ -8722,20 +8727,13 @@ fn all_thread_infos_for_workspace( let session_id = thread.session_id().clone(); let is_background = agent_panel.is_retained_thread(&conversation_thread_id); - let status = if is_waiting_for_input { - AgentThreadStatus::WaitingForConfirmation - } else if thread.had_error() { - AgentThreadStatus::Error - } else { - match thread.status() { - ThreadStatus::Unconfirmed => AgentThreadStatus::Unconfirmed, - ThreadStatus::Generating => AgentThreadStatus::Running, - ThreadStatus::Idle if thread.background_task_count() > 0 => { - AgentThreadStatus::Background - } - ThreadStatus::Idle => AgentThreadStatus::Completed, - } - }; + let status = thread_row_status( + has_pending_tool_call, + has_pending_elicitation, + thread.had_error(), + thread.status(), + thread.background_task_count(), + ); let diff_stats = thread.action_log().read(cx).diff_stats(cx); @@ -9000,3 +8998,29 @@ fn dump_single_workspace(workspace: &Workspace, output: &mut String, cx: &gpui:: writeln!(output).ok(); } + +/// The status a thread's sidebar row shows. The order is the point: a permission prompt +/// outranks a question (0030), both outrank an error, and background work (0029) marks +/// only a thread that is otherwise idle -- it never hides a prompt, a question or an error. +fn thread_row_status( + pending_tool_call: bool, + pending_elicitation: bool, + had_error: bool, + status: ThreadStatus, + background_task_count: usize, +) -> AgentThreadStatus { + if pending_tool_call { + AgentThreadStatus::WaitingForConfirmation + } else if pending_elicitation { + AgentThreadStatus::WaitingForAnswer + } else if had_error { + AgentThreadStatus::Error + } else { + match status { + ThreadStatus::Unconfirmed => AgentThreadStatus::Unconfirmed, + ThreadStatus::Generating => AgentThreadStatus::Running, + ThreadStatus::Idle if background_task_count > 0 => AgentThreadStatus::Background, + ThreadStatus::Idle => AgentThreadStatus::Completed, + } + } +} diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 16bee07..1144fce 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -689,6 +689,7 @@ pub(super) fn visible_entries_as_strings( AgentThreadStatus::Background => " (background)", AgentThreadStatus::Error => " (error)", AgentThreadStatus::WaitingForConfirmation => " (waiting)", + AgentThreadStatus::WaitingForAnswer => " (question)", _ => "", }; let notified = if sidebar @@ -4725,7 +4726,9 @@ async fn test_elicitation_marks_sidebar_thread_waiting_until_responded(cx: &mut .expect("Elicitation should be accepted") }); cx.run_until_parked(); - assert_eq!(thread_status(cx), AgentThreadStatus::WaitingForConfirmation); + // Zeo: a pending elicitation is a question waiting for an answer, its own status, + // not the permission state upstream gives it (see this patch's header). + assert_eq!(thread_status(cx), AgentThreadStatus::WaitingForAnswer); thread.update(cx, |thread, cx| { thread.respond_to_elicitation( @@ -4896,6 +4899,67 @@ async fn test_background_tasks_outlive_the_turn_in_the_sidebar(cx: &mut TestAppC ); } +#[gpui::test] +async fn test_pending_question_shows_its_own_status(cx: &mut TestAppContext) { + let project = init_test_project_with_agent_panel("/my-project", cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); + + let connection = StubAgentConnection::new(); + open_thread_with_connection(&panel, connection, cx); + send_message(&panel, cx); + + let session_id = active_session_id(&panel, cx); + save_test_thread_metadata(&session_id, &project, cx).await; + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec!["v [my-project]", " Hello * (running)"] + ); + + // The turn stays open while the agent waits for an answer, so without its own + // status the row would keep reading as "working". + let thread = cx.update(|_, cx| panel.read(cx).active_agent_thread(cx).unwrap()); + let (elicitation_id, _response) = cx.update(|_, cx| { + thread.update(cx, |thread, cx| { + thread + .request_elicitation_with_id( + acp_v2::CreateElicitationRequest::new( + acp_v2::ElicitationFormMode::new( + acp_v2::ElicitationSessionScope::new(acp_v2::SessionId::new( + session_id.0.clone(), + )), + acp_v2::ElicitationSchema::new().string("database", true), + ), + "Which database should the service use?", + ), + cx, + ) + .expect("the question should be accepted") + }) + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec!["v [my-project]", " Hello * (question)"] + ); + + cx.update(|_, cx| { + thread.update(cx, |thread, cx| { + thread.cancel_elicitation(&elicitation_id, cx) + }); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec!["v [my-project]", " Hello * (running)"] + ); +} + pub(super) fn type_in_search( sidebar: &Entity, query: &str, @@ -16328,3 +16392,52 @@ async fn test_dropped_folders_open_as_projects(cx: &mut TestAppContext) { ] ); } + +// 0029/0030: the row's status, as a truth table over what can be pending at once. A +// permission prompt outranks a question, both outrank an error, and background work only +// ever marks an otherwise idle thread. +#[test] +fn test_thread_row_status_precedence() { + use super::thread_row_status as status; + use acp_thread::ThreadStatus::{Generating, Idle}; + + // (tool call, question, error, thread status, background tasks) -> row + let cases = [ + ( + (true, true, true, Idle, 3), + AgentThreadStatus::WaitingForConfirmation, + ), + ( + (true, false, false, Generating, 0), + AgentThreadStatus::WaitingForConfirmation, + ), + ( + (false, true, true, Idle, 3), + AgentThreadStatus::WaitingForAnswer, + ), + ( + (false, true, false, Generating, 0), + AgentThreadStatus::WaitingForAnswer, + ), + ((false, false, true, Idle, 3), AgentThreadStatus::Error), + ( + (false, false, false, Generating, 3), + AgentThreadStatus::Running, + ), + ( + (false, false, false, Idle, 1), + AgentThreadStatus::Background, + ), + ((false, false, false, Idle, 0), AgentThreadStatus::Completed), + ]; + for ((tool_call, question, error, thread, background), expected) in cases { + let case = format!( + "tool call {tool_call}, question {question}, error {error}, {thread:?}, {background} background" + ); + assert_eq!( + status(tool_call, question, error, thread, background), + expected, + "{case}" + ); + } +} diff --git a/crates/ui/src/components/ai/thread_item.rs b/crates/ui/src/components/ai/thread_item.rs index bb94064..2e68b18 100644 --- a/crates/ui/src/components/ai/thread_item.rs +++ b/crates/ui/src/components/ai/thread_item.rs @@ -17,6 +17,9 @@ pub enum AgentThreadStatus { /// backgrounded shell, an async subagent — which a new turn may follow. Background, WaitingForConfirmation, + /// The agent asked a question (a form, such as `AskUserQuestion`) and is waiting for + /// the user to write an answer -- as opposed to a permission, which takes one click. + WaitingForAnswer, Error, } @@ -373,6 +376,12 @@ impl RenderOnce for ThreadItem { .size(IconSize::Small) .color(Color::Muted), ) + } else if self.status == AgentThreadStatus::WaitingForAnswer { + Some( + Icon::new(IconName::Chat) + .size(IconSize::Small) + .color(Color::Accent), + ) } else if self.notified { Some( Icon::new(IconName::Circle) @@ -536,6 +545,7 @@ impl RenderOnce for ThreadItem { self.status, AgentThreadStatus::Error | AgentThreadStatus::WaitingForConfirmation + | AgentThreadStatus::WaitingForAnswer | AgentThreadStatus::Background ); @@ -765,6 +775,15 @@ impl RenderOnce for ThreadItem { ) .child(Label::new("Waiting for Confirmation")) .into_any_element(), + AgentThreadStatus::WaitingForAnswer => h_flex() + .gap_1() + .child( + Icon::new(IconName::Chat) + .size(IconSize::Small) + .color(Color::Accent), + ) + .child(Label::new("Waiting for Your Answer")) + .into_any_element(), AgentThreadStatus::Background => h_flex() .gap_1() .child( @@ -824,6 +843,16 @@ impl Component for ThreadItem { ) .into_any_element(), ), + single_example( + "Waiting for an Answer", + container() + .child( + ThreadItem::new("ti-2d", "Which database should the service use?") + .timestamp("3h") + .status(AgentThreadStatus::WaitingForAnswer), + ) + .into_any_element(), + ), single_example( "Error", container()