From 857f81b04bfa8e52fc07f9e8d3bc14726957bf6e Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Fri, 10 Apr 2026 00:02:16 -0700 Subject: [PATCH] Fix more folder mutation things (#53585) Continuation of https://github.com/zed-industries/zed/pull/53566, now with proper thread root mutation. 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 --- crates/agent_ui/src/agent_panel.rs | 2 +- crates/agent_ui/src/thread_import.rs | 7 +- crates/agent_ui/src/thread_metadata_store.rs | 452 ++++- .../agent_ui/src/thread_worktree_archive.rs | 4 +- crates/agent_ui/src/threads_archive_view.rs | 7 +- crates/sidebar/src/sidebar.rs | 189 +- crates/sidebar/src/sidebar_tests.rs | 1577 ++++++++++++++--- crates/workspace/src/multi_workspace.rs | 226 ++- crates/workspace/src/multi_workspace_tests.rs | 154 -- crates/workspace/src/workspace.rs | 2 +- 10 files changed, 2039 insertions(+), 581 deletions(-) diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 6c555af7fa630f5f5cc5995f36ec8ee7007508b9..2ff4cd18a78fd53c5d540e66670d6e6c9e51aa47 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -6485,7 +6485,7 @@ mod tests { let metadata = store .entry(session_id) .unwrap_or_else(|| panic!("{label} thread metadata should exist")); - metadata.folder_paths.clone() + metadata.folder_paths().clone() }); let mut sorted = metadata_paths.ordered_paths().cloned().collect::>(); sorted.sort(); diff --git a/crates/agent_ui/src/thread_import.rs b/crates/agent_ui/src/thread_import.rs index 41a23f894d8f406cbdbdcb03db3879437a45e40f..686ca5d6cd4fdfede7eb4a5ed70c90074972fdf4 100644 --- a/crates/agent_ui/src/thread_import.rs +++ b/crates/agent_ui/src/thread_import.rs @@ -18,12 +18,12 @@ use ui::{ prelude::*, }; use util::ResultExt; -use workspace::{ModalView, MultiWorkspace, PathList, Workspace}; +use workspace::{ModalView, MultiWorkspace, Workspace}; use crate::{ Agent, AgentPanel, agent_connection_store::AgentConnectionStore, - thread_metadata_store::{ThreadMetadata, ThreadMetadataStore}, + thread_metadata_store::{ThreadMetadata, ThreadMetadataStore, ThreadWorktreePaths}, }; pub struct AcpThreadImportOnboarding; @@ -527,8 +527,7 @@ fn collect_importable_threads( .unwrap_or_else(|| crate::DEFAULT_THREAD_TITLE.into()), updated_at: session.updated_at.unwrap_or_else(|| Utc::now()), created_at: session.created_at, - folder_paths, - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&folder_paths), remote_connection: remote_connection.clone(), archived: true, }); diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 101ea3c7369dae6dd88e8bc4499f048532d91a43..4ba68b400a60320e95bfd645ee662f6483dc6cf4 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -64,8 +64,7 @@ fn migrate_thread_metadata(cx: &mut App) -> Task> { title: entry.title, updated_at: entry.updated_at, created_at: entry.created_at, - folder_paths: entry.folder_paths, - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&entry.folder_paths), remote_connection: None, archived: true, }) @@ -82,11 +81,11 @@ fn migrate_thread_metadata(cx: &mut App) -> Task> { if is_first_migration { let mut per_project: HashMap> = HashMap::default(); for entry in &mut to_migrate { - if entry.folder_paths.is_empty() { + if entry.worktree_paths.is_empty() { continue; } per_project - .entry(entry.folder_paths.clone()) + .entry(entry.worktree_paths.folder_path_list().clone()) .or_default() .push(entry); } @@ -164,8 +163,8 @@ fn migrate_thread_remote_connections(cx: &mut App, migration_task: Task); impl Global for GlobalThreadMetadataStore {} +/// Paired worktree paths for a thread. Each folder path has a corresponding +/// main worktree path at the same position. The two lists are always the +/// same length and are modified together via `add_path` / `remove_main_path`. +/// +/// For non-linked worktrees, the main path and folder path are identical. +/// For linked worktrees, the main path is the original repo and the folder +/// path is the linked worktree location. +/// +/// Internally stores two `PathList`s with matching insertion order so that +/// `ordered_paths()` on both yields positionally-paired results. +#[derive(Default, Debug, Clone)] +pub struct ThreadWorktreePaths { + folder_paths: PathList, + main_worktree_paths: PathList, +} + +impl PartialEq for ThreadWorktreePaths { + fn eq(&self, other: &Self) -> bool { + self.folder_paths == other.folder_paths + && self.main_worktree_paths == other.main_worktree_paths + } +} + +impl ThreadWorktreePaths { + /// Build from a project's current state. Each visible worktree is paired + /// with its main repo path (resolved via git), falling back to the + /// worktree's own path if no git repo is found. + pub fn from_project(project: &project::Project, cx: &App) -> Self { + let (mains, folders): (Vec, Vec) = project + .visible_worktrees(cx) + .map(|worktree| { + let snapshot = worktree.read(cx).snapshot(); + let folder_path = snapshot.abs_path().to_path_buf(); + let main_path = snapshot + .root_repo_common_dir() + .and_then(|dir| Some(dir.parent()?.to_path_buf())) + .unwrap_or_else(|| folder_path.clone()); + (main_path, folder_path) + }) + .unzip(); + Self { + folder_paths: PathList::new(&folders), + main_worktree_paths: PathList::new(&mains), + } + } + + /// Build from two parallel `PathList`s that already share the same + /// insertion order. Used for deserialization from DB. + /// + /// Returns an error if the two lists have different lengths, which + /// indicates corrupted data from a prior migration bug. + pub fn from_path_lists( + main_worktree_paths: PathList, + folder_paths: PathList, + ) -> anyhow::Result { + anyhow::ensure!( + main_worktree_paths.paths().len() == folder_paths.paths().len(), + "main_worktree_paths has {} entries but folder_paths has {}", + main_worktree_paths.paths().len(), + folder_paths.paths().len(), + ); + Ok(Self { + folder_paths, + main_worktree_paths, + }) + } + + /// Build for non-linked worktrees where main == folder for every path. + pub fn from_folder_paths(folder_paths: &PathList) -> Self { + Self { + folder_paths: folder_paths.clone(), + main_worktree_paths: folder_paths.clone(), + } + } + + pub fn is_empty(&self) -> bool { + self.folder_paths.is_empty() + } + + /// The folder paths (for workspace matching / `threads_by_paths` index). + pub fn folder_path_list(&self) -> &PathList { + &self.folder_paths + } + + /// The main worktree paths (for group key / `threads_by_main_paths` index). + pub fn main_worktree_path_list(&self) -> &PathList { + &self.main_worktree_paths + } + + /// Iterate the (main_worktree_path, folder_path) pairs in insertion order. + pub fn ordered_pairs(&self) -> impl Iterator { + self.main_worktree_paths + .ordered_paths() + .zip(self.folder_paths.ordered_paths()) + } + + /// Add a new path pair. If the exact (main, folder) pair already exists, + /// this is a no-op. Rebuilds both internal `PathList`s to maintain + /// consistent ordering. + pub fn add_path(&mut self, main_path: &Path, folder_path: &Path) { + let already_exists = self + .ordered_pairs() + .any(|(m, f)| m.as_path() == main_path && f.as_path() == folder_path); + if already_exists { + return; + } + let (mut mains, mut folders): (Vec, Vec) = self + .ordered_pairs() + .map(|(m, f)| (m.clone(), f.clone())) + .unzip(); + mains.push(main_path.to_path_buf()); + folders.push(folder_path.to_path_buf()); + self.main_worktree_paths = PathList::new(&mains); + self.folder_paths = PathList::new(&folders); + } + + /// Remove all pairs whose main worktree path matches the given path. + /// This removes the corresponding entries from both lists. + pub fn remove_main_path(&mut self, main_path: &Path) { + let (mains, folders): (Vec, Vec) = self + .ordered_pairs() + .filter(|(m, _)| m.as_path() != main_path) + .map(|(m, f)| (m.clone(), f.clone())) + .unzip(); + self.main_worktree_paths = PathList::new(&mains); + self.folder_paths = PathList::new(&folders); + } +} + /// Lightweight metadata for any thread (native or ACP), enough to populate /// the sidebar list and route to the correct load path when clicked. #[derive(Debug, Clone, PartialEq)] @@ -204,17 +332,25 @@ pub struct ThreadMetadata { pub title: SharedString, pub updated_at: DateTime, pub created_at: Option>, - pub folder_paths: PathList, - pub main_worktree_paths: PathList, + pub worktree_paths: ThreadWorktreePaths, pub remote_connection: Option, pub archived: bool, } +impl ThreadMetadata { + pub fn folder_paths(&self) -> &PathList { + self.worktree_paths.folder_path_list() + } + pub fn main_worktree_paths(&self) -> &PathList { + self.worktree_paths.main_worktree_path_list() + } +} + impl From<&ThreadMetadata> for acp_thread::AgentSessionInfo { fn from(meta: &ThreadMetadata) -> Self { Self { session_id: meta.session_id.clone(), - work_dirs: Some(meta.folder_paths.clone()), + work_dirs: Some(meta.folder_paths().clone()), title: Some(meta.title.clone()), updated_at: Some(meta.updated_at), created_at: meta.created_at, @@ -398,12 +534,12 @@ impl ThreadMetadataStore { for row in rows { this.threads_by_paths - .entry(row.folder_paths.clone()) + .entry(row.folder_paths().clone()) .or_default() .insert(row.session_id.clone()); - if !row.main_worktree_paths.is_empty() { + if !row.main_worktree_paths().is_empty() { this.threads_by_main_paths - .entry(row.main_worktree_paths.clone()) + .entry(row.main_worktree_paths().clone()) .or_default() .insert(row.session_id.clone()); } @@ -438,17 +574,17 @@ impl ThreadMetadataStore { fn save_internal(&mut self, metadata: ThreadMetadata) { if let Some(thread) = self.threads.get(&metadata.session_id) { - if thread.folder_paths != metadata.folder_paths { - if let Some(session_ids) = self.threads_by_paths.get_mut(&thread.folder_paths) { + if thread.folder_paths() != metadata.folder_paths() { + if let Some(session_ids) = self.threads_by_paths.get_mut(thread.folder_paths()) { session_ids.remove(&metadata.session_id); } } - if thread.main_worktree_paths != metadata.main_worktree_paths - && !thread.main_worktree_paths.is_empty() + if thread.main_worktree_paths() != metadata.main_worktree_paths() + && !thread.main_worktree_paths().is_empty() { if let Some(session_ids) = self .threads_by_main_paths - .get_mut(&thread.main_worktree_paths) + .get_mut(thread.main_worktree_paths()) { session_ids.remove(&metadata.session_id); } @@ -459,13 +595,13 @@ impl ThreadMetadataStore { .insert(metadata.session_id.clone(), metadata.clone()); self.threads_by_paths - .entry(metadata.folder_paths.clone()) + .entry(metadata.folder_paths().clone()) .or_default() .insert(metadata.session_id.clone()); - if !metadata.main_worktree_paths.is_empty() { + if !metadata.main_worktree_paths().is_empty() { self.threads_by_main_paths - .entry(metadata.main_worktree_paths.clone()) + .entry(metadata.main_worktree_paths().clone()) .or_default() .insert(metadata.session_id.clone()); } @@ -483,7 +619,11 @@ impl ThreadMetadataStore { ) { if let Some(thread) = self.threads.get(session_id) { self.save_internal(ThreadMetadata { - folder_paths: work_dirs, + worktree_paths: ThreadWorktreePaths::from_path_lists( + thread.main_worktree_paths().clone(), + work_dirs.clone(), + ) + .unwrap_or_else(|_| ThreadWorktreePaths::from_folder_paths(&work_dirs)), ..thread.clone() }); cx.notify(); @@ -524,7 +664,7 @@ impl ThreadMetadataStore { cx: &mut Context, ) { if let Some(thread) = self.threads.get(session_id).cloned() { - let mut paths: Vec = thread.folder_paths.paths().to_vec(); + let mut paths: Vec = thread.folder_paths().paths().to_vec(); for (old_path, new_path) in path_replacements { if let Some(pos) = paths.iter().position(|p| p == old_path) { paths[pos] = new_path.clone(); @@ -532,7 +672,11 @@ impl ThreadMetadataStore { } let new_folder_paths = PathList::new(&paths); self.save_internal(ThreadMetadata { - folder_paths: new_folder_paths, + worktree_paths: ThreadWorktreePaths::from_path_lists( + thread.main_worktree_paths().clone(), + new_folder_paths.clone(), + ) + .unwrap_or_else(|_| ThreadWorktreePaths::from_folder_paths(&new_folder_paths)), ..thread }); cx.notify(); @@ -546,7 +690,7 @@ impl ThreadMetadataStore { cx: &mut Context, ) { if let Some(thread) = self.threads.get(session_id).cloned() { - let mut paths: Vec = thread.folder_paths.paths().to_vec(); + let mut paths: Vec = thread.folder_paths().paths().to_vec(); for (old_path, new_path) in path_replacements { for path in &mut paths { if path == old_path { @@ -556,13 +700,69 @@ impl ThreadMetadataStore { } let new_folder_paths = PathList::new(&paths); self.save_internal(ThreadMetadata { - folder_paths: new_folder_paths, + worktree_paths: ThreadWorktreePaths::from_path_lists( + thread.main_worktree_paths().clone(), + new_folder_paths.clone(), + ) + .unwrap_or_else(|_| ThreadWorktreePaths::from_folder_paths(&new_folder_paths)), ..thread }); cx.notify(); } } + /// Apply a mutation to the worktree paths of all threads whose current + /// `main_worktree_paths` matches `current_main_paths`, then re-index. + pub fn change_worktree_paths( + &mut self, + current_main_paths: &PathList, + mutate: impl Fn(&mut ThreadWorktreePaths), + cx: &mut Context, + ) { + let session_ids: Vec<_> = self + .threads_by_main_paths + .get(current_main_paths) + .into_iter() + .flatten() + .cloned() + .collect(); + + if session_ids.is_empty() { + return; + } + + for session_id in &session_ids { + if let Some(thread) = self.threads.get_mut(session_id) { + if let Some(ids) = self + .threads_by_main_paths + .get_mut(thread.main_worktree_paths()) + { + ids.remove(session_id); + } + if let Some(ids) = self.threads_by_paths.get_mut(thread.folder_paths()) { + ids.remove(session_id); + } + + mutate(&mut thread.worktree_paths); + + self.threads_by_main_paths + .entry(thread.main_worktree_paths().clone()) + .or_default() + .insert(session_id.clone()); + self.threads_by_paths + .entry(thread.folder_paths().clone()) + .or_default() + .insert(session_id.clone()); + + self.pending_thread_ops_tx + .try_send(DbOperation::Upsert(thread.clone())) + .log_err(); + } + } + + cx.notify(); + } + pub fn create_archived_worktree( &self, worktree_path: String, @@ -655,13 +855,13 @@ impl ThreadMetadataStore { pub fn delete(&mut self, session_id: acp::SessionId, cx: &mut Context) { if let Some(thread) = self.threads.get(&session_id) { - if let Some(session_ids) = self.threads_by_paths.get_mut(&thread.folder_paths) { + if let Some(session_ids) = self.threads_by_paths.get_mut(thread.folder_paths()) { session_ids.remove(&session_id); } - if !thread.main_worktree_paths.is_empty() { + if !thread.main_worktree_paths().is_empty() { if let Some(session_ids) = self .threads_by_main_paths - .get_mut(&thread.main_worktree_paths) + .get_mut(thread.main_worktree_paths()) { session_ids.remove(&session_id); } @@ -802,16 +1002,9 @@ impl ThreadMetadataStore { let agent_id = thread_ref.connection().agent_id(); let project = thread_ref.project().read(cx); - let folder_paths = { - let paths: Vec> = project - .visible_worktrees(cx) - .map(|worktree| worktree.read(cx).abs_path()) - .collect(); - PathList::new(&paths) - }; + let worktree_paths = ThreadWorktreePaths::from_project(project, cx); let project_group_key = project.project_group_key(cx); - let main_worktree_paths = project_group_key.path_list().clone(); let remote_connection = project_group_key.host(); // Threads without a folder path (e.g. started in an empty @@ -820,7 +1013,7 @@ impl ThreadMetadataStore { // them from the archive. let archived = existing_thread .map(|t| t.archived) - .unwrap_or(folder_paths.is_empty()); + .unwrap_or(worktree_paths.is_empty()); let metadata = ThreadMetadata { session_id, @@ -828,8 +1021,7 @@ impl ThreadMetadataStore { title, created_at: Some(created_at), updated_at, - folder_paths, - main_worktree_paths, + worktree_paths, remote_connection, archived, }; @@ -919,19 +1111,19 @@ impl ThreadMetadataDb { let title = row.title.to_string(); let updated_at = row.updated_at.to_rfc3339(); let created_at = row.created_at.map(|dt| dt.to_rfc3339()); - let serialized = row.folder_paths.serialize(); - let (folder_paths, folder_paths_order) = if row.folder_paths.is_empty() { + let serialized = row.folder_paths().serialize(); + let (folder_paths, folder_paths_order) = if row.folder_paths().is_empty() { (None, None) } else { (Some(serialized.paths), Some(serialized.order)) }; - let main_serialized = row.main_worktree_paths.serialize(); - let (main_worktree_paths, main_worktree_paths_order) = if row.main_worktree_paths.is_empty() - { - (None, None) - } else { - (Some(main_serialized.paths), Some(main_serialized.order)) - }; + let main_serialized = row.main_worktree_paths().serialize(); + let (main_worktree_paths, main_worktree_paths_order) = + if row.main_worktree_paths().is_empty() { + (None, None) + } else { + (Some(main_serialized.paths), Some(main_serialized.order)) + }; let remote_connection = row .remote_connection .as_ref() @@ -1136,6 +1328,10 @@ impl Column for ThreadMetadata { .transpose() .context("deserialize thread metadata remote connection")?; + let worktree_paths = + ThreadWorktreePaths::from_path_lists(main_worktree_paths, folder_paths) + .unwrap_or_else(|_| ThreadWorktreePaths::default()); + Ok(( ThreadMetadata { session_id: acp::SessionId::new(id), @@ -1143,8 +1339,7 @@ impl Column for ThreadMetadata { title: title.into(), updated_at, created_at, - folder_paths, - main_worktree_paths, + worktree_paths, remote_connection, archived, }, @@ -1227,8 +1422,7 @@ mod tests { title: title.to_string().into(), updated_at, created_at: Some(updated_at), - folder_paths, - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&folder_paths), remote_connection: None, } } @@ -1459,8 +1653,7 @@ mod tests { title: "Existing Metadata".into(), updated_at: now - chrono::Duration::seconds(10), created_at: Some(now - chrono::Duration::seconds(10)), - folder_paths: project_a_paths.clone(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&project_a_paths), remote_connection: None, archived: false, }; @@ -1569,8 +1762,7 @@ mod tests { title: "Existing Metadata".into(), updated_at: existing_updated_at, created_at: Some(existing_updated_at), - folder_paths: project_paths.clone(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&project_paths), remote_connection: None, archived: false, }; @@ -1747,7 +1939,7 @@ mod tests { // Project A: 5 most recent should be unarchived, 2 oldest should be archived let mut project_a_entries: Vec<_> = list .iter() - .filter(|m| m.folder_paths == project_a_paths) + .filter(|m| *m.folder_paths() == project_a_paths) .collect(); assert_eq!(project_a_entries.len(), 7); project_a_entries.sort_by(|a, b| b.updated_at.cmp(&a.updated_at)); @@ -1770,7 +1962,7 @@ mod tests { // Project B: all 3 should be unarchived (under the limit) let project_b_entries: Vec<_> = list .iter() - .filter(|m| m.folder_paths == project_b_paths) + .filter(|m| *m.folder_paths() == project_b_paths) .collect(); assert_eq!(project_b_entries.len(), 3); assert!(project_b_entries.iter().all(|m| !m.archived)); @@ -1934,7 +2126,7 @@ mod tests { let without_worktree = store .entry(&session_without_worktree) .expect("missing metadata for thread without project association"); - assert!(without_worktree.folder_paths.is_empty()); + assert!(without_worktree.folder_paths().is_empty()); assert!( without_worktree.archived, "expected thread without project association to be archived" @@ -1944,7 +2136,7 @@ mod tests { .entry(&session_with_worktree) .expect("missing metadata for thread with project association"); assert_eq!( - with_worktree.folder_paths, + *with_worktree.folder_paths(), PathList::new(&[Path::new("/project-a")]) ); assert!( @@ -2578,7 +2770,7 @@ mod tests { store.entry(&acp::SessionId::new("session-multi")).cloned() }); let entry = entry.unwrap(); - let paths = entry.folder_paths.paths(); + let paths = entry.folder_paths().paths(); assert_eq!(paths.len(), 3); assert!(paths.contains(&PathBuf::from("/restored/worktree-a"))); assert!(paths.contains(&PathBuf::from("/restored/worktree-b"))); @@ -2623,7 +2815,7 @@ mod tests { .cloned() }); let entry = entry.unwrap(); - let paths = entry.folder_paths.paths(); + let paths = entry.folder_paths().paths(); assert_eq!(paths.len(), 2); assert!(paths.contains(&PathBuf::from("/new/worktree-a"))); assert!(paths.contains(&PathBuf::from("/other/path"))); @@ -2669,7 +2861,7 @@ mod tests { store.entry(&acp::SessionId::new("session-multi")).cloned() }); let entry = entry.unwrap(); - let paths = entry.folder_paths.paths(); + let paths = entry.folder_paths().paths(); assert_eq!(paths.len(), 3); assert!(paths.contains(&PathBuf::from("/restored/worktree-a"))); assert!(paths.contains(&PathBuf::from("/restored/worktree-b"))); @@ -2714,7 +2906,7 @@ mod tests { .cloned() }); let entry = entry.unwrap(); - let paths = entry.folder_paths.paths(); + let paths = entry.folder_paths().paths(); assert_eq!(paths.len(), 2); assert!(paths.contains(&PathBuf::from("/new/worktree-a"))); assert!(paths.contains(&PathBuf::from("/other/path"))); @@ -2786,4 +2978,136 @@ mod tests { assert!(paths.contains(&Path::new("/projects/worktree-a"))); assert!(paths.contains(&Path::new("/projects/worktree-b"))); } + + // ── ThreadWorktreePaths tests ────────────────────────────────────── + + /// Helper to build a `ThreadWorktreePaths` from (main, folder) pairs. + fn make_worktree_paths(pairs: &[(&str, &str)]) -> ThreadWorktreePaths { + let (mains, folders): (Vec<&Path>, Vec<&Path>) = pairs + .iter() + .map(|(m, f)| (Path::new(*m), Path::new(*f))) + .unzip(); + ThreadWorktreePaths::from_path_lists(PathList::new(&mains), PathList::new(&folders)) + .unwrap() + } + + #[test] + fn test_thread_worktree_paths_full_add_then_remove_cycle() { + // Full scenario from the issue: + // 1. Start with linked worktree selectric → zed + // 2. Add cloud + // 3. Remove zed + + let mut paths = make_worktree_paths(&[("/projects/zed", "/worktrees/selectric/zed")]); + + // Step 2: add cloud + paths.add_path(Path::new("/projects/cloud"), Path::new("/projects/cloud")); + + assert_eq!(paths.ordered_pairs().count(), 2); + assert_eq!( + paths.folder_path_list(), + &PathList::new(&[ + Path::new("/worktrees/selectric/zed"), + Path::new("/projects/cloud"), + ]) + ); + assert_eq!( + paths.main_worktree_path_list(), + &PathList::new(&[Path::new("/projects/zed"), Path::new("/projects/cloud"),]) + ); + + // Step 3: remove zed + paths.remove_main_path(Path::new("/projects/zed")); + + assert_eq!(paths.ordered_pairs().count(), 1); + assert_eq!( + paths.folder_path_list(), + &PathList::new(&[Path::new("/projects/cloud")]) + ); + assert_eq!( + paths.main_worktree_path_list(), + &PathList::new(&[Path::new("/projects/cloud")]) + ); + } + + #[test] + fn test_thread_worktree_paths_add_is_idempotent() { + let mut paths = make_worktree_paths(&[("/projects/zed", "/projects/zed")]); + + paths.add_path(Path::new("/projects/zed"), Path::new("/projects/zed")); + + assert_eq!(paths.ordered_pairs().count(), 1); + } + + #[test] + fn test_thread_worktree_paths_remove_nonexistent_is_noop() { + let mut paths = make_worktree_paths(&[("/projects/zed", "/worktrees/selectric/zed")]); + + paths.remove_main_path(Path::new("/projects/nonexistent")); + + assert_eq!(paths.ordered_pairs().count(), 1); + } + + #[test] + fn test_thread_worktree_paths_from_path_lists_preserves_association() { + let folder = PathList::new(&[ + Path::new("/worktrees/selectric/zed"), + Path::new("/projects/cloud"), + ]); + let main = PathList::new(&[Path::new("/projects/zed"), Path::new("/projects/cloud")]); + + let paths = ThreadWorktreePaths::from_path_lists(main, folder).unwrap(); + + let pairs: Vec<_> = paths + .ordered_pairs() + .map(|(m, f)| (m.clone(), f.clone())) + .collect(); + assert_eq!(pairs.len(), 2); + assert!(pairs.contains(&( + PathBuf::from("/projects/zed"), + PathBuf::from("/worktrees/selectric/zed") + ))); + assert!(pairs.contains(&( + PathBuf::from("/projects/cloud"), + PathBuf::from("/projects/cloud") + ))); + } + + #[test] + fn test_thread_worktree_paths_main_deduplicates_linked_worktrees() { + // Two linked worktrees of the same main repo: the main_worktree_path_list + // deduplicates because PathList stores unique sorted paths, but + // ordered_pairs still has both entries. + let paths = make_worktree_paths(&[ + ("/projects/zed", "/worktrees/selectric/zed"), + ("/projects/zed", "/worktrees/feature/zed"), + ]); + + // main_worktree_path_list has the duplicate main path twice + // (PathList keeps all entries from its input) + assert_eq!(paths.ordered_pairs().count(), 2); + assert_eq!( + paths.folder_path_list(), + &PathList::new(&[ + Path::new("/worktrees/selectric/zed"), + Path::new("/worktrees/feature/zed"), + ]) + ); + assert_eq!( + paths.main_worktree_path_list(), + &PathList::new(&[Path::new("/projects/zed"), Path::new("/projects/zed"),]) + ); + } + + #[test] + fn test_thread_worktree_paths_mismatched_lengths_returns_error() { + let folder = PathList::new(&[ + Path::new("/worktrees/selectric/zed"), + Path::new("/projects/cloud"), + ]); + let main = PathList::new(&[Path::new("/projects/zed")]); + + let result = ThreadWorktreePaths::from_path_lists(main, folder); + assert!(result.is_err()); + } } diff --git a/crates/agent_ui/src/thread_worktree_archive.rs b/crates/agent_ui/src/thread_worktree_archive.rs index 86c9fb946a911868439c991503dd0ace60e12aa8..4398a2154d4abd550535b247ab1a9e518f84b39d 100644 --- a/crates/agent_ui/src/thread_worktree_archive.rs +++ b/crates/agent_ui/src/thread_worktree_archive.rs @@ -192,7 +192,7 @@ pub fn path_is_referenced_by_other_unarchived_threads( .filter(|thread| !thread.archived) .any(|thread| { thread - .folder_paths + .folder_paths() .paths() .iter() .any(|other_path| other_path.as_path() == path) @@ -428,7 +428,7 @@ pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Resul .entries() .filter(|thread| { thread - .folder_paths + .folder_paths() .paths() .iter() .any(|p| p.as_path() == root.root_path) diff --git a/crates/agent_ui/src/threads_archive_view.rs b/crates/agent_ui/src/threads_archive_view.rs index 44d3e71c170111f6d647c74f237d57705d55f183..6e73584ef87f11810e4c860cc6ff4c8d8ff015a9 100644 --- a/crates/agent_ui/src/threads_archive_view.rs +++ b/crates/agent_ui/src/threads_archive_view.rs @@ -340,7 +340,7 @@ impl ThreadsArchiveView { return; } - if thread.folder_paths.is_empty() { + if thread.folder_paths().is_empty() { self.show_project_picker_for_thread(thread, window, cx); return; } @@ -537,7 +537,7 @@ impl ThreadsArchiveView { }) .timestamp(timestamp) .highlight_positions(highlight_positions.clone()) - .project_paths(thread.folder_paths.paths_owned()) + .project_paths(thread.folder_paths().paths_owned()) .focused(is_focused) .hovered(is_hovered) .on_hover(cx.listener(move |this, is_hovered, _window, cx| { @@ -930,7 +930,8 @@ impl ProjectPickerDelegate { window: &mut Window, cx: &mut Context>, ) { - self.thread.folder_paths = paths.clone(); + self.thread.worktree_paths = + super::thread_metadata_store::ThreadWorktreePaths::from_folder_paths(&paths); ThreadMetadataStore::global(cx).update(cx, |store, cx| { store.update_working_directories(&self.thread.session_id, paths, cx); }); diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 3cd7e0059ac165bcd5e738591363cb600abcd60f..9c126929a4705de4d3ffc9e6472332e86a07c2e8 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -4,7 +4,7 @@ use acp_thread::ThreadStatus; use action_log::DiffStats; use agent_client_protocol::{self as acp}; use agent_settings::AgentSettings; -use agent_ui::thread_metadata_store::{ThreadMetadata, ThreadMetadataStore}; +use agent_ui::thread_metadata_store::{ThreadMetadata, ThreadMetadataStore, ThreadWorktreePaths}; use agent_ui::thread_worktree_archive; use agent_ui::threads_archive_view::{ ThreadsArchiveView, ThreadsArchiveViewEvent, format_history_entry_timestamp, @@ -283,10 +283,8 @@ impl ListEntry { } } ListEntry::ProjectHeader { key, .. } => multi_workspace - .workspaces() - .find(|ws| PathList::new(&ws.read(cx).root_paths(cx)) == *key.path_list()) + .workspaces_for_project_group(key, cx) .cloned() - .into_iter() .collect(), ListEntry::ViewMore { .. } => Vec::new(), } @@ -365,35 +363,63 @@ fn workspace_path_list(workspace: &Entity, cx: &App) -> PathList { /// /// For each path in the thread's `folder_paths`, produces a /// [`WorktreeInfo`] with a short display name, full path, and whether -/// the worktree is the main checkout or a linked git worktree. -fn worktree_info_from_thread_paths( - folder_paths: &PathList, - group_key: &project::ProjectGroupKey, -) -> impl Iterator { - let main_paths = group_key.path_list().paths(); - folder_paths.paths().iter().filter_map(|path| { - let is_main = main_paths.iter().any(|mp| mp.as_path() == path.as_path()); - if is_main { - let name = path.file_name()?.to_string_lossy().to_string(); - Some(WorktreeInfo { - name: SharedString::from(name), - full_path: SharedString::from(path.display().to_string()), +/// the worktree is the main checkout or a linked git worktree. When +/// multiple main paths exist and a linked worktree's short name alone +/// wouldn't identify which main project it belongs to, the main project +/// name is prefixed for disambiguation (e.g. `project:feature`). +/// +fn worktree_info_from_thread_paths(worktree_paths: &ThreadWorktreePaths) -> Vec { + let mut infos: Vec = Vec::new(); + let mut linked_short_names: Vec<(SharedString, SharedString)> = Vec::new(); + let mut unique_main_count = HashSet::new(); + + for (main_path, folder_path) in worktree_paths.ordered_pairs() { + unique_main_count.insert(main_path.clone()); + let is_linked = main_path != folder_path; + + if is_linked { + let short_name = linked_worktree_short_name(main_path, folder_path).unwrap_or_default(); + let project_name = main_path + .file_name() + .map(|n| SharedString::from(n.to_string_lossy().to_string())) + .unwrap_or_default(); + linked_short_names.push((short_name.clone(), project_name)); + infos.push(WorktreeInfo { + name: short_name, + full_path: SharedString::from(folder_path.display().to_string()), highlight_positions: Vec::new(), - kind: ui::WorktreeKind::Main, - }) + kind: ui::WorktreeKind::Linked, + }); } else { - let main_path = main_paths - .iter() - .find(|mp| mp.file_name() == path.file_name()) - .or(main_paths.first())?; - Some(WorktreeInfo { - name: linked_worktree_short_name(main_path, path).unwrap_or_default(), - full_path: SharedString::from(path.display().to_string()), + let Some(name) = folder_path.file_name() else { + continue; + }; + infos.push(WorktreeInfo { + name: SharedString::from(name.to_string_lossy().to_string()), + full_path: SharedString::from(folder_path.display().to_string()), highlight_positions: Vec::new(), - kind: ui::WorktreeKind::Linked, - }) + kind: ui::WorktreeKind::Main, + }); } - }) + } + + // When the group has multiple main worktree paths and the thread's + // folder paths don't all share the same short name, prefix each + // linked worktree chip with its main project name so the user knows + // which project it belongs to. + let all_same_name = infos.len() > 1 && infos.iter().all(|i| i.name == infos[0].name); + + if unique_main_count.len() > 1 && !all_same_name { + for (info, (_short_name, project_name)) in infos + .iter_mut() + .filter(|i| i.kind == ui::WorktreeKind::Linked) + .zip(linked_short_names.iter()) + { + info.name = SharedString::from(format!("{}:{}", project_name, info.name)); + } + } + + infos } /// Shows a [`RemoteConnectionModal`] on the given workspace and establishes @@ -480,6 +506,34 @@ impl Sidebar { MultiWorkspaceEvent::WorkspaceRemoved(_) => { this.update_entries(cx); } + MultiWorkspaceEvent::WorktreePathAdded { + old_main_paths, + added_path, + } => { + let added_path = added_path.clone(); + ThreadMetadataStore::global(cx).update(cx, |store, cx| { + store.change_worktree_paths( + old_main_paths, + |paths| paths.add_path(&added_path, &added_path), + cx, + ); + }); + this.update_entries(cx); + } + MultiWorkspaceEvent::WorktreePathRemoved { + old_main_paths, + removed_path, + } => { + let removed_path = removed_path.clone(); + ThreadMetadataStore::global(cx).update(cx, |store, cx| { + store.change_worktree_paths( + old_main_paths, + |paths| paths.remove_main_path(&removed_path), + cx, + ); + }); + this.update_entries(cx); + } }, ) .detach(); @@ -947,35 +1001,33 @@ impl Sidebar { // Open; otherwise use Closed. let resolve_workspace = |row: &ThreadMetadata| -> ThreadEntryWorkspace { workspace_by_path_list - .get(&row.folder_paths) + .get(row.folder_paths()) .map(|ws| ThreadEntryWorkspace::Open((*ws).clone())) .unwrap_or_else(|| ThreadEntryWorkspace::Closed { - folder_paths: row.folder_paths.clone(), + folder_paths: row.folder_paths().clone(), project_group_key: group_key.clone(), }) }; // Build a ThreadEntry from a metadata row. - let make_thread_entry = |row: ThreadMetadata, - workspace: ThreadEntryWorkspace| - -> ThreadEntry { - let (icon, icon_from_external_svg) = resolve_agent_icon(&row.agent_id); - let worktrees: Vec = - worktree_info_from_thread_paths(&row.folder_paths, &group_key).collect(); - ThreadEntry { - metadata: row, - icon, - icon_from_external_svg, - status: AgentThreadStatus::default(), - workspace, - is_live: false, - is_background: false, - is_title_generating: false, - highlight_positions: Vec::new(), - worktrees, - diff_stats: DiffStats::default(), - } - }; + let make_thread_entry = + |row: ThreadMetadata, workspace: ThreadEntryWorkspace| -> ThreadEntry { + let (icon, icon_from_external_svg) = resolve_agent_icon(&row.agent_id); + let worktrees = worktree_info_from_thread_paths(&row.worktree_paths); + ThreadEntry { + metadata: row, + icon, + icon_from_external_svg, + status: AgentThreadStatus::default(), + workspace, + is_live: false, + is_background: false, + is_title_generating: false, + highlight_positions: Vec::new(), + worktrees, + diff_stats: DiffStats::default(), + } + }; // Main code path: one query per group via main_worktree_paths. // The main_worktree_paths column is set on all new threads and @@ -1190,12 +1242,15 @@ impl Sidebar { // Emit a DraftThread entry when the active draft belongs to this group. if is_draft_for_group { if let Some(ActiveEntry::Draft(draft_ws)) = &self.active_entry { - let ws_path_list = workspace_path_list(draft_ws, cx); - let worktrees = worktree_info_from_thread_paths(&ws_path_list, &group_key); + let ws_worktree_paths = ThreadWorktreePaths::from_project( + draft_ws.read(cx).project().read(cx), + cx, + ); + let worktrees = worktree_info_from_thread_paths(&ws_worktree_paths); entries.push(ListEntry::DraftThread { key: group_key.clone(), workspace: None, - worktrees: worktrees.collect(), + worktrees, }); } } @@ -1218,13 +1273,16 @@ impl Sidebar { if Some(ws.entity_id()) == draft_ws_id { continue; } - let ws_path_list = workspace_path_list(ws, cx); + let ws_worktree_paths = + ThreadWorktreePaths::from_project(ws.read(cx).project().read(cx), cx); let has_linked_worktrees = - worktree_info_from_thread_paths(&ws_path_list, &group_key) + worktree_info_from_thread_paths(&ws_worktree_paths) + .iter() .any(|wt| wt.kind == ui::WorktreeKind::Linked); if !has_linked_worktrees { continue; } + let ws_path_list = workspace_path_list(ws, cx); let store = thread_store.read(cx); let has_threads = store.entries_for_path(&ws_path_list).next().is_some() || store @@ -1234,8 +1292,7 @@ impl Sidebar { if has_threads { continue; } - let worktrees: Vec = - worktree_info_from_thread_paths(&ws_path_list, &group_key).collect(); + let worktrees = worktree_info_from_thread_paths(&ws_worktree_paths); entries.push(ListEntry::DraftThread { key: group_key.clone(), @@ -2170,7 +2227,7 @@ impl Sidebar { panel.load_agent_thread( Agent::from(metadata.agent_id.clone()), metadata.session_id.clone(), - Some(metadata.folder_paths.clone()), + Some(metadata.folder_paths().clone()), Some(metadata.title.clone()), focus, window, @@ -2368,7 +2425,7 @@ impl Sidebar { _ => None, }; - if metadata.folder_paths.paths().is_empty() { + if metadata.folder_paths().paths().is_empty() { ThreadMetadataStore::global(cx) .update(cx, |store, cx| store.unarchive(&session_id, cx)); @@ -2380,7 +2437,7 @@ impl Sidebar { if let Some(workspace) = active_workspace { self.activate_thread_locally(&metadata, &workspace, false, window, cx); } else { - let path_list = metadata.folder_paths.clone(); + let path_list = metadata.folder_paths().clone(); if let Some((target_window, workspace)) = self.find_open_workspace_for_path_list(&path_list, cx) { @@ -2398,7 +2455,7 @@ impl Sidebar { let task = store .read(cx) .get_archived_worktrees_for_thread(session_id.0.to_string(), cx); - let path_list = metadata.folder_paths.clone(); + let path_list = metadata.folder_paths().clone(); let task_session_id = session_id.clone(); let restore_task = cx.spawn_in(window, async move |this, cx| { @@ -2494,7 +2551,7 @@ impl Sidebar { cx.update(|_window, cx| store.read(cx).entry(&session_id).cloned())?; if let Some(updated_metadata) = updated_metadata { - let new_paths = updated_metadata.folder_paths.clone(); + let new_paths = updated_metadata.folder_paths().clone(); cx.update(|_window, cx| { store.update(cx, |store, cx| { @@ -2669,7 +2726,7 @@ impl Sidebar { .read(cx) .entry(session_id) .cloned(); - let thread_folder_paths = metadata.as_ref().map(|m| m.folder_paths.clone()); + let thread_folder_paths = metadata.as_ref().map(|m| m.folder_paths().clone()); // Compute which linked worktree roots should be archived from disk if // this thread is archived. This must happen before we remove any @@ -2696,7 +2753,7 @@ impl Sidebar { } } metadata - .folder_paths + .folder_paths() .ordered_paths() .filter_map(|path| { thread_worktree_archive::build_root_plan(path, &workspaces, cx) @@ -2902,7 +2959,7 @@ impl Sidebar { if let Some(metadata) = neighbor { if let Some(workspace) = self.multi_workspace.upgrade().and_then(|mw| { mw.read(cx) - .workspace_for_paths(&metadata.folder_paths, None, cx) + .workspace_for_paths(metadata.folder_paths(), None, cx) }) { self.activate_workspace(&workspace, window, cx); Self::load_agent_thread_in_workspace(&workspace, metadata, true, window, cx); diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 6a3da0a1d07ae66b4012b87e4533ed163115f4c3..ea4ec36674878ca958a2f73af0adf749a40157f6 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -3,7 +3,7 @@ use acp_thread::{AcpThread, PermissionOptions, StubAgentConnection}; use agent::ThreadStore; use agent_ui::{ test_support::{active_session_id, open_thread_with_connection, send_message}, - thread_metadata_store::ThreadMetadata, + thread_metadata_store::{ThreadMetadata, ThreadWorktreePaths}, }; use chrono::DateTime; use fs::{FakeFs, Fs}; @@ -226,24 +226,14 @@ fn save_thread_metadata( cx: &mut TestAppContext, ) { cx.update(|cx| { - let (folder_paths, main_worktree_paths) = { - let project_ref = project.read(cx); - let paths: Vec> = project_ref - .visible_worktrees(cx) - .map(|worktree| worktree.read(cx).abs_path()) - .collect(); - let folder_paths = PathList::new(&paths); - let main_worktree_paths = project_ref.project_group_key(cx).path_list().clone(); - (folder_paths, main_worktree_paths) - }; + let worktree_paths = ThreadWorktreePaths::from_project(project.read(cx), cx); let metadata = ThreadMetadata { session_id, agent_id: agent::ZED_AGENT_ID.clone(), title, updated_at, created_at, - folder_paths, - main_worktree_paths, + worktree_paths, archived: false, remote_connection: None, }; @@ -252,6 +242,33 @@ fn save_thread_metadata( cx.run_until_parked(); } +fn save_thread_metadata_with_main_paths( + session_id: &str, + title: &str, + folder_paths: PathList, + main_worktree_paths: PathList, + cx: &mut TestAppContext, +) { + let session_id = acp::SessionId::new(Arc::from(session_id)); + let title = SharedString::from(title.to_string()); + let updated_at = chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(); + let metadata = ThreadMetadata { + session_id, + agent_id: agent::ZED_AGENT_ID.clone(), + title, + updated_at, + created_at: None, + worktree_paths: ThreadWorktreePaths::from_path_lists(main_worktree_paths, folder_paths) + .unwrap(), + archived: false, + remote_connection: None, + }; + cx.update(|cx| { + ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx)); + }); + cx.run_until_parked(); +} + fn focus_sidebar(sidebar: &Entity, cx: &mut gpui::VisualTestContext) { sidebar.update_in(cx, |_, window, cx| { cx.focus_self(window); @@ -323,6 +340,11 @@ fn visible_entries_as_strings( } else { "" }; + let is_active = sidebar + .active_entry + .as_ref() + .is_some_and(|active| active.matches_entry(entry)); + let active_indicator = if is_active { " (active)" } else { "" }; match entry { ListEntry::ProjectHeader { label, @@ -339,7 +361,7 @@ fn visible_entries_as_strings( } ListEntry::Thread(thread) => { let title = thread.metadata.title.as_ref(); - let active = if thread.is_live { " *" } else { "" }; + let live = if thread.is_live { " *" } else { "" }; let status_str = match thread.status { AgentThreadStatus::Running => " (running)", AgentThreadStatus::Error => " (error)", @@ -355,7 +377,7 @@ fn visible_entries_as_strings( "" }; let worktree = format_linked_worktree_chips(&thread.worktrees); - format!(" {title}{worktree}{active}{status_str}{notified}{selected}") + format!(" {title}{worktree}{live}{status_str}{notified}{active_indicator}{selected}") } ListEntry::ViewMore { is_fully_expanded, .. @@ -375,7 +397,7 @@ fn visible_entries_as_strings( if workspace.is_some() { format!(" [+ New Thread{}]{}", worktree, selected) } else { - format!(" [~ Draft{}]{}", worktree, selected) + format!(" [~ Draft{}]{}{}", worktree, active_indicator, selected) } } } @@ -544,7 +566,10 @@ async fn test_single_workspace_no_threads(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]"] + vec![ + // + "v [my-project]", + ] ); } @@ -580,6 +605,7 @@ async fn test_single_workspace_with_saved_threads(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Fix crash in project panel", " Add inline diff view", @@ -610,7 +636,11 @@ async fn test_workspace_lifecycle(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project-a]", " Thread A1"] + vec![ + // + "v [project-a]", + " Thread A1", + ] ); // Add a second workspace @@ -621,7 +651,11 @@ async fn test_workspace_lifecycle(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project-a]", " Thread A1",] + vec![ + // + "v [project-a]", + " Thread A1", + ] ); } @@ -640,6 +674,7 @@ async fn test_view_more_pagination(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Thread 12", " Thread 11", @@ -750,7 +785,11 @@ async fn test_collapse_and_expand_group(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Thread 1"] + vec![ + // + "v [my-project]", + " Thread 1", + ] ); // Collapse @@ -761,7 +800,10 @@ async fn test_collapse_and_expand_group(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["> [my-project]"] + vec![ + // + "> [my-project]", + ] ); // Expand @@ -772,7 +814,11 @@ async fn test_collapse_and_expand_group(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Thread 1"] + vec![ + // + "v [my-project]", + " Thread 1", + ] ); } @@ -808,8 +854,7 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { metadata: ThreadMetadata { session_id: acp::SessionId::new(Arc::from("t-1")), agent_id: AgentId::new("zed-agent"), - folder_paths: PathList::default(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::default(), title: "Completed thread".into(), updated_at: Utc::now(), created_at: Some(Utc::now()), @@ -832,8 +877,7 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { metadata: ThreadMetadata { session_id: acp::SessionId::new(Arc::from("t-2")), agent_id: AgentId::new("zed-agent"), - folder_paths: PathList::default(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::default(), title: "Running thread".into(), updated_at: Utc::now(), created_at: Some(Utc::now()), @@ -856,13 +900,12 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { metadata: ThreadMetadata { session_id: acp::SessionId::new(Arc::from("t-3")), agent_id: AgentId::new("zed-agent"), - remote_connection: None, - folder_paths: PathList::default(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::default(), title: "Error thread".into(), updated_at: Utc::now(), created_at: Some(Utc::now()), archived: false, + remote_connection: None, }, icon: IconName::ZedAgent, icon_from_external_svg: None, @@ -881,13 +924,12 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { metadata: ThreadMetadata { session_id: acp::SessionId::new(Arc::from("t-4")), agent_id: AgentId::new("zed-agent"), - remote_connection: None, - folder_paths: PathList::default(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::default(), title: "Waiting thread".into(), updated_at: Utc::now(), created_at: Some(Utc::now()), archived: false, + remote_connection: None, }, icon: IconName::ZedAgent, icon_from_external_svg: None, @@ -906,13 +948,12 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { metadata: ThreadMetadata { session_id: acp::SessionId::new(Arc::from("t-5")), agent_id: AgentId::new("zed-agent"), - remote_connection: None, - folder_paths: PathList::default(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::default(), title: "Notified thread".into(), updated_at: Utc::now(), created_at: Some(Utc::now()), archived: false, + remote_connection: None, }, icon: IconName::ZedAgent, icon_from_external_svg: None, @@ -949,6 +990,7 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [expanded-project]", " Completed thread", " Running thread * (running) <== selected", @@ -1112,10 +1154,14 @@ async fn test_keyboard_confirm_on_project_header_toggles_collapse(cx: &mut TestA assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Thread 1"] + vec![ + // + "v [my-project]", + " Thread 1", + ] ); - // Focus the sidebar and select the header (index 0) + // Focus the sidebar and select the header focus_sidebar(&sidebar, cx); sidebar.update_in(cx, |sidebar, _window, _cx| { sidebar.selection = Some(0); @@ -1127,7 +1173,10 @@ async fn test_keyboard_confirm_on_project_header_toggles_collapse(cx: &mut TestA assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["> [my-project] <== selected"] + vec![ + // + "> [my-project] <== selected", + ] ); // Confirm again expands the group @@ -1136,7 +1185,11 @@ async fn test_keyboard_confirm_on_project_header_toggles_collapse(cx: &mut TestA assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project] <== selected", " Thread 1",] + vec![ + // + "v [my-project] <== selected", + " Thread 1", + ] ); } @@ -1187,7 +1240,11 @@ async fn test_keyboard_expand_and_collapse_selected_entry(cx: &mut TestAppContex assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Thread 1"] + vec![ + // + "v [my-project]", + " Thread 1", + ] ); // Focus sidebar and manually select the header (index 0). Press left to collapse. @@ -1201,7 +1258,10 @@ async fn test_keyboard_expand_and_collapse_selected_entry(cx: &mut TestAppContex assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["> [my-project] <== selected"] + vec![ + // + "> [my-project] <== selected", + ] ); // Press right to expand @@ -1210,7 +1270,11 @@ async fn test_keyboard_expand_and_collapse_selected_entry(cx: &mut TestAppContex assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project] <== selected", " Thread 1",] + vec![ + // + "v [my-project] <== selected", + " Thread 1", + ] ); // Press right again on already-expanded header moves selection down @@ -1237,7 +1301,11 @@ async fn test_keyboard_collapse_from_child_selects_parent(cx: &mut TestAppContex assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Thread 1 <== selected",] + vec![ + // + "v [my-project]", + " Thread 1 <== selected", + ] ); // Pressing left on a child collapses the parent group and selects it @@ -1247,7 +1315,10 @@ async fn test_keyboard_collapse_from_child_selects_parent(cx: &mut TestAppContex assert_eq!(sidebar.read_with(cx, |s, _| s.selection), Some(0)); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["> [my-project] <== selected"] + vec![ + // + "> [my-project] <== selected", + ] ); } @@ -1261,7 +1332,10 @@ async fn test_keyboard_navigation_on_empty_list(cx: &mut TestAppContext) { // An empty project has only the header. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [empty-project]"] + vec![ + // + "v [empty-project]", + ] ); // Focus sidebar — focus_in does not set a selection @@ -1393,7 +1467,12 @@ async fn test_parallel_threads_shown_with_live_status(cx: &mut TestAppContext) { entries[1..].sort(); assert_eq!( entries, - vec!["v [my-project]", " Hello *", " Hello * (running)",] + vec![ + // + "v [my-project]", + " Hello * (active)", + " Hello * (running)", + ] ); } @@ -1486,7 +1565,11 @@ async fn test_background_thread_completion_triggers_notification(cx: &mut TestAp // Thread A is still running; no notification yet. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project-a]", " Hello * (running)",] + vec![ + // + "v [project-a]", + " Hello * (running) (active)", + ] ); // Complete thread A's turn (transition Running → Completed). @@ -1496,7 +1579,11 @@ async fn test_background_thread_completion_triggers_notification(cx: &mut TestAp // The completed background thread shows a notification indicator. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project-a]", " Hello * (!)",] + vec![ + // + "v [project-a]", + " Hello * (!) (active)", + ] ); } @@ -1536,6 +1623,7 @@ async fn test_search_narrows_visible_threads_to_matches(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Fix crash in project panel", " Add inline diff view", @@ -1548,7 +1636,11 @@ async fn test_search_narrows_visible_threads_to_matches(cx: &mut TestAppContext) type_in_search(&sidebar, "diff", cx); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Add inline diff view <== selected",] + vec![ + // + "v [my-project]", + " Add inline diff view <== selected", + ] ); // User changes query to something with no matches — list is empty. @@ -1583,6 +1675,7 @@ async fn test_search_matches_regardless_of_case(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Fix Crash In Project Panel <== selected", ] @@ -1593,6 +1686,7 @@ async fn test_search_matches_regardless_of_case(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Fix Crash In Project Panel <== selected", ] @@ -1623,7 +1717,12 @@ async fn test_escape_clears_search_and_restores_full_list(cx: &mut TestAppContex // Confirm the full list is showing. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Alpha thread", " Beta thread",] + vec![ + // + "v [my-project]", + " Alpha thread", + " Beta thread", + ] ); // User types a search query to filter down. @@ -1631,7 +1730,11 @@ async fn test_escape_clears_search_and_restores_full_list(cx: &mut TestAppContex type_in_search(&sidebar, "alpha", cx); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Alpha thread <== selected",] + vec![ + // + "v [my-project]", + " Alpha thread <== selected", + ] ); // User presses Escape — filter clears, full list is restored. @@ -1641,6 +1744,7 @@ async fn test_escape_clears_search_and_restores_full_list(cx: &mut TestAppContex assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Alpha thread <== selected", " Beta thread", @@ -1697,6 +1801,7 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project-a]", " Fix bug in sidebar", " Add tests for editor", @@ -1707,7 +1812,11 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC type_in_search(&sidebar, "sidebar", cx); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project-a]", " Fix bug in sidebar <== selected",] + vec![ + // + "v [project-a]", + " Fix bug in sidebar <== selected", + ] ); // "typo" only matches in the second workspace — the first header disappears. @@ -1723,6 +1832,7 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project-a]", " Fix bug in sidebar <== selected", " Add tests for editor", @@ -1782,6 +1892,7 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [alpha-project]", " Fix bug in sidebar <== selected", " Add tests for editor", @@ -1793,7 +1904,11 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { type_in_search(&sidebar, "sidebar", cx); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [alpha-project]", " Fix bug in sidebar <== selected",] + vec![ + // + "v [alpha-project]", + " Fix bug in sidebar <== selected", + ] ); // "alpha sidebar" matches the workspace name "alpha-project" (fuzzy: a-l-p-h-a-s-i-d-e-b-a-r @@ -1803,7 +1918,11 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { type_in_search(&sidebar, "fix", cx); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [alpha-project]", " Fix bug in sidebar <== selected",] + vec![ + // + "v [alpha-project]", + " Fix bug in sidebar <== selected", + ] ); // A query that matches a workspace name AND a thread in that same workspace. @@ -1812,6 +1931,7 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [alpha-project]", " Fix bug in sidebar <== selected", " Add tests for editor", @@ -1825,6 +1945,7 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [alpha-project]", " Fix bug in sidebar <== selected", " Add tests for editor", @@ -1874,7 +1995,11 @@ async fn test_search_finds_threads_hidden_behind_view_more(cx: &mut TestAppConte let filtered = visible_entries_as_strings(&sidebar, cx); assert_eq!( filtered, - vec!["v [my-project]", " Hidden gem thread <== selected",] + vec![ + // + "v [my-project]", + " Hidden gem thread <== selected", + ] ); assert!( !filtered.iter().any(|e| e.contains("View More")), @@ -1910,14 +2035,21 @@ async fn test_search_finds_threads_inside_collapsed_groups(cx: &mut TestAppConte assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["> [my-project] <== selected"] + vec![ + // + "> [my-project] <== selected", + ] ); // User types a search — the thread appears even though its group is collapsed. type_in_search(&sidebar, "important", cx); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["> [my-project]", " Important thread <== selected",] + vec![ + // + "> [my-project]", + " Important thread <== selected", + ] ); } @@ -1951,6 +2083,7 @@ async fn test_search_then_keyboard_navigate_and_confirm(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Fix crash in panel <== selected", " Fix lint warnings", @@ -1963,6 +2096,7 @@ async fn test_search_then_keyboard_navigate_and_confirm(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Fix crash in panel", " Fix lint warnings <== selected", @@ -1974,6 +2108,7 @@ async fn test_search_then_keyboard_navigate_and_confirm(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [my-project]", " Fix crash in panel <== selected", " Fix lint warnings", @@ -2014,7 +2149,11 @@ async fn test_confirm_on_historical_thread_activates_workspace(cx: &mut TestAppC assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Historical Thread",] + vec![ + // + "v [my-project]", + " Historical Thread", + ] ); // Switch to workspace 1 so we can verify the confirm switches back. @@ -2075,7 +2214,12 @@ async fn test_click_clears_selection_and_focus_in_restores_it(cx: &mut TestAppCo assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Thread A", " Thread B",] + vec![ + // + "v [my-project]", + " Thread A", + " Thread B", + ] ); // Keyboard confirm preserves selection. @@ -2127,7 +2271,11 @@ async fn test_thread_title_update_propagates_to_sidebar(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Hello *"] + vec![ + // + "v [my-project]", + " Hello * (active)", + ] ); // Simulate the agent generating a title. The notification chain is: @@ -2149,7 +2297,11 @@ async fn test_thread_title_update_propagates_to_sidebar(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Friendly Greeting with AI *"] + vec![ + // + "v [my-project]", + " Friendly Greeting with AI * (active)", + ] ); } @@ -2202,8 +2354,7 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { title: "Test".into(), updated_at: Utc::now(), created_at: None, - folder_paths: PathList::default(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::default(), archived: false, remote_connection: None, }, @@ -2259,8 +2410,7 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { title: "Thread B".into(), updated_at: Utc::now(), created_at: None, - folder_paths: PathList::default(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::default(), archived: false, remote_connection: None, }, @@ -2312,167 +2462,935 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { save_test_thread_metadata(&session_id_b2, &project_b, cx).await; cx.run_until_parked(); - // Panel B is not the active workspace's panel (workspace A is - // active), so opening a thread there should not change focused_thread. - // This prevents running threads in background workspaces from causing - // the selection highlight to jump around. - sidebar.read_with(cx, |sidebar, _cx| { - assert_active_thread( - sidebar, - &session_id_a, - "Opening a thread in a non-active panel should not change focused_thread", - ); - }); + // Panel B is not the active workspace's panel (workspace A is + // active), so opening a thread there should not change focused_thread. + // This prevents running threads in background workspaces from causing + // the selection highlight to jump around. + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_thread( + sidebar, + &session_id_a, + "Opening a thread in a non-active panel should not change focused_thread", + ); + }); + + workspace_b.update_in(cx, |workspace, window, cx| { + workspace.focus_handle(cx).focus(window, cx); + }); + cx.run_until_parked(); + + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_thread( + sidebar, + &session_id_a, + "Defocusing the sidebar should not change focused_thread", + ); + }); + + // Switching workspaces via the multi_workspace (simulates clicking + // a workspace header) should clear focused_thread. + multi_workspace.update_in(cx, |mw, window, cx| { + let workspace = mw.workspaces().find(|w| *w == &workspace_b).cloned(); + if let Some(workspace) = workspace { + mw.activate(workspace, window, cx); + } + }); + cx.run_until_parked(); + + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_thread( + sidebar, + &session_id_b2, + "Switching workspace should seed focused_thread from the new active panel", + ); + assert!( + has_thread_entry(sidebar, &session_id_b2), + "The seeded thread should be present in the entries" + ); + }); + + // ── 8. Focusing the agent panel thread keeps focused_thread ──── + // Workspace B still has session_id_b2 loaded in the agent panel. + // Clicking into the thread (simulated by focusing its view) should + // keep focused_thread since it was already seeded on workspace switch. + panel_b.update_in(cx, |panel, window, cx| { + if let Some(thread_view) = panel.active_conversation_view() { + thread_view.read(cx).focus_handle(cx).focus(window, cx); + } + }); + cx.run_until_parked(); + + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_thread( + sidebar, + &session_id_b2, + "Focusing the agent panel thread should set focused_thread", + ); + assert!( + has_thread_entry(sidebar, &session_id_b2), + "The focused thread should be present in the entries" + ); + }); +} + +#[gpui::test] +async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContext) { + let project = init_test_project_with_agent_panel("/project-a", cx).await; + let fs = cx.update(|cx| ::global(cx)); + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); + + // Start a thread and send a message so it has history. + let connection = StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( + acp::ContentChunk::new("Done".into()), + )]); + open_thread_with_connection(&panel, connection, cx); + send_message(&panel, cx); + let session_id = active_session_id(&panel, cx); + save_test_thread_metadata(&session_id, &project, cx).await; + cx.run_until_parked(); + + // Verify the thread appears in the sidebar. + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a]", + " Hello * (active)", + ] + ); + + // The "New Thread" button should NOT be in "active/draft" state + // because the panel has a thread with messages. + sidebar.read_with(cx, |sidebar, _cx| { + assert!( + matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { .. })), + "Panel has a thread with messages, so active_entry should be Thread, got {:?}", + sidebar.active_entry, + ); + }); + + // Now add a second folder to the workspace, changing the path_list. + fs.as_fake() + .insert_tree("/project-b", serde_json::json!({ "src": {} })) + .await; + project + .update(cx, |project, cx| { + project.find_or_create_worktree("/project-b", true, cx) + }) + .await + .expect("should add worktree"); + cx.run_until_parked(); + + // The workspace path_list is now [project-a, project-b]. The active + // thread's metadata was re-saved with the new paths by the agent panel's + // project subscription. The old [project-a] key is replaced by the new + // key since no other workspace claims it. + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a, project-b]", + " Hello * (active)", + ] + ); + + // The "New Thread" button must still be clickable (not stuck in + // "active/draft" state). Verify that `active_thread_is_draft` is + // false — the panel still has the old thread with messages. + sidebar.read_with(cx, |sidebar, _cx| { + assert!( + matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { .. })), + "After adding a folder the panel still has a thread with messages, \ + so active_entry should be Thread, got {:?}", + sidebar.active_entry, + ); + }); + + // Actually click "New Thread" by calling create_new_thread and + // verify a new draft is created. + let workspace = multi_workspace.read_with(cx, |mw, _cx| mw.workspace().clone()); + sidebar.update_in(cx, |sidebar, window, cx| { + sidebar.create_new_thread(&workspace, window, cx); + }); + cx.run_until_parked(); + + // After creating a new thread, the panel should now be in draft + // state (no messages on the new thread). + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_draft( + sidebar, + &workspace, + "After creating a new thread active_entry should be Draft", + ); + }); +} + +#[gpui::test] +async fn test_worktree_add_and_remove_migrates_threads(cx: &mut TestAppContext) { + // When a worktree is added to a project, the project group key changes + // and all historical threads should be migrated to the new key. Removing + // the worktree should migrate them back. + let (_fs, project) = init_multi_project_test(&["/project-a", "/project-b"], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + + // Save two threads against the initial project group [/project-a]. + save_n_test_threads(2, &project, cx).await; + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a]", + " Thread 2", + " Thread 1", + ] + ); + + // Verify the metadata store has threads under the old key. + let old_key_paths = PathList::new(&[PathBuf::from("/project-a")]); + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx).read(cx); + assert_eq!( + store.entries_for_main_worktree_path(&old_key_paths).count(), + 2, + "should have 2 threads under old key before add" + ); + }); + + // Add a second worktree to the same project. + project + .update(cx, |project, cx| { + project.find_or_create_worktree("/project-b", true, cx) + }) + .await + .expect("should add worktree"); + cx.run_until_parked(); + + // The project group key should now be [/project-a, /project-b]. + let new_key_paths = PathList::new(&[PathBuf::from("/project-a"), PathBuf::from("/project-b")]); + + // Verify multi-workspace state: exactly one project group key, the new one. + multi_workspace.read_with(cx, |mw, _cx| { + let keys: Vec<_> = mw.project_group_keys().cloned().collect(); + assert_eq!( + keys.len(), + 1, + "should have exactly 1 project group key after add" + ); + assert_eq!( + keys[0].path_list(), + &new_key_paths, + "the key should be the new combined path list" + ); + }); + + // Verify threads were migrated to the new key. + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx).read(cx); + assert_eq!( + store.entries_for_main_worktree_path(&old_key_paths).count(), + 0, + "should have 0 threads under old key after migration" + ); + assert_eq!( + store.entries_for_main_worktree_path(&new_key_paths).count(), + 2, + "should have 2 threads under new key after migration" + ); + }); + + // Sidebar should show threads under the new header. + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a, project-b]", + " Thread 2", + " Thread 1", + ] + ); + + // Now remove the second worktree. + let worktree_id = project.read_with(cx, |project, cx| { + project + .visible_worktrees(cx) + .find(|wt| wt.read(cx).abs_path().as_ref() == Path::new("/project-b")) + .map(|wt| wt.read(cx).id()) + .expect("should find project-b worktree") + }); + project.update(cx, |project, cx| { + project.remove_worktree(worktree_id, cx); + }); + cx.run_until_parked(); + + // The key should revert to [/project-a]. + multi_workspace.read_with(cx, |mw, _cx| { + let keys: Vec<_> = mw.project_group_keys().cloned().collect(); + assert_eq!( + keys.len(), + 1, + "should have exactly 1 project group key after remove" + ); + assert_eq!( + keys[0].path_list(), + &old_key_paths, + "the key should revert to the original path list" + ); + }); + + // Threads should be migrated back to the old key. + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx).read(cx); + assert_eq!( + store.entries_for_main_worktree_path(&new_key_paths).count(), + 0, + "should have 0 threads under new key after revert" + ); + assert_eq!( + store.entries_for_main_worktree_path(&old_key_paths).count(), + 2, + "should have 2 threads under old key after revert" + ); + }); + + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a]", + " Thread 2", + " Thread 1", + ] + ); +} + +#[gpui::test] +async fn test_worktree_add_and_remove_preserves_thread_path_associations(cx: &mut TestAppContext) { + // Verifies that adding/removing folders to a project correctly updates + // each thread's worktree_paths (both folder_paths and main_worktree_paths) + // while preserving per-path associations for linked worktrees. + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/project", + serde_json::json!({ + ".git": {}, + "src": {}, + }), + ) + .await; + fs.add_linked_worktree_for_repo( + Path::new("/project/.git"), + false, + git::repository::Worktree { + path: PathBuf::from("/wt-feature"), + ref_name: Some("refs/heads/feature".into()), + sha: "aaa".into(), + is_main: false, + }, + ) + .await; + fs.insert_tree("/other-project", serde_json::json!({ ".git": {} })) + .await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + // Start with a linked worktree workspace: visible root is /wt-feature, + // main repo is /project. + let project = + project::Project::test(fs.clone() as Arc, ["/wt-feature".as_ref()], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let _sidebar = setup_sidebar(&multi_workspace, cx); + + // Save a thread. It should have folder_paths=[/wt-feature], main=[/project]. + save_named_thread_metadata("thread-1", "Thread 1", &project, cx).await; + + let session_id = acp::SessionId::new(Arc::from("thread-1")); + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx).read(cx); + let thread = store.entry(&session_id).expect("thread should exist"); + assert_eq!( + thread.folder_paths().paths(), + &[PathBuf::from("/wt-feature")], + "initial folder_paths should be the linked worktree" + ); + assert_eq!( + thread.main_worktree_paths().paths(), + &[PathBuf::from("/project")], + "initial main_worktree_paths should be the main repo" + ); + }); + + // Add /other-project to the workspace. + project + .update(cx, |project, cx| { + project.find_or_create_worktree("/other-project", true, cx) + }) + .await + .expect("should add worktree"); + cx.run_until_parked(); + + // Thread should now have both paths, with correct associations. + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx).read(cx); + let thread = store.entry(&session_id).expect("thread should exist"); + let pairs: Vec<_> = thread + .worktree_paths + .ordered_pairs() + .map(|(m, f)| (m.clone(), f.clone())) + .collect(); + assert!( + pairs.contains(&(PathBuf::from("/project"), PathBuf::from("/wt-feature"))), + "linked worktree association should be preserved, got: {:?}", + pairs + ); + assert!( + pairs.contains(&( + PathBuf::from("/other-project"), + PathBuf::from("/other-project") + )), + "new folder should have main == folder, got: {:?}", + pairs + ); + }); + + // Remove /other-project. + let worktree_id = project.read_with(cx, |project, cx| { + project + .visible_worktrees(cx) + .find(|wt| wt.read(cx).abs_path().as_ref() == Path::new("/other-project")) + .map(|wt| wt.read(cx).id()) + .expect("should find other-project worktree") + }); + project.update(cx, |project, cx| { + project.remove_worktree(worktree_id, cx); + }); + cx.run_until_parked(); + + // Thread should be back to original state. + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx).read(cx); + let thread = store.entry(&session_id).expect("thread should exist"); + assert_eq!( + thread.folder_paths().paths(), + &[PathBuf::from("/wt-feature")], + "folder_paths should revert to just the linked worktree" + ); + assert_eq!( + thread.main_worktree_paths().paths(), + &[PathBuf::from("/project")], + "main_worktree_paths should revert to just the main repo" + ); + let pairs: Vec<_> = thread + .worktree_paths + .ordered_pairs() + .map(|(m, f)| (m.clone(), f.clone())) + .collect(); + assert_eq!( + pairs, + vec![(PathBuf::from("/project"), PathBuf::from("/wt-feature"))], + "linked worktree association should be preserved through add+remove cycle" + ); + }); +} + +#[gpui::test] +async fn test_worktree_add_key_collision_removes_duplicate_workspace(cx: &mut TestAppContext) { + // When a worktree is added to workspace A and the resulting key matches + // an existing workspace B's key (and B has the same root paths), B + // should be removed as a true duplicate. + let (fs, project_a) = init_multi_project_test(&["/project-a", "/project-b"], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + + // Save a thread against workspace A [/project-a]. + save_named_thread_metadata("thread-a", "Thread A", &project_a, cx).await; + + // Create workspace B with both worktrees [/project-a, /project-b]. + let project_b = project::Project::test( + fs.clone() as Arc, + ["/project-a".as_ref(), "/project-b".as_ref()], + cx, + ) + .await; + let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_b.clone(), window, cx) + }); + cx.run_until_parked(); + + // Switch back to workspace A so it's the active workspace when the collision happens. + let workspace_a = + multi_workspace.read_with(cx, |mw, _| mw.workspaces().next().unwrap().clone()); + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(workspace_a, window, cx); + }); + cx.run_until_parked(); + + // Save a thread against workspace B [/project-a, /project-b]. + save_named_thread_metadata("thread-b", "Thread B", &project_b, cx).await; + + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + + // Both project groups should be visible. + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a, project-b]", + " Thread B", + "v [project-a]", + " Thread A", + ] + ); + + let workspace_b_id = workspace_b.entity_id(); + + // Now add /project-b to workspace A's project, causing a key collision. + project_a + .update(cx, |project, cx| { + project.find_or_create_worktree("/project-b", true, cx) + }) + .await + .expect("should add worktree"); + cx.run_until_parked(); + + // Workspace B should have been removed (true duplicate — same root paths). + multi_workspace.read_with(cx, |mw, _cx| { + let workspace_ids: Vec<_> = mw.workspaces().map(|ws| ws.entity_id()).collect(); + assert!( + !workspace_ids.contains(&workspace_b_id), + "workspace B should have been removed after key collision" + ); + }); + + // There should be exactly one project group key now. + let combined_paths = PathList::new(&[PathBuf::from("/project-a"), PathBuf::from("/project-b")]); + multi_workspace.read_with(cx, |mw, _cx| { + let keys: Vec<_> = mw.project_group_keys().cloned().collect(); + assert_eq!( + keys.len(), + 1, + "should have exactly 1 project group key after collision" + ); + assert_eq!( + keys[0].path_list(), + &combined_paths, + "the remaining key should be the combined paths" + ); + }); + + // Both threads should be visible under the merged group. + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a, project-b]", + " Thread A", + " Thread B", + ] + ); +} + +#[gpui::test] +async fn test_worktree_collision_keeps_active_workspace(cx: &mut TestAppContext) { + // When workspace A adds a folder that makes it collide with workspace B, + // and B is the *active* workspace, A (the incoming one) should be + // dropped so the user stays on B. A linked worktree sibling of A + // should migrate into B's group. + init_test(cx); + let fs = FakeFs::new(cx.executor()); + + // Set up /project-a with a linked worktree. + fs.insert_tree( + "/project-a", + serde_json::json!({ + ".git": { + "worktrees": { + "feature": { + "commondir": "../../", + "HEAD": "ref: refs/heads/feature", + }, + }, + }, + "src": {}, + }), + ) + .await; + fs.insert_tree( + "/wt-feature", + serde_json::json!({ + ".git": "gitdir: /project-a/.git/worktrees/feature", + "src": {}, + }), + ) + .await; + fs.add_linked_worktree_for_repo( + Path::new("/project-a/.git"), + false, + git::repository::Worktree { + path: PathBuf::from("/wt-feature"), + ref_name: Some("refs/heads/feature".into()), + sha: "aaa".into(), + is_main: false, + }, + ) + .await; + fs.insert_tree("/project-b", serde_json::json!({ ".git": {}, "src": {} })) + .await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; + project_a.update(cx, |p, cx| p.git_scans_complete(cx)).await; + + // Linked worktree sibling of A. + let project_wt = project::Project::test(fs.clone(), ["/wt-feature".as_ref()], cx).await; + project_wt + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + + // Workspace B has both folders already. + let project_b = project::Project::test( + fs.clone() as Arc, + ["/project-a".as_ref(), "/project-b".as_ref()], + cx, + ) + .await; + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + + // Add agent panels to all workspaces. + let workspace_a_entity = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + add_agent_panel(&workspace_a_entity, cx); + + // Add the linked worktree workspace (sibling of A). + let workspace_wt = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_wt.clone(), window, cx) + }); + add_agent_panel(&workspace_wt, cx); + cx.run_until_parked(); + + // Add workspace B (will become active). + let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_b.clone(), window, cx) + }); + add_agent_panel(&workspace_b, cx); + cx.run_until_parked(); + + // Save threads in each group. + save_named_thread_metadata("thread-a", "Thread A", &project_a, cx).await; + save_thread_metadata_with_main_paths( + "thread-wt", + "Worktree Thread", + PathList::new(&[PathBuf::from("/wt-feature")]), + PathList::new(&[PathBuf::from("/project-a")]), + cx, + ); + save_named_thread_metadata("thread-b", "Thread B", &project_b, cx).await; + + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + + // B is active, A and wt-feature are in one group, B in another. + assert_eq!( + multi_workspace.read_with(cx, |mw, _| mw.workspace().entity_id()), + workspace_b.entity_id(), + "workspace B should be active" + ); + multi_workspace.read_with(cx, |mw, _cx| { + assert_eq!(mw.project_group_keys().count(), 2, "should have 2 groups"); + assert_eq!(mw.workspaces().count(), 3, "should have 3 workspaces"); + }); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a, project-b]", + " [~ Draft] (active)", + " Thread B", + "v [project-a]", + " Thread A", + " Worktree Thread {wt-feature}", + ] + ); + + let workspace_a = multi_workspace.read_with(cx, |mw, _| { + mw.workspaces() + .find(|ws| { + ws.entity_id() != workspace_b.entity_id() + && ws.entity_id() != workspace_wt.entity_id() + }) + .unwrap() + .clone() + }); + + // Add /project-b to workspace A's project, causing a collision with B. + project_a + .update(cx, |project, cx| { + project.find_or_create_worktree("/project-b", true, cx) + }) + .await + .expect("should add worktree"); + cx.run_until_parked(); + + // Workspace A (the incoming duplicate) should have been dropped. + multi_workspace.read_with(cx, |mw, _cx| { + let workspace_ids: Vec<_> = mw.workspaces().map(|ws| ws.entity_id()).collect(); + assert!( + !workspace_ids.contains(&workspace_a.entity_id()), + "workspace A should have been dropped" + ); + }); + + // The active workspace should still be B. + assert_eq!( + multi_workspace.read_with(cx, |mw, _| mw.workspace().entity_id()), + workspace_b.entity_id(), + "workspace B should still be active" + ); + + // The linked worktree sibling should have migrated into B's group + // (it got the folder add and now shares the same key). + multi_workspace.read_with(cx, |mw, _cx| { + let workspace_ids: Vec<_> = mw.workspaces().map(|ws| ws.entity_id()).collect(); + assert!( + workspace_ids.contains(&workspace_wt.entity_id()), + "linked worktree workspace should still exist" + ); + assert_eq!( + mw.project_group_keys().count(), + 1, + "should have 1 group after merge" + ); + assert_eq!( + mw.workspaces().count(), + 2, + "should have 2 workspaces (B + linked worktree)" + ); + }); + + // The linked worktree workspace should have gotten the new folder. + let wt_worktree_count = + project_wt.read_with(cx, |project, cx| project.visible_worktrees(cx).count()); + assert_eq!( + wt_worktree_count, 2, + "linked worktree project should have gotten /project-b" + ); + + // After: everything merged under one group. Thread A migrated, + // worktree thread shows its chip, B's thread and draft remain. + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a, project-b]", + " [~ Draft] (active)", + " Thread A", + " Worktree Thread {project-a:wt-feature}", + " Thread B", + ] + ); +} + +#[gpui::test] +async fn test_worktree_add_syncs_linked_worktree_sibling(cx: &mut TestAppContext) { + // When a worktree is added to the main workspace, a linked worktree + // sibling (different root paths, same project group key) should also + // get the new folder added to its project. + init_test(cx); + let fs = FakeFs::new(cx.executor()); + + fs.insert_tree( + "/project", + serde_json::json!({ + ".git": { + "worktrees": { + "feature": { + "commondir": "../../", + "HEAD": "ref: refs/heads/feature", + }, + }, + }, + "src": {}, + }), + ) + .await; + + fs.insert_tree( + "/wt-feature", + serde_json::json!({ + ".git": "gitdir: /project/.git/worktrees/feature", + "src": {}, + }), + ) + .await; + + fs.add_linked_worktree_for_repo( + Path::new("/project/.git"), + false, + git::repository::Worktree { + path: PathBuf::from("/wt-feature"), + ref_name: Some("refs/heads/feature".into()), + sha: "aaa".into(), + is_main: false, + }, + ) + .await; + + // Create a second independent project to add as a folder later. + fs.insert_tree( + "/other-project", + serde_json::json!({ ".git": {}, "src": {} }), + ) + .await; + + cx.update(|cx| ::set_global(fs.clone(), cx)); - workspace_b.update_in(cx, |workspace, window, cx| { - workspace.focus_handle(cx).focus(window, cx); - }); - cx.run_until_parked(); + let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; + let worktree_project = project::Project::test(fs.clone(), ["/wt-feature".as_ref()], cx).await; - sidebar.read_with(cx, |sidebar, _cx| { - assert_active_thread( - sidebar, - &session_id_a, - "Defocusing the sidebar should not change focused_thread", - ); - }); + main_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + worktree_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; - // Switching workspaces via the multi_workspace (simulates clicking - // a workspace header) should clear focused_thread. - multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces().find(|w| *w == &workspace_b).cloned(); - if let Some(workspace) = workspace { - mw.activate(workspace, window, cx); - } + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(main_project.clone(), window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + + // Add agent panel to the main workspace. + let main_workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + add_agent_panel(&main_workspace, cx); + + // Open the linked worktree as a separate workspace. + let wt_workspace = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(worktree_project.clone(), window, cx) }); + add_agent_panel(&wt_workspace, cx); cx.run_until_parked(); - sidebar.read_with(cx, |sidebar, _cx| { - assert_active_thread( - sidebar, - &session_id_b2, - "Switching workspace should seed focused_thread from the new active panel", - ); - assert!( - has_thread_entry(sidebar, &session_id_b2), - "The seeded thread should be present in the entries" + // Both workspaces should share the same project group key [/project]. + multi_workspace.read_with(cx, |mw, _cx| { + assert_eq!( + mw.project_group_keys().count(), + 1, + "should have 1 project group key before add" ); + assert_eq!(mw.workspaces().count(), 2, "should have 2 workspaces"); }); - // ── 8. Focusing the agent panel thread keeps focused_thread ──── - // Workspace B still has session_id_b2 loaded in the agent panel. - // Clicking into the thread (simulated by focusing its view) should - // keep focused_thread since it was already seeded on workspace switch. - panel_b.update_in(cx, |panel, window, cx| { - if let Some(thread_view) = panel.active_conversation_view() { - thread_view.read(cx).focus_handle(cx).focus(window, cx); - } - }); - cx.run_until_parked(); + // Save threads against each workspace. + save_named_thread_metadata("main-thread", "Main Thread", &main_project, cx).await; + save_named_thread_metadata("wt-thread", "Worktree Thread", &worktree_project, cx).await; - sidebar.read_with(cx, |sidebar, _cx| { - assert_active_thread( - sidebar, - &session_id_b2, - "Focusing the agent panel thread should set focused_thread", - ); - assert!( - has_thread_entry(sidebar, &session_id_b2), - "The focused thread should be present in the entries" + // Verify both threads are under the old key [/project]. + let old_key_paths = PathList::new(&[PathBuf::from("/project")]); + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx).read(cx); + assert_eq!( + store.entries_for_main_worktree_path(&old_key_paths).count(), + 2, + "should have 2 threads under old key before add" ); }); -} - -#[gpui::test] -async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContext) { - let project = init_test_project_with_agent_panel("/project-a", cx).await; - let fs = cx.update(|cx| ::global(cx)); - let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - // Start a thread and send a message so it has history. - let connection = StubAgentConnection::new(); - connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( - acp::ContentChunk::new("Done".into()), - )]); - open_thread_with_connection(&panel, connection, cx); - send_message(&panel, cx); - let session_id = active_session_id(&panel, cx); - save_test_thread_metadata(&session_id, &project, cx).await; + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); cx.run_until_parked(); - // Verify the thread appears in the sidebar. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project-a]", " Hello *",] + vec![ + // + "v [project]", + " [~ Draft {wt-feature}] (active)", + " Worktree Thread {wt-feature}", + " Main Thread", + ] ); - // The "New Thread" button should NOT be in "active/draft" state - // because the panel has a thread with messages. - sidebar.read_with(cx, |sidebar, _cx| { - assert!( - matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { .. })), - "Panel has a thread with messages, so active_entry should be Thread, got {:?}", - sidebar.active_entry, - ); - }); - - // Now add a second folder to the workspace, changing the path_list. - fs.as_fake() - .insert_tree("/project-b", serde_json::json!({ "src": {} })) - .await; - project + // Add /other-project as a folder to the main workspace. + main_project .update(cx, |project, cx| { - project.find_or_create_worktree("/project-b", true, cx) + project.find_or_create_worktree("/other-project", true, cx) }) .await .expect("should add worktree"); cx.run_until_parked(); - // The workspace path_list is now [project-a, project-b]. The active - // thread's metadata was re-saved with the new paths by the agent panel's - // project subscription, so it stays visible under the updated group. - // The old [project-a] group persists in the sidebar (empty) because - // project_group_keys is append-only. + // The linked worktree workspace should have gotten the new folder too. + let wt_worktree_count = + worktree_project.read_with(cx, |project, cx| project.visible_worktrees(cx).count()); assert_eq!( - visible_entries_as_strings(&sidebar, cx), - vec![ - "v [project-a, project-b]", // - " Hello *", - "v [project-a]", - ] + wt_worktree_count, 2, + "linked worktree project should have gotten the new folder" ); - // The "New Thread" button must still be clickable (not stuck in - // "active/draft" state). Verify that `active_thread_is_draft` is - // false — the panel still has the old thread with messages. - sidebar.read_with(cx, |sidebar, _cx| { - assert!( - matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { .. })), - "After adding a folder the panel still has a thread with messages, \ - so active_entry should be Thread, got {:?}", - sidebar.active_entry, + // Both workspaces should still exist under one key. + multi_workspace.read_with(cx, |mw, _cx| { + assert_eq!(mw.workspaces().count(), 2, "both workspaces should survive"); + assert_eq!( + mw.project_group_keys().count(), + 1, + "should still have 1 project group key" ); }); - // Actually click "New Thread" by calling create_new_thread and - // verify a new draft is created. - let workspace = multi_workspace.read_with(cx, |mw, _cx| mw.workspace().clone()); - sidebar.update_in(cx, |sidebar, window, cx| { - sidebar.create_new_thread(&workspace, window, cx); + // Threads should have been migrated to the new key. + let new_key_paths = + PathList::new(&[PathBuf::from("/other-project"), PathBuf::from("/project")]); + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx).read(cx); + assert_eq!( + store.entries_for_main_worktree_path(&old_key_paths).count(), + 0, + "should have 0 threads under old key after migration" + ); + assert_eq!( + store.entries_for_main_worktree_path(&new_key_paths).count(), + 2, + "should have 2 threads under new key after migration" + ); }); + + // Both threads should still be visible in the sidebar. + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); cx.run_until_parked(); - // After creating a new thread, the panel should now be in draft - // state (no messages on the new thread). - sidebar.read_with(cx, |sidebar, _cx| { - assert_active_draft( - sidebar, - &workspace, - "After creating a new thread active_entry should be Draft", - ); - }); + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [other-project, project]", + " [~ Draft {project:wt-feature}] (active)", + " Worktree Thread {project:wt-feature}", + " Main Thread", + ] + ); } #[gpui::test] @@ -2500,7 +3418,11 @@ async fn test_cmd_n_shows_new_thread_entry(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Hello *"] + vec![ + // + "v [my-project]", + " Hello * (active)", + ] ); // Simulate cmd-n @@ -2515,7 +3437,12 @@ async fn test_cmd_n_shows_new_thread_entry(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " [~ Draft]", " Hello *"], + vec![ + // + "v [my-project]", + " [~ Draft] (active)", + " Hello *", + ], "After Cmd-N the sidebar should show a highlighted Draft entry" ); @@ -2548,7 +3475,11 @@ async fn test_draft_with_server_session_shows_as_draft(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " Hello *"] + vec![ + // + "v [my-project]", + " Hello * (active)", + ] ); // Open a new draft thread via a server connection. This gives the @@ -2560,7 +3491,12 @@ async fn test_draft_with_server_session_shows_as_draft(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " [~ Draft]", " Hello *"], + vec![ + // + "v [my-project]", + " [~ Draft] (active)", + " Hello *", + ], ); let workspace = multi_workspace.read_with(cx, |mw, _cx| mw.workspace().clone()); @@ -2654,7 +3590,11 @@ async fn test_cmd_n_shows_new_thread_entry_in_absorbed_worktree(cx: &mut TestApp assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " Hello {wt-feature-a} *"] + vec![ + // + "v [project]", + " Hello {wt-feature-a} * (active)", + ] ); // Simulate Cmd-N in the worktree workspace. @@ -2669,9 +3609,10 @@ async fn test_cmd_n_shows_new_thread_entry_in_absorbed_worktree(cx: &mut TestApp assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project]", - " [~ Draft {wt-feature-a}]", - " Hello {wt-feature-a} *" + " [~ Draft {wt-feature-a}] (active)", + " Hello {wt-feature-a} *", ], "After Cmd-N in an absorbed worktree, the sidebar should show \ a highlighted Draft entry under the main repo header" @@ -2746,7 +3687,11 @@ async fn test_search_matches_worktree_name(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " Fix Bug {rosewood} <== selected"], + vec![ + // + "v [project]", + " Fix Bug {rosewood} <== selected", + ], ); } @@ -2767,16 +3712,28 @@ async fn test_git_worktree_added_live_updates_sidebar(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - // Save a thread against a worktree path that doesn't exist yet. - save_named_thread_metadata("wt-thread", "Worktree Thread", &worktree_project, cx).await; + // Save a thread against a worktree path with the correct main + // worktree association (as if the git state had been resolved). + save_thread_metadata_with_main_paths( + "wt-thread", + "Worktree Thread", + PathList::new(&[PathBuf::from("/wt/rosewood")]), + PathList::new(&[PathBuf::from("/project")]), + cx, + ); multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); - // Thread is not visible yet — no worktree knows about this path. + // Thread is visible because its main_worktree_paths match the group. + // The chip name is derived from the path even before git discovery. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]"] + vec![ + // + "v [project]", + " Worktree Thread {rosewood}", + ] ); // Now add the worktree to the git state and trigger a rescan. @@ -2797,7 +3754,11 @@ async fn test_git_worktree_added_live_updates_sidebar(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " Worktree Thread {rosewood}",] + vec![ + // + "v [project]", + " Worktree Thread {rosewood}", + ] ); } @@ -2867,6 +3828,7 @@ async fn test_two_worktree_workspaces_absorbed_when_main_added(cx: &mut TestAppC assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project]", " Thread A {wt-feature-a}", " Thread B {wt-feature-b}", @@ -2888,6 +3850,7 @@ async fn test_two_worktree_workspaces_absorbed_when_main_added(cx: &mut TestAppC assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project]", " Thread A {wt-feature-a}", " Thread B {wt-feature-b}", @@ -2963,6 +3926,7 @@ async fn test_threadless_workspace_shows_new_thread_with_worktree_chip(cx: &mut assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project]", " [+ New Thread {wt-feature-b}]", " Thread A {wt-feature-a}", @@ -3042,8 +4006,9 @@ async fn test_multi_worktree_thread_shows_multiple_chips(cx: &mut TestAppContext assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project_a, project_b]", - " Cross Worktree Thread {olivetti}, {selectric}", + " Cross Worktree Thread {project_a:olivetti}, {project_b:selectric}", ] ); } @@ -3115,6 +4080,7 @@ async fn test_same_named_worktree_chips_are_deduplicated(cx: &mut TestAppContext assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project_a, project_b]", " Same Branch Thread {olivetti}", ] @@ -3219,8 +4185,9 @@ async fn test_absorbed_worktree_running_thread_shows_live_status(cx: &mut TestAp assert_eq!( entries, vec![ + // "v [project]", - " [~ Draft]", + " [~ Draft] (active)", " Hello {wt-feature-a} * (running)", ] ); @@ -3306,8 +4273,9 @@ async fn test_absorbed_worktree_completion_triggers_notification(cx: &mut TestAp assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [project]", - " [~ Draft]", + " [~ Draft] (active)", " Hello {wt-feature-a} * (running)", ] ); @@ -3317,7 +4285,12 @@ async fn test_absorbed_worktree_completion_triggers_notification(cx: &mut TestAp assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " [~ Draft]", " Hello {wt-feature-a} * (!)",] + vec![ + // + "v [project]", + " [~ Draft] (active)", + " Hello {wt-feature-a} * (!)", + ] ); } @@ -3373,7 +4346,11 @@ async fn test_clicking_worktree_thread_opens_workspace_when_none_exists(cx: &mut // Thread should appear under the main repo with a worktree chip. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " WT Thread {wt-feature-a}"], + vec![ + // + "v [project]", + " WT Thread {wt-feature-a}", + ], ); // Only 1 workspace should exist. @@ -3462,7 +4439,11 @@ async fn test_clicking_worktree_thread_does_not_briefly_render_as_separate_proje assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " WT Thread {wt-feature-a}"], + vec![ + // + "v [project]", + " WT Thread {wt-feature-a}", + ], ); focus_sidebar(&sidebar, cx); @@ -3699,8 +4680,9 @@ async fn test_activate_archived_thread_with_saved_paths_activates_matching_works title: "Archived Thread".into(), updated_at: Utc::now(), created_at: None, - folder_paths: PathList::new(&[PathBuf::from("/project-b")]), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&PathList::new(&[ + PathBuf::from("/project-b"), + ])), archived: false, remote_connection: None, }, @@ -3713,7 +4695,7 @@ async fn test_activate_archived_thread_with_saved_paths_activates_matching_works assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()), workspace_b, - "should have activated the workspace matching the saved path_list" + "should have switched to the workspace matching the saved paths" ); } @@ -3765,8 +4747,9 @@ async fn test_activate_archived_thread_cwd_fallback_with_matching_workspace( title: "CWD Thread".into(), updated_at: Utc::now(), created_at: None, - folder_paths: PathList::new(&[std::path::PathBuf::from("/project-b")]), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&PathList::new(&[ + std::path::PathBuf::from("/project-b"), + ])), archived: false, remote_connection: None, }, @@ -3829,8 +4812,7 @@ async fn test_activate_archived_thread_no_paths_no_cwd_uses_active_workspace( title: "Contextless Thread".into(), updated_at: Utc::now(), created_at: None, - folder_paths: PathList::default(), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::default(), archived: false, remote_connection: None, }, @@ -3885,8 +4867,7 @@ async fn test_activate_archived_thread_saved_paths_opens_new_workspace(cx: &mut title: "New WS Thread".into(), updated_at: Utc::now(), created_at: None, - folder_paths: path_list_b, - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&path_list_b), archived: false, remote_connection: None, }, @@ -3940,8 +4921,9 @@ async fn test_activate_archived_thread_reuses_workspace_in_another_window(cx: &m title: "Cross Window Thread".into(), updated_at: Utc::now(), created_at: None, - folder_paths: PathList::new(&[PathBuf::from("/project-b")]), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&PathList::new(&[ + PathBuf::from("/project-b"), + ])), archived: false, remote_connection: None, }, @@ -4018,8 +5000,9 @@ async fn test_activate_archived_thread_reuses_workspace_in_another_window_with_t title: "Cross Window Thread".into(), updated_at: Utc::now(), created_at: None, - folder_paths: PathList::new(&[PathBuf::from("/project-b")]), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&PathList::new(&[ + PathBuf::from("/project-b"), + ])), archived: false, remote_connection: None, }, @@ -4099,8 +5082,9 @@ async fn test_activate_archived_thread_prefers_current_window_for_matching_paths title: "Current Window Thread".into(), updated_at: Utc::now(), created_at: None, - folder_paths: PathList::new(&[PathBuf::from("/project-a")]), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&PathList::new(&[ + PathBuf::from("/project-a"), + ])), archived: false, remote_connection: None, }, @@ -4512,6 +5496,7 @@ async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut Test assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ + // "v [other, project]", "v [project]", " Worktree Thread {wt-feature-a}", @@ -6020,8 +7005,9 @@ async fn test_legacy_thread_with_canonical_path_opens_main_repo_workspace(cx: &m title: "Legacy Main Thread".into(), updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), created_at: None, - folder_paths: PathList::new(&[PathBuf::from("/project")]), - main_worktree_paths: PathList::default(), + worktree_paths: ThreadWorktreePaths::from_folder_paths(&PathList::new(&[ + PathBuf::from("/project"), + ])), archived: false, remote_connection: None, }; @@ -6300,19 +7286,23 @@ mod property_test { SwitchToThread { index: usize }, SwitchToProjectGroup { index: usize }, AddLinkedWorktree { project_group_index: usize }, + AddWorktreeToProject { project_group_index: usize }, + RemoveWorktreeFromProject { project_group_index: usize }, } - // Distribution (out of 20 slots): - // SaveThread: 5 slots (~25%) - // SaveWorktreeThread: 2 slots (~10%) - // ToggleAgentPanel: 1 slot (~5%) - // CreateDraftThread: 1 slot (~5%) - // AddProject: 1 slot (~5%) - // ArchiveThread: 2 slots (~10%) - // SwitchToThread: 2 slots (~10%) - // SwitchToProjectGroup: 2 slots (~10%) - // AddLinkedWorktree: 4 slots (~20%) - const DISTRIBUTION_SLOTS: u32 = 20; + // Distribution (out of 24 slots): + // SaveThread: 5 slots (~21%) + // SaveWorktreeThread: 2 slots (~8%) + // ToggleAgentPanel: 1 slot (~4%) + // CreateDraftThread: 1 slot (~4%) + // AddProject: 1 slot (~4%) + // ArchiveThread: 2 slots (~8%) + // SwitchToThread: 2 slots (~8%) + // SwitchToProjectGroup: 2 slots (~8%) + // AddLinkedWorktree: 4 slots (~17%) + // AddWorktreeToProject: 2 slots (~8%) + // RemoveWorktreeFromProject: 2 slots (~8%) + const DISTRIBUTION_SLOTS: u32 = 24; impl TestState { fn generate_operation(&self, raw: u32, project_group_count: usize) -> Operation { @@ -6354,6 +7344,18 @@ mod property_test { 16..=19 => Operation::SaveThread { project_group_index: extra % project_group_count, }, + 20..=21 if project_group_count > 0 => Operation::AddWorktreeToProject { + project_group_index: extra % project_group_count, + }, + 20..=21 => Operation::SaveThread { + project_group_index: extra % project_group_count, + }, + 22..=23 if project_group_count > 0 => Operation::RemoveWorktreeFromProject { + project_group_index: extra % project_group_count, + }, + 22..=23 => Operation::SaveThread { + project_group_index: extra % project_group_count, + }, _ => unreachable!(), } } @@ -6376,8 +7378,8 @@ mod property_test { title, updated_at, created_at: None, - folder_paths: path_list, - main_worktree_paths, + worktree_paths: ThreadWorktreePaths::from_path_lists(main_worktree_paths, path_list) + .unwrap(), archived: false, remote_connection: None, }; @@ -6612,6 +7614,57 @@ mod property_test { main_workspace_path: main_path.clone(), }); } + Operation::AddWorktreeToProject { + project_group_index, + } => { + let workspace = multi_workspace.read_with(cx, |mw, cx| { + let key = mw.project_group_keys().nth(project_group_index).unwrap(); + mw.workspaces_for_project_group(key, cx).next().cloned() + }); + let Some(workspace) = workspace else { return }; + let project = workspace.read_with(cx, |ws, _| ws.project().clone()); + + let new_path = state.next_workspace_path(); + state + .fs + .insert_tree(&new_path, serde_json::json!({ ".git": {}, "src": {} })) + .await; + + let result = project + .update(cx, |project, cx| { + project.find_or_create_worktree(&new_path, true, cx) + }) + .await; + if result.is_err() { + return; + } + cx.run_until_parked(); + } + Operation::RemoveWorktreeFromProject { + project_group_index, + } => { + let workspace = multi_workspace.read_with(cx, |mw, cx| { + let key = mw.project_group_keys().nth(project_group_index).unwrap(); + mw.workspaces_for_project_group(key, cx).next().cloned() + }); + let Some(workspace) = workspace else { return }; + let project = workspace.read_with(cx, |ws, _| ws.project().clone()); + + let worktree_count = project.read_with(cx, |p, cx| p.visible_worktrees(cx).count()); + if worktree_count <= 1 { + return; + } + + let worktree_id = project.read_with(cx, |p, cx| { + p.visible_worktrees(cx).last().map(|wt| wt.read(cx).id()) + }); + if let Some(worktree_id) = worktree_id { + project.update(cx, |project, cx| { + project.remove_worktree(worktree_id, cx); + }); + cx.run_until_parked(); + } + } } } @@ -6636,9 +7689,35 @@ mod property_test { fn validate_sidebar_properties(sidebar: &Sidebar, cx: &App) -> anyhow::Result<()> { verify_every_group_in_multiworkspace_is_shown(sidebar, cx)?; + verify_no_duplicate_threads(sidebar)?; verify_all_threads_are_shown(sidebar, cx)?; verify_active_state_matches_current_workspace(sidebar, cx)?; verify_all_workspaces_are_reachable(sidebar, cx)?; + verify_workspace_group_key_integrity(sidebar, cx)?; + Ok(()) + } + + fn verify_no_duplicate_threads(sidebar: &Sidebar) -> anyhow::Result<()> { + let mut seen: HashSet = HashSet::default(); + let mut duplicates: Vec<(acp::SessionId, String)> = Vec::new(); + + for entry in &sidebar.contents.entries { + if let Some(session_id) = entry.session_id() { + if !seen.insert(session_id.clone()) { + let title = match entry { + ListEntry::Thread(thread) => thread.metadata.title.to_string(), + _ => "".to_string(), + }; + duplicates.push((session_id.clone(), title)); + } + } + } + + anyhow::ensure!( + duplicates.is_empty(), + "threads appear more than once in sidebar: {:?}", + duplicates, + ); Ok(()) } @@ -6890,6 +7969,15 @@ mod property_test { Ok(()) } + fn verify_workspace_group_key_integrity(sidebar: &Sidebar, cx: &App) -> anyhow::Result<()> { + let Some(multi_workspace) = sidebar.multi_workspace.upgrade() else { + anyhow::bail!("sidebar should still have an associated multi-workspace"); + }; + multi_workspace + .read(cx) + .assert_project_group_key_integrity(cx) + } + #[gpui::property_test(config = ProptestConfig { cases: 50, ..Default::default() @@ -7110,8 +8198,11 @@ async fn test_remote_project_integration_does_not_briefly_render_as_separate_pro title: "Worktree Thread".into(), updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 1).unwrap(), created_at: None, - folder_paths: PathList::new(&[PathBuf::from("/project-wt-1")]), - main_worktree_paths, + worktree_paths: ThreadWorktreePaths::from_path_lists( + main_worktree_paths, + PathList::new(&[PathBuf::from("/project-wt-1")]), + ) + .unwrap(), archived: false, remote_connection: None, }; diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index f4e8b47399e1420a4b01d380ad4a6532a0934a2d..9ef81194639e625b4944c48be41b7518fee0bbe3 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -101,6 +101,14 @@ pub enum MultiWorkspaceEvent { ActiveWorkspaceChanged, WorkspaceAdded(Entity), WorkspaceRemoved(EntityId), + WorktreePathAdded { + old_main_paths: PathList, + added_path: PathBuf, + }, + WorktreePathRemoved { + old_main_paths: PathList, + removed_path: PathBuf, + }, } pub enum SidebarEvent { @@ -302,7 +310,7 @@ pub struct MultiWorkspace { workspaces: Vec>, active_workspace: ActiveWorkspace, project_group_keys: Vec, - provisional_project_group_keys: HashMap, + workspace_group_keys: HashMap, sidebar: Option>, sidebar_open: bool, sidebar_overlay: Option, @@ -355,7 +363,7 @@ impl MultiWorkspace { Self { window_id: window.window_handle().window_id(), project_group_keys: Vec::new(), - provisional_project_group_keys: HashMap::default(), + workspace_group_keys: HashMap::default(), workspaces: Vec::new(), active_workspace: ActiveWorkspace::Transient(workspace), sidebar: None, @@ -559,19 +567,11 @@ impl MultiWorkspace { cx.subscribe_in(&project, window, { let workspace = workspace.downgrade(); move |this, _project, event, _window, cx| match event { - project::Event::WorktreeAdded(_) | project::Event::WorktreeRemoved(_) => { + project::Event::WorktreeAdded(_) + | project::Event::WorktreeRemoved(_) + | project::Event::WorktreeUpdatedRootRepoCommonDir(_) => { if let Some(workspace) = workspace.upgrade() { - this.add_project_group_key(workspace.read(cx).project_group_key(cx)); - } - } - project::Event::WorktreeUpdatedRootRepoCommonDir(_) => { - if let Some(workspace) = workspace.upgrade() { - this.maybe_clear_provisional_project_group_key(&workspace, cx); - this.add_project_group_key( - this.project_group_key_for_workspace(&workspace, cx), - ); - this.remove_stale_project_group_keys(cx); - cx.notify(); + this.handle_workspace_key_change(&workspace, cx); } } _ => {} @@ -587,7 +587,124 @@ impl MultiWorkspace { .detach(); } - pub fn add_project_group_key(&mut self, project_group_key: ProjectGroupKey) { + fn handle_workspace_key_change( + &mut self, + workspace: &Entity, + cx: &mut Context, + ) { + let workspace_id = workspace.entity_id(); + let old_key = self.project_group_key_for_workspace(workspace, cx); + let new_key = workspace.read(cx).project_group_key(cx); + + if new_key.path_list().paths().is_empty() || old_key == new_key { + return; + } + + let active_workspace = self.workspace().clone(); + + self.set_workspace_group_key(workspace, new_key.clone()); + + let changed_root_paths = workspace.read(cx).root_paths(cx); + let old_paths = old_key.path_list().paths(); + let new_paths = new_key.path_list().paths(); + + // Remove workspaces that already had the new key and have the same + // root paths (true duplicates that this workspace is replacing). + // + // NOTE: These are dropped without prompting for unsaved changes because + // the user explicitly added a folder that makes this workspace + // identical to the duplicate — they are intentionally overwriting it. + let duplicate_workspaces: Vec> = self + .workspaces + .iter() + .filter(|ws| { + ws.entity_id() != workspace_id + && self.project_group_key_for_workspace(ws, cx) == new_key + && ws.read(cx).root_paths(cx) == changed_root_paths + }) + .cloned() + .collect(); + + if duplicate_workspaces.contains(&active_workspace) { + // The active workspace is among the duplicates — drop the + // incoming workspace instead so the user stays where they are. + self.detach_workspace(workspace, cx); + self.workspaces.retain(|w| w != workspace); + } else { + for ws in &duplicate_workspaces { + self.detach_workspace(ws, cx); + self.workspaces.retain(|w| w != ws); + } + } + + // Propagate folder adds/removes to linked worktree siblings + // (different root paths, same old key) so they stay in the group. + let group_workspaces: Vec> = self + .workspaces + .iter() + .filter(|ws| { + ws.entity_id() != workspace_id + && self.project_group_key_for_workspace(ws, cx) == old_key + }) + .cloned() + .collect(); + + for workspace in &group_workspaces { + // Pre-set this to stop later WorktreeAdded events from triggering + self.set_workspace_group_key(&workspace, new_key.clone()); + + let project = workspace.read(cx).project().clone(); + + for added_path in new_paths.iter().filter(|p| !old_paths.contains(p)) { + project + .update(cx, |project, cx| { + project.find_or_create_worktree(added_path, true, cx) + }) + .detach_and_log_err(cx); + } + + for removed_path in old_paths.iter().filter(|p| !new_paths.contains(p)) { + project.update(cx, |project, cx| { + project.remove_worktree_for_main_worktree_path(removed_path, cx); + }); + } + } + + // Restore the active workspace after removals may have shifted + // the index. If the previously active workspace was removed, + // fall back to the workspace whose key just changed. + if let ActiveWorkspace::Persistent(_) = &self.active_workspace { + let target = if self.workspaces.contains(&active_workspace) { + &active_workspace + } else { + workspace + }; + if let Some(new_index) = self.workspaces.iter().position(|ws| ws == target) { + self.active_workspace = ActiveWorkspace::Persistent(new_index); + } + } + + self.remove_stale_project_group_keys(cx); + + let old_main_paths = old_key.path_list().clone(); + for added_path in new_paths.iter().filter(|p| !old_paths.contains(p)) { + cx.emit(MultiWorkspaceEvent::WorktreePathAdded { + old_main_paths: old_main_paths.clone(), + added_path: added_path.clone(), + }); + } + for removed_path in old_paths.iter().filter(|p| !new_paths.contains(p)) { + cx.emit(MultiWorkspaceEvent::WorktreePathRemoved { + old_main_paths: old_main_paths.clone(), + removed_path: removed_path.clone(), + }); + } + + self.serialize(cx); + cx.notify(); + } + + fn add_project_group_key(&mut self, project_group_key: ProjectGroupKey) { if project_group_key.path_list().paths().is_empty() { return; } @@ -598,12 +715,12 @@ impl MultiWorkspace { self.project_group_keys.insert(0, project_group_key); } - pub fn set_provisional_project_group_key( + pub(crate) fn set_workspace_group_key( &mut self, workspace: &Entity, project_group_key: ProjectGroupKey, ) { - self.provisional_project_group_keys + self.workspace_group_keys .insert(workspace.entity_id(), project_group_key.clone()); self.add_project_group_key(project_group_key); } @@ -613,28 +730,12 @@ impl MultiWorkspace { workspace: &Entity, cx: &App, ) -> ProjectGroupKey { - self.provisional_project_group_keys + self.workspace_group_keys .get(&workspace.entity_id()) .cloned() .unwrap_or_else(|| workspace.read(cx).project_group_key(cx)) } - fn maybe_clear_provisional_project_group_key( - &mut self, - workspace: &Entity, - cx: &App, - ) { - let live_key = workspace.read(cx).project_group_key(cx); - if self - .provisional_project_group_keys - .get(&workspace.entity_id()) - .is_some_and(|key| *key == live_key) - { - self.provisional_project_group_keys - .remove(&workspace.entity_id()); - } - } - fn remove_stale_project_group_keys(&mut self, cx: &App) { let workspace_keys: HashSet = self .workspaces @@ -1045,7 +1146,6 @@ impl MultiWorkspace { self.promote_transient(old, cx); } else { self.detach_workspace(&old, cx); - cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(old.entity_id())); } } } else { @@ -1056,7 +1156,6 @@ impl MultiWorkspace { }); if let Some(old) = self.active_workspace.set_transient(workspace) { self.detach_workspace(&old, cx); - cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(old.entity_id())); } } @@ -1083,7 +1182,7 @@ impl MultiWorkspace { /// Returns the index of the newly inserted workspace. fn promote_transient(&mut self, workspace: Entity, cx: &mut Context) -> usize { let project_group_key = self.project_group_key_for_workspace(&workspace, cx); - self.add_project_group_key(project_group_key); + self.set_workspace_group_key(&workspace, project_group_key); self.workspaces.push(workspace.clone()); cx.emit(MultiWorkspaceEvent::WorkspaceAdded(workspace)); self.workspaces.len() - 1 @@ -1099,10 +1198,10 @@ impl MultiWorkspace { for workspace in std::mem::take(&mut self.workspaces) { if workspace != active { self.detach_workspace(&workspace, cx); - cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(workspace.entity_id())); } } self.project_group_keys.clear(); + self.workspace_group_keys.clear(); self.active_workspace = ActiveWorkspace::Transient(active); cx.notify(); } @@ -1128,7 +1227,7 @@ impl MultiWorkspace { workspace.set_multi_workspace(weak_self, cx); }); - self.add_project_group_key(project_group_key); + self.set_workspace_group_key(&workspace, project_group_key); self.workspaces.push(workspace.clone()); cx.emit(MultiWorkspaceEvent::WorkspaceAdded(workspace)); cx.notify(); @@ -1136,10 +1235,12 @@ impl MultiWorkspace { } } - /// Clears session state and DB binding for a workspace that is being - /// removed or replaced. The DB row is preserved so the workspace still - /// appears in the recent-projects list. + /// Detaches a workspace: clears session state, DB binding, cached + /// group key, and emits `WorkspaceRemoved`. The DB row is preserved + /// so the workspace still appears in the recent-projects list. fn detach_workspace(&mut self, workspace: &Entity, cx: &mut Context) { + self.workspace_group_keys.remove(&workspace.entity_id()); + cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(workspace.entity_id())); workspace.update(cx, |workspace, _cx| { workspace.session_id.take(); workspace._schedule_serialize_workspace.take(); @@ -1313,6 +1414,46 @@ impl MultiWorkspace { tasks } + #[cfg(any(test, feature = "test-support"))] + pub fn assert_project_group_key_integrity(&self, cx: &App) -> anyhow::Result<()> { + let stored_keys: HashSet<&ProjectGroupKey> = self.project_group_keys().collect(); + + let workspace_group_keys: HashSet<&ProjectGroupKey> = + self.workspace_group_keys.values().collect(); + let extra_keys = &workspace_group_keys - &stored_keys; + anyhow::ensure!( + extra_keys.is_empty(), + "workspace_group_keys values not in project_group_keys: {:?}", + extra_keys, + ); + + let cached_ids: HashSet = self.workspace_group_keys.keys().copied().collect(); + let workspace_ids: HashSet = + self.workspaces.iter().map(|ws| ws.entity_id()).collect(); + anyhow::ensure!( + cached_ids == workspace_ids, + "workspace_group_keys entity IDs don't match workspaces.\n\ + only in cache: {:?}\n\ + only in workspaces: {:?}", + &cached_ids - &workspace_ids, + &workspace_ids - &cached_ids, + ); + + for workspace in self.workspaces() { + let live_key = workspace.read(cx).project_group_key(cx); + let cached_key = &self.workspace_group_keys[&workspace.entity_id()]; + anyhow::ensure!( + *cached_key == live_key, + "workspace {:?} has live key {:?} but cached key {:?}", + workspace.entity_id(), + live_key, + cached_key, + ); + } + + Ok(()) + } + #[cfg(any(test, feature = "test-support"))] pub fn set_random_database_id(&mut self, cx: &mut Context) { self.workspace().update(cx, |workspace, _cx| { @@ -1471,7 +1612,6 @@ impl MultiWorkspace { for workspace in &removed_workspaces { this.detach_workspace(workspace, cx); - cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(workspace.entity_id())); } let removed_any = !removed_workspaces.is_empty(); diff --git a/crates/workspace/src/multi_workspace_tests.rs b/crates/workspace/src/multi_workspace_tests.rs index 259346fe097826b3dcc19fb8fad0b8f07ddd0488..9cab28c0ca4ab34b2189985e898285dd82dd4f32 100644 --- a/crates/workspace/src/multi_workspace_tests.rs +++ b/crates/workspace/src/multi_workspace_tests.rs @@ -185,157 +185,3 @@ async fn test_project_group_keys_duplicate_not_added(cx: &mut TestAppContext) { ); }); } - -#[gpui::test] -async fn test_project_group_keys_on_worktree_added(cx: &mut TestAppContext) { - init_test(cx); - let fs = FakeFs::new(cx.executor()); - fs.insert_tree("/root_a", json!({ "file.txt": "" })).await; - fs.insert_tree("/root_b", json!({ "file.txt": "" })).await; - let project = Project::test(fs, ["/root_a".as_ref()], cx).await; - - let initial_key = project.read_with(cx, |p, cx| p.project_group_key(cx)); - - let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - - multi_workspace.update(cx, |mw, cx| { - mw.open_sidebar(cx); - }); - - // Add a second worktree to the same project. - let (worktree, _) = project - .update(cx, |project, cx| { - project.find_or_create_worktree("/root_b", true, cx) - }) - .await - .unwrap(); - worktree - .read_with(cx, |tree, _| tree.as_local().unwrap().scan_complete()) - .await; - cx.run_until_parked(); - - let updated_key = project.read_with(cx, |p, cx| p.project_group_key(cx)); - assert_ne!( - initial_key, updated_key, - "key should change after adding a worktree" - ); - - multi_workspace.read_with(cx, |mw, _cx| { - let keys: Vec<&ProjectGroupKey> = mw.project_group_keys().collect(); - assert_eq!( - keys.len(), - 2, - "should have both the original and updated key" - ); - assert_eq!(*keys[0], updated_key); - assert_eq!(*keys[1], initial_key); - }); -} - -#[gpui::test] -async fn test_project_group_keys_on_worktree_removed(cx: &mut TestAppContext) { - init_test(cx); - let fs = FakeFs::new(cx.executor()); - fs.insert_tree("/root_a", json!({ "file.txt": "" })).await; - fs.insert_tree("/root_b", json!({ "file.txt": "" })).await; - let project = Project::test(fs, ["/root_a".as_ref(), "/root_b".as_ref()], cx).await; - - let initial_key = project.read_with(cx, |p, cx| p.project_group_key(cx)); - - let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - - multi_workspace.update(cx, |mw, cx| { - mw.open_sidebar(cx); - }); - - // Remove one worktree. - let worktree_b_id = project.read_with(cx, |project, cx| { - project - .worktrees(cx) - .find(|wt| wt.read(cx).root_name().as_unix_str() == "root_b") - .unwrap() - .read(cx) - .id() - }); - project.update(cx, |project, cx| { - project.remove_worktree(worktree_b_id, cx); - }); - cx.run_until_parked(); - - let updated_key = project.read_with(cx, |p, cx| p.project_group_key(cx)); - assert_ne!( - initial_key, updated_key, - "key should change after removing a worktree" - ); - - multi_workspace.read_with(cx, |mw, _cx| { - let keys: Vec<&ProjectGroupKey> = mw.project_group_keys().collect(); - assert_eq!( - keys.len(), - 2, - "should accumulate both the original and post-removal key" - ); - assert_eq!(*keys[0], updated_key); - assert_eq!(*keys[1], initial_key); - }); -} - -#[gpui::test] -async fn test_project_group_keys_across_multiple_workspaces_and_worktree_changes( - cx: &mut TestAppContext, -) { - init_test(cx); - let fs = FakeFs::new(cx.executor()); - fs.insert_tree("/root_a", json!({ "file.txt": "" })).await; - fs.insert_tree("/root_b", json!({ "file.txt": "" })).await; - fs.insert_tree("/root_c", json!({ "file.txt": "" })).await; - let project_a = Project::test(fs.clone(), ["/root_a".as_ref()], cx).await; - let project_b = Project::test(fs.clone(), ["/root_b".as_ref()], cx).await; - - let key_a = project_a.read_with(cx, |p, cx| p.project_group_key(cx)); - let key_b = project_b.read_with(cx, |p, cx| p.project_group_key(cx)); - - let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); - - multi_workspace.update(cx, |mw, cx| { - mw.open_sidebar(cx); - }); - - multi_workspace.update_in(cx, |mw, window, cx| { - mw.test_add_workspace(project_b, window, cx); - }); - - multi_workspace.read_with(cx, |mw, _cx| { - assert_eq!(mw.project_group_keys().count(), 2); - }); - - // Now add a worktree to project_a. This should produce a third key. - let (worktree, _) = project_a - .update(cx, |project, cx| { - project.find_or_create_worktree("/root_c", true, cx) - }) - .await - .unwrap(); - worktree - .read_with(cx, |tree, _| tree.as_local().unwrap().scan_complete()) - .await; - cx.run_until_parked(); - - let key_a_updated = project_a.read_with(cx, |p, cx| p.project_group_key(cx)); - assert_ne!(key_a, key_a_updated); - - multi_workspace.read_with(cx, |mw, _cx| { - let keys: Vec<&ProjectGroupKey> = mw.project_group_keys().collect(); - assert_eq!( - keys.len(), - 3, - "should have key_a, key_b, and the updated key_a with root_c" - ); - assert_eq!(*keys[0], key_a_updated); - assert_eq!(*keys[1], key_b); - assert_eq!(*keys[2], key_a); - }); -} diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 81224c0e2db520a278bfb21429e211ba9a4f09ae..d40b7abae0c036a5cdd227ec8a547bd3c10b262c 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -9886,7 +9886,7 @@ async fn open_remote_project_inner( }); if let Some(project_group_key) = provisional_project_group_key.clone() { - multi_workspace.set_provisional_project_group_key(&new_workspace, project_group_key); + multi_workspace.set_workspace_group_key(&new_workspace, project_group_key); } multi_workspace.activate(new_workspace.clone(), window, cx); new_workspace