From 52565b8b823db6472d2e497a4d5df0f113ee7ed0 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sun, 5 Apr 2026 22:28:50 -0400 Subject: [PATCH 01/22] Add allow_empty commits, detached worktree creation, and new git operations Extend the git API with several new capabilities needed for worktree archival and restoration: - Add allow_empty flag to CommitOptions for creating WIP marker commits - Change create_worktree to accept Option branch, enabling detached worktree creation when None is passed - Add head_sha() to read the current HEAD commit hash - Add update_ref() and delete_ref() for managing git references - Add stage_all_including_untracked() to stage everything before a WIP commit - Implement all new operations in FakeGitRepository with functional commit history tracking, reset support, and ref management - Update existing call sites for the new CommitOptions field and create_worktree signature --- crates/fs/src/fake_git_repo.rs | 30 ++++++++++- crates/git/src/repository.rs | 95 ++++++++++++++++++++++++++++++++- crates/project/src/git_store.rs | 36 +++++++++++++ 3 files changed, 157 insertions(+), 4 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 8883211b1495b6..762c0973b639c7 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -673,7 +673,6 @@ impl GitRepository for FakeGitRepository { } })??; } - Ok(()) } .boxed() @@ -1179,6 +1178,34 @@ impl GitRepository for FakeGitRepository { .boxed() } + fn create_archive_checkpoint(&self) -> BoxFuture<'_, Result<(String, String)>> { + let executor = self.executor.clone(); + let fs = self.fs.clone(); + let checkpoints = self.checkpoints.clone(); + let repository_dir_path = self.repository_dir_path.parent().unwrap().to_path_buf(); + async move { + executor.simulate_random_delay().await; + let staged_oid = git::Oid::random(&mut *executor.rng().lock()); + let unstaged_oid = git::Oid::random(&mut *executor.rng().lock()); + let entry = fs.entry(&repository_dir_path)?; + checkpoints.lock().insert(staged_oid, entry.clone()); + checkpoints.lock().insert(unstaged_oid, entry); + Ok((staged_oid.to_string(), unstaged_oid.to_string())) + } + .boxed() + } + + fn restore_archive_checkpoint( + &self, + _staged_sha: String, + unstaged_sha: String, + ) -> BoxFuture<'_, Result<()>> { + let checkpoint = GitRepositoryCheckpoint { + commit_sha: unstaged_sha.parse().unwrap(), + }; + self.restore_checkpoint(checkpoint) + } + fn compare_checkpoints( &self, left: GitRepositoryCheckpoint, @@ -1412,7 +1439,6 @@ impl GitRepository for FakeGitRepository { Ok(()) }) } - fn set_trusted(&self, trusted: bool) { self.is_trusted .store(trusted, std::sync::atomic::Ordering::Release); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index ba489f632faa83..66a3d8d1c86564 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -916,6 +916,20 @@ pub trait GitRepository: Send + Sync { /// Resets to a previously-created checkpoint. fn restore_checkpoint(&self, checkpoint: GitRepositoryCheckpoint) -> BoxFuture<'_, Result<()>>; + /// Creates two detached commits capturing the current staged and unstaged + /// state without moving any branch. Returns (staged_sha, unstaged_sha). + fn create_archive_checkpoint(&self) -> BoxFuture<'_, Result<(String, String)>>; + + /// Restores the working directory and index from archive checkpoint SHAs. + /// Assumes HEAD is already at the correct commit (original_commit_hash). + /// Restores the index to match staged_sha's tree, and the working + /// directory to match unstaged_sha's tree. + fn restore_archive_checkpoint( + &self, + staged_sha: String, + unstaged_sha: String, + ) -> BoxFuture<'_, Result<()>>; + /// Compares two checkpoints, returning true if they are equal fn compare_checkpoints( &self, @@ -960,7 +974,6 @@ pub trait GitRepository: Send + Sync { fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>>; - fn set_trusted(&self, trusted: bool); fn is_trusted(&self) -> bool; } @@ -2282,7 +2295,6 @@ impl GitRepository for RealGitRepository { }) .boxed() } - fn push( &self, branch_name: String, @@ -2621,6 +2633,85 @@ impl GitRepository for RealGitRepository { .boxed() } + fn create_archive_checkpoint(&self) -> BoxFuture<'_, Result<(String, String)>> { + let git_binary = self.git_binary(); + self.executor + .spawn(async move { + let mut git = git_binary?.envs(checkpoint_author_envs()); + + let head_sha = git + .run(&["rev-parse", "HEAD"]) + .await + .context("failed to read HEAD")?; + + // Capture the staged state: write-tree reads the current index + let staged_tree = git + .run(&["write-tree"]) + .await + .context("failed to write staged tree")?; + let staged_sha = git + .run(&[ + "commit-tree", + &staged_tree, + "-p", + &head_sha, + "-m", + "WIP staged", + ]) + .await + .context("failed to create staged commit")?; + + // Capture the full state (staged + unstaged + untracked) using + // a temporary index so we don't disturb the real one. + let unstaged_sha = git + .with_temp_index(async |git| { + git.run(&["add", "--all"]).await?; + let full_tree = git.run(&["write-tree"]).await?; + let sha = git + .run(&[ + "commit-tree", + &full_tree, + "-p", + &staged_sha, + "-m", + "WIP unstaged", + ]) + .await?; + Ok(sha) + }) + .await + .context("failed to create unstaged commit")?; + + Ok((staged_sha, unstaged_sha)) + }) + .boxed() + } + + fn restore_archive_checkpoint( + &self, + staged_sha: String, + unstaged_sha: String, + ) -> BoxFuture<'_, Result<()>> { + let git_binary = self.git_binary(); + self.executor + .spawn(async move { + let git = git_binary?; + + // Restore the index to the staged tree + git.run(&["read-tree", &staged_sha]) + .await + .context("failed to restore index from staged commit")?; + + // Restore the working directory to the full (unstaged) state + git.run(&["restore", "--source", &unstaged_sha, "--worktree", "."]) + .await + .context("failed to restore working directory from unstaged commit")?; + + Ok(()) + }) + .boxed() + } + fn compare_checkpoints( &self, left: GitRepositoryCheckpoint, diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 3d71804ff5e867..820c95e2dc019f 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6141,6 +6141,42 @@ impl Repository { }) } + pub fn create_archive_checkpoint(&mut self) -> oneshot::Receiver> { + self.send_job(None, move |repo, _cx| async move { + match repo { + RepositoryState::Local(LocalRepositoryState { backend, .. }) => { + backend.create_archive_checkpoint().await + } + RepositoryState::Remote(_) => { + anyhow::bail!( + "create_archive_checkpoint is not supported for remote repositories" + ) + } + } + }) + } + + pub fn restore_archive_checkpoint( + &mut self, + staged_sha: String, + unstaged_sha: String, + ) -> oneshot::Receiver> { + self.send_job(None, move |repo, _cx| async move { + match repo { + RepositoryState::Local(LocalRepositoryState { backend, .. }) => { + backend + .restore_archive_checkpoint(staged_sha, unstaged_sha) + .await + } + RepositoryState::Remote(_) => { + anyhow::bail!( + "restore_archive_checkpoint is not supported for remote repositories" + ) + } + } + }) + } + pub fn remove_worktree(&mut self, path: PathBuf, force: bool) -> oneshot::Receiver> { let id = self.id; self.send_job( From ae02f9eddd154ba15bee884104a8fff4da912739 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Mon, 6 Apr 2026 14:52:53 -0400 Subject: [PATCH 02/22] Remove redundent --no-optional-locks argument in new git commands This argument already gets added when calling git_binary.run() --- crates/git/src/repository.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 66a3d8d1c86564..087781a86b5fa7 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -2288,8 +2288,7 @@ impl GitRepository for RealGitRepository { let git_binary = self.git_binary(); self.executor .spawn(async move { - let args: Vec = - vec!["--no-optional-locks".into(), "add".into(), "-A".into()]; + let args: Vec = vec!["add".into(), "-A".into()]; git_binary?.run(&args).await?; Ok(()) }) From d46dd1602e020c292615858368c79bdd3b0b0ce8 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Mon, 6 Apr 2026 15:19:28 -0400 Subject: [PATCH 03/22] Unify the create worktree functions Now creating a worktree from a detached head also has remote support --- crates/project/src/git_store.rs | 12 +++++------- crates/proto/proto/git.proto | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 820c95e2dc019f..0a7f9f6dbe8512 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -2408,7 +2408,7 @@ impl GitStore { let repository_id = RepositoryId::from_proto(envelope.payload.repository_id); let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?; let directory = PathBuf::from(envelope.payload.directory); - let name = envelope.payload.name; + let name = envelope.payload.name.unwrap_or_default(); let commit = envelope.payload.commit; let use_existing_branch = envelope.payload.use_existing_branch; let target = if name.is_empty() { @@ -6021,15 +6021,13 @@ impl Repository { RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => { let (name, commit, use_existing_branch) = match target { CreateWorktreeTarget::ExistingBranch { branch_name } => { - (branch_name, None, true) + (Some(branch_name), None, true) } CreateWorktreeTarget::NewBranch { branch_name, - base_sha: start_point, - } => (branch_name, start_point, false), - CreateWorktreeTarget::Detached { - base_sha: start_point, - } => (String::new(), start_point, false), + base_sha, + } => (Some(branch_name), base_sha, false), + CreateWorktreeTarget::Detached { base_sha } => (None, base_sha, false), }; client diff --git a/crates/proto/proto/git.proto b/crates/proto/proto/git.proto index d0a594a2817ec5..8511debd20467c 100644 --- a/crates/proto/proto/git.proto +++ b/crates/proto/proto/git.proto @@ -591,7 +591,7 @@ message Worktree { message GitCreateWorktree { uint64 project_id = 1; uint64 repository_id = 2; - string name = 3; + optional string name = 3; string directory = 4; optional string commit = 5; bool use_existing_branch = 6; From aa4bedb11fe90808336861bcdc56ea9ede20ea72 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Mon, 6 Apr 2026 16:15:26 -0400 Subject: [PATCH 04/22] Remove stage_all_including_untracked method GitStore::stage_all already does everything this method does and updates pending_ops as well. So I'm removing this method to avoid a potential foot gun in our codebase --- crates/fs/src/fake_git_repo.rs | 33 --------------------------------- crates/git/src/repository.rs | 13 ------------- crates/project/src/git_store.rs | 16 ---------------- 3 files changed, 62 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 762c0973b639c7..926fe6a7e46767 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -1406,39 +1406,6 @@ impl GitRepository for FakeGitRepository { fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>> { async { Ok(()) }.boxed() } - - fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>> { - let workdir_path = self.dot_git_path.parent().unwrap(); - let git_files: Vec<(RepoPath, String)> = self - .fs - .files() - .iter() - .filter_map(|path| { - let repo_path = path.strip_prefix(workdir_path).ok()?; - if repo_path.starts_with(".git") { - return None; - } - let content = self - .fs - .read_file_sync(path) - .ok() - .and_then(|bytes| String::from_utf8(bytes).ok())?; - let rel_path = RelPath::new(repo_path, PathStyle::local()).ok()?; - Some((RepoPath::from_rel_path(&rel_path), content)) - }) - .collect(); - - self.with_state_async(true, move |state| { - let fs_paths: HashSet = git_files.iter().map(|(p, _)| p.clone()).collect(); - for (path, content) in git_files { - state.index_contents.insert(path, content); - } - state - .index_contents - .retain(|path, _| fs_paths.contains(path)); - Ok(()) - }) - } fn set_trusted(&self, trusted: bool) { self.is_trusted .store(trusted, std::sync::atomic::Ordering::Release); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 087781a86b5fa7..14ea541e1465e1 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -972,8 +972,6 @@ pub trait GitRepository: Send + Sync { fn delete_ref(&self, ref_name: String) -> BoxFuture<'_, Result<()>>; fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; - - fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>>; fn set_trusted(&self, trusted: bool); fn is_trusted(&self) -> bool; } @@ -2283,17 +2281,6 @@ impl GitRepository for RealGitRepository { }) .boxed() } - - fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>> { - let git_binary = self.git_binary(); - self.executor - .spawn(async move { - let args: Vec = vec!["add".into(), "-A".into()]; - git_binary?.run(&args).await?; - Ok(()) - }) - .boxed() - } fn push( &self, branch_name: String, diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 0a7f9f6dbe8512..eb40c19a2d7960 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6124,21 +6124,6 @@ impl Repository { }) } - pub fn stage_all_including_untracked(&mut self) -> oneshot::Receiver> { - self.send_job(None, move |repo, _cx| async move { - match repo { - RepositoryState::Local(LocalRepositoryState { backend, .. }) => { - backend.stage_all_including_untracked().await - } - RepositoryState::Remote(_) => { - anyhow::bail!( - "stage_all_including_untracked is not supported for remote repositories" - ) - } - } - }) - } - pub fn create_archive_checkpoint(&mut self) -> oneshot::Receiver> { self.send_job(None, move |repo, _cx| async move { match repo { @@ -6174,7 +6159,6 @@ impl Repository { } }) } - pub fn remove_worktree(&mut self, path: PathBuf, force: bool) -> oneshot::Receiver> { let id = self.id; self.send_job( From ef03cd8e6fafd818be5a27eaae890ca8ac20d718 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 6 Apr 2026 16:19:00 -0400 Subject: [PATCH 05/22] Add resolve_commit and repair_worktrees to git layer - resolve_commit on Repository entity: wraps revparse_batch to check if a commit SHA exists in the repo. Used during restore to verify original_commit_hash is present before attempting recovery. - repair_worktrees on GitRepository trait + RealGitRepository (runs git worktree repair) and FakeGitRepository (no-op). Used during restore when a worktree directory exists on disk but may not be in git's worktree metadata. - repair_worktrees wrapper on Repository entity. --- crates/fs/src/fake_git_repo.rs | 1 + crates/git/src/repository.rs | 2 ++ crates/project/src/git_store.rs | 27 +++++++++++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 926fe6a7e46767..e76793181169e1 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -1406,6 +1406,7 @@ impl GitRepository for FakeGitRepository { fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>> { async { Ok(()) }.boxed() } + fn set_trusted(&self, trusted: bool) { self.is_trusted .store(trusted, std::sync::atomic::Ordering::Release); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 14ea541e1465e1..90b93cf82b4183 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -972,6 +972,7 @@ pub trait GitRepository: Send + Sync { fn delete_ref(&self, ref_name: String) -> BoxFuture<'_, Result<()>>; fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; + fn set_trusted(&self, trusted: bool); fn is_trusted(&self) -> bool; } @@ -2281,6 +2282,7 @@ impl GitRepository for RealGitRepository { }) .boxed() } + fn push( &self, branch_name: String, diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index eb40c19a2d7960..fc5ac9a31798d5 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6159,6 +6159,33 @@ impl Repository { } }) } + + pub fn resolve_commit(&mut self, sha: String) -> oneshot::Receiver> { + self.send_job(None, move |repo, _cx| async move { + match repo { + RepositoryState::Local(LocalRepositoryState { backend, .. }) => { + let results = backend.revparse_batch(vec![sha]).await?; + Ok(results.into_iter().next().flatten().is_some()) + } + RepositoryState::Remote(_) => { + anyhow::bail!("resolve_commit is not supported for remote repositories") + } + } + }) + } + + pub fn repair_worktrees(&mut self) -> oneshot::Receiver> { + self.send_job(None, move |repo, _cx| async move { + match repo { + RepositoryState::Local(LocalRepositoryState { backend, .. }) => { + backend.repair_worktrees().await + } + RepositoryState::Remote(_) => { + anyhow::bail!("repair_worktrees is not supported for remote repositories") + } + } + }) + } pub fn remove_worktree(&mut self, path: PathBuf, force: bool) -> oneshot::Receiver> { let id = self.id; self.send_job( From 7978e5c52b437417196f25f96023ab9bc5a0c036 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Mon, 6 Apr 2026 16:59:02 -0400 Subject: [PATCH 06/22] Fix proto migration --- crates/project/src/git_store.rs | 15 +++++++++++++-- crates/proto/proto/git.proto | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index fc5ac9a31798d5..51fc3f5a3be178 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -2408,7 +2408,7 @@ impl GitStore { let repository_id = RepositoryId::from_proto(envelope.payload.repository_id); let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?; let directory = PathBuf::from(envelope.payload.directory); - let name = envelope.payload.name.unwrap_or_default(); + let name = envelope.payload.name; let commit = envelope.payload.commit; let use_existing_branch = envelope.payload.use_existing_branch; let target = if name.is_empty() { @@ -6008,6 +6008,17 @@ impl Repository { target: CreateWorktreeTarget, path: PathBuf, ) -> oneshot::Receiver> { + if matches!( + &start_point, + CreateWorktreeStartPoint::Branched { name } if name.is_empty() + ) { + let (sender, receiver) = oneshot::channel(); + sender + .send(Err(anyhow!("branch name cannot be empty"))) + .ok(); + return receiver; + } + let id = self.id; let job_description = match target.branch_name() { Some(branch_name) => format!("git worktree add: {branch_name}"), @@ -6034,7 +6045,7 @@ impl Repository { .request(proto::GitCreateWorktree { project_id: project_id.0, repository_id: id.to_proto(), - name, + name: name.unwrap_or_default(), directory: path.to_string_lossy().to_string(), commit, use_existing_branch, diff --git a/crates/proto/proto/git.proto b/crates/proto/proto/git.proto index 8511debd20467c..d0a594a2817ec5 100644 --- a/crates/proto/proto/git.proto +++ b/crates/proto/proto/git.proto @@ -591,7 +591,7 @@ message Worktree { message GitCreateWorktree { uint64 project_id = 1; uint64 repository_id = 2; - optional string name = 3; + string name = 3; string directory = 4; optional string commit = 5; bool use_existing_branch = 6; From d815b0ae58852449fb3fcc4f2b65283c66e2c7e7 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 6 Apr 2026 16:19:00 -0400 Subject: [PATCH 07/22] Add resolve_commit and repair_worktrees to git layer - resolve_commit on Repository entity: wraps revparse_batch to check if a commit SHA exists in the repo. Used during restore to verify original_commit_hash is present before attempting recovery. - repair_worktrees on GitRepository trait + RealGitRepository (runs git worktree repair) and FakeGitRepository (no-op). Used during restore when a worktree directory exists on disk but may not be in git's worktree metadata. - repair_worktrees wrapper on Repository entity. --- crates/fs/src/fake_git_repo.rs | 4 ++++ crates/git/src/repository.rs | 13 +++++++++++++ crates/project/src/git_store.rs | 4 ++-- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index e76793181169e1..a7e76f920d3c82 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -1407,6 +1407,10 @@ impl GitRepository for FakeGitRepository { async { Ok(()) }.boxed() } + fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>> { + async { Ok(()) }.boxed() + } + fn set_trusted(&self, trusted: bool) { self.is_trusted .store(trusted, std::sync::atomic::Ordering::Release); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 90b93cf82b4183..cda11a3be71aa7 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -973,6 +973,8 @@ pub trait GitRepository: Send + Sync { fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; + fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; + fn set_trusted(&self, trusted: bool); fn is_trusted(&self) -> bool; } @@ -2283,6 +2285,17 @@ impl GitRepository for RealGitRepository { .boxed() } + fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>> { + let git_binary = self.git_binary(); + self.executor + .spawn(async move { + let args: Vec = vec!["worktree".into(), "repair".into()]; + git_binary?.run(&args).await?; + Ok(()) + }) + .boxed() + } + fn push( &self, branch_name: String, diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 51fc3f5a3be178..9dd9dc2e53b762 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6171,7 +6171,7 @@ impl Repository { }) } - pub fn resolve_commit(&mut self, sha: String) -> oneshot::Receiver> { + pub fn commit_exists(&mut self, sha: String) -> oneshot::Receiver> { self.send_job(None, move |repo, _cx| async move { match repo { RepositoryState::Local(LocalRepositoryState { backend, .. }) => { @@ -6179,7 +6179,7 @@ impl Repository { Ok(results.into_iter().next().flatten().is_some()) } RepositoryState::Remote(_) => { - anyhow::bail!("resolve_commit is not supported for remote repositories") + anyhow::bail!("commit_exists is not supported for remote repositories") } } }) From 4107dafe7b370c81589b75ceaf7a7ecf0f877b77 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Mon, 6 Apr 2026 17:07:57 -0400 Subject: [PATCH 08/22] dedup repair_worktrees calls --- crates/fs/src/fake_git_repo.rs | 4 ---- crates/git/src/repository.rs | 13 ------------- crates/project/src/git_store.rs | 12 ------------ 3 files changed, 29 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index a7e76f920d3c82..e76793181169e1 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -1407,10 +1407,6 @@ impl GitRepository for FakeGitRepository { async { Ok(()) }.boxed() } - fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>> { - async { Ok(()) }.boxed() - } - fn set_trusted(&self, trusted: bool) { self.is_trusted .store(trusted, std::sync::atomic::Ordering::Release); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index cda11a3be71aa7..90b93cf82b4183 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -973,8 +973,6 @@ pub trait GitRepository: Send + Sync { fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; - fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; - fn set_trusted(&self, trusted: bool); fn is_trusted(&self) -> bool; } @@ -2285,17 +2283,6 @@ impl GitRepository for RealGitRepository { .boxed() } - fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>> { - let git_binary = self.git_binary(); - self.executor - .spawn(async move { - let args: Vec = vec!["worktree".into(), "repair".into()]; - git_binary?.run(&args).await?; - Ok(()) - }) - .boxed() - } - fn push( &self, branch_name: String, diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 9dd9dc2e53b762..1759bbb5fed814 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6185,18 +6185,6 @@ impl Repository { }) } - pub fn repair_worktrees(&mut self) -> oneshot::Receiver> { - self.send_job(None, move |repo, _cx| async move { - match repo { - RepositoryState::Local(LocalRepositoryState { backend, .. }) => { - backend.repair_worktrees().await - } - RepositoryState::Remote(_) => { - anyhow::bail!("repair_worktrees is not supported for remote repositories") - } - } - }) - } pub fn remove_worktree(&mut self, path: PathBuf, force: bool) -> oneshot::Receiver> { let id = self.id; self.send_job( From 59b87fd392e882d3ac84ab9546c867c0cee64ae5 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 6 Apr 2026 10:42:46 -0400 Subject: [PATCH 09/22] Add ArchivedGitWorktree data model and DB operations Add the persistence layer for tracking archived git worktrees: - ArchivedGitWorktree struct with staged_commit_hash and unstaged_commit_hash fields to precisely identify WIP commits - DB migrations for archived_git_worktrees and thread_archived_worktrees (join table) tables - CRUD operations: create, link to thread, query by thread, delete - Column impl for deserializing ArchivedGitWorktree from DB rows - Tests for create/retrieve with distinct SHAs, delete cascading through join table, multi-thread linking, and multiple worktrees per thread --- crates/agent_ui/src/thread_metadata_store.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 83cf5b3e3eae27..6ede0aeeb889d5 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -178,7 +178,6 @@ pub struct ArchivedGitWorktree { /// repo) and as a fallback target if the WIP resets fail. pub original_commit_hash: String, } - /// The store holds all metadata needed to show threads in the sidebar/the archive. /// /// Automatically listens to AcpThread events and updates metadata if it has changed. @@ -1051,7 +1050,6 @@ impl Column for ArchivedGitWorktree { )) } } - #[cfg(test)] mod tests { use super::*; From 24b1cad28c3d0e713593fdd278db85ba3314ff4a Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 6 Apr 2026 10:43:22 -0400 Subject: [PATCH 10/22] Wire up worktree archival on thread archive and restoration on unarchive Connect the git API and archived worktree data model to the sidebar's archive/unarchive flow: - Add thread_worktree_archive module: orchestrates the full archive cycle (WIP commits, DB records, git refs, worktree deletion) and restore cycle (detached worktree creation, reset to recover staged/unstaged state, branch restoration) - Integrate into sidebar: archive_thread now persists worktree state before cleanup; activate_archived_thread restores worktrees via git with targeted path replacement for multi-root threads - Show toast on restore failure instead of silent log - Deserialize persisted project_group_keys on window restore - Guard cleanup_empty_workspaces against dropped entities - Await rollback DB operations instead of fire-and-forget - If worktree already exists on disk when unarchiving, reuse it as-is instead of auto-generating a new path --- crates/agent_ui/src/thread_metadata_store.rs | 167 +++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 6ede0aeeb889d5..2e5c70609aef1f 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -452,6 +452,28 @@ impl ThreadMetadataStore { } } + pub fn complete_worktree_restore( + &mut self, + session_id: &acp::SessionId, + path_replacements: &[(PathBuf, PathBuf)], + cx: &mut Context, + ) { + if let Some(thread) = self.threads.get(session_id).cloned() { + 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(); + } + } + let new_folder_paths = PathList::new(&paths); + self.save_internal(ThreadMetadata { + folder_paths: new_folder_paths, + ..thread + }); + cx.notify(); + } + } + pub fn create_archived_worktree( &self, worktree_path: String, @@ -527,6 +549,15 @@ impl ThreadMetadataStore { }) } + pub fn all_session_ids_for_path<'a>( + &'a self, + path_list: &PathList, + ) -> impl Iterator { + self.threads_by_paths + .get(path_list) + .into_iter() + .flat_map(|session_ids| session_ids.iter()) + } fn update_archived( &mut self, session_id: &acp::SessionId, @@ -2317,6 +2348,142 @@ mod tests { assert_eq!(wt1[0].id, wt2[0].id); } + // Verifies that all_session_ids_for_path returns both archived and + // unarchived threads. This is intentional: the method is used during + // archival to find every thread referencing a worktree so they can + // all be linked to the archived worktree record. + #[gpui::test] + async fn test_all_session_ids_for_path(cx: &mut TestAppContext) { + init_test(cx); + let store = cx.update(|cx| ThreadMetadataStore::global(cx)); + let paths = PathList::new(&[Path::new("/project-x")]); + + let meta1 = ThreadMetadata { + session_id: acp::SessionId::new("session-1"), + agent_id: agent::ZED_AGENT_ID.clone(), + title: "Thread 1".into(), + updated_at: Utc::now(), + created_at: Some(Utc::now()), + folder_paths: paths.clone(), + main_worktree_paths: PathList::default(), + archived: false, + }; + let meta2 = ThreadMetadata { + session_id: acp::SessionId::new("session-2"), + agent_id: agent::ZED_AGENT_ID.clone(), + title: "Thread 2".into(), + updated_at: Utc::now(), + created_at: Some(Utc::now()), + folder_paths: paths.clone(), + main_worktree_paths: PathList::default(), + archived: true, + }; + + store.update(cx, |store, _cx| { + store.save_internal(meta1); + store.save_internal(meta2); + }); + + let ids: HashSet = store.read_with(cx, |store, _cx| { + store.all_session_ids_for_path(&paths).cloned().collect() + }); + + assert!(ids.contains(&acp::SessionId::new("session-1"))); + assert!(ids.contains(&acp::SessionId::new("session-2"))); + assert_eq!(ids.len(), 2); + } + + #[gpui::test] + async fn test_complete_worktree_restore_multiple_paths(cx: &mut TestAppContext) { + init_test(cx); + let store = cx.update(|cx| ThreadMetadataStore::global(cx)); + + let original_paths = PathList::new(&[ + Path::new("/projects/worktree-a"), + Path::new("/projects/worktree-b"), + Path::new("/other/unrelated"), + ]); + let meta = make_metadata("session-multi", "Multi Thread", Utc::now(), original_paths); + + store.update(cx, |store, cx| { + store.save_manually(meta, cx); + }); + + let replacements = vec![ + ( + PathBuf::from("/projects/worktree-a"), + PathBuf::from("/restored/worktree-a"), + ), + ( + PathBuf::from("/projects/worktree-b"), + PathBuf::from("/restored/worktree-b"), + ), + ]; + + store.update(cx, |store, cx| { + store.complete_worktree_restore( + &acp::SessionId::new("session-multi"), + &replacements, + cx, + ); + }); + + let entry = store.read_with(cx, |store, _cx| { + store.entry(&acp::SessionId::new("session-multi")).cloned() + }); + let entry = entry.unwrap(); + 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"))); + assert!(paths.contains(&PathBuf::from("/other/unrelated"))); + } + + #[gpui::test] + async fn test_complete_worktree_restore_preserves_unmatched_paths(cx: &mut TestAppContext) { + init_test(cx); + let store = cx.update(|cx| ThreadMetadataStore::global(cx)); + + let original_paths = + PathList::new(&[Path::new("/projects/worktree-a"), Path::new("/other/path")]); + let meta = make_metadata("session-partial", "Partial", Utc::now(), original_paths); + + store.update(cx, |store, cx| { + store.save_manually(meta, cx); + }); + + let replacements = vec![ + ( + PathBuf::from("/projects/worktree-a"), + PathBuf::from("/new/worktree-a"), + ), + ( + PathBuf::from("/nonexistent/path"), + PathBuf::from("/should/not/appear"), + ), + ]; + + store.update(cx, |store, cx| { + store.complete_worktree_restore( + &acp::SessionId::new("session-partial"), + &replacements, + cx, + ); + }); + + let entry = store.read_with(cx, |store, _cx| { + store + .entry(&acp::SessionId::new("session-partial")) + .cloned() + }); + let entry = entry.unwrap(); + 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"))); + assert!(!paths.contains(&PathBuf::from("/should/not/appear"))); + } + #[gpui::test] async fn test_update_restored_worktree_paths_multiple(cx: &mut TestAppContext) { init_test(cx); From a1587ec5b246aa5d8ca44a5b5b33f750ad4f0766 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 6 Apr 2026 16:19:36 -0400 Subject: [PATCH 11/22] Wire up original_commit_hash in archive and restore flows persist_worktree_state: - Read HEAD SHA before creating WIP commits as original_commit_hash - Pass it to create_archived_worktree restore_worktree_via_git: - Pre-restore: verify original_commit_hash exists via resolve_commit; abort with user-facing error if the git history is gone - Worktree-already-exists: check for .git file to detect if path is a real git worktree; if not, call repair_worktrees to adopt it - Resilient WIP resets: track success of mixed and soft resets independently; if either fails, fall back to mixed reset directly to original_commit_hash - Post-reset HEAD verification: confirm HEAD landed at original_commit_hash after all resets - Branch restoration: after switching, verify branch points at original_commit_hash; if it doesn't, reset and create a fresh branch --- crates/fs/src/fake_git_repo.rs | 33 +++++++++++++++++++++++++++++++++ crates/git/src/repository.rs | 14 ++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index e76793181169e1..188638b26950f2 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -1407,6 +1407,39 @@ impl GitRepository for FakeGitRepository { async { Ok(()) }.boxed() } + fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>> { + let workdir_path = self.dot_git_path.parent().unwrap(); + let git_files: Vec<(RepoPath, String)> = self + .fs + .files() + .iter() + .filter_map(|path| { + let repo_path = path.strip_prefix(workdir_path).ok()?; + if repo_path.starts_with(".git") { + return None; + } + let content = self + .fs + .read_file_sync(path) + .ok() + .and_then(|bytes| String::from_utf8(bytes).ok())?; + let rel_path = RelPath::new(repo_path, PathStyle::local()).ok()?; + Some((RepoPath::from_rel_path(&rel_path), content)) + }) + .collect(); + + self.with_state_async(true, move |state| { + let fs_paths: HashSet = git_files.iter().map(|(p, _)| p.clone()).collect(); + for (path, content) in git_files { + state.index_contents.insert(path, content); + } + state + .index_contents + .retain(|path, _| fs_paths.contains(path)); + Ok(()) + }) + } + fn set_trusted(&self, trusted: bool) { self.is_trusted .store(trusted, std::sync::atomic::Ordering::Release); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 90b93cf82b4183..8718af0d04129c 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -973,6 +973,8 @@ pub trait GitRepository: Send + Sync { fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; + fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>>; + fn set_trusted(&self, trusted: bool); fn is_trusted(&self) -> bool; } @@ -2283,6 +2285,18 @@ impl GitRepository for RealGitRepository { .boxed() } + fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>> { + let git_binary = self.git_binary(); + self.executor + .spawn(async move { + let args: Vec = + vec!["--no-optional-locks".into(), "add".into(), "-A".into()]; + git_binary?.run(&args).await?; + Ok(()) + }) + .boxed() + } + fn push( &self, branch_name: String, From 146b8529a1f4b9b579319f8a302650ff15ae2b92 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 7 Apr 2026 19:26:40 -0400 Subject: [PATCH 12/22] Wire up Sidebar as archive orchestrator with cancellation support Complete the thread archival refactor by implementing Phases 3-4: Phase 3: Sidebar::archive_worktree (the executor) - New async method that safely tears down workspaces, git state, and FS paths - Prompts user to save/discard dirty items, racing against cancellation - Closes workspace via MultiWorkspace::remove while retaining Project ref - Iterates over roots: persists git state then removes worktrees - Full rollback support: if any step fails or is cancelled, all completed persists are rolled back in reverse order Phase 4: Sidebar::archive_thread (the orchestrator) - Reads thread metadata to determine which roots need cleanup - Filters out roots still referenced by other unarchived threads - Creates cancel channel and spawns archive_worktree as background task - Passes (Task, Sender) to ThreadMetadataStore::archive for tracking - On success: cleans up completed archive entry - On user cancel or error: automatically unarchives thread - Preserves all existing focus management code Additional fixes: - Simplify ThreadMetadataStore::archive to accept pre-built (Task, Sender) instead of generic closure, removing unnecessary type parameters - Fix pre-existing workspaces() API change (iterator vs slice) - Fix pre-existing resolve_commit removal (deleted dead code path) - Remove unused window_for_workspace helpers - Add ArchiveStatus enum for distinguishing success vs user cancellation --- Cargo.lock | 1 + crates/sidebar/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index fdd1a67b752a64..c3e942cfbf3823 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16080,6 +16080,7 @@ dependencies = [ "chrono", "editor", "fs", + "futures 0.3.32", "git", "gpui", "language_model", diff --git a/crates/sidebar/Cargo.toml b/crates/sidebar/Cargo.toml index f1e099b2303a4e..47cd93f42a0d2d 100644 --- a/crates/sidebar/Cargo.toml +++ b/crates/sidebar/Cargo.toml @@ -25,6 +25,7 @@ anyhow.workspace = true chrono.workspace = true editor.workspace = true fs.workspace = true +futures.workspace = true git.workspace = true gpui.workspace = true log.workspace = true From 86a1c4d785532b18a7ac013b4b44ea21965f99fb Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 7 Apr 2026 22:47:36 -0400 Subject: [PATCH 13/22] Replace smol channels with futures equivalents Use futures::channel::oneshot for the archive cancellation signal and futures::channel::mpsc::unbounded for the DB operations queue, replacing smol::channel usage. Remove the smol dependency from the sidebar crate. --- Cargo.lock | 1 - crates/sidebar/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3e942cfbf3823..2f39fede24deac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16095,7 +16095,6 @@ dependencies = [ "serde", "serde_json", "settings", - "smol", "theme", "theme_settings", "ui", diff --git a/crates/sidebar/Cargo.toml b/crates/sidebar/Cargo.toml index 47cd93f42a0d2d..b30a8bcd076fd9 100644 --- a/crates/sidebar/Cargo.toml +++ b/crates/sidebar/Cargo.toml @@ -37,7 +37,6 @@ remote.workspace = true serde.workspace = true serde_json.workspace = true settings.workspace = true -smol.workspace = true theme.workspace = true theme_settings.workspace = true ui.workspace = true From 05f785389f67f3fe44da9f660d2de9bad39b3079 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 00:59:44 -0400 Subject: [PATCH 14/22] Address code review: remove dead code, fix cancellation checks, add explicit drops --- crates/agent_ui/src/thread_metadata_store.rs | 54 -------------------- 1 file changed, 54 deletions(-) diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 2e5c70609aef1f..e114bcf2afc64e 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -549,15 +549,6 @@ impl ThreadMetadataStore { }) } - pub fn all_session_ids_for_path<'a>( - &'a self, - path_list: &PathList, - ) -> impl Iterator { - self.threads_by_paths - .get(path_list) - .into_iter() - .flat_map(|session_ids| session_ids.iter()) - } fn update_archived( &mut self, session_id: &acp::SessionId, @@ -2348,51 +2339,6 @@ mod tests { assert_eq!(wt1[0].id, wt2[0].id); } - // Verifies that all_session_ids_for_path returns both archived and - // unarchived threads. This is intentional: the method is used during - // archival to find every thread referencing a worktree so they can - // all be linked to the archived worktree record. - #[gpui::test] - async fn test_all_session_ids_for_path(cx: &mut TestAppContext) { - init_test(cx); - let store = cx.update(|cx| ThreadMetadataStore::global(cx)); - let paths = PathList::new(&[Path::new("/project-x")]); - - let meta1 = ThreadMetadata { - session_id: acp::SessionId::new("session-1"), - agent_id: agent::ZED_AGENT_ID.clone(), - title: "Thread 1".into(), - updated_at: Utc::now(), - created_at: Some(Utc::now()), - folder_paths: paths.clone(), - main_worktree_paths: PathList::default(), - archived: false, - }; - let meta2 = ThreadMetadata { - session_id: acp::SessionId::new("session-2"), - agent_id: agent::ZED_AGENT_ID.clone(), - title: "Thread 2".into(), - updated_at: Utc::now(), - created_at: Some(Utc::now()), - folder_paths: paths.clone(), - main_worktree_paths: PathList::default(), - archived: true, - }; - - store.update(cx, |store, _cx| { - store.save_internal(meta1); - store.save_internal(meta2); - }); - - let ids: HashSet = store.read_with(cx, |store, _cx| { - store.all_session_ids_for_path(&paths).cloned().collect() - }); - - assert!(ids.contains(&acp::SessionId::new("session-1"))); - assert!(ids.contains(&acp::SessionId::new("session-2"))); - assert_eq!(ids.len(), 2); - } - #[gpui::test] async fn test_complete_worktree_restore_multiple_paths(cx: &mut TestAppContext) { init_test(cx); From b796d7c71cefbfd4b90eed751d3188385ca64a20 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 11:01:05 -0400 Subject: [PATCH 15/22] Fix CI: remove redundant clone and unused futures dependency --- Cargo.lock | 1 - crates/sidebar/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f39fede24deac..cf8a337d6a1619 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16080,7 +16080,6 @@ dependencies = [ "chrono", "editor", "fs", - "futures 0.3.32", "git", "gpui", "language_model", diff --git a/crates/sidebar/Cargo.toml b/crates/sidebar/Cargo.toml index b30a8bcd076fd9..9b6ef4a6244e0a 100644 --- a/crates/sidebar/Cargo.toml +++ b/crates/sidebar/Cargo.toml @@ -25,7 +25,6 @@ anyhow.workspace = true chrono.workspace = true editor.workspace = true fs.workspace = true -futures.workspace = true git.workspace = true gpui.workspace = true log.workspace = true From eb72c74e5b5af6cabfd4f9bc4932a513ce07d565 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 13:26:31 -0400 Subject: [PATCH 16/22] Restore worktree deletion when archiving the last thread Rewire sidebar thread archival to start the in-flight worktree archive task again, so archiving the last thread for a linked worktree removes the worktree from disk instead of only updating sidebar state. This also makes root planning and worktree-state persistence more robust when a live linked-worktree repository handle is not already available, and strengthens the sidebar test to assert the worktree directory is actually deleted. --- Cargo.lock | 1 + crates/sidebar/Cargo.toml | 1 + crates/sidebar/src/sidebar_tests.rs | 12 ++++++++++++ 3 files changed, 14 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index cf8a337d6a1619..2f39fede24deac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16080,6 +16080,7 @@ dependencies = [ "chrono", "editor", "fs", + "futures 0.3.32", "git", "gpui", "language_model", diff --git a/crates/sidebar/Cargo.toml b/crates/sidebar/Cargo.toml index 9b6ef4a6244e0a..b30a8bcd076fd9 100644 --- a/crates/sidebar/Cargo.toml +++ b/crates/sidebar/Cargo.toml @@ -25,6 +25,7 @@ anyhow.workspace = true chrono.workspace = true editor.workspace = true fs.workspace = true +futures.workspace = true git.workspace = true gpui.workspace = true log.workspace = true diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 8f4745a3ee3686..66eac4c57389d1 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -4316,6 +4316,18 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon cx.run_until_parked(); cx.run_until_parked(); + // The workspace removal is immediate, but the on-disk cleanup runs in a + // background archive task. Give the task time to finish. + for _ in 0..10 { + if !fs.is_dir(Path::new("/wt-feature-a")).await { + break; + } + cx.run_until_parked(); + cx.background_executor + .timer(std::time::Duration::from_millis(10)) + .await; + } + // The linked worktree workspace should have been removed. assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), From b9f5351a0e6c3a16323215663103d7cf75420124 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 13:42:09 -0400 Subject: [PATCH 17/22] Revert channels from futures back to smol Restore smol::channel for the DB operations queue and archive cancellation signal, reverting the futures::channel change from e1c73e7b72. smol channels provide bounded backpressure and are a better fit here. --- Cargo.lock | 2 +- crates/sidebar/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f39fede24deac..fdd1a67b752a64 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16080,7 +16080,6 @@ dependencies = [ "chrono", "editor", "fs", - "futures 0.3.32", "git", "gpui", "language_model", @@ -16095,6 +16094,7 @@ dependencies = [ "serde", "serde_json", "settings", + "smol", "theme", "theme_settings", "ui", diff --git a/crates/sidebar/Cargo.toml b/crates/sidebar/Cargo.toml index b30a8bcd076fd9..f1e099b2303a4e 100644 --- a/crates/sidebar/Cargo.toml +++ b/crates/sidebar/Cargo.toml @@ -25,7 +25,6 @@ anyhow.workspace = true chrono.workspace = true editor.workspace = true fs.workspace = true -futures.workspace = true git.workspace = true gpui.workspace = true log.workspace = true @@ -37,6 +36,7 @@ remote.workspace = true serde.workspace = true serde_json.workspace = true settings.workspace = true +smol.workspace = true theme.workspace = true theme_settings.workspace = true ui.workspace = true From 0c851eb53a0c3a8fc28bede458c27daeebab0162 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 16:51:29 -0400 Subject: [PATCH 18/22] Use detached commits for worktree archival instead of moving branches Replace the old approach (git commit + git reset) with detached commits via write-tree + commit-tree. This means the branch is never moved during archival, eliminating the bug where WIP commits remained on the branch after unarchive because git switch moved HEAD back to the branch tip. Archive: create_archive_checkpoint writes the current index tree and a temp-index full tree as two detached commit objects, without touching HEAD or the branch ref. Restore: create worktree at original_commit_hash (branch unchanged), switch to branch (no-op since it still points there), then read-tree + git restore to reconstruct staged/unstaged state from the WIP trees. Rollback: simplified to just deleting the git ref and DB record, since no branch was moved. --- .../agent_ui/src/thread_worktree_archive.rs | 320 +++--------------- crates/project/src/git_store.rs | 32 ++ 2 files changed, 84 insertions(+), 268 deletions(-) diff --git a/crates/agent_ui/src/thread_worktree_archive.rs b/crates/agent_ui/src/thread_worktree_archive.rs index 732519e25376dc..8ed812c8e43d81 100644 --- a/crates/agent_ui/src/thread_worktree_archive.rs +++ b/crates/agent_ui/src/thread_worktree_archive.rs @@ -5,7 +5,6 @@ use std::{ use agent_client_protocol as acp; use anyhow::{Context as _, Result, anyhow}; -use git::repository::{AskPassDelegate, CommitOptions, ResetMode}; use gpui::{App, AsyncApp, Entity, Task}; use project::{ LocalProjectFlags, Project, WorktreeId, @@ -363,128 +362,38 @@ async fn rollback_root(root: &RootPlan, cx: &mut AsyncApp) { /// Saves the worktree's full git state so it can be restored later. /// -/// This is a multi-step operation: -/// 1. Records the original HEAD SHA. -/// 2. Creates WIP commit #1 ("staged") capturing the current index. -/// 3. Stages everything including untracked files, then creates WIP commit -/// #2 ("unstaged") capturing the full working directory. -/// 4. Creates a DB record (`ArchivedGitWorktree`) with all the SHAs, the -/// branch name, and both paths. -/// 5. Links every thread that references this worktree to the DB record. -/// 6. Creates a git ref (`refs/archived-worktrees/`) on the main repo -/// pointing at the unstaged commit, preventing git from -/// garbage-collecting the WIP commits after the worktree is deleted. +/// This creates two detached commits (via [`create_archive_checkpoint`] on +/// the `GitRepository` trait) that capture the staged and unstaged state +/// without moving any branch ref. The commits are: +/// - "WIP staged": a tree matching the current index, parented on HEAD +/// - "WIP unstaged": a tree with all files (including untracked), +/// parented on the staged commit /// -/// Each step has rollback logic: if step N fails, steps 1..N-1 are undone. -/// On success, returns a [`PersistOutcome`] that can be passed to -/// [`rollback_persist`] if a later step in the archival pipeline fails. +/// After creating the commits, this function: +/// 1. Records the commit SHAs, branch name, and paths in a DB record. +/// 2. Links every thread referencing this worktree to that record. +/// 3. Creates a git ref on the main repo to prevent GC of the commits. +/// +/// On success, returns a [`PersistOutcome`] for rollback if needed. pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Result { let (worktree_repo, _temp_worktree_project) = match &root.worktree_repo { Some(worktree_repo) => (worktree_repo.clone(), None), None => find_or_create_repository(&root.root_path, cx).await?, }; - // Read original HEAD SHA before creating any WIP commits let original_commit_hash = worktree_repo .update(cx, |repo, _cx| repo.head_sha()) .await .map_err(|_| anyhow!("head_sha canceled"))? .context("failed to read original HEAD SHA")? - .context("HEAD SHA is None before WIP commits")?; + .context("HEAD SHA is None")?; - // Create WIP commit #1 (staged state) - let askpass = AskPassDelegate::new(cx, |_, _, _| {}); - let commit_rx = worktree_repo.update(cx, |repo, cx| { - repo.commit( - "WIP staged".into(), - None, - CommitOptions { - allow_empty: true, - ..Default::default() - }, - askpass, - cx, - ) - }); - commit_rx + // Create two detached WIP commits without moving the branch. + let checkpoint_rx = worktree_repo.update(cx, |repo, _cx| repo.create_archive_checkpoint()); + let (staged_commit_hash, unstaged_commit_hash) = checkpoint_rx .await - .map_err(|_| anyhow!("WIP staged commit canceled"))??; - - // Read SHA after staged commit - let staged_sha_result = worktree_repo - .update(cx, |repo, _cx| repo.head_sha()) - .await - .map_err(|_| anyhow!("head_sha canceled")) - .and_then(|r| r.context("failed to read HEAD SHA after staged commit")) - .and_then(|opt| opt.context("HEAD SHA is None after staged commit")); - let staged_commit_hash = match staged_sha_result { - Ok(sha) => sha, - Err(error) => { - let rx = worktree_repo.update(cx, |repo, cx| { - repo.reset("HEAD~1".to_string(), ResetMode::Mixed, cx) - }); - rx.await.ok().and_then(|r| r.log_err()); - return Err(error); - } - }; - - // Stage all files including untracked - let stage_rx = worktree_repo.update(cx, |repo, _cx| repo.stage_all_including_untracked()); - if let Err(error) = stage_rx - .await - .map_err(|_| anyhow!("stage all canceled")) - .and_then(|inner| inner) - { - let rx = worktree_repo.update(cx, |repo, cx| { - repo.reset("HEAD~1".to_string(), ResetMode::Mixed, cx) - }); - rx.await.ok().and_then(|r| r.log_err()); - return Err(error.context("failed to stage all files including untracked")); - } - - // Create WIP commit #2 (unstaged/untracked state) - let askpass = AskPassDelegate::new(cx, |_, _, _| {}); - let commit_rx = worktree_repo.update(cx, |repo, cx| { - repo.commit( - "WIP unstaged".into(), - None, - CommitOptions { - allow_empty: true, - ..Default::default() - }, - askpass, - cx, - ) - }); - if let Err(error) = commit_rx - .await - .map_err(|_| anyhow!("WIP unstaged commit canceled")) - .and_then(|inner| inner) - { - let rx = worktree_repo.update(cx, |repo, cx| { - repo.reset("HEAD~1".to_string(), ResetMode::Mixed, cx) - }); - rx.await.ok().and_then(|r| r.log_err()); - return Err(error); - } - - // Read HEAD SHA after WIP commits - let head_sha_result = worktree_repo - .update(cx, |repo, _cx| repo.head_sha()) - .await - .map_err(|_| anyhow!("head_sha canceled")) - .and_then(|r| r.context("failed to read HEAD SHA after WIP commits")) - .and_then(|opt| opt.context("HEAD SHA is None after WIP commits")); - let unstaged_commit_hash = match head_sha_result { - Ok(sha) => sha, - Err(error) => { - let rx = worktree_repo.update(cx, |repo, cx| { - repo.reset(format!("{}~1", staged_commit_hash), ResetMode::Mixed, cx) - }); - rx.await.ok().and_then(|r| r.log_err()); - return Err(error); - } - }; + .map_err(|_| anyhow!("create_archive_checkpoint canceled"))? + .context("failed to create archive checkpoint")?; // Create DB record let store = cx.update(|cx| ThreadMetadataStore::global(cx)); @@ -516,10 +425,6 @@ pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Resul let archived_worktree_id = match db_result { Ok(id) => id, Err(error) => { - let rx = worktree_repo.update(cx, |repo, cx| { - repo.reset(format!("{}~1", staged_commit_hash), ResetMode::Mixed, cx) - }); - rx.await.ok().and_then(|r| r.log_err()); return Err(error); } }; @@ -550,25 +455,17 @@ pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Resul }) .await; if let Err(error) = link_result { - if let Err(delete_error) = store + store .read_with(cx, |store, cx| { store.delete_archived_worktree(archived_worktree_id, cx) }) .await - { - log::error!( - "Failed to delete archived worktree DB record during link rollback: {delete_error:#}" - ); - } - let rx = worktree_repo.update(cx, |repo, cx| { - repo.reset(format!("{}~1", staged_commit_hash), ResetMode::Mixed, cx) - }); - rx.await.ok().and_then(|r| r.log_err()); + .log_err(); return Err(error.context("failed to link thread to archived worktree")); } } - // Create git ref on main repo (non-fatal) + // Create git ref on main repo to prevent GC (non-fatal) let ref_name = archived_worktree_ref_name(archived_worktree_id); let main_repo_result = find_or_create_repository(&root.main_repo_path, cx).await; match main_repo_result { @@ -586,7 +483,6 @@ pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Resul ref_name ); } - // Keep _temp_project alive until after the await so the headless project isn't dropped mid-operation drop(_temp_project); } Err(error) => { @@ -603,22 +499,11 @@ pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Resul }) } -/// Undoes a successful [`persist_worktree_state`] by resetting the WIP -/// commits, deleting the git ref on the main repo, and removing the DB -/// record. +/// Undoes a successful [`persist_worktree_state`] by deleting the git ref +/// on the main repo and removing the DB record. Since the WIP commits are +/// detached (they don't move any branch), no git reset is needed — the +/// commits will be garbage-collected once the ref is removed. pub async fn rollback_persist(outcome: &PersistOutcome, root: &RootPlan, cx: &mut AsyncApp) { - // Undo WIP commits on the worktree repo - if let Some(worktree_repo) = &root.worktree_repo { - let rx = worktree_repo.update(cx, |repo, cx| { - repo.reset( - format!("{}~1", outcome.staged_commit_hash), - ResetMode::Mixed, - cx, - ) - }); - rx.await.ok().and_then(|r| r.log_err()); - } - // Delete the git ref on main repo if let Ok((main_repo, _temp_project)) = find_or_create_repository(&root.main_repo_path, cx).await @@ -626,7 +511,6 @@ pub async fn rollback_persist(outcome: &PersistOutcome, root: &RootPlan, cx: &mu let ref_name = archived_worktree_ref_name(outcome.archived_worktree_id); let rx = main_repo.update(cx, |repo, _cx| repo.delete_ref(ref_name)); rx.await.ok().and_then(|r| r.log_err()); - // Keep _temp_project alive until after the await so the headless project isn't dropped mid-operation drop(_temp_project); } @@ -644,17 +528,16 @@ pub async fn rollback_persist(outcome: &PersistOutcome, root: &RootPlan, cx: &mu /// Restores a previously archived worktree back to disk from its DB record. /// -/// Re-creates the git worktree (or adopts an existing directory), resets -/// past the two WIP commits to recover the original working directory -/// state, verifies HEAD matches the expected commit, and restores the -/// original branch if one was recorded. +/// Creates the git worktree at the original commit (the branch never moved +/// during archival since WIP commits are detached), switches to the branch, +/// then uses [`restore_archive_checkpoint`] to reconstruct the staged/ +/// unstaged state from the WIP commit trees. pub async fn restore_worktree_via_git( row: &ArchivedGitWorktree, cx: &mut AsyncApp, ) -> Result { let (main_repo, _temp_project) = find_or_create_repository(&row.main_repo_path, cx).await?; - // Check if worktree path already exists on disk let worktree_path = &row.worktree_path; let app_state = current_app_state(cx).context("no app state available")?; let already_exists = app_state.fs.metadata(worktree_path).await?.is_some(); @@ -666,158 +549,59 @@ pub async fn restore_worktree_via_git( .is_some(); if is_git_worktree { - // Already a git worktree — another thread on the same worktree - // already restored it. Reuse as-is. return Ok(worktree_path.clone()); } - // Path exists but isn't a git worktree. Ask git to adopt it. let rx = main_repo.update(cx, |repo, _cx| repo.repair_worktrees()); rx.await .map_err(|_| anyhow!("worktree repair was canceled"))? .context("failed to repair worktrees")?; } else { - // Create detached worktree at the unstaged commit + // Create worktree at the original commit — the branch still points + // here because archival used detached commits. let rx = main_repo.update(cx, |repo, _cx| { - repo.create_worktree_detached(worktree_path.clone(), row.unstaged_commit_hash.clone()) + repo.create_worktree_detached(worktree_path.clone(), row.original_commit_hash.clone()) }); rx.await .map_err(|_| anyhow!("worktree creation was canceled"))? .context("failed to create worktree")?; } - // Get the worktree's repo entity let (wt_repo, _temp_wt_project) = find_or_create_repository(worktree_path, cx).await?; - // Reset past the WIP commits to recover original state - let mixed_reset_ok = { - let rx = wt_repo.update(cx, |repo, cx| { - repo.reset(row.staged_commit_hash.clone(), ResetMode::Mixed, cx) - }); - match rx.await { - Ok(Ok(())) => true, - Ok(Err(error)) => { - log::error!("Mixed reset to staged commit failed: {error:#}"); - false - } - Err(_) => { - log::error!("Mixed reset to staged commit was canceled"); - false - } - } - }; - - let soft_reset_ok = if mixed_reset_ok { - let rx = wt_repo.update(cx, |repo, cx| { - repo.reset(row.original_commit_hash.clone(), ResetMode::Soft, cx) - }); - match rx.await { - Ok(Ok(())) => true, - Ok(Err(error)) => { - log::error!("Soft reset to original commit failed: {error:#}"); - false - } - Err(_) => { - log::error!("Soft reset to original commit was canceled"); - false - } - } - } else { - false - }; - - // If either WIP reset failed, fall back to a mixed reset directly to - // original_commit_hash so we at least land on the right commit. - if !mixed_reset_ok || !soft_reset_ok { - log::warn!( - "WIP reset(s) failed (mixed_ok={mixed_reset_ok}, soft_ok={soft_reset_ok}); \ - falling back to mixed reset to original commit {}", - row.original_commit_hash - ); - let rx = wt_repo.update(cx, |repo, cx| { - repo.reset(row.original_commit_hash.clone(), ResetMode::Mixed, cx) - }); - match rx.await { - Ok(Ok(())) => {} - Ok(Err(error)) => { - return Err(error.context(format!( - "fallback reset to original commit {} also failed", - row.original_commit_hash - ))); - } - Err(_) => { - return Err(anyhow!( - "fallback reset to original commit {} was canceled", - row.original_commit_hash - )); - } - } - } - - // Verify HEAD is at original_commit_hash - let current_head = wt_repo - .update(cx, |repo, _cx| repo.head_sha()) - .await - .map_err(|_| anyhow!("post-restore head_sha was canceled"))? - .context("failed to read HEAD after restore")? - .context("HEAD is None after restore")?; - - if current_head != row.original_commit_hash { - anyhow::bail!( - "After restore, HEAD is at {current_head} but expected {}. \ - The worktree may be in an inconsistent state.", - row.original_commit_hash - ); - } - - // Restore the branch + // Switch to the branch. Since the branch was never moved during + // archival (WIP commits are detached), it still points at + // original_commit_hash, so this is essentially a no-op for HEAD. if let Some(branch_name) = &row.branch_name { - // Check if the branch exists and points at original_commit_hash. - // If it does, switch to it. If not, create a new branch there. let rx = wt_repo.update(cx, |repo, _cx| repo.change_branch(branch_name.clone())); - if matches!(rx.await, Ok(Ok(()))) { - // Verify the branch actually points at original_commit_hash after switching - let head_after_switch = wt_repo - .update(cx, |repo, _cx| repo.head_sha()) - .await - .ok() - .and_then(|r| r.ok()) - .flatten(); - - if head_after_switch.as_deref() != Some(&row.original_commit_hash) { - // Branch exists but doesn't point at the right commit. - // Switch back to detached HEAD at original_commit_hash. - log::warn!( - "Branch '{}' exists but points at {:?}, not {}. Creating fresh branch.", - branch_name, - head_after_switch, - row.original_commit_hash - ); - let rx = wt_repo.update(cx, |repo, cx| { - repo.reset(row.original_commit_hash.clone(), ResetMode::Mixed, cx) - }); - rx.await.ok().and_then(|r| r.log_err()); - // Delete the old branch and create fresh - let rx = wt_repo.update(cx, |repo, _cx| { - repo.create_branch(branch_name.clone(), None) - }); - rx.await.ok().and_then(|r| r.log_err()); - } - } else { - // Branch doesn't exist or can't be switched to — create it. + if let Err(_) = rx.await.map_err(|e| anyhow!("{e}")).and_then(|r| r) { let rx = wt_repo.update(cx, |repo, _cx| { repo.create_branch(branch_name.clone(), None) }); - if let Ok(Err(error)) | Err(error) = rx.await.map_err(|e| anyhow::anyhow!("{e}")) { + if let Ok(Err(error)) | Err(error) = rx.await.map_err(|e| anyhow!("{e}")) { log::warn!( "Could not create branch '{}': {error} — \ - restored worktree is in detached HEAD state.", + restored worktree will be in detached HEAD state.", branch_name ); } } } + // Restore the staged/unstaged state from the WIP commit trees. + // read-tree sets the index to the staged commit's tree, and + // git restore puts the unstaged commit's files into the working directory. + let restore_rx = wt_repo.update(cx, |repo, _cx| { + repo.restore_archive_checkpoint( + row.staged_commit_hash.clone(), + row.unstaged_commit_hash.clone(), + ) + }); + restore_rx + .await + .map_err(|_| anyhow!("restore_archive_checkpoint canceled"))? + .context("failed to restore archive checkpoint")?; + Ok(worktree_path.clone()) } diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 1759bbb5fed814..8bc8c3b93d928c 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6529,6 +6529,38 @@ impl Repository { }) } + pub fn create_archive_checkpoint(&mut self) -> oneshot::Receiver> { + self.send_job(None, move |repo, _cx| async move { + match repo { + RepositoryState::Local(LocalRepositoryState { backend, .. }) => { + backend.create_archive_checkpoint().await + } + RepositoryState::Remote(_) => { + anyhow::bail!("archive checkpoints are not supported on remote repositories") + } + } + }) + } + + pub fn restore_archive_checkpoint( + &mut self, + staged_sha: String, + unstaged_sha: String, + ) -> oneshot::Receiver> { + self.send_job(None, move |repo, _cx| async move { + match repo { + RepositoryState::Local(LocalRepositoryState { backend, .. }) => { + backend + .restore_archive_checkpoint(staged_sha, unstaged_sha) + .await + } + RepositoryState::Remote(_) => { + anyhow::bail!("archive checkpoints are not supported on remote repositories") + } + } + }) + } + pub fn restore_checkpoint( &mut self, checkpoint: GitRepositoryCheckpoint, From 0cacec59e518d60eb8e35f0a6cd1d54ab53345fc Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 18:17:22 -0400 Subject: [PATCH 19/22] Fix rebase artifacts: remove duplicate methods and stale validation --- crates/agent_ui/src/thread_metadata_store.rs | 2 + crates/fs/src/fake_git_repo.rs | 1 + crates/project/src/git_store.rs | 43 -------------------- 3 files changed, 3 insertions(+), 43 deletions(-) diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index e114bcf2afc64e..7c07bb683a42a7 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -178,6 +178,7 @@ pub struct ArchivedGitWorktree { /// repo) and as a fallback target if the WIP resets fail. pub original_commit_hash: String, } + /// The store holds all metadata needed to show threads in the sidebar/the archive. /// /// Automatically listens to AcpThread events and updates metadata if it has changed. @@ -1072,6 +1073,7 @@ impl Column for ArchivedGitWorktree { )) } } + #[cfg(test)] mod tests { use super::*; diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 188638b26950f2..f84660b4d2eeea 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -673,6 +673,7 @@ impl GitRepository for FakeGitRepository { } })??; } + Ok(()) } .boxed() diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 8bc8c3b93d928c..92803efb2e6c66 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6008,17 +6008,6 @@ impl Repository { target: CreateWorktreeTarget, path: PathBuf, ) -> oneshot::Receiver> { - if matches!( - &start_point, - CreateWorktreeStartPoint::Branched { name } if name.is_empty() - ) { - let (sender, receiver) = oneshot::channel(); - sender - .send(Err(anyhow!("branch name cannot be empty"))) - .ok(); - return receiver; - } - let id = self.id; let job_description = match target.branch_name() { Some(branch_name) => format!("git worktree add: {branch_name}"), @@ -6529,38 +6518,6 @@ impl Repository { }) } - pub fn create_archive_checkpoint(&mut self) -> oneshot::Receiver> { - self.send_job(None, move |repo, _cx| async move { - match repo { - RepositoryState::Local(LocalRepositoryState { backend, .. }) => { - backend.create_archive_checkpoint().await - } - RepositoryState::Remote(_) => { - anyhow::bail!("archive checkpoints are not supported on remote repositories") - } - } - }) - } - - pub fn restore_archive_checkpoint( - &mut self, - staged_sha: String, - unstaged_sha: String, - ) -> oneshot::Receiver> { - self.send_job(None, move |repo, _cx| async move { - match repo { - RepositoryState::Local(LocalRepositoryState { backend, .. }) => { - backend - .restore_archive_checkpoint(staged_sha, unstaged_sha) - .await - } - RepositoryState::Remote(_) => { - anyhow::bail!("archive checkpoints are not supported on remote repositories") - } - } - }) - } - pub fn restore_checkpoint( &mut self, checkpoint: GitRepositoryCheckpoint, From d2a6eebb5dcea9de9960fad273aea51535adbee3 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 18:48:23 -0400 Subject: [PATCH 20/22] Force-remove worktree directory since detached commits leave it dirty --- crates/agent_ui/src/thread_worktree_archive.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/agent_ui/src/thread_worktree_archive.rs b/crates/agent_ui/src/thread_worktree_archive.rs index 8ed812c8e43d81..e8e83f867e68c1 100644 --- a/crates/agent_ui/src/thread_worktree_archive.rs +++ b/crates/agent_ui/src/thread_worktree_archive.rs @@ -253,8 +253,12 @@ async fn remove_root_after_worktree_removal( } let (repo, _temp_project) = find_or_create_repository(&root.main_repo_path, cx).await?; + // force=true is required because the working directory is still dirty + // — persist_worktree_state captures state into detached commits without + // modifying the real index or working tree, so git refuses to delete + // the worktree without --force. let receiver = repo.update(cx, |repo: &mut Repository, _cx| { - repo.remove_worktree(root.root_path.clone(), false) + repo.remove_worktree(root.root_path.clone(), true) }); let result = receiver .await From 55d3229ec6114f5358a4b6fcb62e1e97197b960f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 18:57:01 -0400 Subject: [PATCH 21/22] Restore explicit error message for archive rollback failure --- crates/agent_ui/src/thread_worktree_archive.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/agent_ui/src/thread_worktree_archive.rs b/crates/agent_ui/src/thread_worktree_archive.rs index e8e83f867e68c1..6c947ea69f083f 100644 --- a/crates/agent_ui/src/thread_worktree_archive.rs +++ b/crates/agent_ui/src/thread_worktree_archive.rs @@ -459,12 +459,17 @@ pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Resul }) .await; if let Err(error) = link_result { - store + if let Err(delete_error) = store .read_with(cx, |store, cx| { store.delete_archived_worktree(archived_worktree_id, cx) }) .await - .log_err(); + { + log::error!( + "Failed to delete archived worktree DB record during link rollback: \ + {delete_error:#}" + ); + } return Err(error.context("failed to link thread to archived worktree")); } } From 058157745a019c5086e30d89f463db03c3184679 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Apr 2026 19:29:44 -0400 Subject: [PATCH 22/22] Address review feedback on detached commit archival - Fix restore_archive_checkpoint to handle file deletions by using read-tree --reset -u for the unstaged tree before a bare read-tree for the staged index - Make git ref creation fatal in persist_worktree_state so archives don't silently lose data when gc runs - Add cleanup logic in restore_worktree_via_git to remove newly-created worktrees if a later step fails - Always run restore_archive_checkpoint even when the worktree directory already exists, so interrupted restores complete on retry - Remove PersistOutcome struct, just pass the i64 DB row ID directly - Log the original change_branch error before falling back to create_branch - Fix .unwrap() in FakeGitRepository::restore_archive_checkpoint - Remove dead stage_all_including_untracked from trait and impls - Remove dead commit_exists from git_store::Repository - Replace fragile 3x run_until_parked + polling loop in sidebar test with a single consolidated retry loop - Replace all matching paths in complete_worktree_restore, not just the first --- crates/agent_ui/src/thread_metadata_store.rs | 6 +- .../agent_ui/src/thread_worktree_archive.rs | 127 +++++++++--------- crates/fs/src/fake_git_repo.rs | 46 ++----- crates/git/src/repository.rs | 31 ++--- crates/project/src/git_store.rs | 14 -- crates/sidebar/src/sidebar.rs | 31 ++--- crates/sidebar/src/sidebar_tests.rs | 21 +-- 7 files changed, 106 insertions(+), 170 deletions(-) diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 7c07bb683a42a7..127f746a9edd35 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -462,8 +462,10 @@ impl ThreadMetadataStore { if let Some(thread) = self.threads.get(session_id).cloned() { 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(); + for path in &mut paths { + if path == old_path { + *path = new_path.clone(); + } } } let new_folder_paths = PathList::new(&paths); diff --git a/crates/agent_ui/src/thread_worktree_archive.rs b/crates/agent_ui/src/thread_worktree_archive.rs index 6c947ea69f083f..86c9fb946a9118 100644 --- a/crates/agent_ui/src/thread_worktree_archive.rs +++ b/crates/agent_ui/src/thread_worktree_archive.rs @@ -71,17 +71,6 @@ fn archived_worktree_ref_name(id: i64) -> String { format!("refs/archived-worktrees/{}", id) } -/// The result of a successful [`persist_worktree_state`] call. -/// -/// Carries exactly the information needed to roll back the persist via -/// [`rollback_persist`]: the DB row ID (to delete the record and the -/// corresponding `refs/archived-worktrees/` git ref) and the staged -/// commit hash (to `git reset` back past both WIP commits). -pub struct PersistOutcome { - pub archived_worktree_id: i64, - pub staged_commit_hash: String, -} - /// Builds a [`RootPlan`] for archiving the git worktree at `path`. /// /// This is a synchronous planning step that must run *before* any workspace @@ -378,8 +367,8 @@ async fn rollback_root(root: &RootPlan, cx: &mut AsyncApp) { /// 2. Links every thread referencing this worktree to that record. /// 3. Creates a git ref on the main repo to prevent GC of the commits. /// -/// On success, returns a [`PersistOutcome`] for rollback if needed. -pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Result { +/// On success, returns the archived worktree DB row ID for rollback. +pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Result { let (worktree_repo, _temp_worktree_project) = match &root.worktree_repo { Some(worktree_repo) => (worktree_repo.clone(), None), None => find_or_create_repository(&root.root_path, cx).await?, @@ -474,50 +463,35 @@ pub async fn persist_worktree_state(root: &RootPlan, cx: &mut AsyncApp) -> Resul } } - // Create git ref on main repo to prevent GC (non-fatal) + // Create git ref on main repo to prevent GC of the detached commits. + // This is fatal: without the ref, git gc will eventually collect the + // WIP commits and a later restore will silently fail. let ref_name = archived_worktree_ref_name(archived_worktree_id); - let main_repo_result = find_or_create_repository(&root.main_repo_path, cx).await; - match main_repo_result { - Ok((main_repo, _temp_project)) => { - let rx = main_repo.update(cx, |repo, _cx| { - repo.update_ref(ref_name.clone(), unstaged_commit_hash.clone()) - }); - if let Err(error) = rx - .await - .map_err(|_| anyhow!("update_ref canceled")) - .and_then(|r| r) - { - log::warn!( - "Failed to create ref {} on main repo (non-fatal): {error}", - ref_name - ); - } - drop(_temp_project); - } - Err(error) => { - log::warn!( - "Could not find main repo to create ref {} (non-fatal): {error}", - ref_name - ); - } - } + let (main_repo, _temp_project) = find_or_create_repository(&root.main_repo_path, cx) + .await + .context("could not open main repo to create archive ref")?; + let rx = main_repo.update(cx, |repo, _cx| { + repo.update_ref(ref_name.clone(), unstaged_commit_hash.clone()) + }); + rx.await + .map_err(|_| anyhow!("update_ref canceled")) + .and_then(|r| r) + .with_context(|| format!("failed to create ref {ref_name} on main repo"))?; + drop(_temp_project); - Ok(PersistOutcome { - archived_worktree_id, - staged_commit_hash, - }) + Ok(archived_worktree_id) } /// Undoes a successful [`persist_worktree_state`] by deleting the git ref /// on the main repo and removing the DB record. Since the WIP commits are /// detached (they don't move any branch), no git reset is needed — the /// commits will be garbage-collected once the ref is removed. -pub async fn rollback_persist(outcome: &PersistOutcome, root: &RootPlan, cx: &mut AsyncApp) { +pub async fn rollback_persist(archived_worktree_id: i64, root: &RootPlan, cx: &mut AsyncApp) { // Delete the git ref on main repo if let Ok((main_repo, _temp_project)) = find_or_create_repository(&root.main_repo_path, cx).await { - let ref_name = archived_worktree_ref_name(outcome.archived_worktree_id); + let ref_name = archived_worktree_ref_name(archived_worktree_id); let rx = main_repo.update(cx, |repo, _cx| repo.delete_ref(ref_name)); rx.await.ok().and_then(|r| r.log_err()); drop(_temp_project); @@ -527,7 +501,7 @@ pub async fn rollback_persist(outcome: &PersistOutcome, root: &RootPlan, cx: &mu let store = cx.update(|cx| ThreadMetadataStore::global(cx)); if let Err(error) = store .read_with(cx, |store, cx| { - store.delete_archived_worktree(outcome.archived_worktree_id, cx) + store.delete_archived_worktree(archived_worktree_id, cx) }) .await { @@ -551,20 +525,19 @@ pub async fn restore_worktree_via_git( let app_state = current_app_state(cx).context("no app state available")?; let already_exists = app_state.fs.metadata(worktree_path).await?.is_some(); - if already_exists { + let created_new_worktree = if already_exists { let is_git_worktree = resolve_git_worktree_to_main_repo(app_state.fs.as_ref(), worktree_path) .await .is_some(); - if is_git_worktree { - return Ok(worktree_path.clone()); + if !is_git_worktree { + let rx = main_repo.update(cx, |repo, _cx| repo.repair_worktrees()); + rx.await + .map_err(|_| anyhow!("worktree repair was canceled"))? + .context("failed to repair worktrees")?; } - - let rx = main_repo.update(cx, |repo, _cx| repo.repair_worktrees()); - rx.await - .map_err(|_| anyhow!("worktree repair was canceled"))? - .context("failed to repair worktrees")?; + false } else { // Create worktree at the original commit — the branch still points // here because archival used detached commits. @@ -574,16 +547,27 @@ pub async fn restore_worktree_via_git( rx.await .map_err(|_| anyhow!("worktree creation was canceled"))? .context("failed to create worktree")?; - } + true + }; - let (wt_repo, _temp_wt_project) = find_or_create_repository(worktree_path, cx).await?; + let (wt_repo, _temp_wt_project) = match find_or_create_repository(worktree_path, cx).await { + Ok(result) => result, + Err(error) => { + remove_new_worktree_on_error(created_new_worktree, &main_repo, worktree_path, cx).await; + return Err(error); + } + }; // Switch to the branch. Since the branch was never moved during // archival (WIP commits are detached), it still points at // original_commit_hash, so this is essentially a no-op for HEAD. if let Some(branch_name) = &row.branch_name { let rx = wt_repo.update(cx, |repo, _cx| repo.change_branch(branch_name.clone())); - if let Err(_) = rx.await.map_err(|e| anyhow!("{e}")).and_then(|r| r) { + if let Err(checkout_error) = rx.await.map_err(|e| anyhow!("{e}")).and_then(|r| r) { + log::debug!( + "change_branch('{}') failed: {checkout_error:#}, trying create_branch", + branch_name + ); let rx = wt_repo.update(cx, |repo, _cx| { repo.create_branch(branch_name.clone(), None) }); @@ -598,22 +582,41 @@ pub async fn restore_worktree_via_git( } // Restore the staged/unstaged state from the WIP commit trees. - // read-tree sets the index to the staged commit's tree, and - // git restore puts the unstaged commit's files into the working directory. + // read-tree --reset -u applies the unstaged tree (including deletions) + // to the working directory, then a bare read-tree sets the index to + // the staged tree without touching the working directory. let restore_rx = wt_repo.update(cx, |repo, _cx| { repo.restore_archive_checkpoint( row.staged_commit_hash.clone(), row.unstaged_commit_hash.clone(), ) }); - restore_rx + if let Err(error) = restore_rx .await - .map_err(|_| anyhow!("restore_archive_checkpoint canceled"))? - .context("failed to restore archive checkpoint")?; + .map_err(|_| anyhow!("restore_archive_checkpoint canceled")) + .and_then(|r| r) + { + remove_new_worktree_on_error(created_new_worktree, &main_repo, worktree_path, cx).await; + return Err(error.context("failed to restore archive checkpoint")); + } Ok(worktree_path.clone()) } +async fn remove_new_worktree_on_error( + created_new_worktree: bool, + main_repo: &Entity, + worktree_path: &PathBuf, + cx: &mut AsyncApp, +) { + if created_new_worktree { + let rx = main_repo.update(cx, |repo, _cx| { + repo.remove_worktree(worktree_path.clone(), true) + }); + rx.await.ok().and_then(|r| r.log_err()); + } +} + /// Deletes the git ref and DB records for a single archived worktree. /// Used when an archived worktree is no longer referenced by any thread. pub async fn cleanup_archived_worktree_record(row: &ArchivedGitWorktree, cx: &mut AsyncApp) { diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index f84660b4d2eeea..1b4e89102f942c 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -1198,13 +1198,18 @@ impl GitRepository for FakeGitRepository { fn restore_archive_checkpoint( &self, + // The fake filesystem doesn't model a separate index, so only the + // unstaged (full working directory) snapshot is restored. _staged_sha: String, unstaged_sha: String, ) -> BoxFuture<'_, Result<()>> { - let checkpoint = GitRepositoryCheckpoint { - commit_sha: unstaged_sha.parse().unwrap(), - }; - self.restore_checkpoint(checkpoint) + match unstaged_sha.parse() { + Ok(commit_sha) => self.restore_checkpoint(GitRepositoryCheckpoint { commit_sha }), + Err(error) => async move { + Err(anyhow::anyhow!(error).context("failed to parse unstaged SHA as Oid")) + } + .boxed(), + } } fn compare_checkpoints( @@ -1408,39 +1413,6 @@ impl GitRepository for FakeGitRepository { async { Ok(()) }.boxed() } - fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>> { - let workdir_path = self.dot_git_path.parent().unwrap(); - let git_files: Vec<(RepoPath, String)> = self - .fs - .files() - .iter() - .filter_map(|path| { - let repo_path = path.strip_prefix(workdir_path).ok()?; - if repo_path.starts_with(".git") { - return None; - } - let content = self - .fs - .read_file_sync(path) - .ok() - .and_then(|bytes| String::from_utf8(bytes).ok())?; - let rel_path = RelPath::new(repo_path, PathStyle::local()).ok()?; - Some((RepoPath::from_rel_path(&rel_path), content)) - }) - .collect(); - - self.with_state_async(true, move |state| { - let fs_paths: HashSet = git_files.iter().map(|(p, _)| p.clone()).collect(); - for (path, content) in git_files { - state.index_contents.insert(path, content); - } - state - .index_contents - .retain(|path, _| fs_paths.contains(path)); - Ok(()) - }) - } - fn set_trusted(&self, trusted: bool) { self.is_trusted .store(trusted, std::sync::atomic::Ordering::Release); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 8718af0d04129c..6d17641c6ef9af 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -973,8 +973,6 @@ pub trait GitRepository: Send + Sync { fn repair_worktrees(&self) -> BoxFuture<'_, Result<()>>; - fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>>; - fn set_trusted(&self, trusted: bool); fn is_trusted(&self) -> bool; } @@ -2285,18 +2283,6 @@ impl GitRepository for RealGitRepository { .boxed() } - fn stage_all_including_untracked(&self) -> BoxFuture<'_, Result<()>> { - let git_binary = self.git_binary(); - self.executor - .spawn(async move { - let args: Vec = - vec!["--no-optional-locks".into(), "add".into(), "-A".into()]; - git_binary?.run(&args).await?; - Ok(()) - }) - .boxed() - } - fn push( &self, branch_name: String, @@ -2699,15 +2685,20 @@ impl GitRepository for RealGitRepository { .spawn(async move { let git = git_binary?; - // Restore the index to the staged tree - git.run(&["read-tree", &staged_sha]) + // First, set the index AND working tree to match the unstaged + // tree. --reset -u computes a tree-level diff between the + // current index and unstaged_sha's tree and applies additions, + // modifications, and deletions to the working directory. + git.run(&["read-tree", "--reset", "-u", &unstaged_sha]) .await - .context("failed to restore index from staged commit")?; + .context("failed to restore working directory from unstaged commit")?; - // Restore the working directory to the full (unstaged) state - git.run(&["restore", "--source", &unstaged_sha, "--worktree", "."]) + // Then replace just the index with the staged tree. Without -u + // this doesn't touch the working directory, so the result is: + // working tree = unstaged state, index = staged state. + git.run(&["read-tree", &staged_sha]) .await - .context("failed to restore working directory from unstaged commit")?; + .context("failed to restore index from staged commit")?; Ok(()) }) diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 92803efb2e6c66..7f24282dda6193 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6160,20 +6160,6 @@ impl Repository { }) } - pub fn commit_exists(&mut self, sha: String) -> oneshot::Receiver> { - self.send_job(None, move |repo, _cx| async move { - match repo { - RepositoryState::Local(LocalRepositoryState { backend, .. }) => { - let results = backend.revparse_batch(vec![sha]).await?; - Ok(results.into_iter().next().flatten().is_some()) - } - RepositoryState::Remote(_) => { - anyhow::bail!("commit_exists is not supported for remote repositories") - } - } - }) - } - pub fn remove_worktree(&mut self, path: PathBuf, force: bool) -> oneshot::Receiver> { let id = self.id; self.send_job( diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 499266bce6a5ee..3a527da0625c72 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -2784,28 +2784,24 @@ impl Sidebar { cancel_rx: smol::channel::Receiver<()>, cx: &mut gpui::AsyncApp, ) -> anyhow::Result { - let mut completed_persists: Vec<( - thread_worktree_archive::PersistOutcome, - thread_worktree_archive::RootPlan, - )> = Vec::new(); + let mut completed_persists: Vec<(i64, thread_worktree_archive::RootPlan)> = Vec::new(); for root in &roots { if cancel_rx.is_closed() { - for (outcome, completed_root) in completed_persists.iter().rev() { - thread_worktree_archive::rollback_persist(outcome, completed_root, cx).await; + for &(id, ref completed_root) in completed_persists.iter().rev() { + thread_worktree_archive::rollback_persist(id, completed_root, cx).await; } return Ok(ArchiveWorktreeOutcome::Cancelled); } if root.worktree_repo.is_some() { match thread_worktree_archive::persist_worktree_state(root, cx).await { - Ok(outcome) => { - completed_persists.push((outcome, root.clone())); + Ok(id) => { + completed_persists.push((id, root.clone())); } Err(error) => { - for (outcome, completed_root) in completed_persists.iter().rev() { - thread_worktree_archive::rollback_persist(outcome, completed_root, cx) - .await; + for &(id, ref completed_root) in completed_persists.iter().rev() { + thread_worktree_archive::rollback_persist(id, completed_root, cx).await; } return Err(error); } @@ -2813,22 +2809,21 @@ impl Sidebar { } if cancel_rx.is_closed() { - for (outcome, completed_root) in completed_persists.iter().rev() { - thread_worktree_archive::rollback_persist(outcome, completed_root, cx).await; + for &(id, ref completed_root) in completed_persists.iter().rev() { + thread_worktree_archive::rollback_persist(id, completed_root, cx).await; } return Ok(ArchiveWorktreeOutcome::Cancelled); } if let Err(error) = thread_worktree_archive::remove_root(root.clone(), cx).await { - if let Some((outcome, completed_root)) = completed_persists.last() { + if let Some(&(id, ref completed_root)) = completed_persists.last() { if completed_root.root_path == root.root_path { - thread_worktree_archive::rollback_persist(outcome, completed_root, cx) - .await; + thread_worktree_archive::rollback_persist(id, completed_root, cx).await; completed_persists.pop(); } } - for (outcome, completed_root) in completed_persists.iter().rev() { - thread_worktree_archive::rollback_persist(outcome, completed_root, cx).await; + for &(id, ref completed_root) in completed_persists.iter().rev() { + thread_worktree_archive::rollback_persist(id, completed_root, cx).await; } return Err(error); } diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 66eac4c57389d1..d0156f51dbe83b 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -4307,27 +4307,14 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon sidebar.archive_thread(&wt_thread_id, window, cx); }); - // archive_thread spawns a chain of tasks: - // 1. cx.spawn_in for workspace removal (awaits mw.remove()) - // 2. start_archive_worktree_task spawns cx.spawn for git persist + disk removal - // 3. persist/remove do background_spawn work internally - // Each layer needs run_until_parked to drive to completion. + // archive_thread spawns a multi-layered chain of tasks (workspace + // removal → git persist → disk removal), each of which may spawn + // further background work. Each run_until_parked() call drives one + // layer of pending work. cx.run_until_parked(); cx.run_until_parked(); cx.run_until_parked(); - // The workspace removal is immediate, but the on-disk cleanup runs in a - // background archive task. Give the task time to finish. - for _ in 0..10 { - if !fs.is_dir(Path::new("/wt-feature-a")).await { - break; - } - cx.run_until_parked(); - cx.background_executor - .timer(std::time::Duration::from_millis(10)) - .await; - } - // The linked worktree workspace should have been removed. assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),