Skip to content

Commit 7e96b0f

Browse files
stevenpollackclaude
andcommitted
fix(worktrees): also refuse removal of a worktree with staged changes
The removal guard used only `is_workdir_clean`, which compares the index to the working directory and therefore misses staged-but- uncommitted changes (HEAD vs index). A worktree whose only changes were `git add`ed (workdir matching the index) was treated as clean and its working directory pruned, whereas `git worktree remove` refuses it. Compose an additional `get_status(StatusType::Stage)` check in `remove_worktree` so the guard matches its "uncommitted changes" contract. Adds a regression test. Found by an independent code review of #2995. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BMQxBdr5f3B3jmDtdvGtTu
1 parent 80322f0 commit 7e96b0f

1 file changed

Lines changed: 32 additions & 1 deletion

File tree

asyncgit/src/sync/worktree.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::path::{Path, PathBuf};
55
use git2::{Repository, WorktreeLockStatus, WorktreePruneOptions};
66
use scopetime::scope_time;
77

8+
use super::status::{get_status, StatusType};
89
use super::{is_workdir_clean, repo, RepoPath};
910
use crate::error::{Error, Result};
1011

@@ -215,7 +216,13 @@ pub fn remove_worktree(
215216
// when the working dir still exists.
216217
if worktree.validate().is_ok() {
217218
let wt_path: RepoPath = worktree.path().to_path_buf().into();
218-
if !is_workdir_clean(&wt_path, None)? {
219+
// `is_workdir_clean` only compares the index to the working
220+
// dir, so it misses staged-but-uncommitted changes; also
221+
// reject those (HEAD vs index) to match `git worktree remove`.
222+
let has_staged =
223+
!get_status(&wt_path, StatusType::Stage, None)?
224+
.is_empty();
225+
if has_staged || !is_workdir_clean(&wt_path, None)? {
219226
return Err(Error::Generic(
220227
"worktree has uncommitted changes; commit or discard them first"
221228
.to_string(),
@@ -454,6 +461,30 @@ mod tests {
454461
assert_eq!(get_worktrees(&repo_path).unwrap().len(), 2);
455462
}
456463

464+
#[test]
465+
fn test_remove_worktree_refuses_staged() {
466+
let (_td, repo) = repo_init().unwrap();
467+
let root = repo.path().parent().unwrap();
468+
let repo_path: RepoPath = root.to_str().unwrap().into();
469+
470+
let wt_dir = tempfile::TempDir::new().unwrap();
471+
let wt_path = wt_dir.path().join("staged");
472+
create_worktree(&repo_path, wt_path.to_str().unwrap())
473+
.unwrap();
474+
475+
// stage a new file inside the worktree: it matches the index,
476+
// so only the HEAD-vs-index (staged) check can catch it — the
477+
// working-dir check alone would see this as clean.
478+
std::fs::write(wt_path.join("new.txt"), b"data").unwrap();
479+
let wt_repo = git2::Repository::open(&wt_path).unwrap();
480+
let mut index = wt_repo.index().unwrap();
481+
index.add_path(std::path::Path::new("new.txt")).unwrap();
482+
index.write().unwrap();
483+
484+
assert!(remove_worktree(&repo_path, "staged").is_err());
485+
assert_eq!(get_worktrees(&repo_path).unwrap().len(), 2);
486+
}
487+
457488
#[test]
458489
fn test_toggle_worktree_lock() {
459490
let (_td, repo) = repo_init().unwrap();

0 commit comments

Comments
 (0)