diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index c36609367dce2f6327dd338a0901644c769e761e..69c5377465a420b2e9f64e16139736fe04b65e5a 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -477,7 +477,7 @@ impl ThreadMetadataStore { } } - pub fn migrate_main_worktree_paths( + pub fn update_main_worktree_paths( &mut self, old_paths: &PathList, new_paths: PathList, diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 1bfd0f10a36d64afff6ea62fe46c21237621453a..8624e8683201a242be6521a67c8992bf9c1206a8 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -425,19 +425,7 @@ fn worktree_info_from_thread_paths( // 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. - // All root paths (main + linked) resolve to the same short name. - let first_name = linked_short_names - .first() - .map(|(name, _)| name) - .or_else(|| infos.first().map(|i| &i.name)); - let all_same_name = first_name.is_some_and(|first| { - infos.iter().all(|i| i.name == *first) - && main_paths.iter().all(|mp| { - mp.file_name() - .and_then(|n| n.to_str()) - .is_some_and(|n| n == first.as_ref()) - }) - }); + let all_same_name = infos.len() > 1 && infos.iter().all(|i| i.name == infos[0].name); if main_paths.len() > 1 && !all_same_name { for (info, (_short_name, project_name)) in infos @@ -537,7 +525,7 @@ impl Sidebar { } MultiWorkspaceEvent::ProjectGroupKeyChanged { old_key, new_key } => { ThreadMetadataStore::global(cx).update(cx, |store, cx| { - store.migrate_main_worktree_paths( + store.update_main_worktree_paths( old_key.path_list(), new_key.path_list().clone(), cx, @@ -1036,6 +1024,18 @@ impl Sidebar { } }; + let main_path_list = group_key.path_list(); + + // Skip threads whose folder_paths include a linked worktree + // that hasn't been discovered by any workspace's git state. + let has_unbacked_linked_worktree = |row: &ThreadMetadata| -> bool { + let main_paths = main_path_list.paths(); + row.folder_paths.paths().iter().any(|path| { + let is_main = main_paths.iter().any(|mp| mp.as_path() == path.as_path()); + !is_main && !linked_to_main.contains_key(&**path) + }) + }; + // Main code path: one query per group via main_worktree_paths. // The main_worktree_paths column is set on all new threads and // points to the group's canonical paths regardless of which @@ -1048,6 +1048,9 @@ impl Sidebar { if !seen_session_ids.insert(row.session_id.clone()) { continue; } + if has_unbacked_linked_worktree(&row) { + continue; + } let workspace = resolve_workspace(&row); threads.push(make_thread_entry(row, workspace)); } diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index ebdd1ffa69a8f5de57efae186ac7616badde2067..3666d907ab8e2dd3bffe6f2955b02ed350a1c60b 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -251,6 +251,32 @@ 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, + folder_paths, + main_worktree_paths, + archived: false, + }; + 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); @@ -2789,14 +2815,15 @@ async fn test_worktree_add_key_collision_removes_duplicate_workspace(cx: &mut Te cx.run_until_parked(); // Both project groups should be visible. - let entries = visible_entries_as_strings(&sidebar, cx); - assert!( - entries.iter().any(|e| e.contains("[project-a]")), - "should show project-a group: {entries:?}" - ); - assert!( - entries.iter().any(|e| e.contains("[project-a, project-b]")), - "should show project-a,b group: {entries:?}" + 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(); @@ -2839,18 +2866,14 @@ async fn test_worktree_add_key_collision_removes_duplicate_workspace(cx: &mut Te sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); cx.run_until_parked(); - let entries = visible_entries_as_strings(&sidebar, cx); - assert!( - entries.iter().any(|e| e.contains("Thread A")), - "Thread A should be visible: {entries:?}" - ); - assert!( - entries.iter().any(|e| e.contains("Thread B")), - "Thread B should be visible: {entries:?}" - ); - assert!( - entries.iter().filter(|e| e.contains("[project-a")).count() == 1, - "should have exactly 1 project header: {entries:?}" + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + // + "v [project-a, project-b]", + " Thread A", + " Thread B", + ] ); } @@ -3412,13 +3435,21 @@ 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 not visible yet — the linked worktree hasn't been + // discovered by any workspace's git state. assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ @@ -3699,7 +3730,7 @@ async fn test_multi_worktree_thread_shows_multiple_chips(cx: &mut TestAppContext vec![ // "v [project_a, project_b]", - " Cross Worktree Thread {olivetti}, {selectric}", + " Cross Worktree Thread {project_a:olivetti}, {project_b:selectric}", ] ); } diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 4e19b114fd7f8f3b23ba21f6374ebe9a93060c35..e8e66e543ac0bc21d1758b59f011bda51188361c 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -563,12 +563,9 @@ 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(_) => { - if let Some(workspace) = workspace.upgrade() { - this.handle_workspace_key_change(&workspace, cx); - } - } - project::Event::WorktreeUpdatedRootRepoCommonDir(_) => { + project::Event::WorktreeAdded(_) + | project::Event::WorktreeRemoved(_) + | project::Event::WorktreeUpdatedRootRepoCommonDir(_) => { if let Some(workspace) = workspace.upgrade() { this.handle_workspace_key_change(&workspace, cx); } @@ -1407,7 +1404,8 @@ impl MultiWorkspace { ); let cached_ids: HashSet = self.workspace_group_keys.keys().copied().collect(); - let workspace_ids: HashSet = self.workspaces().map(|ws| ws.entity_id()).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\ 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); - }); -}