From d77aba3ee721e4b93c9deb937739eed3b602df45 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Thu, 26 Mar 2026 17:11:14 -0700 Subject: [PATCH] Group threads by canonical path lists (#52524) ## Context With the new sidebar, we are having some bugs around multi-root projects combined with git work trees that can cause threads to be visible in the agent panel but not have an entry in the sidebar. ## How to Review This PR takes a step towards resolving these issue by adding a `ProjectGroupBuilder` which is responsible for gathering the set of projects groups from the open workspaces and then helping to discover threads and map them into this set. ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - N/A --------- Co-authored-by: Mikayla Maki Co-authored-by: Claude Sonnet 4.6 Co-authored-by: Mikayla Maki Co-authored-by: Max Brunsfeld --- crates/sidebar/src/project_group_builder.rs | 330 +++++++++++ crates/sidebar/src/sidebar.rs | 573 ++++++++++---------- 2 files changed, 621 insertions(+), 282 deletions(-) create mode 100644 crates/sidebar/src/project_group_builder.rs diff --git a/crates/sidebar/src/project_group_builder.rs b/crates/sidebar/src/project_group_builder.rs new file mode 100644 index 0000000000000000000000000000000000000000..d03190e028082e086b30956933780090c1be07e5 --- /dev/null +++ b/crates/sidebar/src/project_group_builder.rs @@ -0,0 +1,330 @@ +//! The sidebar groups threads by a canonical path list. +//! +//! Threads have a path list associated with them, but this is the absolute path +//! of whatever worktrees they were associated with. In the sidebar, we want to +//! group all threads by their main worktree, and then we add a worktree chip to +//! the sidebar entry when that thread is in another worktree. +//! +//! This module is provides the functions and structures necessary to do this +//! lookup and mapping. + +use std::{ + collections::{HashMap, HashSet}, + path::{Path, PathBuf}, + sync::Arc, +}; + +use gpui::{App, Entity}; +use ui::SharedString; +use workspace::{MultiWorkspace, PathList, Workspace}; + +/// Identifies a project group by a set of paths the workspaces in this group +/// have. +/// +/// Paths are mapped to their main worktree path first so we can group +/// workspaces by main repos. +#[derive(PartialEq, Eq, Hash, Clone)] +pub struct ProjectGroupName { + path_list: PathList, +} + +impl ProjectGroupName { + pub fn display_name(&self) -> SharedString { + let mut names = Vec::with_capacity(self.path_list.paths().len()); + for abs_path in self.path_list.paths() { + if let Some(name) = abs_path.file_name() { + names.push(name.to_string_lossy().to_string()); + } + } + if names.is_empty() { + // TODO: Can we do something better in this case? + "Empty Workspace".into() + } else { + names.join(", ").into() + } + } + + pub fn path_list(&self) -> &PathList { + &self.path_list + } +} + +#[derive(Default)] +pub struct ProjectGroup { + pub workspaces: Vec>, + /// Root paths of all open workspaces in this group. Used to skip + /// redundant thread-store queries for linked worktrees that already + /// have an open workspace. + covered_paths: HashSet>, +} + +impl ProjectGroup { + fn add_workspace(&mut self, workspace: &Entity, cx: &App) { + if !self.workspaces.contains(workspace) { + self.workspaces.push(workspace.clone()); + } + for path in workspace.read(cx).root_paths(cx) { + self.covered_paths.insert(path); + } + } + + pub fn first_workspace(&self) -> &Entity { + self.workspaces + .first() + .expect("groups always have at least one workspace") + } +} + +pub struct ProjectGroupBuilder { + /// Maps git repositories' work_directory_abs_path to their original_repo_abs_path + directory_mappings: HashMap, + project_group_names: Vec, + project_groups: Vec, +} + +impl ProjectGroupBuilder { + fn new() -> Self { + Self { + directory_mappings: HashMap::new(), + project_group_names: Vec::new(), + project_groups: Vec::new(), + } + } + + pub fn from_multiworkspace(mw: &MultiWorkspace, cx: &App) -> Self { + let mut builder = Self::new(); + + // First pass: collect all directory mappings from every workspace + // so we know how to canonicalize any path (including linked + // worktree paths discovered by the main repo's workspace). + for workspace in mw.workspaces() { + builder.add_workspace_mappings(workspace.read(cx), cx); + } + + // Second pass: group each workspace using canonical paths derived + // from the full set of mappings. + for workspace in mw.workspaces() { + let group_name = builder.canonical_workspace_paths(workspace, cx); + builder + .project_group_entry(&group_name) + .add_workspace(workspace, cx); + } + builder + } + + fn project_group_entry(&mut self, name: &ProjectGroupName) -> &mut ProjectGroup { + match self.project_group_names.iter().position(|n| n == name) { + Some(idx) => &mut self.project_groups[idx], + None => { + let idx = self.project_group_names.len(); + self.project_group_names.push(name.clone()); + self.project_groups.push(ProjectGroup::default()); + &mut self.project_groups[idx] + } + } + } + + fn add_mapping(&mut self, work_directory: &Path, original_repo: &Path) { + let old = self + .directory_mappings + .insert(PathBuf::from(work_directory), PathBuf::from(original_repo)); + if let Some(old) = old { + debug_assert_eq!( + &old, original_repo, + "all worktrees should map to the same main worktree" + ); + } + } + + pub fn add_workspace_mappings(&mut self, workspace: &Workspace, cx: &App) { + for repo in workspace.project().read(cx).repositories(cx).values() { + let snapshot = repo.read(cx).snapshot(); + + self.add_mapping( + &snapshot.work_directory_abs_path, + &snapshot.original_repo_abs_path, + ); + + for worktree in snapshot.linked_worktrees.iter() { + self.add_mapping(&worktree.path, &snapshot.original_repo_abs_path); + } + } + } + + /// Derives the canonical group name for a workspace by canonicalizing + /// each of its root paths using the builder's directory mappings. + fn canonical_workspace_paths( + &self, + workspace: &Entity, + cx: &App, + ) -> ProjectGroupName { + let paths: Vec<_> = workspace + .read(cx) + .root_paths(cx) + .iter() + .map(|p| self.canonicalize_path(p).to_path_buf()) + .collect(); + ProjectGroupName { + path_list: PathList::new(&paths), + } + } + + pub fn canonicalize_path<'a>(&'a self, path: &'a Path) -> &'a Path { + self.directory_mappings + .get(path) + .map(AsRef::as_ref) + .unwrap_or(path) + } + + /// Whether the given group should load threads for a linked worktree at + /// `worktree_path`. Returns `false` if the worktree already has an open + /// workspace in the group (its threads are loaded via the workspace loop) + /// or if the worktree's canonical path list doesn't match `group_path_list`. + pub fn group_owns_worktree( + &self, + group: &ProjectGroup, + group_path_list: &PathList, + worktree_path: &Path, + ) -> bool { + let worktree_arc: Arc = Arc::from(worktree_path); + if group.covered_paths.contains(&worktree_arc) { + return false; + } + let canonical = self.canonicalize_path_list(&PathList::new(&[worktree_path])); + canonical == *group_path_list + } + + fn canonicalize_path_list(&self, path_list: &PathList) -> PathList { + let paths: Vec<_> = path_list + .paths() + .iter() + .map(|p| self.canonicalize_path(p).to_path_buf()) + .collect(); + PathList::new(&paths) + } + + pub fn groups(&self) -> impl Iterator { + self.project_group_names + .iter() + .zip(self.project_groups.iter()) + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use super::*; + use fs::FakeFs; + use gpui::TestAppContext; + use settings::SettingsStore; + + fn init_test(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + theme::init(theme::LoadThemes::JustBase, cx); + }); + } + + async fn create_fs_with_main_and_worktree(cx: &mut TestAppContext) -> Arc { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/project", + serde_json::json!({ + ".git": { + "worktrees": { + "feature-a": { + "commondir": "../../", + "HEAD": "ref: refs/heads/feature-a", + }, + }, + }, + "src": {}, + }), + ) + .await; + fs.insert_tree( + "/wt/feature-a", + serde_json::json!({ + ".git": "gitdir: /project/.git/worktrees/feature-a", + "src": {}, + }), + ) + .await; + fs.with_git_state(std::path::Path::new("/project/.git"), false, |state| { + state.worktrees.push(git::repository::Worktree { + path: std::path::PathBuf::from("/wt/feature-a"), + ref_name: Some("refs/heads/feature-a".into()), + sha: "abc".into(), + }); + }) + .expect("git state should be set"); + fs + } + + #[gpui::test] + async fn test_main_repo_maps_to_itself(cx: &mut TestAppContext) { + init_test(cx); + let fs = create_fs_with_main_and_worktree(cx).await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + + let (multi_workspace, cx) = cx.add_window_view(|window, cx| { + workspace::MultiWorkspace::test_new(project.clone(), window, cx) + }); + + multi_workspace.read_with(cx, |mw, cx| { + let mut canonicalizer = ProjectGroupBuilder::new(); + for workspace in mw.workspaces() { + canonicalizer.add_workspace_mappings(workspace.read(cx), cx); + } + + // The main repo path should canonicalize to itself. + assert_eq!( + canonicalizer.canonicalize_path(Path::new("/project")), + Path::new("/project"), + ); + + // An unknown path returns None. + assert_eq!( + canonicalizer.canonicalize_path(Path::new("/something/else")), + Path::new("/something/else"), + ); + }); + } + + #[gpui::test] + async fn test_worktree_checkout_canonicalizes_to_main_repo(cx: &mut TestAppContext) { + init_test(cx); + let fs = create_fs_with_main_and_worktree(cx).await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + // Open the worktree checkout as its own project. + let project = project::Project::test(fs.clone(), ["/wt/feature-a".as_ref()], cx).await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + + let (multi_workspace, cx) = cx.add_window_view(|window, cx| { + workspace::MultiWorkspace::test_new(project.clone(), window, cx) + }); + + multi_workspace.read_with(cx, |mw, cx| { + let mut canonicalizer = ProjectGroupBuilder::new(); + for workspace in mw.workspaces() { + canonicalizer.add_workspace_mappings(workspace.read(cx), cx); + } + + // The worktree checkout path should canonicalize to the main repo. + assert_eq!( + canonicalizer.canonicalize_path(Path::new("/wt/feature-a")), + Path::new("/project"), + ); + }); + } +} diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 501b55a73260f0d453775fc245868669c35ab406..123ca7a6bec8af78f25a0c3bbac5767ced38b55f 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -19,16 +19,14 @@ use gpui::{ use menu::{ Cancel, Confirm, SelectChild, SelectFirst, SelectLast, SelectNext, SelectParent, SelectPrevious, }; -use project::{AgentId, Event as ProjectEvent, linked_worktree_short_name}; +use project::{Event as ProjectEvent, linked_worktree_short_name}; use recent_projects::sidebar_recent_projects::SidebarRecentProjects; use ui::utils::platform_title_bar_height; use settings::Settings as _; use std::collections::{HashMap, HashSet}; use std::mem; -use std::path::Path; use std::rc::Rc; -use std::sync::Arc; use theme::ActiveTheme; use ui::{ AgentThreadStatus, CommonAnimationExt, ContextMenu, Divider, HighlightedLabel, KeyBinding, @@ -47,6 +45,10 @@ use zed_actions::editor::{MoveDown, MoveUp}; use zed_actions::agents_sidebar::FocusSidebarFilter; +use crate::project_group_builder::ProjectGroupBuilder; + +mod project_group_builder; + gpui::actions!( agents_sidebar, [ @@ -118,6 +120,24 @@ struct ThreadEntry { diff_stats: DiffStats, } +impl ThreadEntry { + /// Updates this thread entry with active thread information. + /// + /// The existing [`ThreadEntry`] was likely deserialized from the database + /// but if we have a correspond thread already loaded we want to apply the + /// live information. + fn apply_active_info(&mut self, info: &ActiveThreadInfo) { + self.session_info.title = Some(info.title.clone()); + self.status = info.status; + self.icon = info.icon; + self.icon_from_external_svg = info.icon_from_external_svg.clone(); + self.is_live = true; + self.is_background = info.is_background; + self.is_title_generating = info.is_title_generating; + self.diff_stats = info.diff_stats; + } +} + #[derive(Clone)] enum ListEntry { ProjectHeader { @@ -209,21 +229,6 @@ fn workspace_path_list(workspace: &Entity, cx: &App) -> PathList { PathList::new(&workspace.read(cx).root_paths(cx)) } -fn workspace_label_from_path_list(path_list: &PathList) -> SharedString { - let mut names = Vec::with_capacity(path_list.paths().len()); - for abs_path in path_list.paths() { - if let Some(name) = abs_path.file_name() { - names.push(name.to_string_lossy().to_string()); - } - } - if names.is_empty() { - // TODO: Can we do something better in this case? - "Empty Workspace".into() - } else { - names.join(", ").into() - } -} - /// The sidebar re-derives its entire entry list from scratch on every /// change via `update_entries` → `rebuild_contents`. Avoid adding /// incremental or inter-event coordination state — if something can @@ -542,8 +547,21 @@ impl Sidebar { result } - /// When modifying this thread, aim for a single forward pass over workspaces - /// and threads plus an O(T log T) sort. Avoid adding extra scans over the data. + /// Rebuilds the sidebar contents from current workspace and thread state. + /// + /// Uses [`ProjectGroupBuilder`] to group workspaces by their main git + /// repository, then populates thread entries from the metadata store and + /// merges live thread info from active agent panels. + /// + /// Aim for a single forward pass over workspaces and threads plus an + /// O(T log T) sort. Avoid adding extra scans over the data. + /// + /// Properties: + /// + /// - Should always show every workspace in the multiworkspace + /// - If you have no threads, and two workspaces for the worktree and the main workspace, make sure at least one is shown + /// - Should always show every thread, associated with each workspace in the multiworkspace + /// - After every build_contents, our "active" state should exactly match the current workspace's, current agent panel's current thread. fn rebuild_contents(&mut self, cx: &App) { let Some(multi_workspace) = self.multi_workspace.upgrade() else { return; @@ -552,7 +570,6 @@ impl Sidebar { let workspaces = mw.workspaces().to_vec(); let active_workspace = mw.workspaces().get(mw.active_workspace_index()).cloned(); - // Build a lookup for agent icons from the first workspace's AgentServerStore. let agent_server_store = workspaces .first() .map(|ws| ws.read(cx).project().read(cx).agent_server_store().clone()); @@ -607,118 +624,62 @@ impl Sidebar { let mut current_session_ids: HashSet = HashSet::new(); let mut project_header_indices: Vec = Vec::new(); - // Identify absorbed workspaces in a single pass. A workspace is - // "absorbed" when it points at a git worktree checkout whose main - // repo is open as another workspace — its threads appear under the - // main repo's header instead of getting their own. - let mut main_repo_workspace: HashMap, usize> = HashMap::new(); - let mut absorbed: HashMap = HashMap::new(); - let mut pending: HashMap, Vec<(usize, SharedString, Arc)>> = HashMap::new(); - let mut absorbed_workspace_by_path: HashMap, usize> = HashMap::new(); - let workspace_indices_by_path: HashMap, Vec> = workspaces - .iter() - .enumerate() - .flat_map(|(index, workspace)| { - let paths = workspace_path_list(workspace, cx).paths().to_vec(); - paths - .into_iter() - .map(move |path| (Arc::from(path.as_path()), index)) - }) - .fold(HashMap::new(), |mut map, (path, index)| { - map.entry(path).or_default().push(index); - map - }); - - for (i, workspace) in workspaces.iter().enumerate() { - for snapshot in root_repository_snapshots(workspace, cx) { - if snapshot.is_main_worktree() { - main_repo_workspace - .entry(snapshot.work_directory_abs_path.clone()) - .or_insert(i); - - for git_worktree in snapshot.linked_worktrees() { - let worktree_path: Arc = Arc::from(git_worktree.path.as_path()); - if let Some(worktree_indices) = - workspace_indices_by_path.get(worktree_path.as_ref()) - { - for &worktree_idx in worktree_indices { - if worktree_idx == i { - continue; - } - - let worktree_name = linked_worktree_short_name( - &snapshot.original_repo_abs_path, - &git_worktree.path, - ) - .unwrap_or_default(); - absorbed.insert(worktree_idx, (i, worktree_name.clone())); - absorbed_workspace_by_path - .insert(worktree_path.clone(), worktree_idx); - } - } - } - - if let Some(waiting) = pending.remove(&snapshot.work_directory_abs_path) { - for (ws_idx, name, ws_path) in waiting { - absorbed.insert(ws_idx, (i, name)); - absorbed_workspace_by_path.insert(ws_path, ws_idx); - } - } - } else { - let name: SharedString = snapshot - .work_directory_abs_path - .file_name() - .unwrap_or_default() - .to_string_lossy() - .to_string() - .into(); - if let Some(&main_idx) = - main_repo_workspace.get(&snapshot.original_repo_abs_path) - { - absorbed.insert(i, (main_idx, name)); - absorbed_workspace_by_path - .insert(snapshot.work_directory_abs_path.clone(), i); - } else { - pending - .entry(snapshot.original_repo_abs_path.clone()) - .or_default() - .push((i, name, snapshot.work_directory_abs_path.clone())); - } - } - } - } + // Use ProjectGroupBuilder to canonically group workspaces by their + // main git repository. This replaces the manual absorbed-workspace + // detection that was here before. + let project_groups = ProjectGroupBuilder::from_multiworkspace(mw, cx); let has_open_projects = workspaces .iter() .any(|ws| !workspace_path_list(ws, cx).paths().is_empty()); - let active_ws_index = active_workspace - .as_ref() - .and_then(|active| workspaces.iter().position(|ws| ws == active)); - - for (ws_index, workspace) in workspaces.iter().enumerate() { - if absorbed.contains_key(&ws_index) { - continue; + let resolve_agent = |row: &ThreadMetadata| -> (Agent, IconName, Option) { + match &row.agent_id { + None => (Agent::NativeAgent, IconName::ZedAgent, None), + Some(id) => { + let custom_icon = agent_server_store + .as_ref() + .and_then(|store| store.read(cx).agent_icon(id)); + ( + Agent::Custom { id: id.clone() }, + IconName::Terminal, + custom_icon, + ) + } } + }; - let path_list = workspace_path_list(workspace, cx); + for (group_name, group) in project_groups.groups() { + let path_list = group_name.path_list().clone(); if path_list.paths().is_empty() { continue; } - let label = workspace_label_from_path_list(&path_list); + let label = group_name.display_name(); let is_collapsed = self.collapsed_groups.contains(&path_list); let should_load_threads = !is_collapsed || !query.is_empty(); - let is_active = active_ws_index.is_some_and(|active_idx| { - active_idx == ws_index - || absorbed - .get(&active_idx) - .is_some_and(|(main_idx, _)| *main_idx == ws_index) - }); - - let mut live_infos: Vec<_> = all_thread_infos_for_workspace(workspace, cx).collect(); + let is_active = active_workspace + .as_ref() + .is_some_and(|active| group.workspaces.contains(active)); + + // Pick a representative workspace for the group: prefer the active + // workspace if it belongs to this group, otherwise use the first. + // + // This is the workspace that will be activated by the project group + // header. + let representative_workspace = active_workspace + .as_ref() + .filter(|_| is_active) + .unwrap_or_else(|| group.first_workspace()); + + // Collect live thread infos from all workspaces in this group. + let live_infos: Vec<_> = group + .workspaces + .iter() + .flat_map(|ws| all_thread_infos_for_workspace(ws, cx)) + .collect(); let mut threads: Vec = Vec::new(); let mut has_running_threads = false; @@ -726,138 +687,124 @@ impl Sidebar { if should_load_threads { let mut seen_session_ids: HashSet = HashSet::new(); - - // Read threads from the store cache for this workspace's path list. let thread_store = SidebarThreadMetadataStore::global(cx); - let workspace_rows: Vec<_> = - thread_store.read(cx).entries_for_path(&path_list).collect(); - for row in workspace_rows { - seen_session_ids.insert(row.session_id.clone()); - let (agent, icon, icon_from_external_svg) = match &row.agent_id { - None => (Agent::NativeAgent, IconName::ZedAgent, None), - Some(id) => { - let custom_icon = agent_server_store - .as_ref() - .and_then(|store| store.read(cx).agent_icon(&id)); - ( - Agent::Custom { id: id.clone() }, - IconName::Terminal, - custom_icon, - ) - } - }; - threads.push(ThreadEntry { - agent, - session_info: acp_thread::AgentSessionInfo { - session_id: row.session_id.clone(), - work_dirs: None, - title: Some(row.title.clone()), - updated_at: Some(row.updated_at), - created_at: row.created_at, - meta: None, - }, - icon, - icon_from_external_svg, - status: AgentThreadStatus::default(), - workspace: ThreadEntryWorkspace::Open(workspace.clone()), - is_live: false, - is_background: false, - is_title_generating: false, - highlight_positions: Vec::new(), - worktree_name: None, - worktree_full_path: None, - worktree_highlight_positions: Vec::new(), - diff_stats: DiffStats::default(), - }); - } - // Load threads from linked git worktrees of this workspace's repos. - { - let mut linked_worktree_queries: Vec<(PathList, SharedString, Arc)> = - Vec::new(); - for snapshot in root_repository_snapshots(workspace, cx) { - if snapshot.is_linked_worktree() { - continue; - } + // Load threads from each workspace in the group. + for workspace in &group.workspaces { + let ws_path_list = workspace_path_list(workspace, cx); + + // Determine if this workspace covers a git worktree (its + // path canonicalizes to the main repo, not itself). If so, + // threads from it get a worktree chip in the sidebar. + let worktree_info: Option<(SharedString, SharedString)> = + ws_path_list.paths().first().and_then(|path| { + let canonical = project_groups.canonicalize_path(path); + if canonical != path.as_path() { + let name = + linked_worktree_short_name(canonical, path).unwrap_or_default(); + let full_path: SharedString = path.display().to_string().into(); + Some((name, full_path)) + } else { + None + } + }); - let main_worktree_path = snapshot.original_repo_abs_path.clone(); - - for git_worktree in snapshot.linked_worktrees() { - let worktree_name = - linked_worktree_short_name(&main_worktree_path, &git_worktree.path) - .unwrap_or_default(); - linked_worktree_queries.push(( - PathList::new(std::slice::from_ref(&git_worktree.path)), - worktree_name, - Arc::from(git_worktree.path.as_path()), - )); + let workspace_threads: Vec<_> = thread_store + .read(cx) + .entries_for_path(&ws_path_list) + .collect(); + for thread in workspace_threads { + if !seen_session_ids.insert(thread.session_id.clone()) { + continue; } + let (agent, icon, icon_from_external_svg) = resolve_agent(&thread); + threads.push(ThreadEntry { + agent, + session_info: acp_thread::AgentSessionInfo { + session_id: thread.session_id.clone(), + work_dirs: None, + title: Some(thread.title.clone()), + updated_at: Some(thread.updated_at), + created_at: thread.created_at, + meta: None, + }, + icon, + icon_from_external_svg, + status: AgentThreadStatus::default(), + workspace: ThreadEntryWorkspace::Open(workspace.clone()), + is_live: false, + is_background: false, + is_title_generating: false, + highlight_positions: Vec::new(), + worktree_name: worktree_info.as_ref().map(|(name, _)| name.clone()), + worktree_full_path: worktree_info + .as_ref() + .map(|(_, path)| path.clone()), + worktree_highlight_positions: Vec::new(), + diff_stats: DiffStats::default(), + }); } + } - for (worktree_path_list, worktree_name, worktree_path) in - &linked_worktree_queries - { - let target_workspace = match absorbed_workspace_by_path - .get(worktree_path.as_ref()) - { - Some(&idx) => { - live_infos - .extend(all_thread_infos_for_workspace(&workspaces[idx], cx)); - ThreadEntryWorkspace::Open(workspaces[idx].clone()) - } - None => ThreadEntryWorkspace::Closed(worktree_path_list.clone()), - }; + // Load threads from linked git worktrees that don't have an + // open workspace in this group. Only include worktrees that + // belong to this group (not shared with another group). + let linked_worktree_path_lists = group + .workspaces + .iter() + .flat_map(|ws| root_repository_snapshots(ws, cx)) + .filter(|snapshot| !snapshot.is_linked_worktree()) + .flat_map(|snapshot| { + snapshot + .linked_worktrees() + .iter() + .filter(|wt| { + project_groups.group_owns_worktree(group, &path_list, &wt.path) + }) + .map(|wt| PathList::new(std::slice::from_ref(&wt.path))) + .collect::>() + }); - let worktree_rows: Vec<_> = thread_store - .read(cx) - .entries_for_path(worktree_path_list) - .collect(); - for row in worktree_rows { - if !seen_session_ids.insert(row.session_id.clone()) { - continue; - } - let (agent, icon, icon_from_external_svg) = match &row.agent_id { - None => (Agent::NativeAgent, IconName::ZedAgent, None), - Some(name) => { - let custom_icon = - agent_server_store.as_ref().and_then(|store| { - store.read(cx).agent_icon(&AgentId(name.clone().into())) - }); - ( - Agent::Custom { - id: AgentId::new(name.clone()), - }, - IconName::Terminal, - custom_icon, - ) - } - }; - threads.push(ThreadEntry { - agent, - session_info: acp_thread::AgentSessionInfo { - session_id: row.session_id.clone(), - work_dirs: None, - title: Some(row.title.clone()), - updated_at: Some(row.updated_at), - created_at: row.created_at, - meta: None, - }, - icon, - icon_from_external_svg, - status: AgentThreadStatus::default(), - workspace: target_workspace.clone(), - is_live: false, - is_background: false, - is_title_generating: false, - highlight_positions: Vec::new(), - worktree_name: Some(worktree_name.clone()), - worktree_full_path: Some( - worktree_path.display().to_string().into(), - ), - worktree_highlight_positions: Vec::new(), - diff_stats: DiffStats::default(), - }); + for worktree_path_list in linked_worktree_path_lists { + for row in thread_store.read(cx).entries_for_path(&worktree_path_list) { + if !seen_session_ids.insert(row.session_id.clone()) { + continue; } + let worktree_info = row.folder_paths.paths().first().and_then(|path| { + let canonical = project_groups.canonicalize_path(path); + if canonical != path.as_path() { + let name = + linked_worktree_short_name(canonical, path).unwrap_or_default(); + let full_path: SharedString = path.display().to_string().into(); + Some((name, full_path)) + } else { + None + } + }); + let (agent, icon, icon_from_external_svg) = resolve_agent(&row); + threads.push(ThreadEntry { + agent, + session_info: acp_thread::AgentSessionInfo { + session_id: row.session_id.clone(), + work_dirs: None, + title: Some(row.title.clone()), + updated_at: Some(row.updated_at), + created_at: row.created_at, + meta: None, + }, + icon, + icon_from_external_svg, + status: AgentThreadStatus::default(), + workspace: ThreadEntryWorkspace::Closed(row.folder_paths.clone()), + is_live: false, + is_background: false, + is_title_generating: false, + highlight_positions: Vec::new(), + worktree_name: worktree_info.as_ref().map(|(name, _)| name.clone()), + worktree_full_path: worktree_info.map(|(_, path)| path), + worktree_highlight_positions: Vec::new(), + diff_stats: DiffStats::default(), + }); } } @@ -878,19 +825,12 @@ impl Sidebar { // Merge live info into threads and update notification state // in a single pass. for thread in &mut threads { - let session_id = &thread.session_info.session_id; - - if let Some(info) = live_info_by_session.get(session_id) { - thread.session_info.title = Some(info.title.clone()); - thread.status = info.status; - thread.icon = info.icon; - thread.icon_from_external_svg = info.icon_from_external_svg.clone(); - thread.is_live = true; - thread.is_background = info.is_background; - thread.is_title_generating = info.is_title_generating; - thread.diff_stats = info.diff_stats; + if let Some(info) = live_info_by_session.get(&thread.session_info.session_id) { + thread.apply_active_info(info); } + let session_id = &thread.session_info.session_id; + let is_thread_workspace_active = match &thread.workspace { ThreadEntryWorkspace::Open(thread_workspace) => active_workspace .as_ref() @@ -916,7 +856,7 @@ impl Sidebar { b_time.cmp(&a_time) }); } else { - for info in &live_infos { + for info in live_infos { if info.status == AgentThreadStatus::Running { has_running_threads = true; } @@ -964,7 +904,7 @@ impl Sidebar { entries.push(ListEntry::ProjectHeader { path_list: path_list.clone(), label, - workspace: workspace.clone(), + workspace: representative_workspace.clone(), highlight_positions: workspace_highlight_positions, has_running_threads, waiting_thread_count, @@ -988,7 +928,7 @@ impl Sidebar { entries.push(ListEntry::ProjectHeader { path_list: path_list.clone(), label, - workspace: workspace.clone(), + workspace: representative_workspace.clone(), highlight_positions: Vec::new(), has_running_threads, waiting_thread_count, @@ -1002,7 +942,7 @@ impl Sidebar { if show_new_thread_entry { entries.push(ListEntry::NewThread { path_list: path_list.clone(), - workspace: workspace.clone(), + workspace: representative_workspace.clone(), is_active_draft: is_draft_for_workspace, }); } @@ -1611,7 +1551,7 @@ impl Sidebar { true, &path_list, &label, - &workspace, + workspace, &highlight_positions, *has_running_threads, *waiting_thread_count, @@ -3018,9 +2958,7 @@ impl Sidebar { bar.child(toggle_button).child(action_buttons) } } -} -impl Sidebar { fn toggle_archive(&mut self, _: &ToggleArchive, window: &mut Window, cx: &mut Context) { match &self.view { SidebarView::ThreadList => self.show_archive(window, cx), @@ -3211,24 +3149,8 @@ fn all_thread_infos_for_workspace( workspace: &Entity, cx: &App, ) -> impl Iterator { - enum ThreadInfoIterator> { - Empty, - Threads(T), - } - - impl> Iterator for ThreadInfoIterator { - type Item = ActiveThreadInfo; - - fn next(&mut self) -> Option { - match self { - ThreadInfoIterator::Empty => None, - ThreadInfoIterator::Threads(threads) => threads.next(), - } - } - } - let Some(agent_panel) = workspace.read(cx).panel::(cx) else { - return ThreadInfoIterator::Empty; + return None.into_iter().flatten(); }; let agent_panel = agent_panel.read(cx); @@ -3274,7 +3196,7 @@ fn all_thread_infos_for_workspace( } }); - ThreadInfoIterator::Threads(threads) + Some(threads).into_iter().flatten() } #[cfg(test)] @@ -5833,10 +5755,9 @@ mod tests { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ - "v [wt-feature-a]", - " Thread A", - "v [wt-feature-b]", - " Thread B", + "v [project]", + " Thread A {wt-feature-a}", + " Thread B {wt-feature-b}", ] ); @@ -7139,4 +7060,92 @@ mod tests { entries_after ); } + + #[gpui::test] + async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut TestAppContext) { + // When a multi-root workspace (e.g. [/other, /project]) shares a + // repo with a single-root workspace (e.g. [/project]), linked + // worktree threads from the shared repo should only appear under + // the dedicated group [project], not under [other, project]. + init_test(cx); + let fs = FakeFs::new(cx.executor()); + + fs.insert_tree( + "/project", + serde_json::json!({ + ".git": { + "worktrees": { + "feature-a": { + "commondir": "../../", + "HEAD": "ref: refs/heads/feature-a", + }, + }, + }, + "src": {}, + }), + ) + .await; + fs.insert_tree( + "/wt-feature-a", + serde_json::json!({ + ".git": "gitdir: /project/.git/worktrees/feature-a", + "src": {}, + }), + ) + .await; + fs.insert_tree( + "/other", + serde_json::json!({ + ".git": {}, + "src": {}, + }), + ) + .await; + + fs.with_git_state(std::path::Path::new("/project/.git"), false, |state| { + state.worktrees.push(git::repository::Worktree { + path: std::path::PathBuf::from("/wt-feature-a"), + ref_name: Some("refs/heads/feature-a".into()), + sha: "aaa".into(), + }); + }) + .unwrap(); + + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let project_only = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; + project_only + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + + let multi_root = + project::Project::test(fs.clone(), ["/other".as_ref(), "/project".as_ref()], cx).await; + multi_root + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + + let (multi_workspace, cx) = cx.add_window_view(|window, cx| { + MultiWorkspace::test_new(project_only.clone(), window, cx) + }); + multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(multi_root.clone(), window, cx); + }); + let sidebar = setup_sidebar(&multi_workspace, cx); + + let wt_paths = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); + save_named_thread_metadata("wt-thread", "Worktree Thread", &wt_paths, cx).await; + + multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + "v [project]", + " Worktree Thread {wt-feature-a}", + "v [other, project]", + " [+ New Thread]", + ] + ); + } }