diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 786ed9e0aa2663d5c55ba00e49ae834448e79260..67698ea587581191411e4313d9198b1cb2914a0e 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -958,6 +958,7 @@ pub struct AcpThread { terminals: HashMap>, pending_terminal_output: HashMap>>, pending_terminal_exit: HashMap, + had_error: bool, } impl From<&AcpThread> for ActionLogTelemetry { @@ -1193,6 +1194,7 @@ impl AcpThread { terminals: HashMap::default(), pending_terminal_output: HashMap::default(), pending_terminal_exit: HashMap::default(), + had_error: false, } } @@ -1236,6 +1238,24 @@ impl AcpThread { } } + pub fn had_error(&self) -> bool { + self.had_error + } + + pub fn is_waiting_for_confirmation(&self) -> bool { + for entry in self.entries.iter().rev() { + match entry { + AgentThreadEntry::UserMessage(_) => return false, + AgentThreadEntry::ToolCall(ToolCall { + status: ToolCallStatus::WaitingForConfirmation { .. }, + .. + }) => return true, + AgentThreadEntry::ToolCall(_) | AgentThreadEntry::AssistantMessage(_) => {} + } + } + false + } + pub fn token_usage(&self) -> Option<&TokenUsage> { self.token_usage.as_ref() } @@ -1926,6 +1946,7 @@ impl AcpThread { f: impl 'static + AsyncFnOnce(WeakEntity, &mut AsyncApp) -> Result, ) -> BoxFuture<'static, Result>> { self.clear_completed_plan_entries(cx); + self.had_error = false; let (tx, rx) = oneshot::channel(); let cancel_task = self.cancel(cx); @@ -1949,7 +1970,6 @@ impl AcpThread { this.update(cx, |this, cx| { this.project .update(cx, |project, cx| project.set_agent_location(None, cx)); - let Ok(response) = response else { // tx dropped, just return return Ok(None); @@ -1970,6 +1990,7 @@ impl AcpThread { match response { Ok(r) => { if r.stop_reason == acp::StopReason::MaxTokens { + this.had_error = true; cx.emit(AcpThreadEvent::Error); log::error!("Max tokens reached. Usage: {:?}", this.token_usage); return Err(anyhow!("Max tokens reached")); @@ -1982,6 +2003,7 @@ impl AcpThread { // Handle refusal - distinguish between user prompt and tool call refusals if let acp::StopReason::Refusal = r.stop_reason { + this.had_error = true; if let Some((user_msg_ix, _)) = this.last_user_message() { // Check if there's a completed tool call with results after the last user message // This indicates the refusal is in response to tool output, not the user's prompt @@ -2019,6 +2041,7 @@ impl AcpThread { Ok(Some(r)) } Err(e) => { + this.had_error = true; cx.emit(AcpThreadEvent::Error); log::error!("Error in run turn: {:?}", e); Err(e) diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 2dfefcc5be7948daa0340dd61131d8bf53dcaaf9..1e64a5e89a38108d0715a9a181d27b34c68d574f 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -20,7 +20,10 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use theme::ActiveTheme; use ui::utils::TRAFFIC_LIGHT_PADDING; -use ui::{Divider, DividerColor, KeyBinding, ListSubHeader, Tab, ThreadItem, Tooltip, prelude::*}; +use ui::{ + AgentThreadStatus, Divider, DividerColor, KeyBinding, ListSubHeader, Tab, ThreadItem, Tooltip, + prelude::*, +}; use ui_input::ErasedEditor; use util::ResultExt as _; use workspace::{ @@ -28,12 +31,6 @@ use workspace::{ SidebarEvent, ToggleWorkspaceSidebar, Workspace, }; -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum AgentThreadStatus { - Running, - Completed, -} - #[derive(Clone, Debug)] struct AgentThreadInfo { title: SharedString, @@ -123,9 +120,15 @@ impl WorkspaceThreadEntry { let icon = thread_view.agent_icon; let title = thread.title(); - let status = match thread.status() { - ThreadStatus::Generating => AgentThreadStatus::Running, - ThreadStatus::Idle => AgentThreadStatus::Completed, + let status = if thread.is_waiting_for_confirmation() { + AgentThreadStatus::WaitingForConfirmation + } else if thread.had_error() { + AgentThreadStatus::Error + } else { + match thread.status() { + ThreadStatus::Generating => AgentThreadStatus::Running, + ThreadStatus::Idle => AgentThreadStatus::Completed, + } }; Some(AgentThreadInfo { title, @@ -213,7 +216,7 @@ impl WorkspacePickerDelegate { SidebarEntry::WorkspaceThread(thread) => thread .thread_info .as_ref() - .map(|info| (thread.index, info.status.clone())), + .map(|info| (thread.index, info.status)), _ => None, }) .collect(); @@ -626,12 +629,12 @@ impl PickerDelegate for WorkspacePickerDelegate { let has_notification = self.notified_workspaces.contains(&workspace_index); let thread_subtitle = thread_info.as_ref().map(|info| info.title.clone()); + let status = thread_info + .as_ref() + .map_or(AgentThreadStatus::default(), |info| info.status); let running = matches!( - thread_info, - Some(AgentThreadInfo { - status: AgentThreadStatus::Running, - .. - }) + status, + AgentThreadStatus::Running | AgentThreadStatus::WaitingForConfirmation ); Some( @@ -646,6 +649,7 @@ impl PickerDelegate for WorkspacePickerDelegate { ) .running(running) .generation_done(has_notification) + .status(status) .selected(selected) .worktree(worktree_label.clone()) .worktree_highlight_positions(positions.clone()) @@ -1177,7 +1181,7 @@ mod tests { cx: &mut gpui::VisualTestContext, ) { sidebar.update_in(cx, |s, _window, _cx| { - s.set_test_thread_info(index, SharedString::from(title.to_string()), status.clone()); + s.set_test_thread_info(index, SharedString::from(title.to_string()), status); }); multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); diff --git a/crates/ui/src/components/ai/thread_item.rs b/crates/ui/src/components/ai/thread_item.rs index bc7ecd3df68f76e7c7583d6ad567833d4544a4f0..6cc710690ea0103bf2de4253bc405eb52be5af69 100644 --- a/crates/ui/src/components/ai/thread_item.rs +++ b/crates/ui/src/components/ai/thread_item.rs @@ -5,6 +5,15 @@ use crate::{ use gpui::{AnyView, ClickEvent, SharedString}; +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum AgentThreadStatus { + #[default] + Completed, + Running, + WaitingForConfirmation, + Error, +} + #[derive(IntoElement, RegisterComponent)] pub struct ThreadItem { id: ElementId, @@ -13,6 +22,7 @@ pub struct ThreadItem { timestamp: SharedString, running: bool, generation_done: bool, + status: AgentThreadStatus, selected: bool, hovered: bool, added: Option, @@ -35,6 +45,7 @@ impl ThreadItem { timestamp: "".into(), running: false, generation_done: false, + status: AgentThreadStatus::default(), selected: false, hovered: false, added: None, @@ -69,6 +80,11 @@ impl ThreadItem { self } + pub fn status(mut self, status: AgentThreadStatus) -> Self { + self.status = status; + self + } + pub fn selected(mut self, selected: bool) -> Self { self.selected = selected; self @@ -143,22 +159,51 @@ impl RenderOnce for ThreadItem { .color(Color::Muted) .size(IconSize::Small); - let icon = if self.generation_done { - icon_container().child(DecoratedIcon::new( - agent_icon, - Some( - IconDecoration::new( - IconDecorationKind::Dot, - cx.theme().colors().surface_background, - cx, - ) - .color(cx.theme().colors().text_accent) - .position(gpui::Point { - x: px(-2.), - y: px(-2.), - }), - ), - )) + let decoration = if self.status == AgentThreadStatus::WaitingForConfirmation { + Some( + IconDecoration::new( + IconDecorationKind::Triangle, + cx.theme().colors().surface_background, + cx, + ) + .color(cx.theme().status().warning) + .position(gpui::Point { + x: px(-2.), + y: px(-2.), + }), + ) + } else if self.status == AgentThreadStatus::Error { + Some( + IconDecoration::new( + IconDecorationKind::X, + cx.theme().colors().surface_background, + cx, + ) + .color(cx.theme().status().error) + .position(gpui::Point { + x: px(-2.), + y: px(-2.), + }), + ) + } else if self.generation_done { + Some( + IconDecoration::new( + IconDecorationKind::Dot, + cx.theme().colors().surface_background, + cx, + ) + .color(cx.theme().colors().text_accent) + .position(gpui::Point { + x: px(-2.), + y: px(-2.), + }), + ) + } else { + None + }; + + let icon = if let Some(decoration) = decoration { + icon_container().child(DecoratedIcon::new(agent_icon, Some(decoration))) } else { icon_container().child(agent_icon) }; @@ -311,6 +356,26 @@ impl Component for ThreadItem { ) .into_any_element(), ), + single_example( + "Waiting for Confirmation", + container() + .child( + ThreadItem::new("ti-2b", "Execute shell command in terminal") + .timestamp("12:15 AM") + .status(AgentThreadStatus::WaitingForConfirmation), + ) + .into_any_element(), + ), + single_example( + "Error", + container() + .child( + ThreadItem::new("ti-2c", "Failed to connect to language server") + .timestamp("12:20 AM") + .status(AgentThreadStatus::Error), + ) + .into_any_element(), + ), single_example( "Running Agent", container() diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index 38ec26896dedad1cfb11c4a86341fc6015c4af14..ba6d18a80a102f40a4f62bba992fab7639987fc1 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -527,8 +527,26 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()> } } - // Run Test 8: Tool Permissions Settings UI visual test - println!("\n--- Test 8: tool_permissions_settings ---"); + // Run Test 8: ThreadItem icon decorations visual tests + println!("\n--- Test 8: thread_item_icon_decorations ---"); + match run_thread_item_icon_decorations_visual_tests(app_state.clone(), &mut cx, update_baseline) + { + Ok(TestResult::Passed) => { + println!("✓ thread_item_icon_decorations: PASSED"); + passed += 1; + } + Ok(TestResult::BaselineUpdated(_)) => { + println!("✓ thread_item_icon_decorations: Baseline updated"); + updated += 1; + } + Err(e) => { + eprintln!("✗ thread_item_icon_decorations: FAILED - {}", e); + failed += 1; + } + } + + // Run Test 9: Tool Permissions Settings UI visual test + println!("\n--- Test 9: tool_permissions_settings ---"); match run_tool_permissions_visual_tests(app_state.clone(), &mut cx, update_baseline) { Ok(TestResult::Passed) => { println!("✓ tool_permissions_settings: PASSED"); @@ -544,8 +562,8 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()> } } - // Run Test 9: Settings UI sub-page auto-open visual tests - println!("\n--- Test 9: settings_ui_subpage_auto_open (2 variants) ---"); + // Run Test 10: Settings UI sub-page auto-open visual tests + println!("\n--- Test 10: settings_ui_subpage_auto_open (2 variants) ---"); match run_settings_ui_subpage_visual_tests(app_state.clone(), &mut cx, update_baseline) { Ok(TestResult::Passed) => { println!("✓ settings_ui_subpage_auto_open: PASSED"); @@ -2701,12 +2719,12 @@ fn run_multi_workspace_sidebar_visual_tests( sidebar.set_test_thread_info( 0, "Refine thread view scrolling behavior".into(), - sidebar::AgentThreadStatus::Completed, + ui::AgentThreadStatus::Completed, ); sidebar.set_test_thread_info( 1, "Add line numbers option to FileEditBlock".into(), - sidebar::AgentThreadStatus::Running, + ui::AgentThreadStatus::Running, ); }); }); @@ -2840,6 +2858,151 @@ impl gpui::Render for ErrorWrappingTestView { } } +#[cfg(target_os = "macos")] +struct ThreadItemIconDecorationsTestView; + +#[cfg(target_os = "macos")] +impl gpui::Render for ThreadItemIconDecorationsTestView { + fn render( + &mut self, + _window: &mut gpui::Window, + cx: &mut gpui::Context, + ) -> impl gpui::IntoElement { + use ui::{IconName, Label, LabelSize, ThreadItem, prelude::*}; + + let section_label = |text: &str| { + Label::new(text.to_string()) + .size(LabelSize::Small) + .color(Color::Muted) + }; + + let container = || { + v_flex() + .w_80() + .border_1() + .border_color(cx.theme().colors().border_variant) + .bg(cx.theme().colors().panel_background) + }; + + v_flex() + .size_full() + .bg(cx.theme().colors().background) + .p_4() + .gap_3() + .child( + Label::new("ThreadItem Icon Decorations") + .size(LabelSize::Large) + .color(Color::Default), + ) + .child(section_label("No decoration (default idle)")) + .child( + container() + .child(ThreadItem::new("ti-none", "Default idle thread").timestamp("1:00 AM")), + ) + .child(section_label("Blue dot (generation done)")) + .child( + container().child( + ThreadItem::new("ti-done", "Generation completed successfully") + .timestamp("1:05 AM") + .generation_done(true), + ), + ) + .child(section_label("Yellow triangle (waiting for confirmation)")) + .child( + container().child( + ThreadItem::new("ti-waiting", "Waiting for user confirmation") + .timestamp("1:10 AM") + .status(ui::AgentThreadStatus::WaitingForConfirmation), + ), + ) + .child(section_label("Red X (error)")) + .child( + container().child( + ThreadItem::new("ti-error", "Failed to connect to server") + .timestamp("1:15 AM") + .status(ui::AgentThreadStatus::Error), + ), + ) + .child(section_label("Spinner (running)")) + .child( + container().child( + ThreadItem::new("ti-running", "Generating response...") + .icon(IconName::AiClaude) + .timestamp("1:20 AM") + .running(true), + ), + ) + .child(section_label( + "Spinner + yellow triangle (running + waiting)", + )) + .child( + container().child( + ThreadItem::new("ti-running-waiting", "Running but needs confirmation") + .icon(IconName::AiClaude) + .timestamp("1:25 AM") + .running(true) + .status(ui::AgentThreadStatus::WaitingForConfirmation), + ), + ) + } +} + +#[cfg(target_os = "macos")] +fn run_thread_item_icon_decorations_visual_tests( + _app_state: Arc, + cx: &mut VisualTestAppContext, + update_baseline: bool, +) -> Result { + let window_size = size(px(400.0), px(600.0)); + let bounds = Bounds { + origin: point(px(0.0), px(0.0)), + size: window_size, + }; + + let window = cx + .update(|cx| { + cx.open_window( + WindowOptions { + window_bounds: Some(WindowBounds::Windowed(bounds)), + focus: false, + show: false, + ..Default::default() + }, + |_window, cx| cx.new(|_| ThreadItemIconDecorationsTestView), + ) + }) + .context("Failed to open thread item icon decorations test window")?; + + cx.run_until_parked(); + + cx.update_window(window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + let test_result = run_visual_test( + "thread_item_icon_decorations", + window.into(), + cx, + update_baseline, + )?; + + cx.update_window(window.into(), |_, window, _cx| { + window.remove_window(); + }) + .log_err(); + + cx.run_until_parked(); + + for _ in 0..15 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + Ok(test_result) +} + #[cfg(target_os = "macos")] fn run_error_wrapping_visual_tests( _app_state: Arc,