From 1123140e40f47ad7b12815c16de0d49e42e36617 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Fri, 27 Feb 2026 20:35:59 -0300 Subject: [PATCH] agent_ui: Add round of improvements to subagent UI (#50357) --- crates/agent/src/agent.rs | 2 +- crates/agent/src/tools/spawn_agent_tool.rs | 10 +- .../src/connection_view/thread_view.rs | 242 +++++++++++------- 3 files changed, 153 insertions(+), 101 deletions(-) diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index 85b943da4bb65b038100b2b842d81bc34662325d..8de0aaee0c05c07e0b3c86a1b7570a1a61dc5332 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -1822,7 +1822,7 @@ impl SubagentHandle for NativeSubagentHandle { .map(|m| m.to_markdown()) .context("No response from subagent") }), - SubagentPromptResult::Cancelled => Err(anyhow!("User cancelled")), + SubagentPromptResult::Cancelled => Err(anyhow!("User canceled")), SubagentPromptResult::Error(message) => Err(anyhow!("{message}")), SubagentPromptResult::ContextWindowWarning => { thread.update(cx, |thread, cx| thread.cancel(cx)).await; diff --git a/crates/agent/src/tools/spawn_agent_tool.rs b/crates/agent/src/tools/spawn_agent_tool.rs index 2c5c40c704464639ca43b7da32ab8ae0239e3b6a..7713da050996f6fb4c07d56f51a218dfb88d5db5 100644 --- a/crates/agent/src/tools/spawn_agent_tool.rs +++ b/crates/agent/src/tools/spawn_agent_tool.rs @@ -142,9 +142,13 @@ impl AgentTool for SpawnAgentTool { } Err(e) => { let error = e.to_string(); - event_stream.update_fields( - acp::ToolCallUpdateFields::new().content(vec![error.clone().into()]), - ); + // workaround for now because the agent loop will always mark this as ToolCallStatus::Failed + let canceled = error == "User canceled"; + event_stream.update_fields(acp::ToolCallUpdateFields::new().content(vec![ + acp::ToolCallContent::Content(acp::Content::new(error.clone()).meta( + acp::Meta::from_iter([("cancelled".into(), canceled.into())]), + )), + ])); Err(SpawnAgentToolOutput::Error { session_id: Some(subagent_session_id), error, diff --git a/crates/agent_ui/src/connection_view/thread_view.rs b/crates/agent_ui/src/connection_view/thread_view.rs index 20d860c5c14fd8c5c50be3b2bc8eefb89d9d7db6..9f38ba9ba778b6c23f7a1ee4adecea501c98bfdb 100644 --- a/crates/agent_ui/src/connection_view/thread_view.rs +++ b/crates/agent_ui/src/connection_view/thread_view.rs @@ -1,3 +1,4 @@ +use acp_thread::ContentBlock; use cloud_api_types::{SubmitAgentThreadFeedbackBody, SubmitAgentThreadFeedbackCommentsBody}; use editor::actions::OpenExcerpts; use gpui::{Corner, List}; @@ -3840,6 +3841,7 @@ impl ThreadView { entry_ix, tool_call, &self.focus_handle(cx), + false, window, cx, ) @@ -4689,6 +4691,7 @@ impl ThreadView { terminal: &Entity, tool_call: &ToolCall, focus_handle: &FocusHandle, + is_subagent: bool, window: &Window, cx: &Context, ) -> AnyElement { @@ -4907,12 +4910,14 @@ impl ThreadView { .and_then(|entry| entry.terminal(terminal)); v_flex() - .my_1p5() - .mx_5() - .border_1() - .when(tool_failed || command_failed, |card| card.border_dashed()) - .border_color(border_color) - .rounded_md() + .when(!is_subagent, |this| { + this.my_1p5() + .mx_5() + .border_1() + .when(tool_failed || command_failed, |card| card.border_dashed()) + .border_color(border_color) + .rounded_md() + }) .overflow_hidden() .child( v_flex() @@ -4989,6 +4994,7 @@ impl ThreadView { entry_ix: usize, tool_call: &ToolCall, focus_handle: &FocusHandle, + is_subagent: bool, window: &Window, cx: &Context, ) -> Div { @@ -5013,6 +5019,7 @@ impl ThreadView { terminal, tool_call, focus_handle, + is_subagent, window, cx, ) @@ -5023,6 +5030,7 @@ impl ThreadView { entry_ix, tool_call, focus_handle, + is_subagent, window, cx, )) @@ -5036,6 +5044,7 @@ impl ThreadView { entry_ix: usize, tool_call: &ToolCall, focus_handle: &FocusHandle, + is_subagent: bool, window: &Window, cx: &Context, ) -> Div { @@ -5256,7 +5265,9 @@ impl ThreadView { v_flex() .map(|this| { - if use_card_layout { + if is_subagent { + this + } else if use_card_layout { this.my_1p5() .rounded_md() .border_1() @@ -5268,14 +5279,16 @@ impl ThreadView { this.my_1() } }) - .map(|this| { - if has_location && !use_card_layout { - this.ml_4() - } else { - this.ml_5() - } + .when(!is_subagent, |this| { + this.map(|this| { + if has_location && !use_card_layout { + this.ml_4() + } else { + this.ml_5() + } + }) + .mr_5() }) - .mr_5() .map(|this| { if is_terminal_tool { let label_source = tool_call.label.read(cx).source(); @@ -6069,6 +6082,7 @@ impl ThreadView { terminal, tool_call, focus_handle, + false, window, cx, ), @@ -6352,6 +6366,15 @@ impl ThreadView { .map(|log| log.read(cx).changed_buffers(cx)) .unwrap_or_default(); + let is_pending_tool_call = thread + .as_ref() + .and_then(|thread| { + self.conversation + .read(cx) + .pending_tool_call(thread.read(cx).session_id(), cx) + }) + .is_some(); + 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); @@ -6360,11 +6383,20 @@ impl ThreadView { tool_call.status, ToolCallStatus::Pending | ToolCallStatus::InProgress ); - let is_canceled_or_failed = matches!( + + let is_failed = matches!( tool_call.status, - ToolCallStatus::Canceled | ToolCallStatus::Failed | ToolCallStatus::Rejected + ToolCallStatus::Failed | ToolCallStatus::Rejected ); + let is_cancelled = matches!(tool_call.status, ToolCallStatus::Canceled) + || tool_call.content.iter().any(|c| match c { + ToolCallContent::ContentBlock(ContentBlock::Markdown { markdown }) => { + markdown.read(cx).source() == "User canceled" + } + _ => false, + }); + let thread_title = thread .as_ref() .map(|t| t.read(cx).title()) @@ -6373,29 +6405,49 @@ impl ThreadView { let has_tool_call_label = !tool_call_label.is_empty(); let has_title = thread_title.is_some() || has_tool_call_label; - let has_no_title_or_canceled = !has_title || is_canceled_or_failed; + let has_no_title_or_canceled = !has_title || is_failed || is_cancelled; let title: SharedString = if let Some(thread_title) = thread_title { thread_title } else if !tool_call_label.is_empty() { tool_call_label.into() - } else if is_canceled_or_failed { + } else if is_cancelled { "Subagent Canceled".into() + } else if is_failed { + "Subagent Failed".into() } else { - "Spawning agent…".into() + "Spawning Agent…".into() }; let card_header_id = format!("subagent-header-{}", entry_ix); + let status_icon = format!("status-icon-{}", 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() .size(LabelSize::Small) .into_any_element() - } else if is_canceled_or_failed { - Icon::new(IconName::Close) - .size(IconSize::Small) - .color(Color::Error) + } else if is_cancelled { + div() + .id(status_icon) + .child( + Icon::new(IconName::Circle) + .size(IconSize::Small) + .color(Color::Custom( + cx.theme().colors().icon_disabled.opacity(0.5), + )), + ) + .tooltip(Tooltip::text("Subagent Cancelled")) + .into_any_element() + } else if is_failed { + div() + .id(status_icon) + .child( + Icon::new(IconName::Close) + .size(IconSize::Small) + .color(Color::Error), + ) + .tooltip(Tooltip::text("Subagent Failed")) .into_any_element() } else { Icon::new(IconName::Check) @@ -6414,6 +6466,8 @@ impl ThreadView { "Click to Preview" }; + let error_message = self.subagent_error_message(&tool_call.status, tool_call, cx); + v_flex() .w_full() .rounded_md() @@ -6474,7 +6528,7 @@ impl ThreadView { ) }), ) - .when(!has_no_title_or_canceled, |this| { + .when(!has_no_title_or_canceled && !is_pending_tool_call, |this| { this.tooltip(move |_, cx| { Tooltip::with_meta( title.to_string(), @@ -6484,7 +6538,7 @@ impl ThreadView { ) }) }) - .when(has_expandable_content, |this| { + .when(has_expandable_content && !is_pending_tool_call, |this| { this.cursor_pointer() .hover(|s| s.bg(cx.theme().colors().element_hover)) .child( @@ -6546,14 +6600,16 @@ impl ThreadView { 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, - )) + this.child(Divider::horizontal().color(DividerColor::Border)) + .child(thread_view.read(cx).render_any_tool_call( + active_session_id, + entry_ix, + tool_call, + focus_handle, + true, + window, + cx, + )) } else { this } @@ -6567,6 +6623,14 @@ impl ThreadView { window, cx, )) + .when_some(error_message, |this, message| { + this.child( + Callout::new() + .severity(Severity::Error) + .icon(IconName::XCircle) + .title(message), + ) + }) .child( h_flex() .id(entry_ix) @@ -6574,8 +6638,8 @@ impl ThreadView { .w_full() .justify_center() .border_t_1() - .when(is_canceled_or_failed, |this| this.border_dashed()) - .border_color(cx.theme().colors().border_variant) + .when(is_failed, |this| this.border_dashed()) + .border_color(self.tool_card_border_color(cx)) .hover(|s| s.bg(cx.theme().colors().element_hover)) .child( Icon::new(IconName::Maximize) @@ -6611,6 +6675,30 @@ impl ThreadView { ) -> impl IntoElement { const MAX_PREVIEW_ENTRIES: usize = 8; + let parent_thread = self.thread.read(cx); + let mut started_subagent_count = 0usize; + let mut turn_has_our_call = false; + for entry in parent_thread.entries().iter() { + match entry { + AgentThreadEntry::UserMessage(_) => { + if turn_has_our_call { + break; + } + started_subagent_count = 0; + turn_has_our_call = false; + } + AgentThreadEntry::ToolCall(tc) + if tc.is_subagent() && !matches!(tc.status, ToolCallStatus::Pending) => + { + started_subagent_count += 1; + if tc.id == tool_call.id { + turn_has_our_call = true; + } + } + _ => {} + } + } + let subagent_view = thread_view.read(cx); let session_id = subagent_view.thread.read(cx).session_id().clone(); @@ -6635,7 +6723,11 @@ impl ThreadView { let entries = subagent_view.thread.read(cx).entries(); let total_entries = entries.len(); - let start_ix = total_entries.saturating_sub(MAX_PREVIEW_ENTRIES); + let start_ix = if started_subagent_count > 1 { + total_entries.saturating_sub(MAX_PREVIEW_ENTRIES) + } else { + 0 + }; let scroll_handle = self .subagent_scroll_handles @@ -6656,35 +6748,7 @@ impl ThreadView { }) .collect(); - let error_message = - self.subagent_error_message(subagent_view, &tool_call.status, tool_call, cx); - - let parent_thread = self.thread.read(cx); - let mut started_subagent_count = 0usize; - let mut turn_has_our_call = false; - for entry in parent_thread.entries().iter() { - match entry { - AgentThreadEntry::UserMessage(_) => { - if turn_has_our_call { - break; - } - started_subagent_count = 0; - turn_has_our_call = false; - } - AgentThreadEntry::ToolCall(tc) - if tc.is_subagent() && !matches!(tc.status, ToolCallStatus::Pending) => - { - started_subagent_count += 1; - if tc.id == tool_call.id { - turn_has_our_call = true; - } - } - _ => {} - } - } - v_flex() - .relative() .w_full() .border_t_1() .when(is_canceled_or_failed, |this| this.border_dashed()) @@ -6692,22 +6756,12 @@ impl ThreadView { .overflow_hidden() .child( div() - .id(format!("subagent-entries-{}", session_id)) - .flex_1() - .min_h_0() .pb_1() - .overflow_hidden() + .min_h_0() + .id(format!("subagent-entries-{}", session_id)) .track_scroll(&scroll_handle) .children(rendered_entries), ) - .when_some(error_message, |this, message| { - this.child( - Callout::new() - .severity(Severity::Error) - .icon(IconName::XCircle) - .title(message), - ) - }) .when(started_subagent_count > 1, |this| { this.h_56().child(overlay) }) @@ -6716,37 +6770,31 @@ impl ThreadView { fn subagent_error_message( &self, - subagent_view: &ThreadView, status: &ToolCallStatus, tool_call: &ToolCall, cx: &App, ) -> Option { - if matches!(status, ToolCallStatus::Canceled | ToolCallStatus::Rejected) { - return None; - } - - subagent_view - .thread_error - .as_ref() - .and_then(|e| match e { - ThreadError::Refusal => Some("The agent refused to respond to this prompt.".into()), - ThreadError::Other { message, .. } => Some(message.clone()), - ThreadError::PaymentRequired | ThreadError::AuthenticationRequired(_) => None, - }) - .or_else(|| { - tool_call.content.iter().find_map(|content| { - if let ToolCallContent::ContentBlock(block) = content { - if let acp_thread::ContentBlock::Markdown { markdown } = block { - let source = markdown.read(cx).source().to_string(); - if !source.is_empty() { + if matches!(status, ToolCallStatus::Failed) { + tool_call.content.iter().find_map(|content| { + if let ToolCallContent::ContentBlock(block) = content { + if let acp_thread::ContentBlock::Markdown { markdown } = block { + let source = markdown.read(cx).source().to_string(); + if !source.is_empty() { + if source == "User canceled" { + return None; + } else { return Some(SharedString::from(source)); } } } - None - }) + } + None }) + } else { + None + } } + fn render_rules_item(&self, cx: &Context) -> Option { let project_context = self .as_native_thread(cx)?