diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index df6399990c873ab1d81070d62eff349057e6bbb4..4ffa1518f0442fbd1c8192bd87aa6419ad2a8497 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -10,7 +10,7 @@ use editor::Editor; use feature_flags::{AgentV2FeatureFlag, FeatureFlagViewExt as _}; use gpui::{ Action as _, AnyElement, App, Context, Entity, FocusHandle, Focusable, ListState, Pixels, - Render, SharedString, WeakEntity, Window, actions, list, prelude::*, px, + Render, SharedString, WeakEntity, Window, WindowHandle, actions, list, prelude::*, px, }; use menu::{Cancel, Confirm, SelectFirst, SelectLast, SelectNext, SelectPrevious}; use project::{AgentId, Event as ProjectEvent}; @@ -1656,27 +1656,48 @@ impl Sidebar { } } - fn activate_thread( - &mut self, + fn find_workspace_across_windows( + &self, + cx: &App, + predicate: impl Fn(&Entity, &App) -> bool, + ) -> Option<(WindowHandle, Entity)> { + cx.windows() + .into_iter() + .filter_map(|window| window.downcast::()) + .find_map(|window| { + let workspace = window.read(cx).ok().and_then(|multi_workspace| { + multi_workspace + .workspaces() + .iter() + .find(|workspace| predicate(workspace, cx)) + .cloned() + })?; + Some((window, workspace)) + }) + } + + fn find_workspace_in_current_window( + &self, + cx: &App, + predicate: impl Fn(&Entity, &App) -> bool, + ) -> Option> { + self.multi_workspace.upgrade().and_then(|multi_workspace| { + multi_workspace + .read(cx) + .workspaces() + .iter() + .find(|workspace| predicate(workspace, cx)) + .cloned() + }) + } + + fn load_agent_thread_in_workspace( + workspace: &Entity, agent: Agent, session_info: acp_thread::AgentSessionInfo, - workspace: &Entity, window: &mut Window, - cx: &mut Context, + cx: &mut App, ) { - let Some(multi_workspace) = self.multi_workspace.upgrade() else { - return; - }; - - // Set focused_thread eagerly so the sidebar highlight updates - // immediately, rather than waiting for a deferred AgentPanel - // event which can race with ActiveWorkspaceChanged clearing it. - self.focused_thread = Some(session_info.session_id.clone()); - - multi_workspace.update(cx, |multi_workspace, cx| { - multi_workspace.activate(workspace.clone(), cx); - }); - workspace.update(cx, |workspace, cx| { workspace.open_panel::(window, cx); }); @@ -1694,10 +1715,95 @@ impl Sidebar { ); }); } + } + + fn activate_thread_locally( + &mut self, + agent: Agent, + session_info: acp_thread::AgentSessionInfo, + workspace: &Entity, + window: &mut Window, + cx: &mut Context, + ) { + let Some(multi_workspace) = self.multi_workspace.upgrade() else { + return; + }; + + // Set focused_thread eagerly so the sidebar highlight updates + // immediately, rather than waiting for a deferred AgentPanel + // event which can race with ActiveWorkspaceChanged clearing it. + self.focused_thread = Some(session_info.session_id.clone()); + + multi_workspace.update(cx, |multi_workspace, cx| { + multi_workspace.activate(workspace.clone(), cx); + }); + + Self::load_agent_thread_in_workspace(workspace, agent, session_info, window, cx); self.update_entries(false, cx); } + fn activate_thread_in_other_window( + &self, + agent: Agent, + session_info: acp_thread::AgentSessionInfo, + workspace: Entity, + target_window: WindowHandle, + cx: &mut Context, + ) { + let target_session_id = session_info.session_id.clone(); + + let activated = target_window + .update(cx, |multi_workspace, window, cx| { + window.activate_window(); + multi_workspace.activate(workspace.clone(), cx); + Self::load_agent_thread_in_workspace(&workspace, agent, session_info, window, cx); + }) + .log_err() + .is_some(); + + if activated { + if let Some(target_sidebar) = target_window + .read(cx) + .ok() + .and_then(|multi_workspace| { + multi_workspace.sidebar().map(|sidebar| sidebar.to_any()) + }) + .and_then(|sidebar| sidebar.downcast::().ok()) + { + target_sidebar.update(cx, |sidebar, cx| { + sidebar.focused_thread = Some(target_session_id); + sidebar.update_entries(false, cx); + }); + } + } + } + + fn activate_thread( + &mut self, + agent: Agent, + session_info: acp_thread::AgentSessionInfo, + workspace: &Entity, + window: &mut Window, + cx: &mut Context, + ) { + if self + .find_workspace_in_current_window(cx, |candidate, _| candidate == workspace) + .is_some() + { + self.activate_thread_locally(agent, session_info, &workspace, window, cx); + return; + } + + let Some((target_window, workspace)) = + self.find_workspace_across_windows(cx, |candidate, _| candidate == workspace) + else { + return; + }; + + self.activate_thread_in_other_window(agent, session_info, workspace, target_window, cx); + } + fn open_workspace_and_activate_thread( &mut self, agent: Agent, @@ -1725,18 +1831,24 @@ impl Sidebar { .detach_and_log_err(cx); } - fn find_open_workspace_for_path_list( + fn find_current_workspace_for_path_list( &self, path_list: &PathList, cx: &App, ) -> Option> { - let multi_workspace = self.multi_workspace.upgrade()?; - multi_workspace - .read(cx) - .workspaces() - .iter() - .find(|workspace| workspace_path_list(workspace, cx).paths() == path_list.paths()) - .cloned() + self.find_workspace_in_current_window(cx, |workspace, cx| { + workspace_path_list(workspace, cx).paths() == path_list.paths() + }) + } + + fn find_open_workspace_for_path_list( + &self, + path_list: &PathList, + cx: &App, + ) -> Option<(WindowHandle, Entity)> { + self.find_workspace_across_windows(cx, |workspace, cx| { + workspace_path_list(workspace, cx).paths() == path_list.paths() + }) } fn activate_archived_thread( @@ -1747,8 +1859,18 @@ impl Sidebar { cx: &mut Context, ) { if let Some(path_list) = &session_info.work_dirs { - if let Some(workspace) = self.find_open_workspace_for_path_list(&path_list, cx) { - self.activate_thread(agent, session_info, &workspace, window, cx); + if let Some(workspace) = self.find_current_workspace_for_path_list(path_list, cx) { + self.activate_thread_locally(agent, session_info, &workspace, window, cx); + } else if let Some((target_window, workspace)) = + self.find_open_workspace_for_path_list(path_list, cx) + { + self.activate_thread_in_other_window( + agent, + session_info, + workspace, + target_window, + cx, + ); } else { let path_list = path_list.clone(); self.open_workspace_and_activate_thread(agent, session_info, path_list, window, cx); @@ -1764,7 +1886,7 @@ impl Sidebar { }); if let Some(workspace) = active_workspace { - self.activate_thread(agent, session_info, &workspace, window, cx); + self.activate_thread_locally(agent, session_info, &workspace, window, cx); } } @@ -5500,4 +5622,226 @@ mod tests { "should have opened a second workspace for the archived thread's saved paths" ); } + + #[gpui::test] + async fn test_activate_archived_thread_reuses_workspace_in_another_window( + cx: &mut TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/project-a", serde_json::json!({ "src": {} })) + .await; + fs.insert_tree("/project-b", serde_json::json!({ "src": {} })) + .await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; + let project_b = project::Project::test(fs.clone(), ["/project-b".as_ref()], cx).await; + + let multi_workspace_a = + cx.add_window(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); + let multi_workspace_b = + cx.add_window(|window, cx| MultiWorkspace::test_new(project_b, window, cx)); + + let multi_workspace_a_entity = multi_workspace_a.root(cx).unwrap(); + + let cx_a = &mut gpui::VisualTestContext::from_window(multi_workspace_a.into(), cx); + let sidebar = setup_sidebar(&multi_workspace_a_entity, cx_a); + + let session_id = acp::SessionId::new(Arc::from("archived-cross-window")); + + sidebar.update_in(cx_a, |sidebar, window, cx| { + sidebar.activate_archived_thread( + Agent::NativeAgent, + acp_thread::AgentSessionInfo { + session_id: session_id.clone(), + work_dirs: Some(PathList::new(&[PathBuf::from("/project-b")])), + title: Some("Cross Window Thread".into()), + updated_at: None, + created_at: None, + meta: None, + }, + window, + cx, + ); + }); + cx_a.run_until_parked(); + + assert_eq!( + multi_workspace_a + .read_with(cx_a, |mw, _| mw.workspaces().len()) + .unwrap(), + 1, + "should not add the other window's workspace into the current window" + ); + assert_eq!( + multi_workspace_b + .read_with(cx_a, |mw, _| mw.workspaces().len()) + .unwrap(), + 1, + "should reuse the existing workspace in the other window" + ); + assert!( + cx_a.read(|cx| cx.active_window().unwrap()) == *multi_workspace_b, + "should activate the window that already owns the matching workspace" + ); + sidebar.read_with(cx_a, |sidebar, _| { + assert_eq!( + sidebar.focused_thread, None, + "source window's sidebar should not eagerly claim focus for a thread opened in another window" + ); + }); + } + + #[gpui::test] + async fn test_activate_archived_thread_reuses_workspace_in_another_window_with_target_sidebar( + cx: &mut TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/project-a", serde_json::json!({ "src": {} })) + .await; + fs.insert_tree("/project-b", serde_json::json!({ "src": {} })) + .await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; + let project_b = project::Project::test(fs.clone(), ["/project-b".as_ref()], cx).await; + + let multi_workspace_a = + cx.add_window(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); + let multi_workspace_b = + cx.add_window(|window, cx| MultiWorkspace::test_new(project_b.clone(), window, cx)); + + let multi_workspace_a_entity = multi_workspace_a.root(cx).unwrap(); + let multi_workspace_b_entity = multi_workspace_b.root(cx).unwrap(); + + let cx_a = &mut gpui::VisualTestContext::from_window(multi_workspace_a.into(), cx); + let sidebar_a = setup_sidebar(&multi_workspace_a_entity, cx_a); + + let cx_b = &mut gpui::VisualTestContext::from_window(multi_workspace_b.into(), cx); + let sidebar_b = setup_sidebar(&multi_workspace_b_entity, cx_b); + let workspace_b = multi_workspace_b_entity.read_with(cx_b, |mw, _| mw.workspace().clone()); + let _panel_b = add_agent_panel(&workspace_b, &project_b, cx_b); + + let session_id = acp::SessionId::new(Arc::from("archived-cross-window-with-sidebar")); + + sidebar_a.update_in(cx_a, |sidebar, window, cx| { + sidebar.activate_archived_thread( + Agent::NativeAgent, + acp_thread::AgentSessionInfo { + session_id: session_id.clone(), + work_dirs: Some(PathList::new(&[PathBuf::from("/project-b")])), + title: Some("Cross Window Thread".into()), + updated_at: None, + created_at: None, + meta: None, + }, + window, + cx, + ); + }); + cx_a.run_until_parked(); + + assert_eq!( + multi_workspace_a + .read_with(cx_a, |mw, _| mw.workspaces().len()) + .unwrap(), + 1, + "should not add the other window's workspace into the current window" + ); + assert_eq!( + multi_workspace_b + .read_with(cx_a, |mw, _| mw.workspaces().len()) + .unwrap(), + 1, + "should reuse the existing workspace in the other window" + ); + assert!( + cx_a.read(|cx| cx.active_window().unwrap()) == *multi_workspace_b, + "should activate the window that already owns the matching workspace" + ); + sidebar_a.read_with(cx_a, |sidebar, _| { + assert_eq!( + sidebar.focused_thread, None, + "source window's sidebar should not eagerly claim focus for a thread opened in another window" + ); + }); + sidebar_b.read_with(cx_b, |sidebar, _| { + assert_eq!( + sidebar.focused_thread.as_ref(), + Some(&session_id), + "target window's sidebar should eagerly focus the activated archived thread" + ); + }); + } + + #[gpui::test] + async fn test_activate_archived_thread_prefers_current_window_for_matching_paths( + cx: &mut TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/project-a", serde_json::json!({ "src": {} })) + .await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let project_b = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; + let project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; + + let multi_workspace_b = + cx.add_window(|window, cx| MultiWorkspace::test_new(project_b, window, cx)); + let multi_workspace_a = + cx.add_window(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); + + let multi_workspace_a_entity = multi_workspace_a.root(cx).unwrap(); + + let cx_a = &mut gpui::VisualTestContext::from_window(multi_workspace_a.into(), cx); + let sidebar_a = setup_sidebar(&multi_workspace_a_entity, cx_a); + + let session_id = acp::SessionId::new(Arc::from("archived-current-window")); + + sidebar_a.update_in(cx_a, |sidebar, window, cx| { + sidebar.activate_archived_thread( + Agent::NativeAgent, + acp_thread::AgentSessionInfo { + session_id: session_id.clone(), + work_dirs: Some(PathList::new(&[PathBuf::from("/project-a")])), + title: Some("Current Window Thread".into()), + updated_at: None, + created_at: None, + meta: None, + }, + window, + cx, + ); + }); + cx_a.run_until_parked(); + + assert!( + cx_a.read(|cx| cx.active_window().unwrap()) == *multi_workspace_a, + "should keep activation in the current window when it already has a matching workspace" + ); + sidebar_a.read_with(cx_a, |sidebar, _| { + assert_eq!( + sidebar.focused_thread.as_ref(), + Some(&session_id), + "current window's sidebar should eagerly focus the activated archived thread" + ); + }); + assert_eq!( + multi_workspace_a + .read_with(cx_a, |mw, _| mw.workspaces().len()) + .unwrap(), + 1, + "current window should continue reusing its existing workspace" + ); + assert_eq!( + multi_workspace_b + .read_with(cx_a, |mw, _| mw.workspaces().len()) + .unwrap(), + 1, + "other windows should not be activated just because they also match the saved paths" + ); + } }