Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/content/remove.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 a `wt remove <branch>` 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

Worktrunk has two force flags for different situations:
Expand Down
2 changes: 2 additions & 0 deletions plugins/worktrunk/skills/worktrunk/reference/remove.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 a `wt remove <branch>` 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

Worktrunk has two force flags for different situations:
Expand Down
2 changes: 2 additions & 0 deletions skills/worktrunk/reference/remove.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 a `wt remove <branch>` 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

Worktrunk has two force flags for different situations:
Expand Down
107 changes: 105 additions & 2 deletions src/commands/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -58,13 +58,76 @@ 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 <path>` 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.
///
/// 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>(
repo: &Repository,
config: &UserConfig,
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()
.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,
/// another worktree, or branch-only, and prepares the removal plan.
/// Errors are collected (not fatal) to support partial success.
fn validate_remove_targets(
repo: &Repository,
config: &UserConfig,
branches: Vec<String>,
keep_branch: bool,
force_delete: bool,
Expand Down Expand Up @@ -126,7 +189,45 @@ 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.
//
// 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)
}
// 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
Expand Down Expand Up @@ -386,6 +487,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,
Expand Down Expand Up @@ -493,6 +595,7 @@ mod tests {

let plans = validate_remove_targets(
&repo,
&UserConfig::default(),
vec!["missing-worktree".to_string(), "branch-only".to_string()],
false,
false,
Expand Down
23 changes: 16 additions & 7 deletions src/commands/repository_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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 <bold>{branch}</> gained a worktree @ <bold>{path}</> since it was selected; to remove that worktree, run <bold>{}</>",
suggest_command("remove", &[&path], &[])
"Branch <bold>{branch}</> gained a worktree @ <bold>{path}</> since it was selected; to remove that worktree, run <bold>wt remove {path}</>"
));
}
// Check the branch exists locally, so a typo or a remote-only
Expand Down Expand Up @@ -578,10 +580,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,
Expand Down
45 changes: 45 additions & 0 deletions src/git/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -883,6 +905,13 @@ impl GitError {
cformat!("Branch <bold>{branch}</> has no worktree")
}

GitError::DetachedWorktreeForBranch { branch, path } => {
let path_display = format_path_for_display(path);
cformat!(
"Branch <bold>{branch}</> has no worktree; the one @ <bold>{path_display}</> is detached"
)
}

GitError::WorktreeSelectorNotFound { selector } => {
cformat!("No branch or worktree named <bold>{selector}</>")
}
Expand Down Expand Up @@ -1441,6 +1470,22 @@ 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 <underline>wt remove {display_path}</>"
))
)
}

GitError::WorktreeSelectorNotFound { .. } => {
let title = self.title();
write!(
Expand Down
7 changes: 3 additions & 4 deletions src/git/repository/worktrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 <underline>{cmd}</>"))
Expand Down
8 changes: 8 additions & 0 deletions src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading