agent_ui: Add round of improvements to subagent UI (#50357)

Danilo Leal created

Change summary

crates/agent/src/agent.rs                          |   2 
crates/agent/src/tools/spawn_agent_tool.rs         |  10 
crates/agent_ui/src/connection_view/thread_view.rs | 242 +++++++++------
3 files changed, 153 insertions(+), 101 deletions(-)

Detailed changes

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;

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,

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<acp_thread::Terminal>,
         tool_call: &ToolCall,
         focus_handle: &FocusHandle,
+        is_subagent: bool,
         window: &Window,
         cx: &Context<Self>,
     ) -> 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<Self>,
     ) -> 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<Self>,
     ) -> 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<SharedString> {
-        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<Self>) -> Option<AnyElement> {
         let project_context = self
             .as_native_thread(cx)?