From e2780f6fbd05aeff5983f4a07987b2ff5eb8ea3b Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 15 Apr 2026 15:54:03 -0400 Subject: [PATCH] Only archive worktrees that Zed created (#53991) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When archiving a thread, we now check whether the linked worktree lives inside the Zed-managed worktrees directory (configured via `git.worktree_directory`). If it doesn't — meaning the user created it themselves outside of Zed — we skip both the directory deletion and the `git worktree remove`, leaving it untouched. Previously, archiving a thread would delete *any* linked worktree associated with the thread's folder paths, even ones the user had created manually via `git worktree add`. Release Notes: - Fixed archiving an agent thread deleting linked git worktrees the user created outside of Zed's managed worktrees directory. --- .../agent_ui/src/thread_worktree_archive.rs | 244 ++++++++++++++++-- crates/sidebar/src/sidebar_tests.rs | 49 ++-- 2 files changed, 257 insertions(+), 36 deletions(-) diff --git a/crates/agent_ui/src/thread_worktree_archive.rs b/crates/agent_ui/src/thread_worktree_archive.rs index 74fac10a9f3158ca12ea5b8a423d7e265af9d150..8040d67a5aca9f0f1a1c45ef4066e1fccefcafa8 100644 --- a/crates/agent_ui/src/thread_worktree_archive.rs +++ b/crates/agent_ui/src/thread_worktree_archive.rs @@ -67,6 +67,16 @@ fn archived_worktree_ref_name(id: i64) -> String { format!("refs/archived-worktrees/{}", id) } +/// Resolves the Zed-managed worktrees base directory for a given repo. +/// +/// This intentionally reads the *global* `git.worktree_directory` setting +/// rather than any project-local override, because Zed always uses the +/// global value when creating worktrees and the archive check must match. +fn worktrees_base_for_repo(main_repo_path: &Path, cx: &App) -> Option { + let setting = &ProjectSettings::get_global(cx).git.worktree_directory; + worktrees_directory_for_repo(main_repo_path, setting).log_err() +} + /// Builds a [`RootPlan`] for archiving the git worktree at `path`. /// /// This is a synchronous planning step that must run *before* any workspace @@ -134,6 +144,15 @@ pub fn build_root_plan( // Main worktrees must be left alone — git refuses to remove them. let (linked_snapshot, repo) = linked_repo?; let main_repo_path = linked_snapshot.original_repo_abs_path.to_path_buf(); + + // Only archive worktrees that live inside the Zed-managed worktrees + // directory (configured via `git.worktree_directory`). Worktrees the + // user created outside that directory should be left untouched. + let worktrees_base = worktrees_base_for_repo(&main_repo_path, cx)?; + if !path.starts_with(&worktrees_base) { + return None; + } + let branch_name = linked_snapshot .branch .as_ref() @@ -246,10 +265,7 @@ async fn remove_empty_parent_dirs_up_to_worktrees_base( main_repo_path: PathBuf, cx: &mut AsyncApp, ) { - let worktrees_base = cx.update(|cx| { - let setting = &ProjectSettings::get_global(cx).git.worktree_directory; - worktrees_directory_for_repo(&main_repo_path, setting).log_err() - }); + let worktrees_base = cx.update(|cx| worktrees_base_for_repo(&main_repo_path, cx)); if let Some(worktrees_base) = worktrees_base { cx.background_executor() @@ -814,7 +830,7 @@ mod tests { use super::*; use fs::{FakeFs, Fs as _}; use git::repository::Worktree as GitWorktree; - use gpui::TestAppContext; + use gpui::{BorrowAppContext, TestAppContext}; use project::Project; use serde_json::json; use settings::SettingsStore; @@ -997,7 +1013,7 @@ mod tests { Path::new("/project/.git"), true, GitWorktree { - path: PathBuf::from("/linked-worktree"), + path: PathBuf::from("/worktrees/project/feature/project"), ref_name: Some("refs/heads/feature".into()), sha: "abc123".into(), is_main: false, @@ -1008,7 +1024,10 @@ mod tests { let project = Project::test( fs.clone(), - [Path::new("/project"), Path::new("/linked-worktree")], + [ + Path::new("/project"), + Path::new("/worktrees/project/feature/project"), + ], cx, ) .await; @@ -1027,7 +1046,7 @@ mod tests { workspace.read_with(cx, |_workspace, cx| { // The linked worktree SHOULD produce a root plan. let plan = build_root_plan( - Path::new("/linked-worktree"), + Path::new("/worktrees/project/feature/project"), std::slice::from_ref(&workspace), cx, ); @@ -1036,7 +1055,10 @@ mod tests { "build_root_plan should return Some for a linked worktree", ); let plan = plan.unwrap(); - assert_eq!(plan.root_path, PathBuf::from("/linked-worktree")); + assert_eq!( + plan.root_path, + PathBuf::from("/worktrees/project/feature/project") + ); assert_eq!(plan.main_repo_path, PathBuf::from("/project")); // The main worktree should still return None. @@ -1050,6 +1072,176 @@ mod tests { }); } + #[gpui::test] + async fn test_build_root_plan_returns_none_for_external_linked_worktree( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/project", + json!({ + ".git": {}, + "src": { "main.rs": "fn main() {}" } + }), + ) + .await; + fs.set_branch_name(Path::new("/project/.git"), Some("main")); + fs.insert_branches(Path::new("/project/.git"), &["main", "feature"]); + + fs.add_linked_worktree_for_repo( + Path::new("/project/.git"), + true, + GitWorktree { + path: PathBuf::from("/external-worktree"), + ref_name: Some("refs/heads/feature".into()), + sha: "abc123".into(), + is_main: false, + is_bare: false, + }, + ) + .await; + + let project = Project::test( + fs.clone(), + [Path::new("/project"), Path::new("/external-worktree")], + cx, + ) + .await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + + let multi_workspace = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace + .read_with(cx, |mw, _cx| mw.workspace().clone()) + .unwrap(); + + cx.run_until_parked(); + + workspace.read_with(cx, |_workspace, cx| { + let plan = build_root_plan( + Path::new("/external-worktree"), + std::slice::from_ref(&workspace), + cx, + ); + assert!( + plan.is_none(), + "build_root_plan should return None for a linked worktree \ + outside the Zed-managed worktrees directory", + ); + }); + } + + #[gpui::test] + async fn test_build_root_plan_with_custom_worktree_directory(cx: &mut TestAppContext) { + init_test(cx); + + // Override the worktree_directory setting to a non-default location. + // With main repo at /project and setting "../custom-worktrees", the + // resolved base is /custom-worktrees/project. + cx.update(|cx| { + cx.update_global::(|store, cx| { + store.update_user_settings(cx, |s| { + s.git.get_or_insert(Default::default()).worktree_directory = + Some("../custom-worktrees".to_string()); + }); + }); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/project", + json!({ + ".git": {}, + "src": { "main.rs": "fn main() {}" } + }), + ) + .await; + fs.set_branch_name(Path::new("/project/.git"), Some("main")); + fs.insert_branches(Path::new("/project/.git"), &["main", "feature", "feature2"]); + + // Worktree inside the custom managed directory. + fs.add_linked_worktree_for_repo( + Path::new("/project/.git"), + true, + GitWorktree { + path: PathBuf::from("/custom-worktrees/project/feature/project"), + ref_name: Some("refs/heads/feature".into()), + sha: "abc123".into(), + is_main: false, + is_bare: false, + }, + ) + .await; + + // Worktree outside the custom managed directory (at the default + // `../worktrees` location, which is not what the setting says). + fs.add_linked_worktree_for_repo( + Path::new("/project/.git"), + true, + GitWorktree { + path: PathBuf::from("/worktrees/project/feature2/project"), + ref_name: Some("refs/heads/feature2".into()), + sha: "def456".into(), + is_main: false, + is_bare: false, + }, + ) + .await; + + let project = Project::test( + fs.clone(), + [ + Path::new("/project"), + Path::new("/custom-worktrees/project/feature/project"), + Path::new("/worktrees/project/feature2/project"), + ], + cx, + ) + .await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + + let multi_workspace = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace + .read_with(cx, |mw, _cx| mw.workspace().clone()) + .unwrap(); + + cx.run_until_parked(); + + workspace.read_with(cx, |_workspace, cx| { + // Worktree inside the custom managed directory SHOULD be archivable. + let plan = build_root_plan( + Path::new("/custom-worktrees/project/feature/project"), + std::slice::from_ref(&workspace), + cx, + ); + assert!( + plan.is_some(), + "build_root_plan should return Some for a worktree inside \ + the custom worktree_directory", + ); + + // Worktree at the default location SHOULD NOT be archivable + // because the setting points elsewhere. + let plan = build_root_plan( + Path::new("/worktrees/project/feature2/project"), + std::slice::from_ref(&workspace), + cx, + ); + assert!( + plan.is_none(), + "build_root_plan should return None for a worktree outside \ + the custom worktree_directory, even if it would match the default", + ); + }); + } + #[gpui::test] async fn test_remove_root_deletes_directory_and_git_metadata(cx: &mut TestAppContext) { init_test(cx); @@ -1070,7 +1262,7 @@ mod tests { Path::new("/project/.git"), true, GitWorktree { - path: PathBuf::from("/linked-worktree"), + path: PathBuf::from("/worktrees/project/feature/project"), ref_name: Some("refs/heads/feature".into()), sha: "abc123".into(), is_main: false, @@ -1081,7 +1273,10 @@ mod tests { let project = Project::test( fs.clone(), - [Path::new("/project"), Path::new("/linked-worktree")], + [ + Path::new("/project"), + Path::new("/worktrees/project/feature/project"), + ], cx, ) .await; @@ -1101,14 +1296,17 @@ mod tests { let root = workspace .read_with(cx, |_workspace, cx| { build_root_plan( - Path::new("/linked-worktree"), + Path::new("/worktrees/project/feature/project"), std::slice::from_ref(&workspace), cx, ) }) .expect("should produce a root plan for the linked worktree"); - assert!(fs.is_dir(Path::new("/linked-worktree")).await); + assert!( + fs.is_dir(Path::new("/worktrees/project/feature/project")) + .await + ); // Remove the root. let task = cx.update(|cx| cx.spawn(async move |cx| remove_root(root, cx).await)); @@ -1119,7 +1317,8 @@ mod tests { // The FakeFs directory should be gone (removed by the FakeGitRepository // backend's remove_worktree implementation). assert!( - !fs.is_dir(Path::new("/linked-worktree")).await, + !fs.is_dir(Path::new("/worktrees/project/feature/project")) + .await, "linked worktree directory should be removed from FakeFs" ); } @@ -1144,7 +1343,7 @@ mod tests { Path::new("/project/.git"), true, GitWorktree { - path: PathBuf::from("/linked-worktree"), + path: PathBuf::from("/worktrees/project/feature/project"), ref_name: Some("refs/heads/feature".into()), sha: "abc123".into(), is_main: false, @@ -1155,7 +1354,10 @@ mod tests { let project = Project::test( fs.clone(), - [Path::new("/project"), Path::new("/linked-worktree")], + [ + Path::new("/project"), + Path::new("/worktrees/project/feature/project"), + ], cx, ) .await; @@ -1174,7 +1376,7 @@ mod tests { let root = workspace .read_with(cx, |_workspace, cx| { build_root_plan( - Path::new("/linked-worktree"), + Path::new("/worktrees/project/feature/project"), std::slice::from_ref(&workspace), cx, ) @@ -1185,7 +1387,7 @@ mod tests { // remove_root, simulating the directory being deleted externally. fs.as_ref() .remove_dir( - Path::new("/linked-worktree"), + Path::new("/worktrees/project/feature/project"), fs::RemoveOptions { recursive: true, ignore_if_not_exists: false, @@ -1193,7 +1395,11 @@ mod tests { ) .await .unwrap(); - assert!(!fs.as_ref().is_dir(Path::new("/linked-worktree")).await); + assert!( + !fs.as_ref() + .is_dir(Path::new("/worktrees/project/feature/project")) + .await + ); // remove_root should still succeed — the std::fs::remove_dir_all // handles NotFound, and git worktree remove handles a missing diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index e77e1296b73cfef791dad5f1fee37369fcac6071..8aaf19f44d45688dbee7cdf258ef98a946a92b17 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -4549,7 +4549,7 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon .await; fs.insert_tree( - "/wt-feature-a", + "/worktrees/project/feature-a/project", serde_json::json!({ ".git": "gitdir: /project/.git/worktrees/feature-a", "src": {}, @@ -4561,7 +4561,7 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon Path::new("/project/.git"), false, git::repository::Worktree { - path: PathBuf::from("/wt-feature-a"), + path: PathBuf::from("/worktrees/project/feature-a/project"), ref_name: Some("refs/heads/feature-a".into()), sha: "abc".into(), is_main: false, @@ -4573,7 +4573,12 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon cx.update(|cx| ::set_global(fs.clone(), cx)); let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; - let worktree_project = project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + let worktree_project = project::Project::test( + fs.clone(), + ["/worktrees/project/feature-a/project".as_ref()], + cx, + ) + .await; main_project .update(cx, |p, cx| p.git_scans_complete(cx)) @@ -4643,7 +4648,8 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon // The linked worktree checkout directory should also be removed from disk. assert!( - !fs.is_dir(Path::new("/wt-feature-a")).await, + !fs.is_dir(Path::new("/worktrees/project/feature-a/project")) + .await, "linked worktree directory should be removed from disk after archiving its last thread" ); @@ -4677,7 +4683,7 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon }); assert_eq!( archived_paths.paths(), - &[PathBuf::from("/wt-feature-a")], + &[PathBuf::from("/worktrees/project/feature-a/project")], "archived thread must retain its folder_paths for restore" ); } @@ -9892,7 +9898,7 @@ async fn test_archive_removes_worktree_even_when_workspace_paths_diverge(cx: &mu .await; fs.insert_tree( - "/wt-feature-a", + "/worktrees/project/feature-a/project", serde_json::json!({ ".git": "gitdir: /project/.git/worktrees/feature-a", "src": { @@ -9906,7 +9912,7 @@ async fn test_archive_removes_worktree_even_when_workspace_paths_diverge(cx: &mu Path::new("/project/.git"), false, git::repository::Worktree { - path: PathBuf::from("/wt-feature-a"), + path: PathBuf::from("/worktrees/project/feature-a/project"), ref_name: Some("refs/heads/feature-a".into()), sha: "abc".into(), is_main: false, @@ -9918,7 +9924,12 @@ async fn test_archive_removes_worktree_even_when_workspace_paths_diverge(cx: &mu cx.update(|cx| ::set_global(fs.clone(), cx)); let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; - let worktree_project = project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + let worktree_project = project::Project::test( + fs.clone(), + ["/worktrees/project/feature-a/project".as_ref()], + cx, + ) + .await; main_project .update(cx, |p, cx| p.git_scans_complete(cx)) @@ -9945,7 +9956,7 @@ async fn test_archive_removes_worktree_even_when_workspace_paths_diverge(cx: &mu "worktree-thread", "Worktree Thread", PathList::new(&[ - PathBuf::from("/wt-feature-a"), + PathBuf::from("/worktrees/project/feature-a/project"), PathBuf::from("/nonexistent"), ]), PathList::new(&[PathBuf::from("/project"), PathBuf::from("/nonexistent")]), @@ -10004,7 +10015,8 @@ async fn test_archive_removes_worktree_even_when_workspace_paths_diverge(cx: &mu // The linked worktree directory should be removed from disk. assert!( - !fs.is_dir(Path::new("/wt-feature-a")).await, + !fs.is_dir(Path::new("/worktrees/project/feature-a/project")) + .await, "linked worktree directory should be removed from disk" ); } @@ -10037,7 +10049,7 @@ async fn test_archive_mixed_workspace_closes_only_archived_worktree_items(cx: &m .await; fs.insert_tree( - "/wt-feature-b", + "/worktrees/main-repo/feature-b/main-repo", serde_json::json!({ ".git": "gitdir: /main-repo/.git/worktrees/feature-b", "src": { @@ -10051,7 +10063,7 @@ async fn test_archive_mixed_workspace_closes_only_archived_worktree_items(cx: &m Path::new("/main-repo/.git"), false, git::repository::Worktree { - path: PathBuf::from("/wt-feature-b"), + path: PathBuf::from("/worktrees/main-repo/feature-b/main-repo"), ref_name: Some("refs/heads/feature-b".into()), sha: "def".into(), is_main: false, @@ -10066,7 +10078,10 @@ async fn test_archive_mixed_workspace_closes_only_archived_worktree_items(cx: &m // linked worktree — this makes it a "mixed" workspace. let mixed_project = project::Project::test( fs.clone(), - ["/main-repo".as_ref(), "/wt-feature-b".as_ref()], + [ + "/main-repo".as_ref(), + "/worktrees/main-repo/feature-b/main-repo".as_ref(), + ], cx, ) .await; @@ -10093,15 +10108,15 @@ async fn test_archive_mixed_workspace_closes_only_archived_worktree_items(cx: &m let main_repo_wt_id = worktree_ids .iter() - .find(|(_, path)| path.ends_with("main-repo")) + .find(|(_, path)| path.as_ref() == Path::new("/main-repo")) .map(|(id, _)| *id) .expect("should find main-repo worktree"); let feature_b_wt_id = worktree_ids .iter() - .find(|(_, path)| path.ends_with("wt-feature-b")) + .find(|(_, path)| path.as_ref() == Path::new("/worktrees/main-repo/feature-b/main-repo")) .map(|(id, _)| *id) - .expect("should find wt-feature-b worktree"); + .expect("should find feature-b worktree"); // Open files from both worktrees. let main_repo_path = project::ProjectPath { @@ -10158,7 +10173,7 @@ async fn test_archive_mixed_workspace_closes_only_archived_worktree_items(cx: &m "feature-b-thread", "Feature B Thread", PathList::new(&[ - PathBuf::from("/wt-feature-b"), + PathBuf::from("/worktrees/main-repo/feature-b/main-repo"), PathBuf::from("/nonexistent"), ]), PathList::new(&[PathBuf::from("/main-repo"), PathBuf::from("/nonexistent")]),