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
- 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.
- 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.
- 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.
- 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.
- 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.
- 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 (master → main), 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.
cleanwas removed fromgit-treesbefore v1.0.0 (#17). This issue is thecomplete 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
rmoverride before it comes backWhy
git worktree removeunlinks the directory. There is no undo. A deleted branchis 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_RMenvironment variable, unset by default. Unset keeps the historicalbehavior (
git worktree remove). When set, it names the command that disposesof a worktree directory:
A
--rm <cmd>flag oncleancan override it per-invocation.This does not break the pure-git constraint in
AGENTS.md. The default pathstill uses only
git; the override is opt-in and the command is named by theuser, not detected or hardcoded by the script.
Requirements the implementation has to meet
git worktree removerefuses toremove a worktree with uncommitted changes or untracked files unless
--force. Going around it drops that check, so it has to be reinstatedexplicitly —
git -C "$p" status --porcelainmust be empty — before theremoval command runs.
command -vthe override at the top ofcleanand fail with a clear error, rather than discovering it is missingpartway through a multi-branch run.
leaves
.git/worktrees/<name>behind;git worktree prunehandles it, but itneeds to run in the override path too.
git branch -d. Trash utilities vary in exit-code discipline; if thedirectory still exists, skip the branch delete and report it — matching how
the old code handled a failed
git worktree remove.eval, and quote the path. SplitTREES_RMinto a command and fixedarguments 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 readas a flag.
cleanoutput makes clear whether a worktree was trashed or deleted.Also decided
TREES_RMshould not apply to branch deletion.git branch -dis alreadyrefusal-based and reflog-recoverable, and there is no sensible "trash" for a
ref. Document that the override covers working trees only.
Part 2 — the
cleanbug historyEvery bug
cleanhas actually had, consolidated so a reimplementation has thewhole record in one place. None of these may come back. The first three were
open when
cleanwas pulled; the last three were found and fixed while it wasstill shipping, and their guards must be rewritten from scratch along with the
command.
Open when
cleanwas pulled1.
clean --goneoffers to delete the default branch (was #19)The
--mergedpass explicitly protected the default branch; the--gonepassdid not. If the default branch's upstream was reported
[gone],clean --gone --applyremoved its worktree and deleted the branch.Reproduced with a
file://fixture where upstreammainwas deleted, so localmainreported[gone]afterfetch --prune:With
--apply,maingotgit worktree removethengit branch -d.Mitigations that limited but did not remove the impact:
cleanreported unless--applywas passed, andgit branch -drefuses unmerged branches — but whenthe default branch is fully merged (the common case for a synced
main),-dsucceeds 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 wasrenamed (
master→main), the branch was deleted and recreated upstream, or afork's default differs from the local checkout.
Fix on reintroduction: skip
$defin the gone loop, mirroring what the mergedpass did (
| grep -vx "$def") — and in every other pass.2.
--older-thanaccepts non-numeric values and exits 0 (was #21)clean --older-than Nnever checked thatNwas a number. A non-numeric valueproduced a nonsense heading, silently found nothing, and exited 0 — invalid
input was indistinguishable from a successful clean run.
findrejects-mtime +abc, but its stderr was discarded by2>/dev/null, sothe failure was invisible. Not an injection vector —
"$days"was quoted, so30 -exec rm -rf {} ;reachedfindas a single argument and was rejected. Acorrectness and trust bug, not a security one.
Fix on reintroduction: validate as a non-negative integer:
That also rejects
-1and3.5. Decide deliberately whether0is valid.3. Missing
--older-thanvalue hangs forever (was thecleanhalf of #18)--older-than) days="${2:-}"; do_stale=1; shift 2 ;;had no arity check. Withset -uo pipefailbut deliberately noset -e,shift 2fails on a singleremaining argument and leaves
$1unchanged, sowhile [ $# -gt 0 ]spinsforever.
"${2:-}"defends againstset -u, which is why it read as safe — thebug was the
shift, not the expansion.git trees clean --older-thanhung and had to be killed. The identicalinit --host/init --dircases were fixed under #18; thecleanhalf is arequirement here. Note the CI wrinkle: a plain assertion hangs the runner, so
bound it with a background PID plus
kill -0(timeoutis not on macOS bydefault).
Fixed while
cleanwas shipping — rewritten guards must preserve these4.
--older-than --applyremoved the worktree you were standing in (#2, fixed)The cwd guard was
[ "$wt" = "$PWD" ] && continue, which never held on macOS(
git worktree listreports/private/tmp/...while$PWDis/tmp/...) andignored subdirectories, so
cd victim/subwas not recognized as "current".The fix canonicalized both sides (
cd … && pwd -P) and tested containment, notjust 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
$2afterworktree, so any path with a space wascut short — breaking
_path_for,list,list --json, and thecleanstaleloop alike. A project at
/tmp/tt space/projreportedpathas/private/tmp/tt. The fix takes the remainder of the line(
sub(/^worktree /, "")). Forcleanthis was a targeting bug: it can miss ormis-aim a removal.
6. Failed
worktree removewas reported as "unmerged" (#6, fixed)On
--apply, whengit worktree removefailed on a dirty or untracked-fileworktree,
git branch -dthen failed too — because the branch was still checkedout — and the fallback printed
! unmerged — use: git branch -D <branch>. Thatis 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 -Donly when-dfails on a genuinely unmerged tip after the worktree is gone.
This one matters doubly for Part 1: routing removal through
TREES_RMbypassesgit worktree remove's own dirty-tree refusal, which is what makes this failurepath reachable in the first place.
Acceptance criteria
Numbered references are to Part 2.
canonicalized containment rather than string equality (4)
--older-thanvalidated as a non-negative integer (2); a missing valueerrors promptly instead of hanging (3)
path containing a space round-trips through every pass (5)
delete;
git branch -Dis suggested only for a genuinely unmerged tipafter the worktree is gone (6)
TREES_RM(and/or--rm) routes worktree disposal through the namedcommand, with the dirty check preserved
--applyend-to-end: a worktree and branch actually removed,the default branch left alone, and a dirty worktree refused
TREES_RMpath with a stub command that records itsarguments instead of deleting, asserting the path is passed after
--AGENTS.md"Report before destroy" is restored alongside the codeREADME.md§ Removing worktrees replaced by acleancommand section