From 7ea5643ca567263ad392ce1aa1b02772ce3f6ffb Mon Sep 17 00:00:00 2001 From: Leo Dion Date: Wed, 5 Aug 2026 14:54:49 -0400 Subject: [PATCH 1/3] Reintroduce rm and clean subcommands (#34) --- .claude/agent-notes.md | 3 +- AGENTS.md | 10 +- README.md | 35 ++++++- git-trees | 210 ++++++++++++++++++++++++++++++++++++++++- tests/smoke.sh | 102 ++++++++++++++++++++ 5 files changed, 352 insertions(+), 8 deletions(-) diff --git a/.claude/agent-notes.md b/.claude/agent-notes.md index 245256b..3b879c4 100644 --- a/.claude/agent-notes.md +++ b/.claude/agent-notes.md @@ -15,6 +15,7 @@ update or remove the stale line rather than leaving both. - Branch names with `/` are supported and slug to `-` in the directory name; do not reject slash branches. - User-facing docs (README, CHANGELOG) must not mention features that never shipped; 1.0.0 is the first release, so there is no prior version to reference. - CHANGELOG follows GitHub release-notes format (`## What's Changed` + PR URLs), listing shipped features only — not Keep a Changelog / Unreleased / pre-1.0 fix archaeology. PR URLs remain even after the git history wipe. -- Do not mention previous code, removed subcommands, or pre-v1.0 archaeology anywhere in the tree (docs, comments, tests). Forward-looking constraints and current git-behavior rationale are fine; unused merged-branch/`rev-list` guidance and a dedicated `clean` unknown-command test are not. +- Do not mention previous code, removed subcommands, or pre-v1.0 archaeology anywhere in the tree (docs, comments, tests). Forward-looking constraints and current git-behavior rationale are fine. - Do not remove an `init` container on agent-seeding failure; return nonzero and leave the directory. - `main` is always the stable release; README curl install pins `main` (not version tags). Do not describe `main` as a development/moving target. +- `clean` supports `--gone` and `--merged` (detecting direct, rebased, and squash-merged PRs); `--older-than` is omitted. diff --git a/AGENTS.md b/AGENTS.md index b43004f..aeec13e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,10 +48,10 @@ consequence is that `feature/x` and `feature-x` compete for one directory; the directory (`_branch_at`). Do not "fix" that by inventing a suffixed variant: a directory whose name the user cannot predict is worse than an error. -**Nothing destructive.** No subcommand removes a worktree or deletes a branch. -Anything that destroys user data must report by default and act only under an -explicit `--apply`, must use `git branch -d` and never `-D`, and must route -directory removal through a user-configurable command. +**Nothing destructive without `--apply`.** `rm` and `clean` report by default and modify state only when `--apply` is explicitly passed. Local branch deletions use `git branch -d` (falling back to `-D` on `clean` once confirmed gone/merged, or on `rm` when `--apply` is passed), and worktree directory removals route through `TREES_RM_CMD` when configured (defaulting to `git worktree remove`). + + + **`track` only ever sets `origin/`.** Same remote, same name. There is no flag for an arbitrary upstream, and `origin` is hardcoded throughout — @@ -118,6 +118,8 @@ What the suite covers: - **install.sh** — places the binary; seeds `~/.config/git-trees/AGENTS.md` from the template under a redirected `HOME`; does not overwrite an existing config file +- **rm** — dry run vs `--apply`, worktree removal by branch or path, safe branch deletion refusal on unmerged branches, and custom `TREES_RM_CMD` routing +- **clean** — `--gone` identification and deletion, `--merged` identification across direct merges, rebased commits, and squash-merged PRs, zero-commit fresh branch preservation, dry run vs `--apply`, and custom `TREES_RM_CMD` routing Two assertion shapes are easy to get wrong: diff --git a/README.md b/README.md index f19cba4..14f7f78 100644 --- a/README.md +++ b/README.md @@ -253,9 +253,39 @@ One entry per branch with upstream, ahead/behind, last commit date, clean/dirty, and path (relative to the project root). Includes branches with no worktree, shown with path `(none)`. `--json` emits the same fields as an array. +### `git trees rm [--apply]` + +Removes a worktree and deletes its branch. Accepts either a branch name or a worktree path. + +By default (without `--apply`), reports what would be removed without making any changes. Pass `--apply` to perform the removal. + +Worktree directories are removed via `TREES_RM_CMD` if configured, or `git worktree remove`. Local branches are deleted using `git branch -d` (falling back to `git branch -D` if needed). + + +### `git trees clean [--merged|--gone] [--apply]` + +Reports or removes stale worktrees and branches. + +Selectors: +- `--gone`: branches whose upstream remote branch was deleted (`[gone]`) +- `--merged`: branches merged into the default branch. Automatically detects direct merges, rebased/cherry-picked commits, and squash-merged PRs while leaving fresh 0-commit branches intact. + +Passing neither selector runs both `--gone` and `--merged`. + +By default (without `--apply`), `clean` operates in dry-run mode and prints matching branches/worktrees without deleting them. Pass `--apply` to execute removals. Worktree directories are removed via `TREES_RM_CMD` if set (defaulting to `git worktree remove`), and branches are deleted using `git branch -d` (falling back to `git branch -D` for gone/squash-merged branches). + + + ## Removing worktrees -`git-trees` does not delete anything. Remove a worktree and its branch with git: +`git-trees` provides `rm` and `clean` for removing worktrees and branches: + +```bash +git trees rm feature-x --apply # remove a single worktree and its branch +git trees clean --apply # remove merged and gone branches/worktrees +``` + +Both subcommands default to dry-run mode (report only) unless `--apply` is passed. Alternatively, you can use plain git: ```bash git worktree remove @@ -271,6 +301,7 @@ git worktree prune | `TREES_ORG` | *(unset)* | Default org; if unset, bare repo names are rejected | | `TREES_AGENTS_TEMPLATE` | `~/.config/git-trees/AGENTS.md` | Seeded at the container root by `init` (and `root --agents`) | | `TREES_NO_PUSH` | *(unset)* | Any non-empty value: `add`/`track` never create a branch on `origin` | +| `TREES_RM_CMD` | *(unset)* | Custom command for worktree directory removal (defaults to `git worktree remove`) | ## Shell wrapper (optional) @@ -288,8 +319,8 @@ trees() { ## Known limitations -- Removing stale worktrees and branches is manual; nothing here deletes. - `list` spawns several processes per branch — fine for dozens, slow for hundreds. + - `add` ignores `base` when the branch already exists rather than failing. - Branch names beginning with `-` are unsupported: `add` parses them as options and reports `unknown option`. There is no `--` end-of-options marker. diff --git a/git-trees b/git-trees index 386efbd..db54da0 100755 --- a/git-trees +++ b/git-trees @@ -8,12 +8,15 @@ # git trees add [base] [--print-path] [--no-push] # git trees track [path] [--no-push] # git trees list [--json] (alias: ls) +# git trees rm [--apply] +# git trees clean [--merged|--gone] [--apply] # # Env (all optional): # TREES_HOST default host for init (default: github.com) # TREES_ORG default org; if unset, bare repo names are rejected # TREES_AGENTS_TEMPLATE AGENTS.md seeded at the container root # TREES_NO_PUSH non-empty: never create a branch on origin +# TREES_RM_CMD custom command for worktree directory removal set -uo pipefail @@ -21,10 +24,68 @@ set -uo pipefail : "${TREES_ORG:=}" : "${TREES_AGENTS_TEMPLATE:=$HOME/.config/git-trees/AGENTS.md}" : "${TREES_NO_PUSH:=}" +: "${TREES_RM_CMD:=}" + # --- internals --------------------------------------------------------------- +_remove_worktree() { + local path="$1" + if [ -n "$TREES_RM_CMD" ]; then + $TREES_RM_CMD "$path" + else + git worktree remove "$path" + fi +} + +_is_branch_merged() { # _is_branch_merged -> 0 if merged, 1 if not + local br="$1" def="$2" mb br_tree patch_id br_commit + + br_commit=$(git rev-parse "$br" 2>/dev/null) || return 1 + mb=$(git merge-base "origin/$def" "$br" 2>/dev/null) || return 1 + + # 1. Direct merge check: br is an ancestor of origin/$def AND br is not identical to origin/$def tip + if git merge-base --is-ancestor "$br" "origin/$def" 2>/dev/null; then + if [ "$br_commit" != "$mb" ]; then + return 0 + fi + if [ "$br_commit" != "$(git rev-parse "origin/$def" 2>/dev/null)" ]; then + return 0 + fi + fi + + # 2. Rebase / Cherry-pick check (git cherry has no '+' lines) + if git rev-parse --verify --quiet "origin/$def" >/dev/null; then + if [ "$(git rev-list --count "$mb..$br" 2>/dev/null)" -gt 0 ]; then + if ! git cherry "origin/$def" "$br" 2>/dev/null | grep -q '^+'; then + return 0 + fi + fi + fi + + # 3. Squash-merge tree match (branch tree exists in origin/$def history) + br_tree=$(git rev-parse "$br^{tree}" 2>/dev/null) + if [ -n "$br_tree" ] && git log -n 100 --format='%T' "origin/$def" 2>/dev/null | grep -qx "$br_tree"; then + if [ "$(git rev-list --count "$mb..$br" 2>/dev/null)" -gt 0 ]; then + return 0 + fi + fi + + # 4. Squash-merge patch-id match + patch_id=$(git diff "$mb..$br" 2>/dev/null | git patch-id | awk '{print $1}') + if [ -n "$patch_id" ]; then + if git log -p "$mb..origin/$def" 2>/dev/null | git patch-id | awk '{print $1}' | grep -qx "$patch_id"; then + return 0 + fi + fi + + return 1 +} + + + _root() { + local common common=$(git rev-parse --git-common-dir 2>/dev/null) || return 1 cd "$(dirname "$common")" 2>/dev/null && pwd @@ -524,6 +585,147 @@ cmd_list() { fi } +# --- rm ---------------------------------------------------------------------- + +cmd_rm() { + local target="" apply=0 br="" path="" root + + while [ $# -gt 0 ]; do + case "$1" in + --apply) apply=1; shift ;; + -*) echo "git trees rm: unknown option $1" >&2; return 1 ;; + *) + if [ -z "$target" ]; then target="$1"; shift + else echo "git trees rm: unexpected argument $1" >&2; return 1 + fi ;; + esac + done + + [ -z "$target" ] && { + echo "usage: git trees rm [--apply]" >&2 + return 1 + } + + root=$(_root) || { echo "git trees rm: not in a git repo" >&2; return 1; } + + if git show-ref --verify --quiet "refs/heads/$target"; then + br="$target" + path=$(_path_for "$br") + elif [ -d "$target" ]; then + path=$(cd "$target" 2>/dev/null && pwd) + br=$(_branch_at "$path") + else + echo "git trees rm: target '$target' not found" >&2 + return 1 + fi + + if [ -z "$br" ] && [ -z "$path" ]; then + echo "git trees rm: target '$target' not found" >&2 + return 1 + fi + + if [ "$apply" -eq 0 ]; then + [ -n "$path" ] && echo "Would remove worktree: $path" + [ -n "$br" ] && echo "Would delete branch: $br" + echo "(report only — pass --apply to execute)" + return 0 + fi + + if [ -n "$path" ]; then + if ! _remove_worktree "$path"; then + echo "git trees rm: worktree removal failed for $path" >&2 + return 1 + fi + git worktree prune >/dev/null 2>&1 + fi + + if [ -n "$br" ]; then + if ! { git branch -d "$br" 2>/dev/null || git branch -D "$br" 2>/dev/null; }; then + echo "git trees rm: could not delete branch $br" >&2 + fi + fi + + git worktree prune >/dev/null 2>&1 + return 0 +} + + +# --- clean ------------------------------------------------------------------- + +cmd_clean() { + local apply=0 do_gone=0 do_merged=0 root def br p + + while [ $# -gt 0 ]; do + case "$1" in + --gone) do_gone=1; shift ;; + --merged) do_merged=1; shift ;; + --apply) apply=1; shift ;; + *) echo "git trees clean: unknown option $1" >&2; return 1 ;; + esac + done + + if [ "$do_gone" -eq 0 ] && [ "$do_merged" -eq 0 ]; then + do_gone=1 + do_merged=1 + fi + + root=$(_root) || { echo "git trees clean: not in a git repo" >&2; return 1; } + def=$(_default_branch) + git fetch --prune origin >/dev/null 2>&1 || true + + if [ "$do_gone" -eq 1 ]; then + echo "== branches with gone upstream ==" + while read -r br; do + [ -z "$br" ] && continue + p=$(_path_for "$br") + echo " $br${p:+ ($p)}" + if [ "$apply" -eq 1 ]; then + if [ -n "$p" ]; then + if ! _remove_worktree "$p"; then + echo " ! worktree remove failed — skipped branch delete for $br" >&2 + continue + fi + git worktree prune >/dev/null 2>&1 + fi + git branch -d "$br" 2>/dev/null || git branch -D "$br" 2>/dev/null \ + || echo " ! could not delete branch $br" >&2 + fi + done < <( + git for-each-ref --format='%(refname:short) %(upstream:track)' refs/heads \ + | grep '\[gone\]' | awk '{print $1}' + ) + fi + + if [ "$do_merged" -eq 1 ]; then + echo "== branches merged into $def ==" + while read -r br; do + [ -z "$br" ] && continue + [ "$br" = "$def" ] && continue + if _is_branch_merged "$br" "$def"; then + p=$(_path_for "$br") + echo " $br${p:+ ($p)}" + if [ "$apply" -eq 1 ]; then + if [ -n "$p" ]; then + if ! _remove_worktree "$p"; then + echo " ! worktree remove failed — skipped branch delete for $br" >&2 + continue + fi + git worktree prune >/dev/null 2>&1 + fi + git branch -d "$br" 2>/dev/null || git branch -D "$br" 2>/dev/null \ + || echo " ! could not delete branch $br" >&2 + fi + fi + done < <(git for-each-ref --format='%(refname:short)' refs/heads) + fi + + git worktree prune >/dev/null 2>&1 + [ "$apply" -eq 0 ] && echo "(report only — pass --apply to execute)" + return 0 +} + + + # --- usage / dispatch -------------------------------------------------------- usage() { @@ -536,12 +738,16 @@ usage: git trees [args] create a worktree (sets upstream) track [path] [--no-push] ensure branch has an upstream list [--json] worktrees + branches without one + rm [--apply] remove worktree and delete branch + clean [--merged|--gone] [--apply] report/remove merged or gone branches + + note: a branch that does not exist on origin is created there by `add`/`track` via `git push -u origin HEAD`. Pass --no-push (or set TREES_NO_PUSH) to skip that and leave the upstream unset. -env (optional): TREES_ORG, TREES_HOST, TREES_AGENTS_TEMPLATE, TREES_NO_PUSH +env (optional): TREES_ORG, TREES_HOST, TREES_AGENTS_TEMPLATE, TREES_NO_PUSH, TREES_RM_CMD — see README note: `add` prints the worktree path but cannot cd your shell. For that: @@ -564,6 +770,8 @@ main() { add) cmd_add "$@" ;; track) cmd_track "$@" ;; list|ls) cmd_list "$@" ;; + rm) cmd_rm "$@" ;; + clean) cmd_clean "$@" ;; help|--help|-h) usage; return 0 ;; *) echo "git trees: unknown command '$cmd'" >&2; usage; return 1 ;; esac diff --git a/tests/smoke.sh b/tests/smoke.sh index e150c53..913f620 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -142,6 +142,9 @@ assert_fail "unknown command exits nonzero" bash "$T" definitely-not-a-command out=$(bash "$T" help 2>&1) assert_contains "help lists add" "$out" "add " assert_contains "help lists list" "$out" "list [--json]" +assert_contains "help lists rm" "$out" "rm " +assert_contains "help lists clean" "$out" "clean [--merged|--gone]" + section "outside a repo" mkdir -p "$TMP/plain" @@ -484,8 +487,107 @@ assert_eq "install.sh rerun exits 0" "$rc" "0" assert_eq "install.sh does not overwrite an existing template" \ "$(cat "$IHOME/.config/git-trees/AGENTS.md")" "CUSTOM" +# --- rm ---------------------------------------------------------------------- + +section "rm" +RM_C=$(new_container rm-c) +cd "$RM_C" || exit 1 + +assert_ok "create worktree to remove" bash "$T" add rm-target --no-push +assert_ok "worktree directory exists" test -d rm-target + +out=$(bash "$T" rm rm-target 2>&1) +assert_ok "dry run leaves worktree intact" test -d rm-target +assert_contains "dry run reports plan" "$out" "Would remove worktree" +assert_contains "dry run instructs to use --apply" "$out" "(report only — pass --apply to execute)" + +assert_ok "rm with --apply" bash "$T" rm rm-target --apply +assert_fail "worktree directory deleted" test -e rm-target +assert_fail "branch deleted" git show-ref --verify --quiet refs/heads/rm-target + +# Unmerged branch: worktree removed, branch preserved with error warning +assert_ok "create unmerged worktree" bash "$T" add rm-unmerged --no-push +echo "unmerged data" > rm-unmerged/unmerged.txt +git -C rm-unmerged add . && git -C rm-unmerged commit -qm "unmerged commit" + +assert_ok "rm --apply removes worktree and unmerged branch" bash "$T" rm rm-unmerged --apply +assert_fail "worktree directory deleted for unmerged" test -e rm-unmerged +assert_fail "unmerged branch deleted" git show-ref --verify --quiet refs/heads/rm-unmerged + + +# Custom TREES_RM_CMD +assert_ok "create worktree for custom TREES_RM_CMD" bash "$T" add rm-custom --no-push +assert_ok "rm with TREES_RM_CMD" env TREES_RM_CMD="rm -rf" bash "$T" rm rm-custom --apply +assert_fail "custom rm deleted directory" test -e rm-custom +assert_fail "custom rm deleted branch" git show-ref --verify --quiet refs/heads/rm-custom + +assert_fail "rm with no argument" bash "$T" rm +assert_fail "rm with nonexistent target" bash "$T" rm nonexistent + + +# --- clean ------------------------------------------------------------------- + +section "clean" +CLEAN_C=$(new_container clean-c) +cd "$CLEAN_C" || exit 1 + +# Setup origin repo changes for clean testing +# 1. Gone upstream branch +assert_ok "add gone-branch" bash "$T" add gone-branch +git -C "$ORIGIN" branch -D gone-branch >/dev/null 2>&1 + +# 2. Direct merged branch +assert_ok "add merged-direct" bash "$T" add merged-direct +echo "direct change" > merged-direct/direct.txt +git -C merged-direct add . && git -C merged-direct commit -qm "direct commit" +git -C merged-direct push -u origin merged-direct >/dev/null 2>&1 +( cd "$ORIGIN" && git checkout -q main && git merge -q merged-direct --no-ff -m "merge direct" ) >/dev/null 2>&1 +git -C "$CLEAN_C" fetch -q origin + +# 3. Rebase merged branch +assert_ok "add merged-rebase" bash "$T" add merged-rebase +echo "rebase change" > merged-rebase/rebase.txt +git -C merged-rebase add . && git -C merged-rebase commit -qm "rebase commit" +git -C merged-rebase push -u origin merged-rebase >/dev/null 2>&1 +( cd "$ORIGIN" && git checkout -q main && git cherry-pick merged-rebase ) >/dev/null 2>&1 +git -C "$CLEAN_C" fetch -q origin + +# 4. Squash merged branch +assert_ok "add merged-squash" bash "$T" add merged-squash +echo "squash change" > merged-squash/squash.txt +git -C merged-squash add . && git -C merged-squash commit -qm "squash commit" +git -C merged-squash push -u origin merged-squash >/dev/null 2>&1 +( cd "$ORIGIN" && git checkout -q main && git merge -q --squash merged-squash && git commit -qm "squash merge commit" ) >/dev/null 2>&1 +git -C "$CLEAN_C" fetch -q origin + +# 5. Fresh 0-commit branch cut from main (MUST NOT be cleaned) +assert_ok "add fresh-branch" bash "$T" add fresh-branch --no-push + + + +# Run dry run +out=$(bash "$T" clean 2>&1) +assert_contains "clean dry run reports gone branch" "$out" "gone-branch" +assert_contains "clean dry run reports direct merged" "$out" "merged-direct" +assert_contains "clean dry run reports rebase merged" "$out" "merged-rebase" +assert_contains "clean dry run reports squash merged" "$out" "merged-squash" +assert_not_contains "clean dry run preserves fresh 0-commit branch" "$out" "fresh-branch" +assert_contains "clean dry run reports report-only notice" "$out" "(report only — pass --apply to execute)" + +# Apply clean +assert_ok "clean --apply" bash "$T" clean --apply +assert_fail "gone-branch deleted" git show-ref --verify --quiet refs/heads/gone-branch +assert_fail "merged-direct deleted" git show-ref --verify --quiet refs/heads/merged-direct +assert_fail "merged-rebase deleted" git show-ref --verify --quiet refs/heads/merged-rebase +assert_fail "merged-squash deleted" git show-ref --verify --quiet refs/heads/merged-squash +assert_ok "fresh-branch preserved after clean" git show-ref --verify --quiet refs/heads/fresh-branch + +# Cleanup fresh-branch +bash "$T" rm fresh-branch --apply >/dev/null 2>&1 + # --- summary ----------------------------------------------------------------- + cd "$TMP" || exit 1 echo if [ "$FAILED" -eq 0 ]; then From 6a4ca0bdaf2fd1dedf06d5f42ef525b1f5b29dc5 Mon Sep 17 00:00:00 2001 From: Leo Dion Date: Wed, 5 Aug 2026 16:11:10 -0400 Subject: [PATCH 2/3] Address CodeRabbit review on #47 Close a data-loss hole in `rm`, correct docs that contradicted the code, and cover the paths the suite claimed but never exercised. `cmd_rm`'s path arm accepted any directory. `git worktree remove` refuses a non-worktree on its own, but `TREES_RM_CMD` does not, so `TREES_RM_CMD="rm -rf" git trees rm . --apply` deleted the container root and the bare store. Gate the path arm on `git worktree list` registration rather than on the resolved branch, since a detached-HEAD worktree legitimately has none. Resolve with `pwd -P`: git records worktrees by physical path, so a symlinked parent (macOS /var) would fail both that check and `_branch_at`. Also in the script: drop an unreachable comparison in `_is_branch_merged` (once a branch is an ancestor, the merge base is that branch); default `rev-list --count` to 0 so a failure cannot leak bash's "integer expression expected"; bound the patch-id scan to the same 100 commits as the tree scan above it; return nonzero from `clean --apply` when a removal failed, matching `cmd_rm`. `AGENTS.md` claimed `rm` refused to delete unmerged branches while line 51 documented the `-d` -> `-D` escalation correctly; the tests asserted deletion. Correct the coverage lines and record the new constraints. Warn in the README that `TREES_RM_CMD` bypasses git's check for uncommitted changes and untracked files. Tests: assert the fixture commits and the `clean` origin setup, which were unasserted and could fail silently into confusing downstream failures; add the path arm (a slugged directory is the only shape that reaches it) and the non-worktree refusal; assert `clean --apply` removes worktree directories, not just refs; run each selector alone; route `clean` through `TREES_RM_CMD`; note that the section must stay last because it mutates the shared origin. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 25 +++++++++++- README.md | 24 ++++++++++-- git-trees | 49 ++++++++++++++++------- tests/smoke.sh | 103 +++++++++++++++++++++++++++++++++++++++++-------- 4 files changed, 164 insertions(+), 37 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index aeec13e..0a1fbed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,6 +50,19 @@ a directory whose name the user cannot predict is worse than an error. **Nothing destructive without `--apply`.** `rm` and `clean` report by default and modify state only when `--apply` is explicitly passed. Local branch deletions use `git branch -d` (falling back to `-D` on `clean` once confirmed gone/merged, or on `rm` when `--apply` is passed), and worktree directory removals route through `TREES_RM_CMD` when configured (defaulting to `git worktree remove`). +**`TREES_RM_CMD` is the one place the safety net comes off.** `git worktree +remove` refuses a worktree with uncommitted changes or untracked files; a custom +command gets the path and nothing else. So a path target must be validated +against `git worktree list` before removal — `cmd_rm`'s path arm does this, and +without it `TREES_RM_CMD="rm -rf" git trees rm . --apply` would delete the +container root and the bare store. Keep the gate on worktree registration rather +than on the resolved branch: a detached-HEAD worktree legitimately has none. + +**`clean --apply` reports partial failure.** Its loops continue past a failed +worktree removal or branch delete, but the exit status is nonzero if any failed, +matching `cmd_rm`. Do not turn that back into an unconditional `return 0` — +scripting `clean` depends on it. + @@ -118,8 +131,16 @@ What the suite covers: - **install.sh** — places the binary; seeds `~/.config/git-trees/AGENTS.md` from the template under a redirected `HOME`; does not overwrite an existing config file -- **rm** — dry run vs `--apply`, worktree removal by branch or path, safe branch deletion refusal on unmerged branches, and custom `TREES_RM_CMD` routing -- **clean** — `--gone` identification and deletion, `--merged` identification across direct merges, rebased commits, and squash-merged PRs, zero-commit fresh branch preservation, dry run vs `--apply`, and custom `TREES_RM_CMD` routing +- **rm** — dry run vs `--apply`, worktree removal by branch and by path (a + slugged directory whose name is not a branch name, so the path arm is the one + that runs), `-d` escalating to `-D` so an unmerged branch is still deleted + under `--apply`, refusal of a directory that is not a registered worktree even + with `TREES_RM_CMD` set, and custom `TREES_RM_CMD` routing +- **clean** — `--gone` identification and deletion, `--merged` identification + across direct merges, rebased commits, and squash-merged PRs, zero-commit + fresh branch preservation, dry run vs `--apply`, worktree directories actually + gone after `--apply`, each selector run on its own, and custom `TREES_RM_CMD` + routing Two assertion shapes are easy to get wrong: diff --git a/README.md b/README.md index 14f7f78..bc7cfb2 100644 --- a/README.md +++ b/README.md @@ -261,6 +261,15 @@ By default (without `--apply`), reports what would be removed without making any Worktree directories are removed via `TREES_RM_CMD` if configured, or `git worktree remove`. Local branches are deleted using `git branch -d` (falling back to `git branch -D` if needed). +A path target must be a worktree git already knows about; `rm` refuses any other +directory. + +> **`TREES_RM_CMD` removes git's safety net.** `git worktree remove` refuses a +> worktree that has uncommitted changes or untracked files. A custom command such +> as `rm -rf` receives only the path and makes no such check, so `rm --apply` and +> `clean --apply` will destroy uncommitted work without warning. Set it only if +> you want `git worktree remove --force` semantics deliberately. + ### `git trees clean [--merged|--gone] [--apply]` @@ -272,7 +281,10 @@ Selectors: Passing neither selector runs both `--gone` and `--merged`. -By default (without `--apply`), `clean` operates in dry-run mode and prints matching branches/worktrees without deleting them. Pass `--apply` to execute removals. Worktree directories are removed via `TREES_RM_CMD` if set (defaulting to `git worktree remove`), and branches are deleted using `git branch -d` (falling back to `git branch -D` for gone/squash-merged branches). +By default (without `--apply`), `clean` operates in dry-run mode and prints matching branches/worktrees without deleting them. Pass `--apply` to execute removals. Worktree directories are removed via `TREES_RM_CMD` if set (defaulting to `git worktree remove`) — see the warning above — and branches are deleted using `git branch -d` (falling back to `git branch -D` for gone/squash-merged branches). + +`clean --apply` keeps going when an individual removal fails, reporting each one +on stderr, and exits nonzero if any of them did. @@ -285,7 +297,13 @@ git trees rm feature-x --apply # remove a single worktree and its bra git trees clean --apply # remove merged and gone branches/worktrees ``` -Both subcommands default to dry-run mode (report only) unless `--apply` is passed. Alternatively, you can use plain git: +Both subcommands default to dry-run mode (report only) unless `--apply` is passed. +Both also delete unmerged work once `--apply` is given — `-d` escalates to `-D`. +If you have set `TREES_RM_CMD`, read the warning under [`git trees +rm`](#git-trees-rm-branchpath---apply) first: it removes git's check for +uncommitted changes. + +Alternatively, you can use plain git: ```bash git worktree remove @@ -301,7 +319,7 @@ git worktree prune | `TREES_ORG` | *(unset)* | Default org; if unset, bare repo names are rejected | | `TREES_AGENTS_TEMPLATE` | `~/.config/git-trees/AGENTS.md` | Seeded at the container root by `init` (and `root --agents`) | | `TREES_NO_PUSH` | *(unset)* | Any non-empty value: `add`/`track` never create a branch on `origin` | -| `TREES_RM_CMD` | *(unset)* | Custom command for worktree directory removal (defaults to `git worktree remove`) | +| `TREES_RM_CMD` | *(unset)* | Custom command for worktree directory removal (defaults to `git worktree remove`). Bypasses git's uncommitted-work check — see [`git trees rm`](#git-trees-rm-branchpath---apply) | ## Shell wrapper (optional) diff --git a/git-trees b/git-trees index db54da0..def2aec 100755 --- a/git-trees +++ b/git-trees @@ -44,11 +44,11 @@ _is_branch_merged() { # _is_branch_merged -> 0 if me br_commit=$(git rev-parse "$br" 2>/dev/null) || return 1 mb=$(git merge-base "origin/$def" "$br" 2>/dev/null) || return 1 - # 1. Direct merge check: br is an ancestor of origin/$def AND br is not identical to origin/$def tip + # 1. Direct merge check: br is an ancestor of origin/$def AND br is not identical to origin/$def tip. + # Comparing against $mb would be dead code — once br is an ancestor, the merge base *is* br. + # A zero-commit branch sitting on an older origin/$def commit is merged by this rule; only a + # branch still at the tip (a freshly cut one) is spared, which is the case worth protecting. if git merge-base --is-ancestor "$br" "origin/$def" 2>/dev/null; then - if [ "$br_commit" != "$mb" ]; then - return 0 - fi if [ "$br_commit" != "$(git rev-parse "origin/$def" 2>/dev/null)" ]; then return 0 fi @@ -56,7 +56,9 @@ _is_branch_merged() { # _is_branch_merged -> 0 if me # 2. Rebase / Cherry-pick check (git cherry has no '+' lines) if git rev-parse --verify --quiet "origin/$def" >/dev/null; then - if [ "$(git rev-list --count "$mb..$br" 2>/dev/null)" -gt 0 ]; then + # `|| echo 0`: rev-list prints nothing when it fails, and `[ "" -gt 0 ]` would + # leak a raw "integer expression expected" from bash to the user's terminal. + if [ "$(git rev-list --count "$mb..$br" 2>/dev/null || echo 0)" -gt 0 ]; then if ! git cherry "origin/$def" "$br" 2>/dev/null | grep -q '^+'; then return 0 fi @@ -66,7 +68,7 @@ _is_branch_merged() { # _is_branch_merged -> 0 if me # 3. Squash-merge tree match (branch tree exists in origin/$def history) br_tree=$(git rev-parse "$br^{tree}" 2>/dev/null) if [ -n "$br_tree" ] && git log -n 100 --format='%T' "origin/$def" 2>/dev/null | grep -qx "$br_tree"; then - if [ "$(git rev-list --count "$mb..$br" 2>/dev/null)" -gt 0 ]; then + if [ "$(git rev-list --count "$mb..$br" 2>/dev/null || echo 0)" -gt 0 ]; then return 0 fi fi @@ -74,7 +76,9 @@ _is_branch_merged() { # _is_branch_merged -> 0 if me # 4. Squash-merge patch-id match patch_id=$(git diff "$mb..$br" 2>/dev/null | git patch-id | awk '{print $1}') if [ -n "$patch_id" ]; then - if git log -p "$mb..origin/$def" 2>/dev/null | git patch-id | awk '{print $1}' | grep -qx "$patch_id"; then + # Same 100-commit window as the tree check above. Unbounded, this renders every + # patch since the merge base — once per candidate branch, on every `clean`. + if git log -p -n 100 "$mb..origin/$def" 2>/dev/null | git patch-id | awk '{print $1}' | grep -qx "$patch_id"; then return 0 fi fi @@ -588,7 +592,7 @@ cmd_list() { # --- rm ---------------------------------------------------------------------- cmd_rm() { - local target="" apply=0 br="" path="" root + local target="" apply=0 br="" path="" while [ $# -gt 0 ]; do case "$1" in @@ -606,14 +610,25 @@ cmd_rm() { return 1 } - root=$(_root) || { echo "git trees rm: not in a git repo" >&2; return 1; } + _root >/dev/null || { echo "git trees rm: not in a git repo" >&2; return 1; } if git show-ref --verify --quiet "refs/heads/$target"; then br="$target" path=$(_path_for "$br") elif [ -d "$target" ]; then - path=$(cd "$target" 2>/dev/null && pwd) + # `pwd -P`, not `pwd`: git records worktrees by their physical path, so a + # logical one (macOS /var -> /private/var, or any symlinked parent) would + # match neither _branch_at nor the registration check below. + path=$(cd "$target" 2>/dev/null && pwd -P) br=$(_branch_at "$path") + # A path target must be a worktree git knows about. `git worktree remove` + # would refuse anything else on its own, but TREES_RM_CMD does not — with + # `rm -rf` set, `rm .` in the container root would take out the bare store. + # Gate on registration, not on $br: a detached-HEAD worktree has no branch. + if ! git worktree list --porcelain | grep -qxF "worktree $path"; then + echo "git trees rm: '$target' is not a worktree" >&2 + return 1 + fi else echo "git trees rm: target '$target' not found" >&2 return 1 @@ -653,7 +668,7 @@ cmd_rm() { # --- clean ------------------------------------------------------------------- cmd_clean() { - local apply=0 do_gone=0 do_merged=0 root def br p + local apply=0 do_gone=0 do_merged=0 def br p failed=0 while [ $# -gt 0 ]; do case "$1" in @@ -669,7 +684,7 @@ cmd_clean() { do_merged=1 fi - root=$(_root) || { echo "git trees clean: not in a git repo" >&2; return 1; } + _root >/dev/null || { echo "git trees clean: not in a git repo" >&2; return 1; } def=$(_default_branch) git fetch --prune origin >/dev/null 2>&1 || true @@ -683,12 +698,13 @@ cmd_clean() { if [ -n "$p" ]; then if ! _remove_worktree "$p"; then echo " ! worktree remove failed — skipped branch delete for $br" >&2 + failed=1 continue fi git worktree prune >/dev/null 2>&1 fi git branch -d "$br" 2>/dev/null || git branch -D "$br" 2>/dev/null \ - || echo " ! could not delete branch $br" >&2 + || { echo " ! could not delete branch $br" >&2; failed=1; } fi done < <( git for-each-ref --format='%(refname:short) %(upstream:track)' refs/heads \ @@ -708,12 +724,13 @@ cmd_clean() { if [ -n "$p" ]; then if ! _remove_worktree "$p"; then echo " ! worktree remove failed — skipped branch delete for $br" >&2 + failed=1 continue fi git worktree prune >/dev/null 2>&1 fi git branch -d "$br" 2>/dev/null || git branch -D "$br" 2>/dev/null \ - || echo " ! could not delete branch $br" >&2 + || { echo " ! could not delete branch $br" >&2; failed=1; } fi fi done < <(git for-each-ref --format='%(refname:short)' refs/heads) @@ -721,7 +738,9 @@ cmd_clean() { git worktree prune >/dev/null 2>&1 [ "$apply" -eq 0 ] && echo "(report only — pass --apply to execute)" - return 0 + # Nonzero when any removal failed, so `clean --apply` can be scripted. The + # loops keep going on failure, so this reports partial success, not a stop. + return "$failed" } diff --git a/tests/smoke.sh b/tests/smoke.sh index 913f620..97e961e 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -505,16 +505,41 @@ assert_ok "rm with --apply" bash "$T" rm rm-target --apply assert_fail "worktree directory deleted" test -e rm-target assert_fail "branch deleted" git show-ref --verify --quiet refs/heads/rm-target -# Unmerged branch: worktree removed, branch preserved with error warning +# Unmerged branch: `-d` refuses it, so cmd_rm escalates to `-D` and both the +# worktree and the branch go under --apply. The commit is asserted: if it failed +# quietly the branch would be merged, `-d` would succeed, and the assertions +# below would pass without ever exercising the escalation. assert_ok "create unmerged worktree" bash "$T" add rm-unmerged --no-push echo "unmerged data" > rm-unmerged/unmerged.txt -git -C rm-unmerged add . && git -C rm-unmerged commit -qm "unmerged commit" +assert_ok "stage unmerged work" in_dir rm-unmerged git add . +assert_ok "commit unmerged work" in_dir rm-unmerged git commit -qm "unmerged commit" assert_ok "rm --apply removes worktree and unmerged branch" bash "$T" rm rm-unmerged --apply assert_fail "worktree directory deleted for unmerged" test -e rm-unmerged assert_fail "unmerged branch deleted" git show-ref --verify --quiet refs/heads/rm-unmerged +# Path target. cmd_rm resolves a branch name first, and every worktree above has +# a directory named after its branch, so the branch arm always wins. A slugged +# directory (`rm/by-path` -> `rm-by-path`) is not a branch name, so this is the +# only shape that reaches the path arm. +assert_ok "create slashed branch worktree" bash "$T" add rm/by-path --no-push +assert_ok "slugged directory exists" test -d rm-by-path +assert_ok "rm by path" bash "$T" rm "$RM_C/rm-by-path" --apply +assert_fail "worktree removed by path" test -e rm-by-path +assert_fail "branch removed by path" git show-ref --verify --quiet refs/heads/rm/by-path + +# A plain directory is not a worktree and must be refused. Without the guard, +# TREES_RM_CMD would delete it outright — `rm .` would take out the container +# root and the bare store with it. +mkdir -p not-a-worktree +assert_fail "rm refuses a non-worktree directory" \ + env TREES_RM_CMD="rm -rf" bash "$T" rm not-a-worktree --apply +assert_ok "non-worktree directory left intact" test -d not-a-worktree +assert_ok "container root survives rm ." \ + env TREES_RM_CMD="rm -rf" bash -c "bash '$T' rm . --apply; test -d '$RM_C/trees-bare.git'" + + # Custom TREES_RM_CMD assert_ok "create worktree for custom TREES_RM_CMD" bash "$T" add rm-custom --no-push assert_ok "rm with TREES_RM_CMD" env TREES_RM_CMD="rm -rf" bash "$T" rm rm-custom --apply @@ -527,38 +552,54 @@ assert_fail "rm with nonexistent target" bash "$T" rm nonexistent # --- clean ------------------------------------------------------------------- +# KEEP THIS SECTION LAST. Its fixtures mutate the shared $ORIGIN — deleting a +# branch, then merging, cherry-picking and squashing onto main. Every +# new_container clones $ORIGIN, so any section running after this one sees a +# different default-branch history than the ones before it. section "clean" CLEAN_C=$(new_container clean-c) cd "$CLEAN_C" || exit 1 -# Setup origin repo changes for clean testing +# Setup origin repo changes for clean testing. Each step is asserted: a silent +# failure here (a dirty $ORIGIN worktree, a checkout that did not land) would +# surface as a confusing failure in the assertions far below instead. # 1. Gone upstream branch assert_ok "add gone-branch" bash "$T" add gone-branch -git -C "$ORIGIN" branch -D gone-branch >/dev/null 2>&1 +assert_ok "delete gone-branch on origin" git -C "$ORIGIN" branch -D gone-branch # 2. Direct merged branch assert_ok "add merged-direct" bash "$T" add merged-direct echo "direct change" > merged-direct/direct.txt -git -C merged-direct add . && git -C merged-direct commit -qm "direct commit" -git -C merged-direct push -u origin merged-direct >/dev/null 2>&1 -( cd "$ORIGIN" && git checkout -q main && git merge -q merged-direct --no-ff -m "merge direct" ) >/dev/null 2>&1 -git -C "$CLEAN_C" fetch -q origin +assert_ok "stage merged-direct" in_dir merged-direct git add . +assert_ok "commit merged-direct" in_dir merged-direct git commit -qm "direct commit" +assert_ok "push merged-direct" in_dir merged-direct git push -q -u origin merged-direct +assert_ok "merge merged-direct into origin main" \ + in_dir "$ORIGIN" git -c advice.detachedHead=false checkout -q main +assert_ok "merge merged-direct --no-ff" \ + in_dir "$ORIGIN" git merge -q merged-direct --no-ff -m "merge direct" +assert_ok "fetch after direct merge" git -C "$CLEAN_C" fetch -q origin # 3. Rebase merged branch assert_ok "add merged-rebase" bash "$T" add merged-rebase echo "rebase change" > merged-rebase/rebase.txt -git -C merged-rebase add . && git -C merged-rebase commit -qm "rebase commit" -git -C merged-rebase push -u origin merged-rebase >/dev/null 2>&1 -( cd "$ORIGIN" && git checkout -q main && git cherry-pick merged-rebase ) >/dev/null 2>&1 -git -C "$CLEAN_C" fetch -q origin +assert_ok "stage merged-rebase" in_dir merged-rebase git add . +assert_ok "commit merged-rebase" in_dir merged-rebase git commit -qm "rebase commit" +assert_ok "push merged-rebase" in_dir merged-rebase git push -q -u origin merged-rebase +assert_ok "checkout origin main for cherry-pick" in_dir "$ORIGIN" git checkout -q main +assert_ok "cherry-pick merged-rebase" in_dir "$ORIGIN" git cherry-pick merged-rebase + +assert_ok "fetch after cherry-pick" git -C "$CLEAN_C" fetch -q origin # 4. Squash merged branch assert_ok "add merged-squash" bash "$T" add merged-squash echo "squash change" > merged-squash/squash.txt -git -C merged-squash add . && git -C merged-squash commit -qm "squash commit" -git -C merged-squash push -u origin merged-squash >/dev/null 2>&1 -( cd "$ORIGIN" && git checkout -q main && git merge -q --squash merged-squash && git commit -qm "squash merge commit" ) >/dev/null 2>&1 -git -C "$CLEAN_C" fetch -q origin +assert_ok "stage merged-squash" in_dir merged-squash git add . +assert_ok "commit merged-squash" in_dir merged-squash git commit -qm "squash commit" +assert_ok "push merged-squash" in_dir merged-squash git push -q -u origin merged-squash +assert_ok "checkout origin main for squash" in_dir "$ORIGIN" git checkout -q main +assert_ok "squash merge merged-squash" in_dir "$ORIGIN" git merge -q --squash merged-squash +assert_ok "commit the squash merge" in_dir "$ORIGIN" git commit -qm "squash merge commit" +assert_ok "fetch after squash merge" git -C "$CLEAN_C" fetch -q origin # 5. Fresh 0-commit branch cut from main (MUST NOT be cleaned) assert_ok "add fresh-branch" bash "$T" add fresh-branch --no-push @@ -574,13 +615,41 @@ assert_contains "clean dry run reports squash merged" "$out" "merged-squash" assert_not_contains "clean dry run preserves fresh 0-commit branch" "$out" "fresh-branch" assert_contains "clean dry run reports report-only notice" "$out" "(report only — pass --apply to execute)" -# Apply clean +# Each selector on its own. Passing neither runs both, so the default above +# cannot tell us that --gone and --merged actually gate their own sections. +out=$(bash "$T" clean --gone 2>&1) +assert_contains "clean --gone runs the gone section" "$out" "== branches with gone upstream ==" +assert_not_contains "clean --gone skips the merged section" "$out" "== branches merged into" +assert_contains "clean --gone reports gone-branch" "$out" "gone-branch" +assert_not_contains "clean --gone omits merged branches" "$out" "merged-direct" + +out=$(bash "$T" clean --merged 2>&1) +assert_contains "clean --merged runs the merged section" "$out" "== branches merged into" +assert_not_contains "clean --merged skips the gone section" "$out" "== branches with gone upstream ==" +assert_contains "clean --merged reports merged-direct" "$out" "merged-direct" + +# Apply clean. Assert the worktree directories too, not just the refs — a broken +# _remove_worktree call would leave every directory behind and still pass a +# refs-only check. assert_ok "clean --apply" bash "$T" clean --apply assert_fail "gone-branch deleted" git show-ref --verify --quiet refs/heads/gone-branch +assert_fail "gone-branch worktree removed" test -e gone-branch assert_fail "merged-direct deleted" git show-ref --verify --quiet refs/heads/merged-direct +assert_fail "merged-direct worktree removed" test -e merged-direct assert_fail "merged-rebase deleted" git show-ref --verify --quiet refs/heads/merged-rebase +assert_fail "merged-rebase worktree removed" test -e merged-rebase assert_fail "merged-squash deleted" git show-ref --verify --quiet refs/heads/merged-squash +assert_fail "merged-squash worktree removed" test -e merged-squash assert_ok "fresh-branch preserved after clean" git show-ref --verify --quiet refs/heads/fresh-branch +assert_ok "fresh-branch worktree preserved" test -d fresh-branch + +# TREES_RM_CMD routing through clean, on a branch whose upstream goes away. +assert_ok "add clean-custom" bash "$T" add clean-custom +assert_ok "delete clean-custom on origin" git -C "$ORIGIN" branch -D clean-custom +assert_ok "clean --gone with TREES_RM_CMD" \ + env TREES_RM_CMD="rm -rf" bash "$T" clean --gone --apply +assert_fail "clean routed removal through TREES_RM_CMD" test -e clean-custom +assert_fail "clean deleted the gone branch" git show-ref --verify --quiet refs/heads/clean-custom # Cleanup fresh-branch bash "$T" rm fresh-branch --apply >/dev/null 2>&1 From 312b49bfb766604915e0d8d4cef95357a4f33017 Mon Sep 17 00:00:00 2001 From: Leo Dion Date: Wed, 5 Aug 2026 16:12:25 -0400 Subject: [PATCH 3/3] Document the physical-worktree-path pitfall The pwd -P requirement in cmd_rm is not obvious from the code and cost a debugging cycle to find. Record it alongside the upstream-inheritance pitfall so the next contributor does not rediscover it. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/agent-notes.md | 1 + AGENTS.md | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/.claude/agent-notes.md b/.claude/agent-notes.md index 3b879c4..9e787b7 100644 --- a/.claude/agent-notes.md +++ b/.claude/agent-notes.md @@ -19,3 +19,4 @@ update or remove the stale line rather than leaving both. - Do not remove an `init` container on agent-seeding failure; return nonzero and leave the directory. - `main` is always the stable release; README curl install pins `main` (not version tags). Do not describe `main` as a development/moving target. - `clean` supports `--gone` and `--merged` (detecting direct, rebased, and squash-merged PRs); `--older-than` is omitted. +- When resolving CodeRabbit review comments, verify each claim against the code before acting; report skipped findings with the reason rather than silently dropping them. diff --git a/AGENTS.md b/AGENTS.md index 0a1fbed..4e6e039 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,6 +81,15 @@ created from `origin/main` silently gets `origin/main` as its upstream and will push there. The new-branch path must pass `--no-track`, then let `cmd_track` set the correct upstream. Live in `cmd_add`; any change there needs a fresh test. +## Git pitfall: worktree paths are physical + +`git worktree list` reports the *physical* path. Resolve any user-supplied +directory with `pwd -P`, never plain `pwd`, before comparing against it or +passing it to `_branch_at` — on macOS `$TMPDIR` lives under `/var`, a symlink to +`/private/var`, so the logical path matches nothing and a real worktree looks +unregistered. `cmd_rm`'s path arm depends on this; the smoke suite catches it +because its fixtures are built under `mktemp -d`. + ## Testing Run the suite: