From f926a2313ab87c9cf40b57a491a1b7a84192da58 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 3 Apr 2026 11:54:23 -0400 Subject: [PATCH] Remove workspace before deleting worktree directory on archive The file watcher was racing with directory deletion, causing 'root path could not be canonicalized' and 'reference HEAD not found' errors. Now we remove the worktree's workspace from the MultiWorkspace (stopping the file watcher) before deleting the directory. Also fix directory deletion order: delete the directory first (it may have uncommitted files that prevent git worktree remove --force from succeeding), then clean up git's worktree registration. On the restore side, clean up any stale worktree registration before creating, to handle registrations left over from previous archives. --- crates/sidebar/src/sidebar.rs | 106 +++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 40 deletions(-) diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 1e5f9cf623b068841888e66ff05edc4c1dc7b59d..7de50c42d538986b58cfa651468a65aec6019305 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -2464,6 +2464,20 @@ impl Sidebar { worktree_path.clone() }; + // Clean up any stale worktree registration at this path (e.g. from + // a previous archive that deleted the directory but left the git + // metadata behind). + let cleanup_result = main_repo.update(cx, |repo, _cx| { + repo.remove_worktree(final_path.clone(), true) + }); + match cleanup_result.await { + Ok(Ok(())) => log::info!( + "Cleaned up stale worktree registration at {}", + final_path.display() + ), + _ => {} // Not registered or already clean — that's fine + } + // Create the worktree via the Repository entity (detached, since // the commit is a WIP snapshot, not a real branch tip). let create_result = main_repo.update(cx, |repo, _cx| { @@ -2903,12 +2917,14 @@ impl Sidebar { .any(|entry| &entry.session_id != session_id); // Collect info for each path that is a linked git worktree. + #[allow(clippy::type_complexity)] let mut linked_worktrees: Vec<( Entity, PathBuf, Option, std::sync::Arc, Option>, + Option>, )> = Vec::new(); for worktree_path in folder_paths.paths() { if let Some(info) = workspaces.iter().find_map(|workspace| { @@ -2933,6 +2949,7 @@ impl Sidebar { branch_name, main_repo_path, main_repo, + Some(workspace.clone()), )) } else { None @@ -2949,8 +2966,14 @@ impl Sidebar { let fs = ::global(cx); - for (worktree_repo, worktree_path, branch_name, main_repo_path, main_repo) in - linked_worktrees + for ( + worktree_repo, + worktree_path, + branch_name, + main_repo_path, + main_repo, + worktree_workspace, + ) in linked_worktrees { let session_id = session_id.clone(); let folder_paths = folder_paths.clone(); @@ -2965,10 +2988,12 @@ impl Sidebar { branch_name, main_repo_path, main_repo, + worktree_workspace, is_last_thread, session_id, folder_paths, fs, + this.clone(), cx, ) .await; @@ -2989,10 +3014,12 @@ impl Sidebar { branch_name: Option, main_repo_path: std::sync::Arc, main_repo: Option>, + worktree_workspace: Option>, is_last_thread: bool, session_id: acp::SessionId, folder_paths: PathList, fs: std::sync::Arc, + sidebar: WeakEntity, cx: &mut AsyncWindowContext, ) -> anyhow::Result<()> { if !is_last_thread { @@ -3379,42 +3406,25 @@ impl Sidebar { } } - // Use `git worktree remove --force` which handles both the git - // bookkeeping and directory deletion. Fall back to manual removal - // only if git can't do it. - let dir_removed = if let Some(main_repo) = &main_repo { - let receiver = main_repo.update(cx, |repo, _cx| { - repo.remove_worktree(worktree_path.clone(), true) - }); - match receiver.await { - Ok(Ok(())) => true, - Ok(Err(err)) => { - log::warn!("git worktree remove failed: {err}, trying manual removal"); - fs.remove_dir( - &worktree_path, - fs::RemoveOptions { - recursive: true, - ignore_if_not_exists: true, - }, - ) - .await - .is_ok() - } - Err(_) => { - log::warn!("git worktree remove was canceled, trying manual removal"); - fs.remove_dir( - &worktree_path, - fs::RemoveOptions { - recursive: true, - ignore_if_not_exists: true, - }, - ) - .await - .is_ok() - } - } - } else { - fs.remove_dir( + // Remove the worktree's workspace from the MultiWorkspace before + // deleting the directory, so the file watcher stops scanning it. + if let Some(worktree_workspace) = &worktree_workspace { + sidebar + .update_in(cx, |sidebar, window, cx| { + if let Some(multi_workspace) = sidebar.multi_workspace.upgrade() { + multi_workspace.update(cx, |mw, cx| { + mw.remove(worktree_workspace, window, cx); + }); + } + }) + .ok(); + } + + // Delete the directory first (it may contain uncommitted files that + // prevent `git worktree remove` from succeeding even with --force), + // then clean up git's worktree registration. + let dir_removed = fs + .remove_dir( &worktree_path, fs::RemoveOptions { recursive: true, @@ -3422,8 +3432,24 @@ impl Sidebar { }, ) .await - .is_ok() - }; + .is_ok(); + + if dir_removed { + if let Some(main_repo) = &main_repo { + let receiver = main_repo.update(cx, |repo, _cx| { + repo.remove_worktree(worktree_path.clone(), true) + }); + match receiver.await { + Ok(Ok(())) => {} + Ok(Err(err)) => { + log::warn!("git worktree remove failed after directory deletion: {err}"); + } + Err(_) => { + log::warn!("git worktree remove was canceled after directory deletion"); + } + } + } + } if !dir_removed { let undo_ok = if commit_ok {