diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index c7a962b277d..cd3f7ae2264 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -1275,6 +1275,11 @@ impl AgentPanel { }, ); + cx.on_release(|this, cx| { + this.dismiss_all_terminal_notifications(cx); + }) + .detach(); + let mut panel = Self { workspace_id, base_view, @@ -2217,16 +2222,7 @@ impl AgentPanel { window: &mut Window, cx: &mut Context, ) { - let is_active = self.active_terminal_id() == Some(terminal_id); - // Only suppress when the user can actually see the bell, i.e. the - // terminal is focused AND the OS window is active. A bell delivered to - // a background window should still be marked unseen. - let user_is_looking = is_active - && window.is_window_active() - && self.terminals.get(&terminal_id).is_some_and(|terminal| { - terminal.view.focus_handle(cx).contains_focused(window, cx) - }); - if user_is_looking { + if self.active_terminal_visible(terminal_id, window, cx) { return; } let newly_notified = { @@ -2244,7 +2240,10 @@ impl AgentPanel { cx.emit(AgentPanelEvent::EntryChanged); cx.notify(); #[cfg(feature = "audio")] - self.play_terminal_notification_sound(cx); + self.play_terminal_notification_sound( + self.terminal_status_visible(terminal_id, window, cx), + cx, + ); self.show_terminal_notification(terminal_id, window, cx); } } @@ -2262,6 +2261,9 @@ impl AgentPanel { return; } let title = terminal.title(cx); + if self.terminal_status_visible(terminal_id, window, cx) { + return; + } let settings = AgentSettings::get_global(cx); match settings.notify_when_agent_waiting { NotifyWhenAgentWaiting::PrimaryScreen => { @@ -2338,6 +2340,8 @@ impl AgentPanel { }) .log_err(); }); + + this.dismiss_terminal_notifications(terminal_id, cx); } AgentNotificationEvent::Dismissed => { this.dismiss_terminal_notifications(terminal_id, cx); @@ -2353,10 +2357,25 @@ impl AgentPanel { } }); - let multi_workspace_subscription = window.root::().flatten().map(|mw| { - cx.observe_in(&mw, window, move |this, _, window, cx| { - this.dismiss_terminal_pop_up_if_visible(terminal_id, &pop_up_weak, window, cx); + let multi_workspace_subscription = { + let pop_up_weak = pop_up_weak.clone(); + window.root::().flatten().map(|mw| { + cx.observe_in(&mw, window, move |this, _, window, cx| { + this.dismiss_terminal_pop_up_if_visible(terminal_id, &pop_up_weak, window, cx); + }) }) + }; + + let this_panel = cx.entity(); + let agent_panel_subscription = cx.subscribe_in(&this_panel, window, { + move |this, _, event: &AgentPanelEvent, window, cx| match event { + AgentPanelEvent::ActiveViewChanged | AgentPanelEvent::ActiveViewFocused => { + this.dismiss_terminal_pop_up_if_visible(terminal_id, &pop_up_weak, window, cx); + } + AgentPanelEvent::EntryChanged + | AgentPanelEvent::TerminalClosed { .. } + | AgentPanelEvent::ThreadInteracted { .. } => {} + } }); let Some(terminal) = self.terminals.get_mut(&terminal_id) else { @@ -2370,12 +2389,15 @@ impl AgentPanel { terminal .notification_subscriptions .push(window_activation_subscription); + terminal + .notification_subscriptions + .push(agent_panel_subscription); if let Some(subscription) = multi_workspace_subscription { terminal.notification_subscriptions.push(subscription); } } - fn dismiss_terminal_notifications(&mut self, terminal_id: TerminalId, cx: &mut Context) { + fn dismiss_terminal_notifications(&mut self, terminal_id: TerminalId, cx: &mut App) { let Some(terminal) = self.terminals.get_mut(&terminal_id) else { return; }; @@ -2390,11 +2412,18 @@ impl AgentPanel { } } - fn terminal_visible_to_user(&self, terminal_id: TerminalId, window: &Window, cx: &App) -> bool { + fn dismiss_all_terminal_notifications(&mut self, cx: &mut App) { + let terminal_ids = self.terminals.keys().copied().collect::>(); + for terminal_id in terminal_ids { + self.dismiss_terminal_notifications(terminal_id, cx); + } + } + + fn active_terminal_visible(&self, terminal_id: TerminalId, window: &Window, cx: &App) -> bool { if !window.is_window_active() { return false; } - if self.active_terminal_id() != Some(terminal_id) { + if !self.terminal_surface_visible(terminal_id) { return false; } let Some(workspace) = self.workspace.upgrade() else { @@ -2402,9 +2431,6 @@ impl AgentPanel { }; if let Some(multi_workspace) = window.root::().flatten() { let multi_workspace = multi_workspace.read(cx); - if multi_workspace.sidebar_open() { - return true; - } if multi_workspace.workspace() != &workspace { return false; } @@ -2412,6 +2438,36 @@ impl AgentPanel { AgentPanel::is_visible(&workspace, cx) } + fn terminal_surface_visible(&self, terminal_id: TerminalId) -> bool { + self.active_terminal_id() == Some(terminal_id) + && matches!(self.visible_surface(), VisibleSurface::Terminal(_)) + } + + fn terminal_status_visible(&self, terminal_id: TerminalId, window: &Window, cx: &App) -> bool { + if !window.is_window_active() { + return false; + } + + if let Some(multi_workspace) = window.root::().flatten() { + let multi_workspace = multi_workspace.read(cx); + if multi_workspace.sidebar_open() && multi_workspace.is_threads_list_view_active(cx) { + return true; + } + + let Some(workspace) = self.workspace.upgrade() else { + return false; + }; + + return multi_workspace.workspace() == &workspace + && self.terminal_surface_visible(terminal_id) + && AgentPanel::is_visible(&workspace, cx); + } + + self.workspace.upgrade().is_some_and(|workspace| { + self.terminal_surface_visible(terminal_id) && AgentPanel::is_visible(&workspace, cx) + }) + } + fn dismiss_terminal_pop_up_if_visible( &mut self, terminal_id: TerminalId, @@ -2419,9 +2475,17 @@ impl AgentPanel { window: &mut Window, cx: &mut Context, ) { - if !self.terminal_visible_to_user(terminal_id, window, cx) { + if !self.terminal_status_visible(terminal_id, window, cx) { return; } + if self.active_terminal_visible(terminal_id, window, cx) + && let Some(terminal) = self.terminals.get_mut(&terminal_id) + && terminal.has_notification + { + terminal.has_notification = false; + cx.emit(AgentPanelEvent::EntryChanged); + cx.notify(); + } if let Some(pop_up) = pop_up.upgrade() { pop_up.update(cx, |notification, cx| { notification.dismiss(cx); @@ -2430,11 +2494,9 @@ impl AgentPanel { } #[cfg(feature = "audio")] - fn play_terminal_notification_sound(&self, cx: &mut App) { - // We only reach this when the user isn't looking at the terminal, - // so it is effectively hidden — only `Never` suppresses the sound. + fn play_terminal_notification_sound(&self, visible: bool, cx: &mut App) { let settings = AgentSettings::get_global(cx); - if settings.play_sound_when_agent_done.should_play(false) { + if settings.play_sound_when_agent_done.should_play(visible) { Audio::play_sound(Sound::AgentDone, cx); } } @@ -3512,6 +3574,13 @@ impl AgentPanel { } } + pub(crate) fn visible_conversation_view(&self) -> Option<&Entity> { + match self.visible_surface() { + VisibleSurface::AgentThread(conversation_view) => Some(conversation_view), + _ => None, + } + } + pub fn conversation_views(&self) -> Vec> { self.active_conversation_view() .into_iter() @@ -5935,7 +6004,7 @@ mod tests { use crate::conversation_view::tests::{StubAgentServer, init_test}; use crate::test_support::{ active_session_id, active_thread_id, open_thread_with_connection, - open_thread_with_custom_connection, send_message, + open_thread_with_custom_connection, register_test_sidebar, send_message, }; use acp_thread::{AgentConnection, StubAgentConnection, ThreadStatus, UserMessageId}; use action_log::ActionLog; @@ -7809,6 +7878,56 @@ mod tests { (panel, cx) } + async fn setup_visible_panel( + cx: &mut TestAppContext, + ) -> (Entity, VisualTestContext) { + setup_visible_panel_with_sidebar(cx, true).await + } + + async fn setup_visible_panel_with_sidebar( + cx: &mut TestAppContext, + threads_list_active: bool, + ) -> (Entity, VisualTestContext) { + init_test(cx); + cx.update(|cx| { + agent::ThreadStore::init_global(cx); + language_model::LanguageModelRegistry::test(cx); + AgentSettings::override_global( + AgentSettings { + notify_when_agent_waiting: NotifyWhenAgentWaiting::PrimaryScreen, + ..AgentSettings::get_global(cx).clone() + }, + cx, + ); + }); + + let fs = FakeFs::new(cx.executor()); + cx.update(|cx| ::set_global(fs.clone(), cx)); + fs.insert_tree("/project", json!({ "file.txt": "" })).await; + let project = Project::test(fs.clone(), [Path::new("/project")], cx).await; + + let multi_workspace = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + + let workspace = multi_workspace + .read_with(cx, |multi_workspace, _cx| { + multi_workspace.workspace().clone() + }) + .unwrap(); + + let mut cx = VisualTestContext::from_window(multi_workspace.into(), cx); + register_test_sidebar(threads_list_active, &mut cx); + + let panel = workspace.update_in(&mut cx, |workspace, window, cx| { + let panel = cx.new(|cx| AgentPanel::new(workspace, None, window, cx)); + workspace.add_panel(panel.clone(), window, cx); + workspace.focus_panel::(window, cx); + panel + }); + + (panel, cx) + } + fn expected_terminal_drop_text(paths: &[PathBuf]) -> String { let mut text = String::new(); for path in paths { @@ -8578,6 +8697,499 @@ mod tests { }); } + #[gpui::test] + async fn test_visible_terminal_bell_is_suppressed(cx: &mut TestAppContext) { + let (panel, mut cx) = setup_visible_panel(cx).await; + let terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Claude", true, window, cx) + }) + .expect("test terminal should be inserted"); + cx.run_until_parked(); + + cx.update(|window, cx| { + assert!(window.is_window_active()); + assert!(panel.read(cx).focus_handle(cx).contains_focused(window, cx)); + }); + + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(terminal_id, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == terminal_id) + .expect("terminal should remain in the panel"); + assert!(!terminal.has_notification); + }); + assert!( + cx.windows() + .iter() + .all(|window| window.downcast::().is_none()) + ); + } + + #[gpui::test] + async fn test_visible_terminal_bell_is_suppressed_without_focus(cx: &mut TestAppContext) { + let (panel, mut cx) = setup_visible_panel(cx).await; + let terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Claude", true, window, cx) + }) + .expect("test terminal should be inserted"); + cx.run_until_parked(); + + let workspace = cx.update(|window, cx| { + window + .root::() + .flatten() + .expect("test window should have a MultiWorkspace root") + .read(cx) + .workspace() + .clone() + }); + workspace.update_in(&mut cx, |workspace, window, cx| { + workspace.focus_handle(cx).focus(window, cx); + }); + cx.update(|window, cx| { + assert!(window.is_window_active()); + assert!(workspace.read(cx).focus_handle(cx).is_focused(window)); + assert!(!panel.read(cx).focus_handle(cx).contains_focused(window, cx)); + }); + + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(terminal_id, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == terminal_id) + .expect("terminal should remain in the panel"); + assert!(!terminal.has_notification); + }); + assert!( + cx.windows() + .iter() + .all(|window| window.downcast::().is_none()) + ); + } + + #[gpui::test] + async fn test_terminal_bell_notifies_when_configuration_overlay_covers_terminal( + cx: &mut TestAppContext, + ) { + let (panel, mut cx) = setup_visible_panel(cx).await; + let terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Claude", true, window, cx) + }) + .expect("test terminal should be inserted"); + cx.run_until_parked(); + + panel.update_in(&mut cx, |panel, window, cx| { + panel.set_overlay(OverlayView::Configuration, true, window, cx); + }); + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(terminal_id, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == terminal_id) + .expect("terminal should remain in the panel"); + assert!(terminal.has_notification); + }); + cx.windows() + .iter() + .find_map(|window| window.downcast::()) + .expect("covered terminal bell should show a notification"); + } + + #[gpui::test] + async fn test_thread_notification_shows_when_configuration_overlay_covers_thread( + cx: &mut TestAppContext, + ) { + let (panel, mut cx) = setup_visible_panel(cx).await; + let connection = StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( + acp::ContentChunk::new("Default response".into()), + )]); + open_thread_with_connection(&panel, connection, &mut cx); + + panel.update_in(&mut cx, |panel, window, cx| { + panel.set_overlay(OverlayView::Configuration, true, window, cx); + }); + send_message(&panel, &mut cx); + + cx.windows() + .iter() + .find_map(|window| window.downcast::()) + .expect("covered thread should show a notification"); + } + + #[gpui::test] + async fn test_terminal_bell_marks_without_popup_when_sidebar_open(cx: &mut TestAppContext) { + let (panel, mut cx) = setup_visible_panel(cx).await; + let first_terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Build", true, window, cx) + }) + .expect("first test terminal should be inserted"); + let second_terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Server", true, window, cx) + }) + .expect("second test terminal should be inserted"); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, _cx| { + assert_eq!(panel.active_terminal_id(), Some(second_terminal_id)); + }); + cx.update(|window, cx| { + let multi_workspace = window + .root::() + .flatten() + .expect("test window should have a MultiWorkspace root"); + multi_workspace.update(cx, |multi_workspace, cx| { + multi_workspace.open_sidebar(cx); + }); + }); + cx.run_until_parked(); + + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(first_terminal_id, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let first_terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == first_terminal_id) + .expect("first terminal should remain in the panel"); + assert!(first_terminal.has_notification); + }); + assert!( + cx.windows() + .iter() + .all(|window| window.downcast::().is_none()) + ); + } + + #[gpui::test] + async fn test_terminal_bell_notifies_when_sidebar_history_open(cx: &mut TestAppContext) { + let (panel, mut cx) = setup_visible_panel_with_sidebar(cx, false).await; + let first_terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Build", true, window, cx) + }) + .expect("first test terminal should be inserted"); + let second_terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Server", true, window, cx) + }) + .expect("second test terminal should be inserted"); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, _cx| { + assert_eq!(panel.active_terminal_id(), Some(second_terminal_id)); + }); + cx.update(|window, cx| { + let multi_workspace = window + .root::() + .flatten() + .expect("test window should have a MultiWorkspace root"); + multi_workspace.update(cx, |multi_workspace, cx| { + multi_workspace.open_sidebar(cx); + }); + }); + cx.run_until_parked(); + + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(first_terminal_id, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let first_terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == first_terminal_id) + .expect("first terminal should remain in the panel"); + assert!(first_terminal.has_notification); + }); + cx.windows() + .iter() + .find_map(|window| window.downcast::()) + .expect("terminal bell should notify when the sidebar thread list is hidden"); + } + + #[gpui::test] + async fn test_terminal_notification_dismissed_when_sidebar_opens(cx: &mut TestAppContext) { + let (panel, mut cx) = setup_visible_panel(cx).await; + let first_terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Build", true, window, cx) + }) + .expect("first test terminal should be inserted"); + let second_terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Server", true, window, cx) + }) + .expect("second test terminal should be inserted"); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, _cx| { + assert_eq!(panel.active_terminal_id(), Some(second_terminal_id)); + }); + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(first_terminal_id, cx); + }); + cx.run_until_parked(); + + cx.windows() + .iter() + .find_map(|window| window.downcast::()) + .expect("inactive terminal bell should show a notification"); + + cx.update(|window, cx| { + let multi_workspace = window + .root::() + .flatten() + .expect("test window should have a MultiWorkspace root"); + multi_workspace.update(cx, |multi_workspace, cx| { + multi_workspace.open_sidebar(cx); + }); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let first_terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == first_terminal_id) + .expect("first terminal should remain in the panel"); + assert!(first_terminal.has_notification); + }); + assert!( + cx.windows() + .iter() + .all(|window| window.downcast::().is_none()) + ); + } + + #[gpui::test] + async fn test_focused_terminal_bell_notifies_when_window_inactive(cx: &mut TestAppContext) { + let (panel, mut cx) = setup_visible_panel(cx).await; + let terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Claude", true, window, cx) + }) + .expect("test terminal should be inserted"); + cx.run_until_parked(); + + cx.update(|window, cx| { + assert!(window.is_window_active()); + assert!(panel.read(cx).focus_handle(cx).contains_focused(window, cx)); + }); + cx.deactivate_window(); + cx.update(|window, _cx| { + assert!(!window.is_window_active()); + }); + + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(terminal_id, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == terminal_id) + .expect("terminal should remain in the panel"); + assert!(terminal.has_notification); + }); + cx.windows() + .iter() + .find_map(|window| window.downcast::()) + .expect("background terminal bell should show a notification"); + } + + #[gpui::test] + async fn test_active_terminal_notification_clears_when_window_reactivates( + cx: &mut TestAppContext, + ) { + let (panel, mut cx) = setup_visible_panel(cx).await; + let terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Claude", true, window, cx) + }) + .expect("test terminal should be inserted"); + cx.run_until_parked(); + + cx.deactivate_window(); + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(terminal_id, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == terminal_id) + .expect("terminal should remain in the panel"); + assert!(terminal.has_notification); + }); + cx.windows() + .iter() + .find_map(|window| window.downcast::()) + .expect("background terminal bell should show a notification"); + + cx.update(|window, _cx| { + window.activate_window(); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == terminal_id) + .expect("terminal should remain in the panel"); + assert!(!terminal.has_notification); + }); + assert!( + cx.windows() + .iter() + .all(|window| window.downcast::().is_none()) + ); + } + + #[gpui::test] + async fn test_terminal_notification_dismissed_when_active_terminal_becomes_visible( + cx: &mut TestAppContext, + ) { + let (panel, mut cx) = setup_panel(cx).await; + cx.update(|_window, cx| { + AgentSettings::override_global( + AgentSettings { + notify_when_agent_waiting: NotifyWhenAgentWaiting::PrimaryScreen, + ..AgentSettings::get_global(cx).clone() + }, + cx, + ); + }); + let terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Claude", true, window, cx) + }) + .expect("test terminal should be inserted"); + cx.run_until_parked(); + + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(terminal_id, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == terminal_id) + .expect("terminal should remain in the panel"); + assert!(terminal.has_notification); + }); + cx.windows() + .iter() + .find_map(|window| window.downcast::()) + .expect("hidden terminal bell should show a notification"); + + let workspace = cx.update(|window, cx| { + window + .root::() + .flatten() + .expect("test window should have a MultiWorkspace root") + .read(cx) + .workspace() + .clone() + }); + workspace.update_in(&mut cx, |workspace, window, cx| { + workspace.add_panel(panel.clone(), window, cx); + workspace.focus_panel::(window, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminal = panel + .terminals(cx) + .into_iter() + .find(|terminal| terminal.id == terminal_id) + .expect("terminal should remain in the panel"); + assert!(!terminal.has_notification); + }); + assert!( + cx.windows() + .iter() + .all(|window| window.downcast::().is_none()) + ); + } + + #[gpui::test] + async fn test_terminal_notification_closed_when_panel_dropped(cx: &mut TestAppContext) { + let (panel, mut cx) = setup_panel(cx).await; + cx.update(|_window, cx| { + AgentSettings::override_global( + AgentSettings { + notify_when_agent_waiting: NotifyWhenAgentWaiting::PrimaryScreen, + ..AgentSettings::get_global(cx).clone() + }, + cx, + ); + }); + let terminal_id = panel + .update_in(&mut cx, |panel, window, cx| { + panel.insert_test_terminal("Claude", true, window, cx) + }) + .expect("test terminal should be inserted"); + let weak_panel = panel.downgrade(); + cx.run_until_parked(); + + panel.update(&mut cx, |panel, cx| { + panel.emit_test_terminal_bell(terminal_id, cx); + }); + cx.run_until_parked(); + + cx.windows() + .iter() + .find_map(|window| window.downcast::()) + .expect("hidden terminal bell should show a notification"); + + drop(panel); + cx.update(|_window, _cx| {}); + cx.run_until_parked(); + + assert!( + !weak_panel.is_upgradable(), + "agent panel should be released after dropping the last handle" + ); + assert!( + cx.windows() + .iter() + .all(|window| window.downcast::().is_none()) + ); + } + #[gpui::test] async fn test_terminal_notification_view_activates_terminal_workspace(cx: &mut TestAppContext) { init_test(cx); @@ -8656,6 +9268,11 @@ mod tests { notification .update(cx, |notification, _window, cx| notification.accept(cx)) .unwrap(); + assert!( + cx.windows() + .iter() + .all(|window| window.downcast::().is_none()) + ); cx.run_until_parked(); multi_workspace diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 5e5f6af52b3..96d6476b9a3 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -2533,18 +2533,24 @@ impl ConversationView { return false; }; - multi_workspace.read(cx).sidebar_open() - || multi_workspace.read(cx).workspace() == &workspace - && AgentPanel::is_visible(&workspace, cx) - && multi_workspace - .read(cx) - .workspace() - .read(cx) - .panel::(cx) - .map_or(false, |p| { - p.read(cx).active_conversation_view().map(|c| c.entity_id()) - == Some(cx.entity_id()) - }) + let multi_workspace = multi_workspace.read(cx); + multi_workspace.sidebar_open() && multi_workspace.is_threads_list_view_active(cx) + || multi_workspace.workspace() == &workspace + && self.is_visible_in_agent_panel(&workspace, cx) + } + + fn is_visible_in_agent_panel(&self, workspace: &Entity, cx: &Context) -> bool { + AgentPanel::is_visible(workspace, cx) + && workspace + .read(cx) + .panel::(cx) + .is_some_and(|panel| { + panel + .read(cx) + .visible_conversation_view() + .map(|conversation_view| conversation_view.entity_id()) + == Some(cx.entity_id()) + }) } fn agent_status_visible(&self, window: &Window, cx: &Context) -> bool { @@ -2557,7 +2563,7 @@ impl ConversationView { } else { self.workspace .upgrade() - .is_some_and(|workspace| AgentPanel::is_visible(&workspace, cx)) + .is_some_and(|workspace| self.is_visible_in_agent_panel(&workspace, cx)) } } @@ -2569,7 +2575,7 @@ impl ConversationView { } else { self.workspace .upgrade() - .is_some_and(|workspace| AgentPanel::is_visible(&workspace, cx)) + .is_some_and(|workspace| self.is_visible_in_agent_panel(&workspace, cx)) }; let settings = AgentSettings::get_global(cx); if settings.play_sound_when_agent_done.should_play(visible) { @@ -3179,6 +3185,7 @@ pub(crate) mod tests { use crate::agent_panel; use crate::completion_provider::AgentContextSource; + use crate::test_support::register_test_sidebar; use crate::thread_metadata_store::ThreadMetadataStore; use super::*; @@ -4164,6 +4171,7 @@ pub(crate) mod tests { .unwrap(); let cx = &mut VisualTestContext::from_window(multi_workspace_handle.into(), cx); + register_test_sidebar(true, cx); // Open the sidebar so that sidebar_open() returns true. multi_workspace_handle @@ -4228,6 +4236,80 @@ pub(crate) mod tests { ); } + #[gpui::test] + async fn test_notification_when_sidebar_open_but_thread_list_hidden(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + + cx.update(|cx| { + cx.update_flags(true, vec!["agent-v2".to_string()]); + agent::ThreadStore::init_global(cx); + language_model::LanguageModelRegistry::test(cx); + ::set_global(fs.clone(), cx); + }); + + let project = Project::test(fs, [], cx).await; + let multi_workspace_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + + let workspace = multi_workspace_handle + .read_with(cx, |mw, _cx| mw.workspace().clone()) + .unwrap(); + + let cx = &mut VisualTestContext::from_window(multi_workspace_handle.into(), cx); + register_test_sidebar(false, cx); + multi_workspace_handle + .update(cx, |mw, _window, cx| { + mw.open_sidebar(cx); + }) + .unwrap(); + cx.run_until_parked(); + + let thread_store = cx.update(|_window, cx| cx.new(|cx| ThreadStore::new(cx))); + let connection_store = + cx.update(|_window, cx| cx.new(|cx| AgentConnectionStore::new(project.clone(), cx))); + + let conversation_view = cx.update(|window, cx| { + cx.new(|cx| { + ConversationView::new( + Rc::new(StubAgentServer::default_response()), + connection_store, + Agent::Custom { id: "Test".into() }, + None, + None, + None, + None, + None, + workspace.downgrade(), + project.clone(), + Some(thread_store), + None, + AgentThreadSource::AgentPanel, + window, + cx, + ) + }) + }); + cx.run_until_parked(); + + 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(); + + assert!( + cx.windows() + .iter() + .any(|window| window.downcast::().is_some()), + "Expected notification when the sidebar is open but the thread list is hidden" + ); + } + #[gpui::test] async fn test_notification_dismissed_when_sidebar_opens(cx: &mut TestAppContext) { init_test(cx); @@ -4250,6 +4332,7 @@ pub(crate) mod tests { .unwrap(); let cx = &mut VisualTestContext::from_window(multi_workspace_handle.into(), cx); + register_test_sidebar(true, cx); let thread_store = cx.update(|_window, cx| cx.new(|cx| ThreadStore::new(cx))); let connection_store = diff --git a/crates/agent_ui/src/test_support.rs b/crates/agent_ui/src/test_support.rs index 509dfdbb559..3f8d5c8c6df 100644 --- a/crates/agent_ui/src/test_support.rs +++ b/crates/agent_ui/src/test_support.rs @@ -1,13 +1,17 @@ use acp_thread::{AgentConnection, StubAgentConnection}; use agent_client_protocol::schema as acp; use agent_servers::{AgentServer, AgentServerDelegate}; -use gpui::{Entity, Task, TestAppContext, VisualTestContext}; +use gpui::{ + App, AppContext as _, Context, Entity, EventEmitter, FocusHandle, Focusable, IntoElement, + Pixels, Render, Task, TestAppContext, VisualTestContext, Window, div, px, +}; use project::AgentId; use project::Project; use settings::SettingsStore; use std::any::Any; use std::cell::RefCell; use std::rc::Rc; +use workspace::{MultiWorkspace, Sidebar as WorkspaceSidebar, SidebarEvent, SidebarSide}; use crate::AgentPanel; use crate::agent_panel; @@ -109,6 +113,71 @@ pub fn init_test(cx: &mut TestAppContext) { }); } +pub struct TestWorkspaceSidebar { + focus_handle: FocusHandle, + threads_list_active: bool, +} + +impl TestWorkspaceSidebar { + fn new(threads_list_active: bool, cx: &mut Context) -> Self { + Self { + focus_handle: cx.focus_handle(), + threads_list_active, + } + } +} + +impl EventEmitter for TestWorkspaceSidebar {} + +impl Focusable for TestWorkspaceSidebar { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl WorkspaceSidebar for TestWorkspaceSidebar { + fn width(&self, _cx: &App) -> Pixels { + px(300.) + } + + fn set_width(&mut self, _width: Option, _cx: &mut Context) {} + + fn has_notifications(&self, _cx: &App) -> bool { + false + } + + fn side(&self, _cx: &App) -> SidebarSide { + SidebarSide::Left + } + + fn is_threads_list_view_active(&self) -> bool { + self.threads_list_active + } +} + +impl Render for TestWorkspaceSidebar { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + div() + } +} + +pub fn register_test_sidebar( + threads_list_active: bool, + cx: &mut VisualTestContext, +) -> Entity { + cx.update(|window, cx| { + let multi_workspace = window + .root::() + .flatten() + .expect("test window should have a MultiWorkspace root"); + let sidebar = cx.new(|cx| TestWorkspaceSidebar::new(threads_list_active, cx)); + multi_workspace.update(cx, |multi_workspace, cx| { + multi_workspace.register_sidebar(sidebar.clone(), cx); + }); + sidebar + }) +} + pub fn open_thread_with_connection( panel: &Entity, connection: StubAgentConnection,