From fdebc23cccd295fe3e01b6d1cf5e3b2f5822d874 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 16 Jun 2026 11:36:59 -0400 Subject: [PATCH] Prevent archival of manually-created worktrees (#58275) This fixes archive cleanup for agent threads so Zed only removes worktrees it explicitly created, rather than treating every linked worktree under the configured managed directory as safe to delete. When Zed creates a worktree, it now records it in the local database along with the creation time of the worktree's git metadata directory (`.git/worktrees//`). Archive planning requires that record, and right before deleting anything, Zed re-stats the directory and compares creation times: if the worktree was removed and recreated outside Zed (or the time can't be read at all), deletion is skipped and the stale record is dropped. Every failure mode fails safe by leaving the directory untouched. For remote (SSH) projects, the stat runs on the remote host via a new `GitWorktreeCreatedAt` request. Worktrees created by a different Zed install (another release channel, or another machine connecting to the same host) have no record in the local database and are therefore never auto-archived, which is the intended conservative behavior. Closes https://github.com/zed-industries/zed/issues/57155 Closes AI-352 Release Notes: - Fixed archiving an agent thread incorrectly deleting manually-created git worktrees. --- crates/agent_ui/src/conversation_view.rs | 3 + crates/agent_ui/src/test_support.rs | 39 +++ .../agent_ui/src/thread_worktree_archive.rs | 324 +++++++++++++++++- crates/collab/src/rpc.rs | 1 + crates/fs/src/fake_git_repo.rs | 33 +- crates/fs/src/fs.rs | 13 + crates/git/src/repository.rs | 76 ++++ crates/git_ui/src/created_worktrees.rs | 150 ++++++++ crates/git_ui/src/git_ui.rs | 1 + crates/git_ui/src/worktree_service.rs | 18 + crates/project/src/git_store.rs | 51 ++- crates/proto/proto/git.proto | 10 + crates/proto/proto/zed.proto | 4 +- crates/proto/src/proto.rs | 4 + crates/remote/src/remote_identity.rs | 31 ++ crates/sidebar/src/sidebar_tests.rs | 78 +++++ 16 files changed, 815 insertions(+), 21 deletions(-) create mode 100644 crates/git_ui/src/created_worktrees.rs diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index bf1dca28b8..2d8eabbee0 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -5627,6 +5627,9 @@ pub(crate) mod tests { cx.update(|cx| { let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); + // Use an isolated DB so parallel tests can't overwrite each + // other's global keys (e.g. the last-created entry kind). + cx.set_global(db::AppDatabase::test_new()); ThreadMetadataStore::init_global(cx); theme_settings::init(theme::LoadThemes::JustBase, cx); editor::init(cx); diff --git a/crates/agent_ui/src/test_support.rs b/crates/agent_ui/src/test_support.rs index 3f8d5c8c6d..2390784ce4 100644 --- a/crates/agent_ui/src/test_support.rs +++ b/crates/agent_ui/src/test_support.rs @@ -10,7 +10,9 @@ use project::Project; use settings::SettingsStore; use std::any::Any; use std::cell::RefCell; +use std::path::Path; use std::rc::Rc; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use workspace::{MultiWorkspace, Sidebar as WorkspaceSidebar, SidebarEvent, SidebarSide}; use crate::AgentPanel; @@ -102,6 +104,9 @@ pub fn init_test(cx: &mut TestAppContext) { cx.update(|cx| { let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); + // Use an isolated DB so parallel tests can't see each other's + // persisted records (e.g. created-worktree records). + cx.set_global(db::AppDatabase::test_new()); cx.set_global(acp_thread::StubSessionCounter( std::sync::atomic::AtomicUsize::new(0), )); @@ -113,6 +118,40 @@ pub fn init_test(cx: &mut TestAppContext) { }); } +/// Returns the creation time assigned to a linked worktree's git metadata +/// directory, mirroring `FakeGitRepository::worktree_created_at` (which uses +/// the FakeFs directory mtime as a stand-in for the creation time). +pub async fn fake_worktree_created_at(fs: &dyn fs::Fs, worktree_path: &Path) -> SystemTime { + let git_file = fs.load(&worktree_path.join(".git")).await.unwrap(); + let git_dir = worktree_path.join(git_file.strip_prefix("gitdir:").unwrap().trim()); + let (seconds, nanos) = fs + .metadata(&git_dir) + .await + .unwrap() + .unwrap() + .mtime + .to_seconds_and_nanos_for_persistence() + .unwrap(); + UNIX_EPOCH + Duration::new(seconds, nanos) +} + +/// Records the worktree in the created-worktrees registry with its actual +/// (fake) creation time, as the worktree creation flow would. Tests that +/// expect a worktree to be archivable must call this after setting it up. +pub async fn record_zed_created_worktree( + fs: &dyn fs::Fs, + worktree_path: &Path, + remote: Option<&remote::RemoteConnectionOptions>, + cx: &mut TestAppContext, +) { + let created_at = fake_worktree_created_at(fs, worktree_path).await; + cx.update(|cx| { + git_ui::created_worktrees::record_created_worktree(worktree_path, remote, created_at, cx) + }) + .await + .unwrap(); +} + pub struct TestWorkspaceSidebar { focus_handle: FocusHandle, threads_list_active: bool, diff --git a/crates/agent_ui/src/thread_worktree_archive.rs b/crates/agent_ui/src/thread_worktree_archive.rs index f3d54657d4..1d392a8635 100644 --- a/crates/agent_ui/src/thread_worktree_archive.rs +++ b/crates/agent_ui/src/thread_worktree_archive.rs @@ -1,6 +1,7 @@ use std::{ path::{Path, PathBuf}, sync::Arc, + time::SystemTime, }; use anyhow::{Context as _, Result, anyhow}; @@ -52,6 +53,11 @@ pub struct RootPlan { /// used to create temporary remote projects when the main repo isn't /// loaded in any open workspace. pub remote_connection: Option, + /// The creation time of the worktree's git metadata directory that was + /// recorded when Zed created the worktree. [`remove_root`] re-stats the + /// directory and refuses to delete anything if the time has changed, + /// which means the worktree was recreated outside Zed. + pub recorded_created_at: SystemTime, } /// A `Project` that references a worktree being archived, paired with the @@ -174,6 +180,15 @@ pub fn build_root_plan( return None; } + // Only archive worktrees that Zed explicitly created. The directory + // check above constrains paths, but the database record is what + // distinguishes a Zed-created worktree from one the user manually + // created under the same directory layout. The recorded creation time + // is re-verified against the filesystem in [`remove_root`] before + // anything is deleted. + let recorded_created_at = + git_ui::created_worktrees::recorded_created_at(&path, remote_connection, cx)?; + let branch_name = linked_snapshot .branch .as_ref() @@ -186,6 +201,7 @@ pub fn build_root_plan( worktree_repo: repo, branch_name, remote_connection: remote_connection.cloned(), + recorded_created_at, }) } @@ -198,6 +214,8 @@ pub fn build_root_plan( /// delete the worktree directory. If the git removal fails, the worktree /// is re-added to each project via [`rollback_root`]. pub async fn remove_root(root: RootPlan, cx: &mut AsyncApp) -> Result<()> { + verify_created_by_zed(&root, cx).await?; + let release_tasks: Vec<_> = root .affected_projects .iter() @@ -217,9 +235,73 @@ pub async fn remove_root(root: RootPlan, cx: &mut AsyncApp) -> Result<()> { return Err(error); } + // The worktree is gone, so its registry record is now stale. If the + // user later creates a new worktree at the same path outside Zed, a + // leftover record would only be saved by the creation time check, so + // remove it eagerly. + cx.update(|cx| { + git_ui::created_worktrees::forget_created_worktree( + &root.root_path, + root.remote_connection.as_ref(), + cx, + ) + }) + .await + .log_err(); + Ok(()) } +/// Confirms that the worktree on disk is still the one Zed created, by +/// comparing the creation time of its git metadata directory against the +/// time recorded when Zed created it. +/// +/// Outcomes: +/// - Creation time matches the recorded one: proceed. +/// - Worktree directory no longer exists: proceed — there is nothing on +/// disk to protect, and removal will only clean up git metadata. +/// - Creation time differs: the worktree was removed and recreated outside +/// Zed. The registry record is removed (so subsequent archival attempts +/// skip the worktree entirely) and an error is returned so the caller +/// leaves the directory untouched. +/// - Creation time cannot be read: return an error but keep the record, +/// since the failure may be transient (e.g. a disconnected remote). +async fn verify_created_by_zed(root: &RootPlan, cx: &mut AsyncApp) -> Result<()> { + let receiver = root.worktree_repo.update(cx, |repo: &mut Repository, _cx| { + repo.worktree_created_at(root.root_path.clone()) + }); + let created_at = receiver + .await + .map_err(|_| anyhow!("worktree creation time check was canceled"))? + .with_context(|| { + format!( + "refusing to delete worktree at {}: failed to verify that Zed created it", + root.root_path.display() + ) + })?; + + match created_at { + None => Ok(()), + Some(created_at) if created_at == root.recorded_created_at => Ok(()), + Some(_) => { + cx.update(|cx| { + git_ui::created_worktrees::forget_created_worktree( + &root.root_path, + root.remote_connection.as_ref(), + cx, + ) + }) + .await + .log_err(); + Err(anyhow!( + "refusing to delete worktree at {}: it is not the worktree Zed created \ + (it was likely removed and recreated outside Zed)", + root.root_path.display() + )) + } + } +} + async fn remove_root_after_worktree_removal( root: &RootPlan, release_tasks: Vec>>, @@ -707,6 +789,18 @@ pub async fn restore_worktree_via_git( return Err(error.context("failed to restore archive checkpoint")); } + if created_new_worktree { + // Re-register the restored worktree as Zed-created so it can be + // archived again later. + git_ui::created_worktrees::record_created_worktree_for_repo( + &wt_repo, + worktree_path, + remote_connection, + cx, + ) + .await; + } + Ok(worktree_path.clone()) } @@ -869,18 +963,34 @@ mod tests { use project::Project; use serde_json::json; use settings::SettingsStore; + use std::time::Duration; use workspace::MultiWorkspace; fn init_test(cx: &mut TestAppContext) { cx.update(|cx| { let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); + // Use an isolated DB so parallel tests can't see each other's + // created-worktree records. + cx.set_global(db::AppDatabase::test_new()); theme_settings::init(theme::LoadThemes::JustBase, cx); editor::init(cx); release_channel::init(semver::Version::new(0, 0, 0), cx); }); } + async fn fake_worktree_created_at(fs: &FakeFs, worktree_path: &Path) -> SystemTime { + crate::test_support::fake_worktree_created_at(fs, worktree_path).await + } + + async fn record_zed_created_worktree( + fs: &FakeFs, + worktree_path: &Path, + cx: &mut TestAppContext, + ) { + crate::test_support::record_zed_created_worktree(fs, worktree_path, None, cx).await + } + #[gpui::test] async fn test_build_root_plan_returns_none_for_main_worktree(cx: &mut TestAppContext) { init_test(cx); @@ -949,6 +1059,7 @@ mod tests { }, ) .await; + record_zed_created_worktree(&fs, Path::new("/worktrees/project/feature/project"), cx).await; let project = Project::test( fs.clone(), @@ -1069,6 +1180,75 @@ mod tests { }); } + #[gpui::test] + async fn test_build_root_plan_returns_none_for_unrecorded_linked_worktree_in_managed_directory( + 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("/worktrees/project/feature/project"), + ref_name: Some("refs/heads/feature".into()), + sha: "abc123".into(), + is_main: false, + is_bare: false, + }, + ) + .await; + // Deliberately don't record the worktree in the created-worktrees + // registry: it represents a worktree the user created manually. + + let project = Project::test( + fs.clone(), + [ + Path::new("/project"), + Path::new("/worktrees/project/feature/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| { + let plan = build_root_plan( + Path::new("/worktrees/project/feature/project"), + None, + std::slice::from_ref(&workspace), + cx, + ); + assert!( + plan.is_none(), + "build_root_plan should return None for a linked worktree Zed didn't create, \ + even when it lives inside the Zed-managed worktrees directory", + ); + }); + } + #[gpui::test] async fn test_build_root_plan_with_custom_worktree_directory(cx: &mut TestAppContext) { init_test(cx); @@ -1110,9 +1290,17 @@ mod tests { }, ) .await; + record_zed_created_worktree( + &fs, + Path::new("/custom-worktrees/project/feature/project"), + cx, + ) + .await; // Worktree outside the custom managed directory (at the default // `../worktrees` location, which is not what the setting says). + // It is recorded as Zed-created so that the directory check, not + // the registry, is what excludes it below. fs.add_linked_worktree_for_repo( Path::new("/project/.git"), true, @@ -1125,6 +1313,8 @@ mod tests { }, ) .await; + record_zed_created_worktree(&fs, Path::new("/worktrees/project/feature2/project"), cx) + .await; let project = Project::test( fs.clone(), @@ -1206,6 +1396,7 @@ mod tests { }, ) .await; + record_zed_created_worktree(&fs, Path::new("/worktrees/project/feature/project"), cx).await; let project = Project::test( fs.clone(), @@ -1287,6 +1478,7 @@ mod tests { }, ) .await; + record_zed_created_worktree(&fs, Path::new("/worktrees/project/feature/project"), cx).await; let project = Project::test( fs.clone(), @@ -1346,6 +1538,116 @@ mod tests { .expect("remove_root should succeed even when directory is already gone"); } + #[gpui::test] + async fn test_remove_root_refuses_when_worktree_recreated_outside_zed(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("/worktrees/project/feature/project"), + ref_name: Some("refs/heads/feature".into()), + sha: "abc123".into(), + is_main: false, + is_bare: false, + }, + ) + .await; + + // Record a creation time that doesn't match the directory on disk, + // simulating a worktree that was removed and recreated outside Zed + // after Zed recorded the original. + let worktree_path = Path::new("/worktrees/project/feature/project"); + let actual_created_at = fake_worktree_created_at(&fs, worktree_path).await; + cx.update(|cx| { + git_ui::created_worktrees::record_created_worktree( + worktree_path, + None, + actual_created_at + Duration::from_secs(1), + cx, + ) + }) + .await + .unwrap(); + + let project = Project::test( + fs.clone(), + [ + Path::new("/project"), + Path::new("/worktrees/project/feature/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(); + + let root = workspace + .read_with(cx, |_workspace, cx| { + build_root_plan( + Path::new("/worktrees/project/feature/project"), + None, + std::slice::from_ref(&workspace), + cx, + ) + }) + .expect("should produce a root plan while the record exists"); + + let task = cx.update(|cx| cx.spawn(async move |cx| remove_root(root, cx).await)); + let error = task + .await + .expect_err("remove_root should refuse to delete a recreated worktree"); + assert!( + error.to_string().contains("not the worktree Zed created"), + "unexpected error: {error:#}" + ); + + cx.run_until_parked(); + + // The directory must be left untouched. + assert!( + fs.is_dir(Path::new("/worktrees/project/feature/project")) + .await, + "worktree directory should not be deleted on creation time mismatch" + ); + + // The stale record should be forgotten, so subsequent archival + // attempts skip the worktree entirely. + workspace.read_with(cx, |_workspace, cx| { + assert!( + git_ui::created_worktrees::recorded_created_at(worktree_path, None, cx).is_none(), + "stale created-worktree record should be removed" + ); + let plan = build_root_plan(worktree_path, None, std::slice::from_ref(&workspace), cx); + assert!( + plan.is_none(), + "build_root_plan should return None after the stale record is removed" + ); + }); + } + #[gpui::test] async fn test_remove_root_returns_error_and_rolls_back_on_remove_dir_failure( cx: &mut TestAppContext, @@ -1376,6 +1678,7 @@ mod tests { }, ) .await; + record_zed_created_worktree(&fs, Path::new("/worktrees/project/feature/project"), cx).await; let project = Project::test( fs.clone(), @@ -1409,25 +1712,10 @@ mod tests { }) .expect("should produce a root plan for the linked worktree"); - // Replace the worktree directory with a file so that fs.remove_dir - // fails with a "not a directory" error. + // Make deleting the worktree directory fail, while leaving the + // worktree itself intact so the created-by-Zed verification passes. let worktree_path = Path::new("/worktrees/project/feature/project"); - fs.remove_dir( - worktree_path, - fs::RemoveOptions { - recursive: true, - ignore_if_not_exists: false, - }, - ) - .await - .unwrap(); - fs.create_file(worktree_path, fs::CreateOptions::default()) - .await - .unwrap(); - assert!( - fs.is_file(worktree_path).await, - "path should now be a file, not a directory" - ); + fs.set_remove_dir_error(worktree_path, "simulated remove_dir failure".to_string()); let task = cx.update(|cx| cx.spawn(async move |cx| remove_root(root, cx).await)); let result = task.await; diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 176d504ae1..337e363db6 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -493,6 +493,7 @@ impl Server { .add_request_handler(forward_mutating_project_request::) .add_request_handler(forward_mutating_project_request::) .add_request_handler(forward_read_only_project_request::) + .add_request_handler(forward_read_only_project_request::) .add_request_handler(forward_read_only_project_request::) .add_request_handler(forward_read_only_project_request::) .add_request_stream_handler( diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index c2522576e8..5d8fc647fe 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -26,7 +26,7 @@ use gpui::{AsyncApp, BackgroundExecutor, SharedString, Task}; use ignore::gitignore::GitignoreBuilder; use parking_lot::Mutex; use rope::Rope; -use std::{path::PathBuf, sync::Arc, sync::atomic::AtomicBool}; +use std::{path::PathBuf, sync::Arc, sync::atomic::AtomicBool, time::SystemTime}; use text::LineEnding; use util::{paths::PathStyle, rel_path::RelPath}; @@ -590,6 +590,37 @@ impl GitRepository for FakeGitRepository { .boxed() } + fn worktree_created_at( + &self, + worktree_path: PathBuf, + ) -> BoxFuture<'_, Result>> { + let fs = self.fs.clone(); + async move { + if fs.metadata(&worktree_path).await?.is_none() { + return Ok(None); + } + let dot_git_path = worktree_path.join(".git"); + let git_file = fs + .load(&dot_git_path) + .await + .with_context(|| format!("failed to read {}", dot_git_path.display()))?; + let git_dir = git_file + .strip_prefix("gitdir:") + .context("worktree .git file missing gitdir pointer")? + .trim(); + let git_dir = worktree_path.join(git_dir); + let metadata = fs + .metadata(&git_dir) + .await? + .with_context(|| format!("missing worktree git dir {}", git_dir.display()))?; + // FakeFs assigns directories a monotonically increasing mtime at + // creation and never updates it afterwards, so a directory's + // mtime doubles as its creation time. + Ok(Some(metadata.mtime.0)) + } + .boxed() + } + fn create_worktree( &self, target: CreateWorktreeTarget, diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 68c57c655d..59fe449b18 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -1336,6 +1336,7 @@ struct FakeFsState { job_event_subscribers: Arc>>, trash: Vec<(TrashedEntry, FakeFsEntry)>, file_to_create_before_watch_add: Option<(PathBuf, PathBuf)>, + remove_dir_errors: std::collections::HashMap, } #[cfg(feature = "test-support")] @@ -1655,6 +1656,7 @@ impl FakeFs { job_event_subscribers: Arc::new(Mutex::new(Vec::new())), trash: Vec::new(), file_to_create_before_watch_add: None, + remove_dir_errors: Default::default(), })), }); @@ -2397,6 +2399,14 @@ impl FakeFs { .unwrap(); } + /// Makes subsequent `remove_dir` calls for `path` fail with `message`. + pub fn set_remove_dir_error(&self, path: impl AsRef, message: String) { + self.state + .lock() + .remove_dir_errors + .insert(normalize_path(path.as_ref()), message); + } + pub fn paths(&self, include_dot_git: bool) -> Vec { let mut result = Vec::new(); let mut queue = collections::VecDeque::new(); @@ -2539,6 +2549,9 @@ impl FakeFs { self.simulate_random_delay().await; let path = normalize_path(path); + if let Some(message) = self.state.lock().remove_dir_errors.get(&path) { + anyhow::bail!("{message}"); + } let parent_path = path.parent().context("cannot remove the root")?; let base_name = path.file_name().context("cannot remove the root")?; diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 3d164af6a0..8547462e8e 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -24,6 +24,7 @@ use std::sync::atomic::AtomicBool; use std::process::{ExitStatus, Output}; use std::str::FromStr; +use std::time::SystemTime; use std::{ cmp::Ordering, path::{Path, PathBuf}, @@ -75,6 +76,17 @@ pub fn original_repo_path_from_common_dir(common_dir: &Path) -> Option } } +fn linked_worktree_git_dir(worktree_path: &Path) -> Result { + let dot_git_path = worktree_path.join(".git"); + let git_file = std::fs::read_to_string(&dot_git_path) + .with_context(|| format!("failed to read {}", dot_git_path.display()))?; + let git_dir = git_file + .strip_prefix("gitdir:") + .context("worktree .git file missing gitdir pointer")? + .trim(); + Ok(worktree_path.join(git_dir)) +} + fn normalize_git_metadata_path(path: PathBuf) -> Result { paths::normalize_lexically(&path) .map_err(|_| anyhow!("git metadata path escapes its filesystem root: {path:?}")) @@ -845,6 +857,24 @@ pub trait GitRepository: Send + Sync { fn worktrees(&self) -> BoxFuture<'_, Result>>; + /// Returns the creation time of a linked worktree's git metadata + /// directory (`.git/worktrees//`), resolved via the worktree's + /// `.git` file. + /// + /// The metadata directory is created by `git worktree add` and removed + /// by `git worktree remove`, so its creation time identifies a + /// particular incarnation of the worktree: if the worktree is removed + /// and recreated at the same path, the creation time changes. + /// + /// Returns `Ok(None)` when the worktree directory does not exist at + /// all, and an error when the directory exists but the time cannot be + /// determined (e.g. on filesystems without birthtime support); callers + /// should fail safe in the error case. + fn worktree_created_at( + &self, + worktree_path: PathBuf, + ) -> BoxFuture<'_, Result>>; + fn create_worktree( &self, target: CreateWorktreeTarget, @@ -1921,6 +1951,34 @@ impl GitRepository for RealGitRepository { .boxed() } + fn worktree_created_at( + &self, + worktree_path: PathBuf, + ) -> BoxFuture<'_, Result>> { + self.executor + .spawn(async move { + match std::fs::metadata(&worktree_path) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(None); + } + Err(error) => { + return Err(error).with_context(|| { + format!("failed to stat {}", worktree_path.display()) + }); + } + Ok(_) => {} + } + let git_dir = linked_worktree_git_dir(&worktree_path)?; + let metadata = std::fs::metadata(&git_dir) + .with_context(|| format!("failed to stat {}", git_dir.display()))?; + let created_at = metadata.created().with_context(|| { + format!("creation time unavailable for {}", git_dir.display()) + })?; + Ok(Some(created_at)) + }) + .boxed() + } + fn create_worktree( &self, target: CreateWorktreeTarget, @@ -4907,6 +4965,24 @@ mod tests { new_worktree.path.canonicalize().unwrap(), worktree_path.canonicalize().unwrap(), ); + + // The new worktree's git metadata directory should report a creation + // time, resolved via the worktree's `.git` file. + let created_at = repo + .worktree_created_at(worktree_path.clone()) + .await + .unwrap(); + assert!( + created_at.is_some(), + "creation time should be available for a freshly created worktree" + ); + + // A path with no worktree at all reports `None`. + let missing = repo + .worktree_created_at(worktrees_dir.join("does-not-exist")) + .await + .unwrap(); + assert_eq!(missing, None); } #[gpui::test] diff --git a/crates/git_ui/src/created_worktrees.rs b/crates/git_ui/src/created_worktrees.rs new file mode 100644 index 0000000000..b026d99a41 --- /dev/null +++ b/crates/git_ui/src/created_worktrees.rs @@ -0,0 +1,150 @@ +//! A persistent registry of git worktrees that Zed itself created. +//! +//! Thread archival deletes a worktree's directory from disk, so it must be +//! certain the worktree was created by Zed rather than by the user. This +//! module records each Zed-created worktree in the local database, keyed by +//! its path (and remote host, for remote projects), along with the creation +//! time of the worktree's git metadata directory at the time Zed created it. +//! +//! Before deleting a worktree, callers re-stat that directory and compare +//! against the recorded time. A mismatch means the worktree was removed and +//! recreated outside Zed, so deletion must be skipped. Every failure mode +//! (no record, unreadable creation time, mismatched time) fails safe by +//! leaving the directory untouched. +//! +//! Because the registry lives in the local database, worktrees created by a +//! different Zed install (e.g. another release channel, or another machine +//! connecting to the same remote host) are treated as manually created and +//! never archived. That is intentional: when in doubt, don't delete. + +use std::{ + future::Future, + path::Path, + time::{Duration, SystemTime, UNIX_EPOCH}, +}; + +use anyhow::{Context as _, Result}; +use db::kvp::KeyValueStore; +use gpui::{App, AsyncApp, Entity}; +use project::git_store::Repository; +use remote::{RemoteConnectionOptions, remote_connection_identity}; +use serde::{Deserialize, Serialize}; +use util::ResultExt as _; + +const NAMESPACE: &str = "created_git_worktrees"; + +#[derive(Serialize, Deserialize)] +struct CreatedWorktreeRecord { + created_at_seconds: u64, + created_at_subsec_nanos: u32, +} + +fn record_key(worktree_path: &Path, remote: Option<&RemoteConnectionOptions>) -> String { + let host = match remote { + None => "local".to_string(), + Some(options) => remote_connection_identity(options).persistence_key(), + }; + // Paths cannot contain newlines in practice, so this separator is + // unambiguous. + format!("{host}\n{}", worktree_path.display()) +} + +/// Records that Zed created the worktree at `worktree_path`, along with the +/// creation time of its git metadata directory. +pub fn record_created_worktree( + worktree_path: &Path, + remote: Option<&RemoteConnectionOptions>, + created_at: SystemTime, + cx: &App, +) -> impl Future> + use<> { + let store = KeyValueStore::global(cx); + let key = record_key(worktree_path, remote); + let value = created_at + .duration_since(UNIX_EPOCH) + .context("worktree creation time predates the unix epoch") + .and_then(|duration| { + serde_json::to_string(&CreatedWorktreeRecord { + created_at_seconds: duration.as_secs(), + created_at_subsec_nanos: duration.subsec_nanos(), + }) + .context("failed to serialize created worktree record") + }); + async move { store.scoped(NAMESPACE).write(key, value?).await } +} + +/// Returns the recorded creation time for a worktree Zed created, or `None` +/// if Zed has no record of creating it. +pub fn recorded_created_at( + worktree_path: &Path, + remote: Option<&RemoteConnectionOptions>, + cx: &App, +) -> Option { + let store = KeyValueStore::global(cx); + let value = store + .scoped(NAMESPACE) + .read(&record_key(worktree_path, remote)) + .log_err()??; + let record: CreatedWorktreeRecord = serde_json::from_str(&value).log_err()?; + Some(UNIX_EPOCH + Duration::new(record.created_at_seconds, record.created_at_subsec_nanos)) +} + +/// Looks up the creation time of a freshly created worktree's git metadata +/// directory through the repository and records it in the registry. +/// +/// Failures are logged and swallowed: the worktree just won't be eligible +/// for automatic archival. +pub async fn record_created_worktree_for_repo( + repo: &Entity, + worktree_path: &Path, + remote: Option<&RemoteConnectionOptions>, + cx: &mut AsyncApp, +) { + let receiver = repo.update(cx, |repo, _cx| { + repo.worktree_created_at(worktree_path.to_path_buf()) + }); + let created_at = match receiver.await { + Ok(Ok(Some(created_at))) => created_at, + Ok(Ok(None)) => { + log::warn!( + "Newly created worktree {} not found on disk; \ + it won't be eligible for automatic archival", + worktree_path.display() + ); + return; + } + Ok(Err(error)) => { + log::warn!( + "Couldn't determine creation time for worktree {}; \ + it won't be eligible for automatic archival: {error:#}", + worktree_path.display() + ); + return; + } + Err(_) => { + log::warn!( + "Worktree creation time lookup was canceled for {}", + worktree_path.display() + ); + return; + } + }; + let record = cx.update(|cx| record_created_worktree(worktree_path, remote, created_at, cx)); + if let Err(error) = record.await { + log::warn!( + "Failed to record created worktree {}: {error:#}", + worktree_path.display() + ); + } +} + +/// Removes the record for a worktree, either because Zed deleted it or +/// because the directory on disk turned out not to be the one Zed created. +pub fn forget_created_worktree( + worktree_path: &Path, + remote: Option<&RemoteConnectionOptions>, + cx: &App, +) -> impl Future> + use<> { + let store = KeyValueStore::global(cx); + let key = record_key(worktree_path, remote); + async move { store.scoped(NAMESPACE).delete(key).await } +} diff --git a/crates/git_ui/src/git_ui.rs b/crates/git_ui/src/git_ui.rs index 5c211a8746..7e5b2ade32 100644 --- a/crates/git_ui/src/git_ui.rs +++ b/crates/git_ui/src/git_ui.rs @@ -35,6 +35,7 @@ mod commit_modal; pub mod commit_tooltip; pub mod commit_view; mod conflict_view; +pub mod created_worktrees; pub mod file_diff_view; pub mod git_graph; pub mod git_panel; diff --git a/crates/git_ui/src/worktree_service.rs b/crates/git_ui/src/worktree_service.rs index 8c0153c49a..a9dcdb8406 100644 --- a/crates/git_ui/src/worktree_service.rs +++ b/crates/git_ui/src/worktree_service.rs @@ -958,8 +958,26 @@ async fn do_create_worktree( let fs = cx.update(|_, cx| ::global(cx))?; + let creation_pairs: Vec<(Entity, PathBuf)> = creation_infos + .iter() + .map(|(repo, path, _)| (repo.clone(), path.clone())) + .collect(); + let created_paths = await_and_rollback_on_failure(creation_infos, fs, cx).await?; + // Record each created worktree so thread archival can later verify that + // Zed created it before deleting it from disk. Failures are non-fatal: + // the worktree just won't be eligible for automatic archival. + for (repo, path) in creation_pairs { + crate::created_worktrees::record_created_worktree_for_repo( + &repo, + &path, + remote_connection_options.as_ref(), + cx, + ) + .await; + } + // `path_remapping` has one entry per source git repo, while `created_paths` // has one per *unique* target worktree. When the former is larger, two or // more source repos were linked worktrees of the same underlying diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 0af840fbbe..d7c0e73c9f 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -78,7 +78,7 @@ use std::{ Arc, atomic::{self, AtomicU64}, }, - time::{Duration, Instant}, + time::{Duration, Instant, SystemTime}, }; use sum_tree::{Edit, SumTree, TreeMap}; use task::Shell; @@ -702,6 +702,7 @@ impl GitStore { client.add_entity_request_handler(Self::handle_create_worktree); client.add_entity_request_handler(Self::handle_remove_worktree); client.add_entity_request_handler(Self::handle_rename_worktree); + client.add_entity_request_handler(Self::handle_worktree_created_at); client.add_entity_request_handler(Self::handle_get_head_sha); client.add_entity_request_handler(Self::handle_edit_ref); client.add_entity_request_handler(Self::handle_repair_worktrees); @@ -2853,6 +2854,26 @@ impl GitStore { Ok(proto::Ack {}) } + async fn handle_worktree_created_at( + this: Entity, + envelope: TypedEnvelope, + mut cx: AsyncApp, + ) -> Result { + let repository_id = RepositoryId::from_proto(envelope.payload.repository_id); + let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?; + let worktree_path = PathBuf::from(envelope.payload.worktree_path); + + let created_at = repository_handle + .update(&mut cx, |repository_handle, _| { + repository_handle.worktree_created_at(worktree_path) + }) + .await??; + + Ok(proto::GitWorktreeCreatedAtResponse { + created_at: created_at.map(Into::into), + }) + } + async fn handle_get_head_sha( this: Entity, envelope: TypedEnvelope, @@ -7497,6 +7518,34 @@ impl Repository { ) } + /// Returns the creation time of a linked worktree's git metadata + /// directory. See [`GitRepository::worktree_created_at`]. For remote + /// projects the stat runs on the remote host, where the worktree's + /// filesystem lives. + pub fn worktree_created_at( + &mut self, + worktree_path: PathBuf, + ) -> oneshot::Receiver>> { + let id = self.id; + self.send_job("worktree_created_at", None, move |repo, _cx| async move { + match repo { + RepositoryState::Local(LocalRepositoryState { backend, .. }) => { + backend.worktree_created_at(worktree_path).await + } + RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => { + let response = client + .request(proto::GitWorktreeCreatedAt { + project_id: project_id.0, + repository_id: id.to_proto(), + worktree_path: worktree_path.to_string_lossy().to_string(), + }) + .await?; + Ok(response.created_at.map(SystemTime::from)) + } + } + }) + } + pub fn create_worktree_detached( &mut self, path: PathBuf, diff --git a/crates/proto/proto/git.proto b/crates/proto/proto/git.proto index ac7e39a83d..c4c838042c 100644 --- a/crates/proto/proto/git.proto +++ b/crates/proto/proto/git.proto @@ -666,6 +666,16 @@ message GitRenameWorktree { string new_path = 4; } +message GitWorktreeCreatedAt { + uint64 project_id = 1; + uint64 repository_id = 2; + string worktree_path = 3; +} + +message GitWorktreeCreatedAtResponse { + optional Timestamp created_at = 1; +} + message RunGitHook { enum GitHook { PRE_COMMIT = 0; diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index ab5ea5ce72..1af251f0ab 100644 --- a/crates/proto/proto/zed.proto +++ b/crates/proto/proto/zed.proto @@ -488,7 +488,9 @@ message Envelope { GetDocumentLinks get_document_links = 453; GetDocumentLinksResponse get_document_links_response = 454; ResolveDocumentLink resolve_document_link = 455; - ResolveDocumentLinkResponse resolve_document_link_response = 456; // current max + ResolveDocumentLinkResponse resolve_document_link_response = 456; + GitWorktreeCreatedAt git_worktree_created_at = 457; + GitWorktreeCreatedAtResponse git_worktree_created_at_response = 458; // current max } reserved 87 to 88; diff --git a/crates/proto/src/proto.rs b/crates/proto/src/proto.rs index 2836c28a55..16b8f38fb0 100644 --- a/crates/proto/src/proto.rs +++ b/crates/proto/src/proto.rs @@ -370,6 +370,8 @@ messages!( (GitCreateWorktree, Background), (GitRemoveWorktree, Background), (GitRenameWorktree, Background), + (GitWorktreeCreatedAt, Background), + (GitWorktreeCreatedAtResponse, Background), (ShareAgentThread, Foreground), (GetSharedAgentThread, Foreground), (GetSharedAgentThreadResponse, Foreground), @@ -588,6 +590,7 @@ request_messages!( (GitCreateWorktree, Ack), (GitRemoveWorktree, Ack), (GitRenameWorktree, Ack), + (GitWorktreeCreatedAt, GitWorktreeCreatedAtResponse), (TrustWorktrees, Ack), (RestrictWorktrees, Ack), (FindSearchCandidatesChunk, Ack), @@ -789,6 +792,7 @@ entity_messages!( GitCreateWorktree, GitRemoveWorktree, GitRenameWorktree, + GitWorktreeCreatedAt, TrustWorktrees, RestrictWorktrees, FindSearchCandidatesChunk, diff --git a/crates/remote/src/remote_identity.rs b/crates/remote/src/remote_identity.rs index 92d9534e7d..f776bdb7a9 100644 --- a/crates/remote/src/remote_identity.rs +++ b/crates/remote/src/remote_identity.rs @@ -26,6 +26,37 @@ pub enum RemoteConnectionIdentity { Mock { id: u64 }, } +impl RemoteConnectionIdentity { + /// A stable string form of this identity, suitable for use in + /// persistence keys (e.g. database keys scoped to a remote host). + pub fn persistence_key(&self) -> String { + match self { + Self::Ssh { + host, + username, + port, + } => format!( + "ssh:{}@{}:{}", + username.as_deref().unwrap_or_default(), + host, + port.map(|port| port.to_string()).unwrap_or_default() + ), + Self::Wsl { distro_name, user } => format!( + "wsl:{}@{}", + user.as_deref().unwrap_or_default(), + distro_name + ), + Self::Docker { + container_id, + name, + remote_user, + } => format!("docker:{remote_user}@{name}:{container_id}"), + #[cfg(any(test, feature = "test-support"))] + Self::Mock { id } => format!("mock:{id}"), + } + } +} + impl From<&RemoteConnectionOptions> for RemoteConnectionIdentity { fn from(options: &RemoteConnectionOptions) -> Self { match options { diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 2991b2d332..e5c7cca5d6 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -28,6 +28,9 @@ fn init_test(cx: &mut TestAppContext) { cx.update(|cx| { let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); + // Use an isolated DB so parallel tests can't see each other's + // persisted records (e.g. created-worktree records). + cx.set_global(db::AppDatabase::test_new()); theme_settings::init(theme::LoadThemes::JustBase, cx); editor::init(cx); ThreadStore::init_global(cx); @@ -2047,6 +2050,13 @@ async fn test_terminal_close_event_on_archived_linked_worktree_removes_workspace }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/project/feature-a/project"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; @@ -2533,6 +2543,13 @@ async fn test_archive_selected_draft_archives_linked_worktree_after_last_draft( }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/project/feature-a/project"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; @@ -2751,6 +2768,13 @@ async fn test_archive_selected_draft_archives_closed_linked_worktree(cx: &mut Te }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/project/feature-a/project"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; @@ -3296,6 +3320,13 @@ async fn test_archive_selected_terminal_archives_closed_linked_worktree(cx: &mut }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/project/feature-a/project"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; @@ -3464,6 +3495,13 @@ async fn test_archive_selected_thread_archives_closed_linked_worktree(cx: &mut T }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/project/feature-a/project"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; @@ -8130,6 +8168,13 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/project/feature-a/project"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); @@ -13488,6 +13533,13 @@ async fn test_archive_removes_worktree_even_when_workspace_paths_diverge(cx: &mu }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/project/feature-a/project"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); @@ -13640,6 +13692,13 @@ async fn test_archive_mixed_workspace_closes_only_archived_worktree_items(cx: &m }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/main-repo/feature-b/main-repo"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); @@ -13861,6 +13920,13 @@ async fn test_discard_mixed_workspace_draft_closes_only_archived_worktree_items( }, ) .await; + agent_ui::test_support::record_zed_created_worktree( + fs.as_ref(), + Path::new("/worktrees/main-repo/feature-b/main-repo"), + None, + cx, + ) + .await; cx.update(|cx| ::set_global(fs.clone(), cx)); @@ -14202,6 +14268,18 @@ async fn test_remote_archive_thread_with_active_connection( // parent repo, so `build_root_plan` targets the linked worktree // specifically and knows which main repo owns it. let remote_connection = project.read_with(cx, |p, cx| p.remote_connection_options(cx)); + + // Record the worktree as Zed-created on the client, keyed by the remote + // connection identity, with the creation time of the gitdir on the + // *remote* filesystem (where the archive flow will re-stat it). + agent_ui::test_support::record_zed_created_worktree( + server_fs.as_ref(), + Path::new("/worktrees/project/feature-a/project"), + remote_connection.as_ref(), + cx, + ) + .await; + let wt_thread_id = acp::SessionId::new(Arc::from("worktree-thread")); cx.update(|_window, cx| { let metadata = ThreadMetadata {