From cc4aacf99f4d7ccaf176673f5ebdbed151c62967 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 9 Aug 2026 14:53:28 -0700 Subject: [PATCH 1/6] fix(remove): refuse to strand a detached worktree when removing by branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` dropped out of the branch-first lookup, degraded to a branch-only deletion, and exited 0 — deleting the ref and leaving the worktree registered. The failing half was silent. The `worktree-path` template is what still connects the two, so the removal matches against it and refuses, naming the path that reaches the worktree. Three states are deliberately not matched: a worktree on some other branch (that branch names it), the main worktree (whose path-based removal refuses too, so the hint would be a dead end), and a prunable entry (stale metadata, not a worktree on disk). The guard sits in `wt remove` rather than `prepare_worktree_removal`, which every producer of a branch-only target shares: `wt step prune` plans its whole sweep from one worktree-list snapshot, so a detached worktree it is about to remove as its own candidate is still registered when the branch's plan is built — refusing there would leave prune unable to clean up either half. `live_sibling_checkout` keeps `exists()` rather than the union predicate, and now says why: the two only disagree on a directory that is present but no longer holds its worktree, where calling it dead deletes a branch a checkout still resolves. Fixes #3769 Co-Authored-By: Claude Opus 5 (1M context) --- docs/content/remove.md | 2 + .../skills/worktrunk/reference/remove.md | 2 + skills/worktrunk/reference/remove.md | 2 + src/cli/mod.rs | 2 + src/commands/remove.rs | 84 +++++++++++++++- src/commands/repository_ext.rs | 15 ++- src/git/error.rs | 43 ++++++++ tests/integration_tests/remove.rs | 98 ++++++++++++++++++- ...gration_tests__help__help_remove_long.snap | 3 + ...branch_with_detached_worktree_message.snap | 63 ++++++++++++ ...ranch_with_prunable_detached_worktree.snap | 64 ++++++++++++ ...lt_branch_with_detached_main_worktree.snap | 63 ++++++++++++ ...ve__remove_detached_worktree_in_multi.snap | 26 +++-- 13 files changed, 452 insertions(+), 15 deletions(-) create mode 100644 tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_message.snap create mode 100644 tests/snapshots/integration__integration_tests__remove__remove_branch_with_prunable_detached_worktree.snap create mode 100644 tests/snapshots/integration__integration_tests__remove__remove_default_branch_with_detached_main_worktree.snap diff --git a/docs/content/remove.md b/docs/content/remove.md index 6b14753f48..680b074542 100644 --- a/docs/content/remove.md +++ b/docs/content/remove.md @@ -56,6 +56,8 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. + ## Force flags Worktrunk has two force flags for different situations: diff --git a/plugins/worktrunk/skills/worktrunk/reference/remove.md b/plugins/worktrunk/skills/worktrunk/reference/remove.md index 0eee006fdb..c3373f6bef 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/remove.md +++ b/plugins/worktrunk/skills/worktrunk/reference/remove.md @@ -55,6 +55,8 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. + ## Force flags Worktrunk has two force flags for different situations: diff --git a/skills/worktrunk/reference/remove.md b/skills/worktrunk/reference/remove.md index 0eee006fdb..c3373f6bef 100644 --- a/skills/worktrunk/reference/remove.md +++ b/skills/worktrunk/reference/remove.md @@ -55,6 +55,8 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. + ## Force flags Worktrunk has two force flags for different situations: diff --git a/src/cli/mod.rs b/src/cli/mod.rs index c4e91f75c2..bd5e118265 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1286,6 +1286,8 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. + ## Force flags Worktrunk has two force flags for different situations: diff --git a/src/commands/remove.rs b/src/commands/remove.rs index 2d1e7545f5..4af450535d 100644 --- a/src/commands/remove.rs +++ b/src/commands/remove.rs @@ -16,7 +16,7 @@ use crate::output::{BackgroundFallbackMode, RemovalExecution, handle_remove_outp use super::hook_plan::{ApprovedHookPlan, HookPlanBuilder}; use super::hooks::HookAnnouncer; use super::repository_ext::RepositoryCliExt; -use super::worktree::{BranchFate, RemovalPlan}; +use super::worktree::{BranchFate, RemovalPlan, compute_worktree_path}; use super::{RemoveTarget, flag_pair}; /// The execution mode `--foreground` selects; the background default falls @@ -58,6 +58,56 @@ impl RemovePlans { } } +/// The removable detached worktree sitting where `branch`'s worktree belongs, +/// if any. +/// +/// Detaching a worktree's HEAD severs the only link git records between it and +/// the branch, so once that happens the `worktree-path` template is all that +/// still connects the two. That is a stronger association than the rest of the +/// module draws — [`is_worktree_at_expected_path`] returns false for a detached +/// worktree, and [`worktree_display_name`] renders one as `dir_name (detached)` +/// without consulting the template — and it is intentional here: the template +/// match is what keeps the removal from stranding it. Three cases are +/// deliberately not matched, because refusing on them would name a removal that +/// can't happen or protect nothing: +/// +/// - a worktree checked out on some *other* branch — that branch names it, so +/// removing this one strands nothing; +/// - the main worktree, which `wt remove ` refuses — the hint would name +/// a command that can't run, and the branch falls through to the accurate +/// [`CannotRemoveDefaultBranch`](worktrunk::git::GitError::CannotRemoveDefaultBranch). +/// A bare repo has no main worktree, so its default-branch checkout is +/// matched like any other and the hint works; +/// - a prunable entry, whose directory is already gone — stale metadata for +/// `wt step prune` to sweep, not a worktree left on disk. +/// +/// A template that won't expand yields no expected path and so no match: the +/// guard can't assert what it can't compute, and refusing every branch-only +/// removal on a broken template would cost more than the case it guards. +/// +/// [`is_worktree_at_expected_path`]: super::worktree::is_worktree_at_expected_path +/// [`worktree_display_name`]: super::worktree::worktree_display_name +fn detached_worktree_for<'a>( + repo: &Repository, + config: &UserConfig, + branch: &str, + worktrees: &'a [worktrunk::git::WorktreeInfo], +) -> Option<&'a Path> { + let expected = compute_worktree_path(repo, branch, config).ok()?; + worktrees + .iter() + .find(|wt| { + wt.branch.is_none() + && !wt.is_prunable() + && worktrunk::path::paths_match(&wt.path, &expected) + // Fail closed: a `git_dir` lookup that fails says nothing about + // whether the worktree is linked, and skipping the guard there + // deletes the ref and strands the worktree. + && repo.worktree_at(&wt.path).is_linked().unwrap_or(true) + }) + .map(|wt| wt.path.as_path()) +} + /// Validate all removal targets, returning categorized plans. /// /// Resolves each branch name, determines whether it's the current worktree, @@ -65,6 +115,7 @@ impl RemovePlans { /// Errors are collected (not fatal) to support partial success. fn validate_remove_targets( repo: &Repository, + config: &UserConfig, branches: Vec, keep_branch: bool, force_delete: bool, @@ -126,7 +177,34 @@ fn validate_remove_targets( // otherwise (see its shared-branch handling). RemoveTarget::WorktreePath(path_canonical) } - ResolvedWorktree::BranchOnly { branch } => RemoveTarget::BranchOnly(branch), + ResolvedWorktree::BranchOnly { branch } => { + // A detached worktree is invisible to the branch-first lookup, + // so a branch whose worktree has since been detached resolves + // here and would have its ref deleted with the worktree left + // registered. Refuse instead, and name the path — the only + // spelling that still reaches it. + // + // The guard belongs to this command rather than to + // `prepare_worktree_removal`, which every producer of a + // `BranchOnly` target shares: `wt step prune` plans its whole + // sweep from one worktree-list snapshot, so a detached + // worktree it is about to remove as its own candidate is still + // registered when the branch's plan is built. Refusing there + // would leave prune unable to clean up either half. + if let Some(detached) = + worktrees.and_then(|wts| detached_worktree_for(repo, config, &branch, wts)) + { + plans.record_error( + GitError::DetachedWorktreeForBranch { + branch, + path: detached.to_path_buf(), + } + .into(), + ); + continue; + } + RemoveTarget::BranchOnly(branch) + } // Resolution tried the argument as a branch and as a worktree path // and matched neither, so a directory sitting there is a leftover // skeleton rather than anything wt can remove. Only a typed @@ -386,6 +464,7 @@ pub fn handle_remove_command(args: RemoveArgs, yes: bool) -> anyhow::Result<()> // Multi-worktree removal: validate ALL first, then approve, then execute let plans = validate_remove_targets( &repo, + &config, branches, !delete_branch, args.force_delete, @@ -493,6 +572,7 @@ mod tests { let plans = validate_remove_targets( &repo, + &UserConfig::default(), vec!["missing-worktree".to_string(), "branch-only".to_string()], false, false, diff --git a/src/commands/repository_ext.rs b/src/commands/repository_ext.rs index 1b308fe583..1ec385a33a 100644 --- a/src/commands/repository_ext.rs +++ b/src/commands/repository_ext.rs @@ -578,10 +578,17 @@ pub(crate) fn compute_integration_reason( /// with an unresolvable `HEAD`, so every removal that could delete a branch /// asks this first. /// -/// Only a live directory counts. A sibling entry whose directory is already -/// gone is stale metadata awaiting `git worktree prune`, not a checkout with -/// anything to lose — retaining a branch for it would strand the branch and -/// point the user at a directory that isn't there. +/// Only a live directory counts: a sibling entry whose directory is gone is +/// stale metadata awaiting `git worktree prune`, not a checkout with anything +/// to lose, and retaining a branch for it would strand the branch and point the +/// user at a directory that isn't there. +/// +/// `exists()` is the test, not [`Repository::worktree_is_unusable`], which the +/// rest of the removal path uses. The two disagree on a directory that is +/// present but no longer holds its worktree, and the disagreement is +/// asymmetric: calling a dead sibling live retains a branch nobody needed, +/// while calling a live one dead deletes a branch a checkout still resolves. +/// This answer only ever gates a deletion, so it takes the conservative test. pub(crate) fn live_sibling_checkout<'a>( worktrees: &'a [WorktreeInfo], branch: &str, diff --git a/src/git/error.rs b/src/git/error.rs index 2acb68b85f..0fed06b90b 100644 --- a/src/git/error.rs +++ b/src/git/error.rs @@ -588,6 +588,28 @@ pub enum GitError { WorktreeNotFound { branch: String, }, + /// A branch with no worktree, whose worktree's place is taken by a + /// registered worktree with a detached HEAD. + /// + /// Detaching severs the only link git records between a worktree and a + /// branch, so the branch-first lookup reports no worktree and an operation + /// addressed by branch degrades to acting on the ref alone. For removal + /// that means deleting the ref, leaving the worktree registered, and + /// reporting success. Refusing is the honest answer, and the path is the + /// only spelling that still reaches the worktree. + /// + /// Distinct from [`GitError::WorktreeNotFound`], where the branch has no + /// checkout anywhere and creating one is the right suggestion. + /// + /// [`GitError::WorktreePathOccupied`] reports the same physical state to + /// `wt switch`, which wants the worktree back on the branch and says so. + /// Removal wants it gone, so the two carry different hints and split on the + /// occupied-by-another-branch case: that one blocks a switch, and leaves a + /// removal nothing to strand. + DetachedWorktreeForBranch { + branch: String, + path: PathBuf, + }, /// A worktree selector matched neither a branch nor a worktree path. /// /// Distinct from [`GitError::WorktreeNotFound`], which means the branch @@ -883,6 +905,13 @@ impl GitError { cformat!("Branch {branch} has no worktree") } + GitError::DetachedWorktreeForBranch { branch, path } => { + let path_display = format_path_for_display(path); + cformat!( + "Branch {branch} has no worktree; the one @ {path_display} is detached" + ) + } + GitError::WorktreeSelectorNotFound { selector } => { cformat!("No branch or worktree named {selector}") } @@ -1441,6 +1470,20 @@ impl GitError { ) } + GitError::DetachedWorktreeForBranch { path, .. } => { + let title = self.title(); + let display_path = format_path_for_display(path); + let remove_cmd = suggest_command("remove", &[&display_path], &[]); + write!( + f, + "{}\n{}", + error_message(&title), + hint_message(cformat!( + "To remove the detached worktree, run {remove_cmd}" + )) + ) + } + GitError::WorktreeSelectorNotFound { .. } => { let title = self.title(); write!( diff --git a/tests/integration_tests/remove.rs b/tests/integration_tests/remove.rs index 6e13a0c62d..b8e0d7f6b8 100644 --- a/tests/integration_tests/remove.rs +++ b/tests/integration_tests/remove.rs @@ -292,6 +292,98 @@ fn test_remove_locked_worktree_directory_missing(mut repo: TestRepo) { ); } +/// Regression test for #3769: `wt remove ` on a worktree whose HEAD has +/// since been detached deleted the branch, left the worktree registered, and +/// exited 0. Detaching severs the only link git records between the two, so the +/// branch-first lookup found no worktree and the removal degraded to +/// branch-only — half the operation, and the silent half. +/// +/// Refusing is defensible because the path still reaches the worktree, which +/// `test_remove_detached_worktree_by_path` pins. +#[rstest] +fn test_remove_branch_whose_worktree_was_detached(mut repo: TestRepo) { + let _worktree_path = repo.add_worktree("detached-later"); + repo.detach_head_in_worktree("detached-later"); + + let output = repo + .wt_command() + .args(["remove", "detached-later", "--foreground", "--yes"]) + .output() + .unwrap(); + assert!( + !output.status.success(), + "wt remove should refuse a branch whose worktree has been detached.\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + + // The branch must survive — deleting it is the half that used to run. + let branch_exists = repo + .git_command() + .args(["branch", "--list", "detached-later"]) + .run() + .unwrap(); + assert!( + !String::from_utf8_lossy(&branch_exists.stdout) + .trim() + .is_empty(), + "Branch should NOT be deleted while a detached worktree occupies its path", + ); + + // ...and so must the worktree it would otherwise have stranded. + let list_after = repo + .git_command() + .args(["worktree", "list", "--porcelain"]) + .run() + .unwrap(); + assert!( + String::from_utf8_lossy(&list_after.stdout).contains("detached-later"), + "Detached worktree should still be registered", + ); +} + +/// The refusal names the worktree and the path-based removal that reaches it. +#[rstest] +fn test_remove_branch_with_detached_worktree_message(mut repo: TestRepo) { + repo.add_worktree("feature-detached-strand"); + repo.detach_head_in_worktree("feature-detached-strand"); + + assert_cmd_snapshot!(make_snapshot_cmd( + &repo, + "remove", + &["feature-detached-strand"], + None + )); +} + +/// The default branch's expected path is the main worktree, so a detached main +/// worktree would match the #3769 guard — but `wt remove ` refuses +/// too, so pointing at it would be a dead end. The default-branch refusal, which +/// is the accurate answer, must still be what surfaces. +#[rstest] +fn test_remove_default_branch_with_detached_main_worktree(repo: TestRepo) { + repo.run_git(&["checkout", "--detach", "HEAD"]); + + assert_cmd_snapshot!(make_snapshot_cmd(&repo, "remove", &["main"], None)); +} + +/// A detached worktree whose directory is already gone is stale metadata, not a +/// worktree the #3769 guard protects — the branch-only removal proceeds rather +/// than refusing and naming a path that isn't on disk. +#[rstest] +fn test_remove_branch_with_prunable_detached_worktree(mut repo: TestRepo) { + let worktree_path = repo.add_worktree("feature-detached-gone"); + repo.detach_head_in_worktree("feature-detached-gone"); + std::fs::remove_dir_all(&worktree_path).unwrap(); + + assert_cmd_snapshot!(make_snapshot_cmd( + &repo, + "remove", + &["-D", "feature-detached-gone"], + None + )); +} + #[rstest] fn test_remove_by_name_from_main(mut repo: TestRepo) { // Create a worktree @@ -2622,8 +2714,10 @@ fn test_remove_detached_worktree_in_multi(mut repo: TestRepo) { // Detach HEAD in feature-b repo.detach_head_in_worktree("feature-b"); - // From main, try to multi-remove both - // feature-a should succeed, feature-b should fail (detached HEAD) + // From main, try to multi-remove both. feature-a is removed; feature-b is + // refused, because detaching dropped it out of the branch-first lookup and + // removing it by branch would delete the ref and strand the worktree + // (#3769). Partial success is the point: one refusal doesn't stop the rest. assert_cmd_snapshot!(make_snapshot_cmd( &repo, "remove", diff --git a/tests/snapshots/integration__integration_tests__help__help_remove_long.snap b/tests/snapshots/integration__integration_tests__help__help_remove_long.snap index 0806c5764f..878926a396 100644 --- a/tests/snapshots/integration__integration_tests__help__help_remove_long.snap +++ b/tests/snapshots/integration__integration_tests__help__help_remove_long.snap @@ -35,6 +35,7 @@ info: WORKTRUNK_TEST_PARENT_SHELL: "" WORKTRUNK_TEST_POWERSHELL_ENV: "0" WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" WORKTRUNK_TEST_ZSH_INSTALLED: "0" --- @@ -147,6 +148,8 @@ Branches matching these conditions and with empty working trees are dimmed in [ Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via git worktree add --force) fails a different test: deleting the ref would leave that worktree unable to resolve HEAD, which is why git branch -d refuses the same delete. Such a branch is retained whatever -D asks, and the surviving checkout is named. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so wt remove  would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. + Force flags Worktrunk has two force flags for different situations: diff --git a/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_message.snap b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_message.snap new file mode 100644 index 0000000000..9160fd423e --- /dev/null +++ b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_message.snap @@ -0,0 +1,63 @@ +--- +source: tests/integration_tests/remove.rs +info: + program: wt + args: + - remove + - feature-detached-strand + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_AUTHOR_EMAIL: test@example.com + GIT_AUTHOR_NAME: Test User + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_EMAIL: test@example.com + GIT_COMMITTER_NAME: Test User + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: false +exit_code: 1 +----- stdout ----- + +----- stderr ----- +✗ Branch feature-detached-strand has no worktree; the one @ _REPO_.feature-detached-strand is detached +↳ To remove the detached worktree, run wt remove _REPO_.feature-detached-strand diff --git a/tests/snapshots/integration__integration_tests__remove__remove_branch_with_prunable_detached_worktree.snap b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_prunable_detached_worktree.snap new file mode 100644 index 0000000000..de6befd2c1 --- /dev/null +++ b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_prunable_detached_worktree.snap @@ -0,0 +1,64 @@ +--- +source: tests/integration_tests/remove.rs +info: + program: wt + args: + - remove + - "-D" + - feature-detached-gone + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_AUTHOR_EMAIL: test@example.com + GIT_AUTHOR_NAME: Test User + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_EMAIL: test@example.com + GIT_COMMITTER_NAME: Test User + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- +○ No worktree found for branch feature-detached-gone +✓ Removed branch feature-detached-gone (--force-delete) diff --git a/tests/snapshots/integration__integration_tests__remove__remove_default_branch_with_detached_main_worktree.snap b/tests/snapshots/integration__integration_tests__remove__remove_default_branch_with_detached_main_worktree.snap new file mode 100644 index 0000000000..2d6a87c459 --- /dev/null +++ b/tests/snapshots/integration__integration_tests__remove__remove_default_branch_with_detached_main_worktree.snap @@ -0,0 +1,63 @@ +--- +source: tests/integration_tests/remove.rs +info: + program: wt + args: + - remove + - main + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_AUTHOR_EMAIL: test@example.com + GIT_AUTHOR_NAME: Test User + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_EMAIL: test@example.com + GIT_COMMITTER_NAME: Test User + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: false +exit_code: 1 +----- stdout ----- + +----- stderr ----- +✗ Cannot remove the default branch main +↳ To force-delete, run wt remove -D main diff --git a/tests/snapshots/integration__integration_tests__remove__remove_detached_worktree_in_multi.snap b/tests/snapshots/integration__integration_tests__remove__remove_detached_worktree_in_multi.snap index 86d45938b4..d6b367aaff 100644 --- a/tests/snapshots/integration__integration_tests__remove__remove_detached_worktree_in_multi.snap +++ b/tests/snapshots/integration__integration_tests__remove__remove_detached_worktree_in_multi.snap @@ -8,12 +8,23 @@ info: - feature-b env: APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" CLICOLOR_FORCE: "1" COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_AUTHOR_EMAIL: test@example.com + GIT_AUTHOR_NAME: Test User GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" - GIT_CONFIG_GLOBAL: "[TEST_GIT_CONFIG]" - GIT_CONFIG_SYSTEM: /dev/null + GIT_COMMITTER_EMAIL: test@example.com + GIT_COMMITTER_NAME: Test User + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" GIT_TERMINAL_PROMPT: "0" HOME: "[TEST_HOME]" LANG: C @@ -36,19 +47,20 @@ info: WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" WORKTRUNK_TEST_NUSHELL_ENV: "0" WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" WORKTRUNK_TEST_POWERSHELL_ENV: "0" WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" WORKTRUNK_TEST_ZSH_INSTALLED: "0" XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" --- -success: true -exit_code: 0 +success: false +exit_code: 1 ----- stdout ----- ----- stderr ----- +✗ Branch feature-b has no worktree; the one @ _REPO_.feature-b is detached +↳ To remove the detached worktree, run wt remove _REPO_.feature-b ◎ Removing feature-a worktree in background ↳ Branch unmerged; to delete, run wt remove -D feature-a -○ No worktree found for branch feature-b -○ Branch feature-b retained; has unmerged changes -↳ To delete the unmerged branch, run wt remove -D feature-b From 27a0e2154238043c94b147606521b865c703fe65 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 9 Aug 2026 15:13:54 -0700 Subject: [PATCH 2/6] fix(errors): stop double-escaping the path in worktree-removal hints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `format_path_for_display` already returns a shell-ready token, so routing its result through `suggest_command` escapes it a second time. A worktree under $HOME rendered as `wt remove '~/repo.feature'`, where the quoting suppresses the tilde expansion the unquoted form in the line above it relies on; a path that genuinely needs escaping came out carrying literal quote characters and resolved to nothing. The three hints that composed the two now interpolate the formatted path directly, as the neighbouring `rm -rf {path}` and `git worktree unlock {path}` hints already do. `format_path_for_display` documents the trap so the next caller doesn't repeat it — `pr_mr_switch_hint` had recorded it only in its own docstring. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/repository_ext.rs | 8 +++++--- src/git/error.rs | 6 ++++-- src/git/repository/worktrees.rs | 7 +++---- src/path.rs | 8 ++++++++ 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/commands/repository_ext.rs b/src/commands/repository_ext.rs index 1ec385a33a..4292261a99 100644 --- a/src/commands/repository_ext.rs +++ b/src/commands/repository_ext.rs @@ -8,7 +8,7 @@ use worktrunk::git::{ parse_porcelain_z, parse_untracked_files, }; use worktrunk::path::format_path_for_display; -use worktrunk::styling::{eprintln, format_with_gutter, suggest_command, warning_message}; +use worktrunk::styling::{eprintln, format_with_gutter, warning_message}; /// Target for worktree removal. #[derive(Debug)] @@ -167,10 +167,12 @@ impl RepositoryCliExt for Repository { .iter() .find(|wt| wt.branch.as_deref() == Some(branch.as_str())) { + // `path` is already shell-ready, so the suggested command + // interpolates it rather than passing it through + // `suggest_command`, which would escape it a second time. let path = format_path_for_display(&wt.path); bail!(cformat!( - "Branch {branch} gained a worktree @ {path} since it was selected; to remove that worktree, run {}", - suggest_command("remove", &[&path], &[]) + "Branch {branch} gained a worktree @ {path} since it was selected; to remove that worktree, run wt remove {path}" )); } // Check the branch exists locally, so a typo or a remote-only diff --git a/src/git/error.rs b/src/git/error.rs index 0fed06b90b..9db0f70519 100644 --- a/src/git/error.rs +++ b/src/git/error.rs @@ -1472,14 +1472,16 @@ impl GitError { GitError::DetachedWorktreeForBranch { path, .. } => { let title = self.title(); + // `format_path_for_display` already returns a shell-ready + // token, so the command is built by interpolation — routing it + // through `suggest_command` would escape it a second time. let display_path = format_path_for_display(path); - let remove_cmd = suggest_command("remove", &[&display_path], &[]); write!( f, "{}\n{}", error_message(&title), hint_message(cformat!( - "To remove the detached worktree, run {remove_cmd}" + "To remove the detached worktree, run wt remove {display_path}" )) ) } diff --git a/src/git/repository/worktrees.rs b/src/git/repository/worktrees.rs index 8f87c49bc3..b563752cd4 100644 --- a/src/git/repository/worktrees.rs +++ b/src/git/repository/worktrees.rs @@ -13,9 +13,7 @@ use super::{ normalize_selector, resolve_input_path, }; use crate::path::{format_path_for_display, paths_match}; -use crate::styling::{ - eprintln, format_with_gutter, hint_message, suggest_command, warning_message, -}; +use crate::styling::{eprintln, format_with_gutter, hint_message, warning_message}; impl Repository { /// List all worktrees for this repository. @@ -757,7 +755,8 @@ fn warn_duplicate_checkout(branch: &str, paths: &[PathBuf]) { // removes exactly the worktree named and retains the branch the others // still hold, so it's safe to suggest for a duplicate. for extra in &paths[1..] { - let cmd = suggest_command("remove", &[&format_path_for_display(extra)], &[]); + // Already shell-ready; `suggest_command` would escape it again. + let cmd = format!("wt remove {}", format_path_for_display(extra)); eprintln!( "{}", hint_message(cformat!("To drop a duplicate, run {cmd}")) diff --git a/src/path.rs b/src/path.rs index 4ded02115e..f18f436d28 100644 --- a/src/path.rs +++ b/src/path.rs @@ -103,6 +103,14 @@ fn needs_shell_escaping(s: &str) -> bool { /// Uses POSIX shell escaping since all our hints target POSIX-compatible shells /// (bash, zsh, fish, and Git Bash on Windows). /// +/// The result is already shell-ready, so a hint embeds it by interpolation +/// (`rm -rf {path}`). Passing it to [`suggest_command`] escapes it a second +/// time: `~/repo` becomes `'~/repo'`, which the shell no longer tilde-expands, +/// and `'/tmp/my repo'` becomes a string carrying literal quote characters that +/// resolves to nothing. +/// +/// [`suggest_command`]: crate::styling::suggest_command +/// /// # Examples /// - `/Users/alex/repo` → `~/repo` (no escaping needed) /// - `/Users/alex/my repo` → `'/Users/alex/my repo'` (needs quoting, use original) From 56ada0bf23bf6f38bae78914282bd3af71cd99c6 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 9 Aug 2026 15:39:24 -0700 Subject: [PATCH 3/6] fix(remove): scope the detached-worktree guard to removals that delete a ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard fired ahead of `prepare_worktree_removal`'s branch-existence check, so a name that is not a local branch but whose templated path holds a detached worktree reported `Branch has no worktree` instead of `No branch named `. Check `exists_locally` first. It also fired under `--no-delete-branch` / `[remove] delete-branch = false`, where the branch-only arm deletes nothing — nothing was going to strand the worktree, so the guard turned a no-op exit 0 into a hard failure. Gate on `deletion_mode.should_keep()`. An unintegrated branch under `SafeDelete` retains its ref too, but only the deletion attempt downstream knows that, so those still refuse rather than exiting 0. That is the conservative direction — the worktree the refusal names is on disk — and a test pins it as a decision rather than an accident. Co-Authored-By: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/remove.rs | 27 +++++++- tests/integration_tests/remove.rs | 63 +++++++++++++++++++ ...with_detached_worktree_keeping_branch.snap | 63 +++++++++++++++++++ ...ch_with_detached_worktree_at_its_path.snap | 63 +++++++++++++++++++ ...branch_with_detached_worktree_refuses.snap | 63 +++++++++++++++++++ 5 files changed, 277 insertions(+), 2 deletions(-) create mode 100644 tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_keeping_branch.snap create mode 100644 tests/snapshots/integration__integration_tests__remove__remove_missing_branch_with_detached_worktree_at_its_path.snap create mode 100644 tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_refuses.snap diff --git a/src/commands/remove.rs b/src/commands/remove.rs index 4af450535d..f4d013276f 100644 --- a/src/commands/remove.rs +++ b/src/commands/remove.rs @@ -85,6 +85,9 @@ impl RemovePlans { /// guard can't assert what it can't compute, and refusing every branch-only /// removal on a broken template would cost more than the case it guards. /// +/// A name that is no local branch at all is rejected before any of that, so the +/// refusal never asserts a branch that isn't there. +/// /// [`is_worktree_at_expected_path`]: super::worktree::is_worktree_at_expected_path /// [`worktree_display_name`]: super::worktree::worktree_display_name fn detached_worktree_for<'a>( @@ -93,6 +96,15 @@ fn detached_worktree_for<'a>( branch: &str, worktrees: &'a [worktrunk::git::WorktreeInfo], ) -> Option<&'a Path> { + // Downstream, `prepare_worktree_removal` is what reports a typo, a deleted + // branch, or a remote-only name; a guard that fires ahead of it would + // assert a branch that doesn't exist. `exists_locally` reports a failed + // lookup as `false` rather than an error, so this can't fail closed — but + // the same call downstream then refuses the removal, so a guard skipped + // that way still never deletes a ref. + if !repo.branch(branch).exists_locally().unwrap_or(true) { + return None; + } let expected = compute_worktree_path(repo, branch, config).ok()?; worktrees .iter() @@ -191,8 +203,19 @@ fn validate_remove_targets( // worktree it is about to remove as its own candidate is still // registered when the branch's plan is built. Refusing there // would leave prune unable to clean up either half. - if let Some(detached) = - worktrees.and_then(|wts| detached_worktree_for(repo, config, &branch, wts)) + // + // Under `Keep` (`--no-delete-branch`, or + // `[remove] delete-branch = false`) this arm deletes nothing, + // so there is no ref to strand the worktree behind — the guard + // would turn a no-op into a failure. `SafeDelete` on an + // unintegrated branch retains its ref too, but only the + // deletion attempt downstream knows that + // (`Repository::integration_reason`), so those refuse here + // rather than exiting 0 — the conservative direction, since + // what the refusal names is still on disk. + if let Some(detached) = worktrees + .filter(|_| !deletion_mode.should_keep()) + .and_then(|wts| detached_worktree_for(repo, config, &branch, wts)) { plans.record_error( GitError::DetachedWorktreeForBranch { diff --git a/tests/integration_tests/remove.rs b/tests/integration_tests/remove.rs index b8e0d7f6b8..e1464fb0f0 100644 --- a/tests/integration_tests/remove.rs +++ b/tests/integration_tests/remove.rs @@ -367,6 +367,69 @@ fn test_remove_default_branch_with_detached_main_worktree(repo: TestRepo) { assert_cmd_snapshot!(make_snapshot_cmd(&repo, "remove", &["main"], None)); } +/// The #3769 guard runs ahead of the branch-existence check in +/// `prepare_worktree_removal`, so it must not claim a branch that isn't there: +/// a name whose templated path holds a detached worktree still reports the +/// missing branch once the ref is gone. +#[rstest] +fn test_remove_missing_branch_with_detached_worktree_at_its_path(mut repo: TestRepo) { + repo.add_worktree("feature-detached-orphan"); + repo.detach_head_in_worktree("feature-detached-orphan"); + repo.run_git(&["branch", "-D", "feature-detached-orphan"]); + + assert_cmd_snapshot!(make_snapshot_cmd( + &repo, + "remove", + &["feature-detached-orphan"], + None + )); +} + +/// Under `--no-delete-branch` the branch-only removal deletes nothing, so there +/// is no ref whose deletion could strand the detached worktree — the #3769 +/// guard must not turn that no-op into a failure. +#[rstest] +fn test_remove_branch_with_detached_worktree_keeping_branch(mut repo: TestRepo) { + repo.add_worktree("feature-detached-keep"); + repo.detach_head_in_worktree("feature-detached-keep"); + + assert_cmd_snapshot!(make_snapshot_cmd( + &repo, + "remove", + &["--no-delete-branch", "feature-detached-keep"], + None + )); +} + +/// An unintegrated branch keeps its ref under `SafeDelete` too, but only the +/// deletion attempt downstream knows that — so unlike the `--no-delete-branch` +/// case above, a detached worktree refuses here rather than exiting 0 with the +/// branch retained. The refusal names a worktree that is still on disk, so this +/// pins the conservative direction rather than an accident. +#[rstest] +fn test_remove_unmerged_branch_with_detached_worktree_refuses(mut repo: TestRepo) { + let worktree_path = repo.add_worktree("feature-detached-unmerged"); + std::fs::write(worktree_path.join("feature.txt"), "new feature").unwrap(); + repo.git_command() + .args(["add", "feature.txt"]) + .current_dir(&worktree_path) + .run() + .unwrap(); + repo.git_command() + .args(["commit", "-m", "Add feature"]) + .current_dir(&worktree_path) + .run() + .unwrap(); + repo.detach_head_in_worktree("feature-detached-unmerged"); + + assert_cmd_snapshot!(make_snapshot_cmd( + &repo, + "remove", + &["feature-detached-unmerged"], + None + )); +} + /// A detached worktree whose directory is already gone is stale metadata, not a /// worktree the #3769 guard protects — the branch-only removal proceeds rather /// than refusing and naming a path that isn't on disk. diff --git a/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_keeping_branch.snap b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_keeping_branch.snap new file mode 100644 index 0000000000..57b01888ea --- /dev/null +++ b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_keeping_branch.snap @@ -0,0 +1,63 @@ +--- +source: tests/integration_tests/remove.rs +info: + program: wt + args: + - remove + - "--no-delete-branch" + - feature-detached-keep + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_AUTHOR_EMAIL: test@example.com + GIT_AUTHOR_NAME: Test User + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_EMAIL: test@example.com + GIT_COMMITTER_NAME: Test User + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- +○ No worktree found for branch feature-detached-keep diff --git a/tests/snapshots/integration__integration_tests__remove__remove_missing_branch_with_detached_worktree_at_its_path.snap b/tests/snapshots/integration__integration_tests__remove__remove_missing_branch_with_detached_worktree_at_its_path.snap new file mode 100644 index 0000000000..d51d91b34a --- /dev/null +++ b/tests/snapshots/integration__integration_tests__remove__remove_missing_branch_with_detached_worktree_at_its_path.snap @@ -0,0 +1,63 @@ +--- +source: tests/integration_tests/remove.rs +info: + program: wt + args: + - remove + - feature-detached-orphan + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_AUTHOR_EMAIL: test@example.com + GIT_AUTHOR_NAME: Test User + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_EMAIL: test@example.com + GIT_COMMITTER_NAME: Test User + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: false +exit_code: 1 +----- stdout ----- + +----- stderr ----- +✗ No branch named feature-detached-orphan +↳ To list branches, run wt list --branches --remotes diff --git a/tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_refuses.snap b/tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_refuses.snap new file mode 100644 index 0000000000..6a75bc3c84 --- /dev/null +++ b/tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_refuses.snap @@ -0,0 +1,63 @@ +--- +source: tests/integration_tests/remove.rs +info: + program: wt + args: + - remove + - feature-detached-unmerged + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_AUTHOR_EMAIL: test@example.com + GIT_AUTHOR_NAME: Test User + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_EMAIL: test@example.com + GIT_COMMITTER_NAME: Test User + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: false +exit_code: 1 +----- stdout ----- + +----- stderr ----- +✗ Branch feature-detached-unmerged has no worktree; the one @ _REPO_.feature-detached-unmerged is detached +↳ To remove the detached worktree, run wt remove _REPO_.feature-detached-unmerged From dee7d77ae7d9217ef5ccc33ac9491380e9d0535b Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 9 Aug 2026 16:03:00 -0700 Subject: [PATCH 4/6] docs(remove): scope the detached-worktree note to removals that delete a ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The paragraph was written when the guard was unconditional, so it promised a refusal that `--no-delete-branch` and `[remove] delete-branch = false` turn off — and those are the users most likely to go looking, since what they actually see is the inaccurate `○ No worktree found for branch …` that #3769 opens with. Co-Authored-By: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) --- docs/content/remove.md | 2 +- plugins/worktrunk/skills/worktrunk/reference/remove.md | 2 +- skills/worktrunk/reference/remove.md | 2 +- src/cli/mod.rs | 2 +- .../integration__integration_tests__help__help_remove_long.snap | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/content/remove.md b/docs/content/remove.md index 680b074542..e09fcb276f 100644 --- a/docs/content/remove.md +++ b/docs/content/remove.md @@ -56,7 +56,7 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so a `wt remove ` that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under `--no-delete-branch` (or `delete-branch = false`) no ref is deleted, so nothing is stranded and the removal stays a no-op. ## Force flags diff --git a/plugins/worktrunk/skills/worktrunk/reference/remove.md b/plugins/worktrunk/skills/worktrunk/reference/remove.md index c3373f6bef..1ea9738f22 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/remove.md +++ b/plugins/worktrunk/skills/worktrunk/reference/remove.md @@ -55,7 +55,7 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so a `wt remove ` that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under `--no-delete-branch` (or `delete-branch = false`) no ref is deleted, so nothing is stranded and the removal stays a no-op. ## Force flags diff --git a/skills/worktrunk/reference/remove.md b/skills/worktrunk/reference/remove.md index c3373f6bef..1ea9738f22 100644 --- a/skills/worktrunk/reference/remove.md +++ b/skills/worktrunk/reference/remove.md @@ -55,7 +55,7 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so a `wt remove ` that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under `--no-delete-branch` (or `delete-branch = false`) no ref is deleted, so nothing is stranded and the removal stays a no-op. ## Force flags diff --git a/src/cli/mod.rs b/src/cli/mod.rs index bd5e118265..7bbf6f4b97 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1286,7 +1286,7 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so `wt remove ` would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so a `wt remove ` that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under `--no-delete-branch` (or `delete-branch = false`) no ref is deleted, so nothing is stranded and the removal stays a no-op. ## Force flags diff --git a/tests/snapshots/integration__integration_tests__help__help_remove_long.snap b/tests/snapshots/integration__integration_tests__help__help_remove_long.snap index 878926a396..7959609ebd 100644 --- a/tests/snapshots/integration__integration_tests__help__help_remove_long.snap +++ b/tests/snapshots/integration__integration_tests__help__help_remove_long.snap @@ -148,7 +148,7 @@ Branches matching these conditions and with empty working trees are dimmed in [ Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via git worktree add --force) fails a different test: deleting the ref would leave that worktree unable to resolve HEAD, which is why git branch -d refuses the same delete. Such a branch is retained whatever -D asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so wt remove  would delete the ref and leave the worktree registered. It refuses instead and names the path, the one spelling that still removes the worktree. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so a wt remove  that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under --no-delete-branch (or delete-branch = false) no ref is deleted, so nothing is stranded and the removal stays a no-op. Force flags From acdbbb68242b69de06e5efad6cdec2631e60d834 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Tue, 11 Aug 2026 07:47:21 -0700 Subject: [PATCH 5/6] fix(remove): name the detached worktree instead of refusing the removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard this branch added was solving the wrong problem. Detaching a worktree's HEAD severs the only link git records between it and the branch, so the branch really has no worktree and deleting the ref alone is the correct operation — and it never lost anything: `SafeDelete` retains an unintegrated branch, so the only refs it deleted were ones worktrunk's own integration test calls lossless, and even a forced `-D` leaves the commits reachable through the detached worktree's HEAD, which is a GC root. What #3769 actually reported was silence. `○ No worktree found for branch ` is true and still reads as "nothing is there" while a directory sits at exactly that path, so the removal now names it and the `wt remove ` that clears it, on stderr and in `--format=json`'s new `detached_worktree`. Refusing instead cost a legitimate branch cleanup and let a branch address a detached worktree — the one thing the worktree model says a branch cannot name. `detached_worktree_for` survives to find the path; it no longer decides whether the command runs, which is also why the guard-scoping it needed (a branch-existence check, a `deletion_mode` gate) goes with it: an extra line of output is harmless where a refusal had to be narrowed case by case. Co-Authored-By: Claude Opus 5 (1M context) --- docs/content/remove.md | 4 +- .../skills/worktrunk/reference/remove.md | 4 +- skills/worktrunk/reference/remove.md | 4 +- src/cli/mod.rs | 4 +- src/commands/picker/mod.rs | 6 + src/commands/remove.rs | 137 +++++++----------- src/commands/repository_ext.rs | 2 + src/commands/worktree/types.rs | 23 +++ src/git/error.rs | 45 ------ src/output/handlers.rs | 54 +++++-- tests/integration_tests/remove.rs | 123 ++++++++++------ ...gration_tests__help__help_remove_long.snap | 4 +- ...with_detached_worktree_keeping_branch.snap | 3 +- ...branch_with_detached_worktree_message.snap | 7 +- ...ve__remove_detached_worktree_in_multi.snap | 10 +- ...ests__remove__remove_json_branch_only.snap | 1 + ...e__remove_json_multi_with_branch_only.snap | 1 + ...ch_with_detached_worktree_reports_it.snap} | 8 +- 18 files changed, 238 insertions(+), 202 deletions(-) rename tests/snapshots/{integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_refuses.snap => integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_reports_it.snap} (84%) diff --git a/docs/content/remove.md b/docs/content/remove.md index e09fcb276f..955569ca5e 100644 --- a/docs/content/remove.md +++ b/docs/content/remove.md @@ -56,7 +56,7 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so a `wt remove ` that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under `--no-delete-branch` (or `delete-branch = false`) no ref is deleted, so nothing is stranded and the removal stays a no-op. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so the branch has no worktree from then on and `wt remove ` deletes the ref alone. The directory stays where it is, and the removal names it and the `wt remove ` that clears it. ## Force flags @@ -100,7 +100,7 @@ Reaping runs before the worktree directory is touched, so it is independent of f ## JSON output -`--format=json` prints one object per removal to stdout: `{kind, branch, path, branch_outcome, branch_checked_out_at}` for a worktree, with `pruned` in place of `path` for a branch-only removal. +`--format=json` prints one object per removal to stdout: `{kind, branch, path, branch_outcome, branch_checked_out_at}` for a worktree, with `pruned` in place of `path` for a branch-only removal, plus `detached_worktree` — the directory left at that branch's path with a detached HEAD, which the branch no longer names and this removal therefore leaves alone. `branch_outcome` names what happened to the branch, so a caller can tell a deletion the removal declined from one it was never asked to make: diff --git a/plugins/worktrunk/skills/worktrunk/reference/remove.md b/plugins/worktrunk/skills/worktrunk/reference/remove.md index 1ea9738f22..3628191646 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/remove.md +++ b/plugins/worktrunk/skills/worktrunk/reference/remove.md @@ -55,7 +55,7 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so a `wt remove ` that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under `--no-delete-branch` (or `delete-branch = false`) no ref is deleted, so nothing is stranded and the removal stays a no-op. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so the branch has no worktree from then on and `wt remove ` deletes the ref alone. The directory stays where it is, and the removal names it and the `wt remove ` that clears it. ## Force flags @@ -102,7 +102,7 @@ Reaping runs before the worktree directory is touched, so it is independent of f ## JSON output -`--format=json` prints one object per removal to stdout: `{kind, branch, path, branch_outcome, branch_checked_out_at}` for a worktree, with `pruned` in place of `path` for a branch-only removal. +`--format=json` prints one object per removal to stdout: `{kind, branch, path, branch_outcome, branch_checked_out_at}` for a worktree, with `pruned` in place of `path` for a branch-only removal, plus `detached_worktree` — the directory left at that branch's path with a detached HEAD, which the branch no longer names and this removal therefore leaves alone. `branch_outcome` names what happened to the branch, so a caller can tell a deletion the removal declined from one it was never asked to make: diff --git a/skills/worktrunk/reference/remove.md b/skills/worktrunk/reference/remove.md index 1ea9738f22..3628191646 100644 --- a/skills/worktrunk/reference/remove.md +++ b/skills/worktrunk/reference/remove.md @@ -55,7 +55,7 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so a `wt remove ` that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under `--no-delete-branch` (or `delete-branch = false`) no ref is deleted, so nothing is stranded and the removal stays a no-op. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so the branch has no worktree from then on and `wt remove ` deletes the ref alone. The directory stays where it is, and the removal names it and the `wt remove ` that clears it. ## Force flags @@ -102,7 +102,7 @@ Reaping runs before the worktree directory is touched, so it is independent of f ## JSON output -`--format=json` prints one object per removal to stdout: `{kind, branch, path, branch_outcome, branch_checked_out_at}` for a worktree, with `pruned` in place of `path` for a branch-only removal. +`--format=json` prints one object per removal to stdout: `{kind, branch, path, branch_outcome, branch_checked_out_at}` for a worktree, with `pruned` in place of `path` for a branch-only removal, plus `detached_worktree` — the directory left at that branch's path with a detached HEAD, which the branch no longer names and this removal therefore leaves alone. `branch_outcome` names what happened to the branch, so a caller can tell a deletion the removal declined from one it was never asked to make: diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 7bbf6f4b97..1875c79f11 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1286,7 +1286,7 @@ Branches matching these conditions and with empty working trees are dimmed in `w Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via `git worktree add --force`) fails a different test: deleting the ref would leave that worktree unable to resolve `HEAD`, which is why `git branch -d` refuses the same delete. Such a branch is retained whatever `-D` asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so a `wt remove ` that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under `--no-delete-branch` (or `delete-branch = false`) no ref is deleted, so nothing is stranded and the removal stays a no-op. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so the branch has no worktree from then on and `wt remove ` deletes the ref alone. The directory stays where it is, and the removal names it and the `wt remove ` that clears it. ## Force flags @@ -1333,7 +1333,7 @@ Reaping runs before the worktree directory is touched, so it is independent of f ## JSON output -`--format=json` prints one object per removal to stdout: `{kind, branch, path, branch_outcome, branch_checked_out_at}` for a worktree, with `pruned` in place of `path` for a branch-only removal. +`--format=json` prints one object per removal to stdout: `{kind, branch, path, branch_outcome, branch_checked_out_at}` for a worktree, with `pruned` in place of `path` for a branch-only removal, plus `detached_worktree` — the directory left at that branch's path with a detached HEAD, which the branch no longer names and this removal therefore leaves alone. `branch_outcome` names what happened to the branch, so a caller can tell a deletion the removal declined from one it was never asked to make: diff --git a/src/commands/picker/mod.rs b/src/commands/picker/mod.rs index 3b09885421..8bf13ee468 100644 --- a/src/commands/picker/mod.rs +++ b/src/commands/picker/mod.rs @@ -2874,6 +2874,7 @@ pub mod tests { target_branch: None, integration_reason: None, branch_checked_out_at: None, + detached_worktree: None, }; AltXRemover::do_removal(&repo, &result, &Approvals::default()).unwrap(); @@ -2901,6 +2902,7 @@ pub mod tests { target_branch: None, integration_reason: None, branch_checked_out_at: None, + detached_worktree: None, }; AltXRemover::do_removal(&repo, &result, &Approvals::default()).unwrap(); @@ -3820,6 +3822,7 @@ pub mod tests { target_branch: None, integration_reason: None, branch_checked_out_at: None, + detached_worktree: None, }; assert_eq!( super::removal_failure_subject(&branch_only), @@ -4277,6 +4280,7 @@ pub mod tests { target_branch: None, integration_reason: None, branch_checked_out_at: None, + detached_worktree: None, }; assert!(super::removal_target_still_present(&repo, &present_branch)); @@ -4287,6 +4291,7 @@ pub mod tests { target_branch: None, integration_reason: None, branch_checked_out_at: None, + detached_worktree: None, }; assert!(!super::removal_target_still_present(&repo, &gone_branch)); } @@ -4308,6 +4313,7 @@ pub mod tests { target_branch: None, integration_reason: integration, branch_checked_out_at: None, + detached_worktree: None, } }; diff --git a/src/commands/remove.rs b/src/commands/remove.rs index f4d013276f..c7c4feea6d 100644 --- a/src/commands/remove.rs +++ b/src/commands/remove.rs @@ -58,35 +58,37 @@ impl RemovePlans { } } -/// The removable detached worktree sitting where `branch`'s worktree belongs, -/// if any. +/// The detached worktree sitting where `branch`'s worktree would go, if any — +/// the one thing a branch-only removal of `branch` can't otherwise mention. /// /// Detaching a worktree's HEAD severs the only link git records between it and -/// the branch, so once that happens the `worktree-path` template is all that -/// still connects the two. That is a stronger association than the rest of the -/// module draws — [`is_worktree_at_expected_path`] returns false for a detached -/// worktree, and [`worktree_display_name`] renders one as `dir_name (detached)` -/// without consulting the template — and it is intentional here: the template -/// match is what keeps the removal from stranding it. Three cases are -/// deliberately not matched, because refusing on them would name a removal that -/// can't happen or protect nothing: +/// the branch, so `branch` genuinely has no worktree and deleting the ref alone +/// is the right operation. But the directory is still on disk, and `No worktree +/// found for branch ` reads as though it isn't. Once the link is gone the +/// `worktree-path` template is all that still connects the two, which is a +/// weaker association than the rest of the module draws — the template records +/// where a worktree *would* go, not that this one belongs to that branch, and +/// [`is_worktree_at_expected_path`] returns false for a detached worktree while +/// [`worktree_display_name`] renders one as `dir_name (detached)` without +/// consulting the template at all. That weakness is why this only ever adds a +/// line: a mention that occasionally points at a coincidence costs nothing, +/// where a refusal built on the same inference would block work. /// -/// - a worktree checked out on some *other* branch — that branch names it, so -/// removing this one strands nothing; -/// - the main worktree, which `wt remove ` refuses — the hint would name -/// a command that can't run, and the branch falls through to the accurate -/// [`CannotRemoveDefaultBranch`](worktrunk::git::GitError::CannotRemoveDefaultBranch). -/// A bare repo has no main worktree, so its default-branch checkout is -/// matched like any other and the hint works; -/// - a prunable entry, whose directory is already gone — stale metadata for -/// `wt step prune` to sweep, not a worktree left on disk. +/// Three cases go unmatched, each because naming them would mislead: /// -/// A template that won't expand yields no expected path and so no match: the -/// guard can't assert what it can't compute, and refusing every branch-only -/// removal on a broken template would cost more than the case it guards. +/// - a worktree checked out on some *other* branch — that branch names it, and +/// it has nothing to do with this removal; +/// - the main worktree, whose path-based removal `wt remove` refuses, so the +/// hint would name a command that can't run. A bare repo has no main +/// worktree, so its default-branch checkout is named like any other and the +/// hint works; +/// - a prunable entry, whose directory is already gone — `wt step prune`'s to +/// sweep, and nothing to point a user at. /// -/// A name that is no local branch at all is rejected before any of that, so the -/// refusal never asserts a branch that isn't there. +/// A template that won't expand yields no expected path and so no match. The +/// branch is known to exist by the time this runs: `prepare_worktree_removal` +/// reports a typo, a deleted branch, or a remote-only name as an error, so a +/// plan to annotate means the lookup already succeeded. /// /// [`is_worktree_at_expected_path`]: super::worktree::is_worktree_at_expected_path /// [`worktree_display_name`]: super::worktree::worktree_display_name @@ -96,15 +98,6 @@ fn detached_worktree_for<'a>( branch: &str, worktrees: &'a [worktrunk::git::WorktreeInfo], ) -> Option<&'a Path> { - // Downstream, `prepare_worktree_removal` is what reports a typo, a deleted - // branch, or a remote-only name; a guard that fires ahead of it would - // assert a branch that doesn't exist. `exists_locally` reports a failed - // lookup as `false` rather than an error, so this can't fail closed — but - // the same call downstream then refuses the removal, so a guard skipped - // that way still never deletes a ref. - if !repo.branch(branch).exists_locally().unwrap_or(true) { - return None; - } let expected = compute_worktree_path(repo, branch, config).ok()?; worktrees .iter() @@ -112,9 +105,9 @@ fn detached_worktree_for<'a>( wt.branch.is_none() && !wt.is_prunable() && worktrunk::path::paths_match(&wt.path, &expected) - // Fail closed: a `git_dir` lookup that fails says nothing about - // whether the worktree is linked, and skipping the guard there - // deletes the ref and strands the worktree. + // A `git_dir` lookup that fails says nothing either way, so + // treat it as linked and name the worktree — an extra line is + // the worst a wrong answer here can cost. && repo.worktree_at(&wt.path).is_linked().unwrap_or(true) }) .map(|wt| wt.path.as_path()) @@ -189,45 +182,7 @@ fn validate_remove_targets( // otherwise (see its shared-branch handling). RemoveTarget::WorktreePath(path_canonical) } - ResolvedWorktree::BranchOnly { branch } => { - // A detached worktree is invisible to the branch-first lookup, - // so a branch whose worktree has since been detached resolves - // here and would have its ref deleted with the worktree left - // registered. Refuse instead, and name the path — the only - // spelling that still reaches it. - // - // The guard belongs to this command rather than to - // `prepare_worktree_removal`, which every producer of a - // `BranchOnly` target shares: `wt step prune` plans its whole - // sweep from one worktree-list snapshot, so a detached - // worktree it is about to remove as its own candidate is still - // registered when the branch's plan is built. Refusing there - // would leave prune unable to clean up either half. - // - // Under `Keep` (`--no-delete-branch`, or - // `[remove] delete-branch = false`) this arm deletes nothing, - // so there is no ref to strand the worktree behind — the guard - // would turn a no-op into a failure. `SafeDelete` on an - // unintegrated branch retains its ref too, but only the - // deletion attempt downstream knows that - // (`Repository::integration_reason`), so those refuse here - // rather than exiting 0 — the conservative direction, since - // what the refusal names is still on disk. - if let Some(detached) = worktrees - .filter(|_| !deletion_mode.should_keep()) - .and_then(|wts| detached_worktree_for(repo, config, &branch, wts)) - { - plans.record_error( - GitError::DetachedWorktreeForBranch { - branch, - path: detached.to_path_buf(), - } - .into(), - ); - continue; - } - RemoveTarget::BranchOnly(branch) - } + ResolvedWorktree::BranchOnly { branch } => RemoveTarget::BranchOnly(branch), // Resolution tried the argument as a branch and as a worktree path // and matched neither, so a directory sitting there is a leftover // skeleton rather than anything wt can remove. Only a typed @@ -251,14 +206,32 @@ fn validate_remove_targets( // Bucket the validated result, not the pre-validation resolution: // a worktree whose directory disappeared degrades to BranchOnly // during preparation and must run with the other branch-only plans. - Ok(result) => match &result { - RemovalPlan::Worktree { - changed_directory: true, + Ok(mut result) => { + // A branch whose worktree has since been detached still has no + // worktree — detaching severed the link — so the branch-only + // removal is right. What it can't see is the directory still + // sitting at that branch's templated path, which the output + // would otherwise never mention. Annotate here, where the + // user's typed branch name and the config are both in hand. + if let RemovalPlan::BranchOnly { + branch_name, + detached_worktree, .. - } => plans.current = Some(result), - RemovalPlan::Worktree { .. } => plans.others.push(result), - RemovalPlan::BranchOnly { .. } => plans.branch_only.push(result), - }, + } = &mut result + && let Some(wts) = worktrees + { + *detached_worktree = detached_worktree_for(repo, config, branch_name, wts) + .map(Path::to_path_buf); + } + match &result { + RemovalPlan::Worktree { + changed_directory: true, + .. + } => plans.current = Some(result), + RemovalPlan::Worktree { .. } => plans.others.push(result), + RemovalPlan::BranchOnly { .. } => plans.branch_only.push(result), + } + } Err(e) => plans.record_error(e), } } diff --git a/src/commands/repository_ext.rs b/src/commands/repository_ext.rs index 4292261a99..3942657d60 100644 --- a/src/commands/repository_ext.rs +++ b/src/commands/repository_ext.rs @@ -319,6 +319,7 @@ impl RepositoryCliExt for Repository { target_branch: None, integration_reason: None, branch_checked_out_at: Some(shared), + detached_worktree: None, }); } let default_branch = self.default_branch(); @@ -337,6 +338,7 @@ impl RepositoryCliExt for Repository { target_branch, integration_reason, branch_checked_out_at: None, + detached_worktree: None, }); } Resolved::Worktree { diff --git a/src/commands/worktree/types.rs b/src/commands/worktree/types.rs index 47815cd02a..a96b65487f 100644 --- a/src/commands/worktree/types.rs +++ b/src/commands/worktree/types.rs @@ -338,6 +338,22 @@ pub enum RemovalPlan { /// checkout of the same branch survives the fallback to branch-only /// deletion. See [`SharedBranchCheckout`]. branch_checked_out_at: Option, + /// A detached worktree occupying the directory `branch_name`'s worktree + /// would use, when one is there. + /// + /// Detaching severs the only link git records between a worktree and a + /// branch, so the branch truly has no worktree and this removal is the + /// right operation — but the directory is still on disk, and saying only + /// "no worktree found" leaves the user believing it isn't. Naming it is + /// the whole job: nothing here acts on it, and `wt remove ` is + /// what removes it. + /// + /// The command fills this rather than the planner, which has no + /// [`UserConfig`](worktrunk::config::UserConfig) to expand the + /// `worktree-path` template with. `None` everywhere else, which is also + /// what `wt step prune` and the picker want — neither has a user's typed + /// branch name to explain. + detached_worktree: Option, }, } @@ -425,6 +441,7 @@ impl RemovalPlan { branch_name, prune_entry, branch_checked_out_at, + detached_worktree, .. } => serde_json::json!({ "kind": "branch_only", @@ -432,6 +449,7 @@ impl RemovalPlan { "pruned": prune_entry.is_some(), "branch_outcome": branch_outcome, "branch_checked_out_at": branch_checked_out_at.as_ref().map(|c| &c.path), + "detached_worktree": detached_worktree, }), } } @@ -465,6 +483,7 @@ mod tests { target_branch: None, integration_reason: None, branch_checked_out_at: None, + detached_worktree: None, }; assert_eq!(branch_only.branch_name(), Some("solo")); } @@ -664,6 +683,7 @@ mod tests { target_branch: None, integration_reason: None, branch_checked_out_at: None, + detached_worktree: None, }; match result { RemovalPlan::BranchOnly { @@ -673,6 +693,7 @@ mod tests { target_branch, integration_reason, branch_checked_out_at, + .. } => { assert_eq!(branch_name, "stale-branch"); assert!(deletion_mode.should_keep()); @@ -695,6 +716,7 @@ mod tests { target_branch: Some("main".to_string()), integration_reason: None, branch_checked_out_at: None, + detached_worktree: None, }; match result { RemovalPlan::BranchOnly { @@ -704,6 +726,7 @@ mod tests { target_branch, integration_reason, branch_checked_out_at, + .. } => { assert_eq!(branch_name, "pruned-branch"); assert!(!deletion_mode.should_keep()); diff --git a/src/git/error.rs b/src/git/error.rs index 9db0f70519..2acb68b85f 100644 --- a/src/git/error.rs +++ b/src/git/error.rs @@ -588,28 +588,6 @@ pub enum GitError { WorktreeNotFound { branch: String, }, - /// A branch with no worktree, whose worktree's place is taken by a - /// registered worktree with a detached HEAD. - /// - /// Detaching severs the only link git records between a worktree and a - /// branch, so the branch-first lookup reports no worktree and an operation - /// addressed by branch degrades to acting on the ref alone. For removal - /// that means deleting the ref, leaving the worktree registered, and - /// reporting success. Refusing is the honest answer, and the path is the - /// only spelling that still reaches the worktree. - /// - /// Distinct from [`GitError::WorktreeNotFound`], where the branch has no - /// checkout anywhere and creating one is the right suggestion. - /// - /// [`GitError::WorktreePathOccupied`] reports the same physical state to - /// `wt switch`, which wants the worktree back on the branch and says so. - /// Removal wants it gone, so the two carry different hints and split on the - /// occupied-by-another-branch case: that one blocks a switch, and leaves a - /// removal nothing to strand. - DetachedWorktreeForBranch { - branch: String, - path: PathBuf, - }, /// A worktree selector matched neither a branch nor a worktree path. /// /// Distinct from [`GitError::WorktreeNotFound`], which means the branch @@ -905,13 +883,6 @@ impl GitError { cformat!("Branch {branch} has no worktree") } - GitError::DetachedWorktreeForBranch { branch, path } => { - let path_display = format_path_for_display(path); - cformat!( - "Branch {branch} has no worktree; the one @ {path_display} is detached" - ) - } - GitError::WorktreeSelectorNotFound { selector } => { cformat!("No branch or worktree named {selector}") } @@ -1470,22 +1441,6 @@ impl GitError { ) } - GitError::DetachedWorktreeForBranch { path, .. } => { - let title = self.title(); - // `format_path_for_display` already returns a shell-ready - // token, so the command is built by interpolation — routing it - // through `suggest_command` would escape it a second time. - let display_path = format_path_for_display(path); - write!( - f, - "{}\n{}", - error_message(&title), - hint_message(cformat!( - "To remove the detached worktree, run wt remove {display_path}" - )) - ) - } - GitError::WorktreeSelectorNotFound { .. } => { let title = self.title(); write!( diff --git a/src/output/handlers.rs b/src/output/handlers.rs index 36148cf119..210f29e4a7 100644 --- a/src/output/handlers.rs +++ b/src/output/handlers.rs @@ -1196,6 +1196,7 @@ pub fn handle_remove_output( target_branch, integration_reason, branch_checked_out_at, + detached_worktree, } => handle_branch_only_output( branch_name, *deletion_mode, @@ -1203,6 +1204,7 @@ pub fn handle_remove_output( *integration_reason, target_branch.as_deref(), branch_checked_out_at.as_ref(), + detached_worktree.as_deref(), quiet, ), } @@ -1216,6 +1218,17 @@ pub fn handle_remove_output( /// /// When `quiet` is true, suppresses the "No worktree found for branch X" /// info line for non-pruned cases (noise in prune/batch context). +/// +/// `detached_worktree` is a directory still sitting where this branch's +/// worktree would go, detached and so nameless in the branch namespace. The +/// removal is correct without it — the branch really has no worktree — but the +/// info line above reads as "nothing there", so it is named alongside, with the +/// path spelling that removes it. Only ever set for a branch the user typed. +/// +/// The parameters are one `RemovalPlan::BranchOnly`'s fields, destructured by +/// the sole caller, plus `quiet` — so the count tracks the variant rather than +/// a signature anyone chose. +#[allow(clippy::too_many_arguments)] fn handle_branch_only_output( branch_name: &str, deletion_mode: BranchDeletionMode, @@ -1223,6 +1236,7 @@ fn handle_branch_only_output( integration_reason: Option, target_branch: Option<&str>, branch_checked_out_at: Option<&SharedBranchCheckout>, + detached_worktree: Option<&Path>, quiet: bool, ) -> anyhow::Result { let pruned = if let Some(path) = prune_entry { @@ -1231,15 +1245,37 @@ fn handle_branch_only_output( } else { false }; - let branch_info = if pruned { - cformat!("Worktree directory missing for {branch_name}; pruned") - } else { - cformat!("No worktree found for branch {branch_name}") + // A detached worktree at this branch's templated path is never a pruned + // entry — that one's directory is already gone — so only the "no worktree" + // wording, the one that reads as "nothing there", has to answer for it. + let branch_info = match (pruned, detached_worktree) { + (true, _) => cformat!("Worktree directory missing for {branch_name}; pruned"), + (false, Some(path)) => { + let path = format_path_for_display(path); + cformat!( + "No worktree found for branch {branch_name}; a detached worktree is @ {path}" + ) + } + (false, None) => cformat!("No worktree found for branch {branch_name}"), + }; + // `branch_info` prints from five places below, so the hint that follows it + // rides along rather than being repeated at each. + let announce_branch_info = || { + eprintln!("{}", info_message(&branch_info)); + if let Some(path) = detached_worktree { + let path = format_path_for_display(path); + eprintln!( + "{}", + hint_message(cformat!( + "To remove the detached worktree, run wt remove {path}" + )) + ); + } }; // If we won't delete the branch, show info and return early if deletion_mode.should_keep() { - eprintln!("{}", info_message(&branch_info)); + announce_branch_info(); // A sibling `--force` checkout kept the branch alive; name it so the // user knows why the pruned branch survived rather than being deleted. if let Some(shared) = branch_checked_out_at { @@ -1294,7 +1330,7 @@ fn handle_branch_only_output( let retained = match &deletion.result.outcome { BranchDeletionOutcome::RetainedCheckedOut { path } => { - eprintln!("{}", info_message(&branch_info)); + announce_branch_info(); eprintln!( "{}", retained_checked_out_branch_message(branch_name, path, false) @@ -1302,12 +1338,12 @@ fn handle_branch_only_output( true } BranchDeletionOutcome::RetainedRaced => { - eprintln!("{}", info_message(&branch_info)); + announce_branch_info(); eprintln!("{}", retained_raced_branch_message(branch_name, false)); true } BranchDeletionOutcome::NotDeleted => { - eprintln!("{}", info_message(&branch_info)); + announce_branch_info(); if deletion.show_unmerged_hint { print_retained_unmerged_branch(branch_name); } @@ -1335,7 +1371,7 @@ fn handle_branch_only_output( ); } else { if !quiet { - eprintln!("{}", info_message(&branch_info)); + announce_branch_info(); } eprintln!( "{}", diff --git a/tests/integration_tests/remove.rs b/tests/integration_tests/remove.rs index e1464fb0f0..a282baa1dc 100644 --- a/tests/integration_tests/remove.rs +++ b/tests/integration_tests/remove.rs @@ -292,17 +292,17 @@ fn test_remove_locked_worktree_directory_missing(mut repo: TestRepo) { ); } -/// Regression test for #3769: `wt remove ` on a worktree whose HEAD has -/// since been detached deleted the branch, left the worktree registered, and -/// exited 0. Detaching severs the only link git records between the two, so the -/// branch-first lookup found no worktree and the removal degraded to -/// branch-only — half the operation, and the silent half. +/// Regression test for #3769. Detaching a worktree's HEAD severs the only link +/// git records between it and the branch, so the branch really has no worktree +/// and deleting the ref alone is the right operation — the directory is left on +/// disk either way, and `wt remove ` is what clears it. /// -/// Refusing is defensible because the path still reaches the worktree, which -/// `test_remove_detached_worktree_by_path` pins. +/// What was wrong was the report: `○ No worktree found for branch ` reads +/// as "nothing is there" while a directory sits at exactly that path. The +/// removal must still happen, and must now name what it left behind. #[rstest] fn test_remove_branch_whose_worktree_was_detached(mut repo: TestRepo) { - let _worktree_path = repo.add_worktree("detached-later"); + let worktree_path = repo.add_worktree("detached-later"); repo.detach_head_in_worktree("detached-later"); let output = repo @@ -311,38 +311,41 @@ fn test_remove_branch_whose_worktree_was_detached(mut repo: TestRepo) { .output() .unwrap(); assert!( - !output.status.success(), - "wt remove should refuse a branch whose worktree has been detached.\nstdout: {}\nstderr: {}", + output.status.success(), + "the branch has no worktree, so removing it is the right operation.\nstdout: {}\nstderr: {}", String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr), ); - // The branch must survive — deleting it is the half that used to run. + // The branch goes, because nothing is checked out on it. let branch_exists = repo .git_command() .args(["branch", "--list", "detached-later"]) .run() .unwrap(); assert!( - !String::from_utf8_lossy(&branch_exists.stdout) + String::from_utf8_lossy(&branch_exists.stdout) .trim() .is_empty(), - "Branch should NOT be deleted while a detached worktree occupies its path", + "Branch should be deleted — no worktree holds it", ); - // ...and so must the worktree it would otherwise have stranded. - let list_after = repo - .git_command() - .args(["worktree", "list", "--porcelain"]) - .run() - .unwrap(); + // The detached worktree is untouched, and the output says so rather than + // leaving the user to discover it. assert!( - String::from_utf8_lossy(&list_after.stdout).contains("detached-later"), - "Detached worktree should still be registered", + worktree_path.exists(), + "Detached worktree must be left alone" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("a detached worktree is @"), + "Output must name the directory it left behind; stderr:\n{stderr}" ); } -/// The refusal names the worktree and the path-based removal that reaches it. +/// The wording: the info line names the detached worktree alongside the branch +/// it no longer belongs to, and the hint gives the path spelling that removes +/// it. #[rstest] fn test_remove_branch_with_detached_worktree_message(mut repo: TestRepo) { repo.add_worktree("feature-detached-strand"); @@ -357,9 +360,9 @@ fn test_remove_branch_with_detached_worktree_message(mut repo: TestRepo) { } /// The default branch's expected path is the main worktree, so a detached main -/// worktree would match the #3769 guard — but `wt remove ` refuses -/// too, so pointing at it would be a dead end. The default-branch refusal, which -/// is the accurate answer, must still be what surfaces. +/// worktree would match — but `wt remove ` refuses the main worktree, +/// so naming it would be a dead end. The default-branch refusal, which is the +/// accurate answer, is what surfaces. #[rstest] fn test_remove_default_branch_with_detached_main_worktree(repo: TestRepo) { repo.run_git(&["checkout", "--detach", "HEAD"]); @@ -367,10 +370,9 @@ fn test_remove_default_branch_with_detached_main_worktree(repo: TestRepo) { assert_cmd_snapshot!(make_snapshot_cmd(&repo, "remove", &["main"], None)); } -/// The #3769 guard runs ahead of the branch-existence check in -/// `prepare_worktree_removal`, so it must not claim a branch that isn't there: -/// a name whose templated path holds a detached worktree still reports the -/// missing branch once the ref is gone. +/// A detached worktree at the templated path must not make a name that is no +/// longer a branch look like one: once the ref is gone, the missing branch is +/// what gets reported. #[rstest] fn test_remove_missing_branch_with_detached_worktree_at_its_path(mut repo: TestRepo) { repo.add_worktree("feature-detached-orphan"); @@ -385,9 +387,9 @@ fn test_remove_missing_branch_with_detached_worktree_at_its_path(mut repo: TestR )); } -/// Under `--no-delete-branch` the branch-only removal deletes nothing, so there -/// is no ref whose deletion could strand the detached worktree — the #3769 -/// guard must not turn that no-op into a failure. +/// `--no-delete-branch` deletes nothing, so the removal is a no-op — but the +/// detached worktree is still worth naming, since "no worktree found" misreads +/// the same way whether or not a ref goes with it. #[rstest] fn test_remove_branch_with_detached_worktree_keeping_branch(mut repo: TestRepo) { repo.add_worktree("feature-detached-keep"); @@ -401,13 +403,11 @@ fn test_remove_branch_with_detached_worktree_keeping_branch(mut repo: TestRepo) )); } -/// An unintegrated branch keeps its ref under `SafeDelete` too, but only the -/// deletion attempt downstream knows that — so unlike the `--no-delete-branch` -/// case above, a detached worktree refuses here rather than exiting 0 with the -/// branch retained. The refusal names a worktree that is still on disk, so this -/// pins the conservative direction rather than an accident. +/// An unintegrated branch keeps its ref under `SafeDelete`, and the detached +/// worktree at its path is named all the same — the note is about the directory +/// on disk, not about what happened to the ref. #[rstest] -fn test_remove_unmerged_branch_with_detached_worktree_refuses(mut repo: TestRepo) { +fn test_remove_unmerged_branch_with_detached_worktree_reports_it(mut repo: TestRepo) { let worktree_path = repo.add_worktree("feature-detached-unmerged"); std::fs::write(worktree_path.join("feature.txt"), "new feature").unwrap(); repo.git_command() @@ -430,9 +430,9 @@ fn test_remove_unmerged_branch_with_detached_worktree_refuses(mut repo: TestRepo )); } -/// A detached worktree whose directory is already gone is stale metadata, not a -/// worktree the #3769 guard protects — the branch-only removal proceeds rather -/// than refusing and naming a path that isn't on disk. +/// A detached worktree whose directory is already gone is stale metadata for +/// `wt step prune` to sweep — there is nothing on disk to point the user at, so +/// the removal says nothing extra. #[rstest] fn test_remove_branch_with_prunable_detached_worktree(mut repo: TestRepo) { let worktree_path = repo.add_worktree("feature-detached-gone"); @@ -2777,10 +2777,9 @@ fn test_remove_detached_worktree_in_multi(mut repo: TestRepo) { // Detach HEAD in feature-b repo.detach_head_in_worktree("feature-b"); - // From main, try to multi-remove both. feature-a is removed; feature-b is - // refused, because detaching dropped it out of the branch-first lookup and - // removing it by branch would delete the ref and strand the worktree - // (#3769). Partial success is the point: one refusal doesn't stop the rest. + // From main, remove both. feature-a's worktree goes; feature-b was detached, + // so its branch has no worktree and only the ref is deleted — with the + // directory left behind named in the output rather than passed over (#3769). assert_cmd_snapshot!(make_snapshot_cmd( &repo, "remove", @@ -4077,6 +4076,40 @@ fn test_remove_json_branch_only(repo: TestRepo) { assert_snapshot!(String::from_utf8_lossy(&output.stdout)); } +/// `--format=json` carries the detached worktree too. The human output names it +/// on stderr, which a script consuming stdout never sees — and "a script +/// checking the exit code sees a clean run" is half of what #3769 reported. +#[rstest] +fn test_remove_json_reports_detached_worktree(mut repo: TestRepo) { + let worktree_path = repo.add_worktree("json-detached"); + repo.detach_head_in_worktree("json-detached"); + + let output = repo + .wt_command() + .args([ + "remove", + "json-detached", + "--format=json", + "--yes", + "--foreground", + ]) + .output() + .unwrap(); + assert!(output.status.success()); + + let json: serde_json::Value = + serde_json::from_str(&String::from_utf8_lossy(&output.stdout)).unwrap(); + let entry = &json[0]; + assert_eq!(entry["kind"], "branch_only"); + assert_eq!( + entry["detached_worktree"] + .as_str() + .map(std::path::Path::new), + Some(worktree_path.as_path()), + "json must name the directory the removal left behind: {json}" + ); +} + #[cfg(not(target_os = "windows"))] #[rstest] fn test_remove_json_multi_with_branch_only(mut repo: TestRepo) { diff --git a/tests/snapshots/integration__integration_tests__help__help_remove_long.snap b/tests/snapshots/integration__integration_tests__help__help_remove_long.snap index 7959609ebd..003355f239 100644 --- a/tests/snapshots/integration__integration_tests__help__help_remove_long.snap +++ b/tests/snapshots/integration__integration_tests__help__help_remove_long.snap @@ -148,7 +148,7 @@ Branches matching these conditions and with empty working trees are dimmed in [ Those six ask whether deleting loses work. A branch checked out in a second worktree (only reachable via git worktree add --force) fails a different test: deleting the ref would leave that worktree unable to resolve HEAD, which is why git branch -d refuses the same delete. Such a branch is retained whatever -D asks, and the surviving checkout is named. -Detaching a worktree's HEAD severs the only link git records between it and the branch, so a wt remove  that would delete the ref refuses instead and names the path, the one spelling that still removes the worktree. Under --no-delete-branch (or delete-branch = false) no ref is deleted, so nothing is stranded and the removal stays a no-op. +Detaching a worktree's HEAD severs the only link git records between it and the branch, so the branch has no worktree from then on and wt remove  deletes the ref alone. The directory stays where it is, and the removal names it and the wt remove  that clears it. Force flags @@ -191,7 +191,7 @@ Reaping runs before the worktree directory is touched, so it is independent of f JSON output ---format=json prints one object per removal to stdout: {kind, branch, path, branch_outcome, branch_checked_out_at} for a worktree, with pruned in place of path for a branch-only removal. +--format=json prints one object per removal to stdout: {kind, branch, path, branch_outcome, branch_checked_out_at} for a worktree, with pruned in place of path for a branch-only removal, plus detached_worktree — the directory left at that branch's path with a detached HEAD, which the branch no longer names and this removal therefore leaves alone. branch_outcome names what happened to the branch, so a caller can tell a deletion the removal declined from one it was never asked to make: diff --git a/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_keeping_branch.snap b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_keeping_branch.snap index 57b01888ea..9d2c25aac2 100644 --- a/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_keeping_branch.snap +++ b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_keeping_branch.snap @@ -60,4 +60,5 @@ exit_code: 0 ----- stdout ----- ----- stderr ----- -○ No worktree found for branch feature-detached-keep +○ No worktree found for branch feature-detached-keep; a detached worktree is @ _REPO_.feature-detached-keep +↳ To remove the detached worktree, run wt remove _REPO_.feature-detached-keep diff --git a/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_message.snap b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_message.snap index 9160fd423e..f6ed9e9dcf 100644 --- a/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_message.snap +++ b/tests/snapshots/integration__integration_tests__remove__remove_branch_with_detached_worktree_message.snap @@ -54,10 +54,11 @@ info: WORKTRUNK_TEST_ZSH_INSTALLED: "0" XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" --- -success: false -exit_code: 1 +success: true +exit_code: 0 ----- stdout ----- ----- stderr ----- -✗ Branch feature-detached-strand has no worktree; the one @ _REPO_.feature-detached-strand is detached +○ No worktree found for branch feature-detached-strand; a detached worktree is @ _REPO_.feature-detached-strand ↳ To remove the detached worktree, run wt remove _REPO_.feature-detached-strand +✓ Removed branch feature-detached-strand (same commit as main, _) diff --git a/tests/snapshots/integration__integration_tests__remove__remove_detached_worktree_in_multi.snap b/tests/snapshots/integration__integration_tests__remove__remove_detached_worktree_in_multi.snap index d6b367aaff..05987312bc 100644 --- a/tests/snapshots/integration__integration_tests__remove__remove_detached_worktree_in_multi.snap +++ b/tests/snapshots/integration__integration_tests__remove__remove_detached_worktree_in_multi.snap @@ -55,12 +55,14 @@ info: WORKTRUNK_TEST_ZSH_INSTALLED: "0" XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" --- -success: false -exit_code: 1 +success: true +exit_code: 0 ----- stdout ----- ----- stderr ----- -✗ Branch feature-b has no worktree; the one @ _REPO_.feature-b is detached -↳ To remove the detached worktree, run wt remove _REPO_.feature-b ◎ Removing feature-a worktree in background ↳ Branch unmerged; to delete, run wt remove -D feature-a +○ No worktree found for branch feature-b; a detached worktree is @ _REPO_.feature-b +↳ To remove the detached worktree, run wt remove _REPO_.feature-b +○ Branch feature-b retained; has unmerged changes +↳ To delete the unmerged branch, run wt remove -D feature-b diff --git a/tests/snapshots/integration__integration_tests__remove__remove_json_branch_only.snap b/tests/snapshots/integration__integration_tests__remove__remove_json_branch_only.snap index c060354f86..f274836478 100644 --- a/tests/snapshots/integration__integration_tests__remove__remove_json_branch_only.snap +++ b/tests/snapshots/integration__integration_tests__remove__remove_json_branch_only.snap @@ -7,6 +7,7 @@ expression: "String::from_utf8_lossy(&output.stdout)" "branch": "orphan-branch", "branch_checked_out_at": null, "branch_outcome": "deleted", + "detached_worktree": null, "kind": "branch_only", "pruned": false } diff --git a/tests/snapshots/integration__integration_tests__remove__remove_json_multi_with_branch_only.snap b/tests/snapshots/integration__integration_tests__remove__remove_json_multi_with_branch_only.snap index 9a7eac0ea2..713f4a447d 100644 --- a/tests/snapshots/integration__integration_tests__remove__remove_json_multi_with_branch_only.snap +++ b/tests/snapshots/integration__integration_tests__remove__remove_json_multi_with_branch_only.snap @@ -14,6 +14,7 @@ expression: "String::from_utf8_lossy(&output.stdout)" "branch": "orphan-branch", "branch_checked_out_at": null, "branch_outcome": "deleted", + "detached_worktree": null, "kind": "branch_only", "pruned": false } diff --git a/tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_refuses.snap b/tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_reports_it.snap similarity index 84% rename from tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_refuses.snap rename to tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_reports_it.snap index 6a75bc3c84..4931cd39aa 100644 --- a/tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_refuses.snap +++ b/tests/snapshots/integration__integration_tests__remove__remove_unmerged_branch_with_detached_worktree_reports_it.snap @@ -54,10 +54,12 @@ info: WORKTRUNK_TEST_ZSH_INSTALLED: "0" XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" --- -success: false -exit_code: 1 +success: true +exit_code: 0 ----- stdout ----- ----- stderr ----- -✗ Branch feature-detached-unmerged has no worktree; the one @ _REPO_.feature-detached-unmerged is detached +○ No worktree found for branch feature-detached-unmerged; a detached worktree is @ _REPO_.feature-detached-unmerged ↳ To remove the detached worktree, run wt remove _REPO_.feature-detached-unmerged +○ Branch feature-detached-unmerged retained; has unmerged changes +↳ To delete the unmerged branch, run wt remove -D feature-detached-unmerged From 581e1129fa8d85c476225bb5f05aa1c0581e1e02 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Tue, 11 Aug 2026 08:06:45 -0700 Subject: [PATCH 6/6] test(remove): pin the main-worktree exclusion, and say what decides it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `compute_worktree_path` returns the repo root for the default branch of a non-bare repo, so a detached main worktree passes every predicate in `detached_worktree_for` except `is_linked()` — which is the only thing stopping the removal from pointing a user at a directory `wt remove` refuses outright. Nothing pinned that: the existing default-branch test never reaches the annotation, because `check_not_default_branch` errors first and only fires when the removal isn't forced. `-D` skips that error and runs the whole way through, so that is where the test belongs. Verified by dropping the predicate, which makes the new snapshot name the repo root. The docstring said the exclusion exists because "the hint would name a command that can't run", which a locked worktree also satisfies while being named all the same. What actually decides it is whether the hint leads anywhere: a lock refusal carries the `git worktree unlock` that clears the way, and the main worktree's carries nothing. Co-Authored-By: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/remove.rs | 10 +-- tests/integration_tests/remove.rs | 14 ++++ ...nch_force_with_detached_main_worktree.snap | 64 +++++++++++++++++++ 3 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 tests/snapshots/integration__integration_tests__remove__remove_default_branch_force_with_detached_main_worktree.snap diff --git a/src/commands/remove.rs b/src/commands/remove.rs index c7c4feea6d..4d6eb83f04 100644 --- a/src/commands/remove.rs +++ b/src/commands/remove.rs @@ -78,10 +78,12 @@ impl RemovePlans { /// /// - a worktree checked out on some *other* branch — that branch names it, and /// it has nothing to do with this removal; -/// - the main worktree, whose path-based removal `wt remove` refuses, so the -/// hint would name a command that can't run. A bare repo has no main -/// worktree, so its default-branch checkout is named like any other and the -/// hint works; +/// - the main worktree, which `wt remove ` refuses with nothing to do +/// about it, so the hint would be a dead end. What decides this is whether +/// the hint leads anywhere, not whether the command succeeds: a *locked* +/// worktree refuses too and is named all the same, because that refusal +/// carries the `git worktree unlock` that clears the way. A bare repo has no +/// main worktree, so its default-branch checkout is named like any other; /// - a prunable entry, whose directory is already gone — `wt step prune`'s to /// sweep, and nothing to point a user at. /// diff --git a/tests/integration_tests/remove.rs b/tests/integration_tests/remove.rs index a282baa1dc..89c5427454 100644 --- a/tests/integration_tests/remove.rs +++ b/tests/integration_tests/remove.rs @@ -370,6 +370,20 @@ fn test_remove_default_branch_with_detached_main_worktree(repo: TestRepo) { assert_cmd_snapshot!(make_snapshot_cmd(&repo, "remove", &["main"], None)); } +/// `-D` is what reaches the annotation for the default branch: without it +/// `check_not_default_branch` errors first, so the case above never exercises +/// the exclusion it appears to be about. `compute_worktree_path` returns the +/// repo root for the default branch of a non-bare repo, and a detached main +/// worktree passes every other predicate in the `find` — `is_linked()` is the +/// only thing keeping the removal from pointing at a directory `wt remove` +/// refuses outright. +#[rstest] +fn test_remove_default_branch_force_with_detached_main_worktree(repo: TestRepo) { + repo.run_git(&["checkout", "--detach", "HEAD"]); + + assert_cmd_snapshot!(make_snapshot_cmd(&repo, "remove", &["-D", "main"], None)); +} + /// A detached worktree at the templated path must not make a name that is no /// longer a branch look like one: once the ref is gone, the missing branch is /// what gets reported. diff --git a/tests/snapshots/integration__integration_tests__remove__remove_default_branch_force_with_detached_main_worktree.snap b/tests/snapshots/integration__integration_tests__remove__remove_default_branch_force_with_detached_main_worktree.snap new file mode 100644 index 0000000000..8c932a3c66 --- /dev/null +++ b/tests/snapshots/integration__integration_tests__remove__remove_default_branch_force_with_detached_main_worktree.snap @@ -0,0 +1,64 @@ +--- +source: tests/integration_tests/remove.rs +info: + program: wt + args: + - remove + - "-D" + - main + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_ALLOW_PROTOCOL: file + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_AUTHOR_EMAIL: test@example.com + GIT_AUTHOR_NAME: Test User + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_EMAIL: test@example.com + GIT_COMMITTER_NAME: Test User + GIT_CONFIG_COUNT: "2" + GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig + GIT_CONFIG_KEY_0: user.useConfigOnly + GIT_CONFIG_KEY_1: rerere.enabled + GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig + GIT_CONFIG_VALUE_0: "true" + GIT_CONFIG_VALUE_1: "false" + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- +○ No worktree found for branch main +✓ Removed branch main (--force-delete)