Skip to content

Reintroduce clean with a configurable removal command #34

Description

@leogdion

clean was removed from git-trees before v1.0.0 (#17). This issue is the
complete record required to bring it back: the removal-command design it must
ship with, and every bug it has ever had.

Read this whole issue before writing a line of the reimplementation. The
guards described in Part 2 went away with the code, so nothing in the current
tree prevents any of these bugs from returning.

Part 1 — an rm override before it comes back

Why

git worktree remove unlinks the directory. There is no undo. A deleted branch
is recoverable from the reflog; uncommitted work in the removed worktree is
not
— which is exactly the state a "stale worktree" is most likely to be in.

Making the removal step configurable turns the worst-case outcome from data
loss into an inconvenience.

Proposed design

A TREES_RM environment variable, unset by default. Unset keeps the historical
behavior (git worktree remove). When set, it names the command that disposes
of a worktree directory:

export TREES_RM=trash        # macOS, Homebrew `trash`
export TREES_RM=trash-put    # Linux, trash-cli
export TREES_RM="gio trash"  # Linux, GLib

A --rm <cmd> flag on clean can override it per-invocation.

This does not break the pure-git constraint in AGENTS.md. The default path
still uses only git; the override is opt-in and the command is named by the
user, not detected or hardcoded by the script.

Requirements the implementation has to meet

  1. Preserve the dirty-worktree refusal. git worktree remove refuses to
    remove a worktree with uncommitted changes or untracked files unless
    --force. Going around it drops that check, so it has to be reinstated
    explicitly — git -C "$p" status --porcelain must be empty — before the
    removal command runs.
  2. Verify the command exists first. command -v the override at the top of
    clean and fail with a clear error, rather than discovering it is missing
    partway through a multi-branch run.
  3. Clean up git's admin files. Removing the directory out from under git
    leaves .git/worktrees/<name> behind; git worktree prune handles it, but it
    needs to run in the override path too.
  4. Confirm the directory is actually gone before proceeding to
    git branch -d. Trash utilities vary in exit-code discipline; if the
    directory still exists, skip the branch delete and report it — matching how
    the old code handled a failed git worktree remove.
  5. No eval, and quote the path. Split TREES_RM into a command and fixed
    arguments with read -r -a (fine on bash 3.2), then invoke it directly.
    Pass -- before the path so a directory name beginning with - is not read
    as a flag.
  6. Say what it did. The report line should name the disposal method, so
    clean output makes clear whether a worktree was trashed or deleted.

Also decided

TREES_RM should not apply to branch deletion. git branch -d is already
refusal-based and reflog-recoverable, and there is no sensible "trash" for a
ref. Document that the override covers working trees only.

Part 2 — the clean bug history

Every bug clean has actually had, consolidated so a reimplementation has the
whole record in one place. None of these may come back. The first three were
open when clean was pulled; the last three were found and fixed while it was
still shipping, and their guards must be rewritten from scratch along with the
command.

Open when clean was pulled

1. clean --gone offers to delete the default branch (was #19)

The --merged pass explicitly protected the default branch; the --gone pass
did not. If the default branch's upstream was reported [gone],
clean --gone --apply removed its worktree and deleted the branch.

Reproduced with a file:// fixture where upstream main was deleted, so local
main reported [gone] after fetch --prune:

$ git for-each-ref --format='%(refname:short) %(upstream:short) %(upstream:track)' refs/heads
doomed origin/doomed [gone]
main origin/main [gone]

$ git trees clean --gone
== branches with gone upstream ==
  doomed  (/…/proj/doomed)
  main  (/…/proj/main)
(report only — pass --apply to execute)

With --apply, main got git worktree remove then git branch -d.
Mitigations that limited but did not remove the impact: clean reported unless
--apply was passed, and git branch -d refuses unmerged branches — but when
the default branch is fully merged (the common case for a synced main), -d
succeeds and the worktree is already gone by then. This was the only issue in
the pre-1.0 review with real blast radius.

Plausible ways to reach [gone] on a default branch: the remote's default was
renamed (mastermain), the branch was deleted and recreated upstream, or a
fork's default differs from the local checkout.

Fix on reintroduction: skip $def in the gone loop, mirroring what the merged
pass did (| grep -vx "$def") — and in every other pass.

2. --older-than accepts non-numeric values and exits 0 (was #21)

clean --older-than N never checked that N was a number. A non-numeric value
produced a nonsense heading, silently found nothing, and exited 0 — invalid
input was indistinguishable from a successful clean run.

$ git trees clean --older-than abc
== branches with gone upstream ==
== branches merged into main ==
== worktrees untouched >abcd ==
(report only — pass --apply to execute)
$ echo $?
0

find rejects -mtime +abc, but its stderr was discarded by 2>/dev/null, so
the failure was invisible. Not an injection vector — "$days" was quoted, so
30 -exec rm -rf {} ; reached find as a single argument and was rejected. A
correctness and trust bug, not a security one.

Fix on reintroduction: validate as a non-negative integer:

case "$days" in
  ''|*[!0-9]*)
    echo "git trees clean: --older-than needs a non-negative integer (days)" >&2
    return 1 ;;
esac

That also rejects -1 and 3.5. Decide deliberately whether 0 is valid.

3. Missing --older-than value hangs forever (was the clean half of #18)

--older-than) days="${2:-}"; do_stale=1; shift 2 ;; had no arity check. With
set -uo pipefail but deliberately no set -e, shift 2 fails on a single
remaining argument and leaves $1 unchanged, so while [ $# -gt 0 ] spins
forever. "${2:-}" defends against set -u, which is why it read as safe — the
bug was the shift, not the expansion.

git trees clean --older-than hung and had to be killed. The identical
init --host / init --dir cases were fixed under #18; the clean half is a
requirement here. Note the CI wrinkle: a plain assertion hangs the runner, so
bound it with a background PID plus kill -0 (timeout is not on macOS by
default).

Fixed while clean was shipping — rewritten guards must preserve these

4. --older-than --apply removed the worktree you were standing in (#2, fixed)

The cwd guard was [ "$wt" = "$PWD" ] && continue, which never held on macOS
(git worktree list reports /private/tmp/... while $PWD is /tmp/...) and
ignored subdirectories, so cd victim/sub was not recognized as "current".

git trees add victim
touch -t 202001010000 victim
cd victim
git trees clean --older-than 1 --apply   # worktree deleted out from under the shell

The fix canonicalized both sides (cd … && pwd -P) and tested containment, not
just equality. It was only ever applied to the stale pass — the gone and
merged passes never got a cwd guard at all, which is why the acceptance criteria
below require it in all three.

5. Worktree paths containing spaces were truncated (#3, fixed)

Porcelain parsing took awk $2 after worktree, so any path with a space was
cut short — breaking _path_for, list, list --json, and the clean stale
loop alike. A project at /tmp/tt space/proj reported path as
/private/tmp/tt. The fix takes the remainder of the line
(sub(/^worktree /, "")). For clean this was a targeting bug: it can miss or
mis-aim a removal.

6. Failed worktree remove was reported as "unmerged" (#6, fixed)

On --apply, when git worktree remove failed on a dirty or untracked-file
worktree, git branch -d then failed too — because the branch was still checked
out — and the fallback printed ! unmerged — use: git branch -D <branch>. That
is the wrong diagnosis, and it advises a force-delete for a branch that is
merged and whose work is uncommitted on disk. The fix skipped the branch delete
and reported the remove failure instead. Suggest git branch -D only when -d
fails on a genuinely unmerged tip after the worktree is gone.

This one matters doubly for Part 1: routing removal through TREES_RM bypasses
git worktree remove's own dirty-tree refusal, which is what makes this failure
path reachable in the first place.

Acceptance criteria

Numbered references are to Part 2.

  • All passes skip the default branch (1) and the worktree containing cwd, by
    canonicalized containment rather than string equality (4)
  • --older-than validated as a non-negative integer (2); a missing value
    errors promptly instead of hanging (3)
  • Worktree paths are parsed as the remainder of the porcelain line, and a
    path containing a space round-trips through every pass (5)
  • A failed removal is reported as a removal failure and skips the branch
    delete; git branch -D is suggested only for a genuinely unmerged tip
    after the worktree is gone (6)
  • TREES_RM (and/or --rm) routes worktree disposal through the named
    command, with the dirty check preserved
  • CI exercises --apply end-to-end: a worktree and branch actually removed,
    the default branch left alone, and a dirty worktree refused
  • CI exercises the TREES_RM path with a stub command that records its
    arguments instead of deleting, asserting the path is passed after --
  • AGENTS.md "Report before destroy" is restored alongside the code
  • README.md § Removing worktrees replaced by a clean command section

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions