diff --git a/.claude/scripts/merge-ready.sh b/.claude/scripts/merge-ready.sh index ec6498c..74e1ee1 100644 --- a/.claude/scripts/merge-ready.sh +++ b/.claude/scripts/merge-ready.sh @@ -75,12 +75,29 @@ decide() { merged=0; skipped=0 for n in $(gh pr list -R "$repo" --base "$base" --state open --json number -q '.[].number'); do - data="$(gh pr view "$n" -R "$repo" --json number,title,isDraft,baseRefName,mergeable,reviews,statusCheckRollup,commits)" + data="$(gh pr view "$n" -R "$repo" --json number,title,isDraft,baseRefName,headRefName,mergeable,reviews,statusCheckRollup,commits)" verdict="$(printf '%s' "$data" | decide)" title="$(printf '%s' "$data" | node -e 'process.stdout.write((JSON.parse(require("fs").readFileSync(0,"utf8")).title)||"")')" + head_branch="$(printf '%s' "$data" | node -e 'process.stdout.write((JSON.parse(require("fs").readFileSync(0,"utf8")).headRefName)||"")')" if [ "$verdict" = "MERGE" ]; then if gh pr merge "$n" -R "$repo" --merge --delete-branch >/dev/null 2>&1; then echo "{\"pr\":$n,\"action\":\"merged\",\"title\":\"$title\"}"; merged=$((merged+1)) + # Auto-cleanup (issue #91): the merged branch's local worktree + local + # branch are now stale. worktree-cleanup.sh applies its OWN safety + # rails (worker-path naming, clean tree, fully merged into $base) and + # NEVER forces — a "skip" line from it is expected and fine, just + # tag it with the PR number and pass it through. + if [ -n "$head_branch" ]; then + while IFS= read -r cleanup_line; do + [ -z "$cleanup_line" ] && continue + printf '%s\n' "$cleanup_line" | node -e ' + const pr = process.argv[1]; + const obj = JSON.parse(require("fs").readFileSync(0,"utf8")); + obj.pr = Number(pr); + console.log(JSON.stringify(obj)); + ' "$n" + done < <(bash "$script_dir/worktree-cleanup.sh" "$base" "$head_branch") + fi else echo "{\"pr\":$n,\"action\":\"merge-failed\",\"title\":\"$title\"}" fi diff --git a/.claude/scripts/worktree-cleanup.sh b/.claude/scripts/worktree-cleanup.sh new file mode 100755 index 0000000..29e4599 --- /dev/null +++ b/.claude/scripts/worktree-cleanup.sh @@ -0,0 +1,151 @@ +#!/usr/bin/env bash +# worktree-cleanup.sh — after merge-ready.sh merges a PR, remove the worker's +# now-dead worktree + delete its local branch, so `.claude/worktrees/` and the +# local branch list don't silently accumulate `prunable` entries forever +# (issue #91). Factored out of merge-ready.sh so it's independently testable +# offline (see worktree-cleanup.test.sh) without a real merge or network call. +# +# Usage: worktree-cleanup.sh [...] +# the branch merged PRs land on (e.g. main) — used to decide +# "fully merged" against origin/ (falling back to +# local `git branch --merged ` if origin is +# unreachable/unset — see merged_via_origin below). +# ... zero or more branch names that were just merged. For each, +# the corresponding worktree (if any) is located via +# `git worktree list --porcelain` and, ONLY if every safety +# rail below holds, the worktree is removed and the local +# branch deleted. +# +# Safety rails — ALL must hold, else the branch is SKIPPED with a logged +# reason and left untouched. NEVER `--force`, NEVER `git branch -D`: +# - the branch has an associated worktree at all +# - that worktree's path matches the worker naming convention: +# .claude/worktrees/agent-* or .claude/worktrees/issue-* (rejects the +# main worktree and anything else, e.g. a hand-made worktree) +# - the worktree has a clean tree (`git status --porcelain` empty) +# - the branch is fully merged into — checked against the +# fetched origin/ (authoritative; see merged_via_origin) OR local +# `git branch --merged ` (issue #91 follow-up: merge-ready.sh calls +# this script right after `gh pr merge`, which only merges on the +# remote, so local is stale at this point — checking it alone +# would report every just-merged branch as unmerged forever) +# - `git worktree remove` succeeds on its own steam (never forced); the +# branch is then deleted via `git branch -d` if it agrees, or — only when +# merged_via_origin independently verified the merge — via a direct ref +# delete (never `-D`/`--force`, never for an unverified branch) +# +# Emits ONE JSON line per branch on stdout, either: +# {"branch":"","worktree_removed":"","branch_deleted":""} +# {"branch":"","worktree_removed":"","action":"skip","reason":"branch-delete-failed"} +# {"branch":"","action":"skip","reason":""} +# consistent with merge-ready.sh's existing JSON-lines output style. +set -uo pipefail + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=resolve-roots.sh +. "$script_dir/resolve-roots.sh" + +base="${1:?usage: worktree-cleanup.sh [...]}" +shift || true + +json_escape() { + node -e 'process.stdout.write(JSON.stringify(process.argv[1]))' "$1" +} + +# Map a branch name -> its worktree's absolute path, by parsing +# `git worktree list --porcelain` records: +# worktree +# HEAD +# branch refs/heads/ (absent when detached) +# +worktree_for_branch() { + local want="$1" + git -C "$root" worktree list --porcelain | node -e ' + const want = process.argv[1]; + const chunks = require("fs").readFileSync(0, "utf8").split(/\n\n+/); + for (const c of chunks) { + let path = null, branch = null; + for (const l of c.split("\n")) { + if (l.startsWith("worktree ")) path = l.slice("worktree ".length); + if (l.startsWith("branch refs/heads/")) branch = l.slice("branch refs/heads/".length); + } + if (path && branch === want) { process.stdout.write(path); process.exit(0); } + } + ' "$want" +} + +is_worker_path() { + case "$1" in + */.claude/worktrees/agent-*|*/.claude/worktrees/issue-*) return 0 ;; + *) return 1 ;; + esac +} + +skip() { + local branch="$1" reason="$2" + echo "{\"branch\":$(json_escape "$branch"),\"action\":\"skip\",\"reason\":$(json_escape "$reason")}" +} + +# merge-ready.sh invokes this script IMMEDIATELY after `gh pr merge` succeeds +# — that merges on the REMOTE only. The LOCAL base branch isn't fast-forwarded +# until merge-ready.sh's post-loop local_sync block, which runs AFTER this +# script returns. A plain local `git branch --merged "$base"` check therefore +# always reports the just-merged branch as unmerged, so cleanup never fires +# (issue #91 follow-up — the feature was inert in production). Fetch the +# authoritative remote base once up front and prefer ancestry against it; +# fall back to the local check (previous behavior) when the fetch is +# unavailable (offline) or origin/$base doesn't exist yet. +git -C "$root" fetch --quiet origin "$base" 2>/dev/null || true + +# True iff $1 is an ancestor of origin/$base — i.e. genuinely merged on the +# remote — independent of whatever local $base happens to point at right +# now. Used both as (part of) the merged-ness gate and, if `git branch -d` +# refuses below because local $base hasn't caught up yet, as the trusted +# basis for a non-force deletion. +merged_via_origin() { + local branch="$1" + git -C "$root" rev-parse --verify --quiet "refs/remotes/origin/$base" >/dev/null 2>&1 \ + && git -C "$root" merge-base --is-ancestor "$branch" "refs/remotes/origin/$base" 2>/dev/null +} + +for branch in "$@"; do + [ -z "$branch" ] && continue + + wt="$(worktree_for_branch "$branch")" + if [ -z "$wt" ]; then + skip "$branch" "no-worktree" + continue + fi + if ! is_worker_path "$wt"; then + skip "$branch" "not-a-worker-worktree-path" + continue + fi + if [ -n "$(git -C "$wt" status --porcelain 2>/dev/null)" ]; then + skip "$branch" "worktree-dirty" + continue + fi + # `git branch --merged` prefixes the current branch with `*` and any branch + # checked out in ANOTHER linked worktree with `+` — strip both markers + # before comparing names. OR'd with merged_via_origin so a branch merged on + # the remote but not yet reflected in local $base still counts as merged. + if ! merged_via_origin "$branch" \ + && ! git -C "$root" branch --merged "$base" 2>/dev/null | sed 's/^[*+ ]*//' | grep -qxF "$branch"; then + skip "$branch" "branch-not-merged" + continue + fi + if ! git -C "$root" worktree remove "$wt" 2>/dev/null; then + skip "$branch" "worktree-remove-failed(dirty-or-locked)" + continue + fi + if git -C "$root" branch -d "$branch" >/dev/null 2>&1; then + echo "{\"branch\":$(json_escape "$branch"),\"worktree_removed\":$(json_escape "$wt"),\"branch_deleted\":$(json_escape "$branch")}" + elif merged_via_origin "$branch" && git -C "$root" update-ref -d "refs/heads/$branch" 2>/dev/null; then + # `git branch -d` refused because it only trusts local $base/upstream, + # which hasn't fast-forwarded yet — but we've independently verified via + # origin/$base ancestry above that this is genuinely merged, so deleting + # the ref directly is safe (still never -D/--force on anything unverified). + echo "{\"branch\":$(json_escape "$branch"),\"worktree_removed\":$(json_escape "$wt"),\"branch_deleted\":$(json_escape "$branch")}" + else + echo "{\"branch\":$(json_escape "$branch"),\"worktree_removed\":$(json_escape "$wt"),\"action\":\"skip\",\"reason\":\"branch-delete-failed(unmerged-elsewhere?)\"}" + fi +done diff --git a/.claude/scripts/worktree-cleanup.test.sh b/.claude/scripts/worktree-cleanup.test.sh new file mode 100755 index 0000000..94572d2 --- /dev/null +++ b/.claude/scripts/worktree-cleanup.test.sh @@ -0,0 +1,220 @@ +#!/usr/bin/env bash +# worktree-cleanup.test.sh — offline smoke test for worktree-cleanup.sh +# (issue #91). Builds a THROWAWAY temp git repo with real `git worktree add` +# worktrees (no real network, no gh, no real PR merge — scenario 6 uses a +# local bare repo as a stand-in "origin", never a real remote) and exercises +# the real worktree-cleanup.sh against it, asserting: +# 1. merged + clean + matching-name worktree -> removed + branch deleted +# 2. dirty worktree -> preserved (never touched) +# 3. unmerged branch -> preserved (never touched) +# 4. non-matching path (main worktree, and an arbitrary non-worker path) +# -> never touched +# 5. no worktree at all -> skip, no crash +# 6. PRODUCTION ordering (issue #91 follow-up): merge landed on the +# authoritative remote but local base is still stale -> removed + branch +# deleted anyway (via origin ancestry, never a forced delete) +# +# Exit 0 on success, non-zero if any assertion fails. Runnable bare: +# bash .claude/scripts/worktree-cleanup.test.sh +set -uo pipefail + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cleanup_src="$script_dir/worktree-cleanup.sh" +resolve_roots_src="$script_dir/resolve-roots.sh" + +work="$(mktemp -d "${TMPDIR:-/tmp}/worktree-cleanup-test.XXXXXX")" +trap 'rm -rf "$work"' EXIT + +fail=0 +ok=0 +check() { + local desc="$1"; shift + if "$@"; then + ok=$((ok + 1)) + echo "ok - $desc" + else + fail=1 + echo "FAIL - $desc" + fi +} + +# Throwaway "consumer project" git repo: real commits + real `git worktree +# add`, with the REAL worktree-cleanup.sh + resolve-roots.sh copied into its +# .claude/scripts/ (mirrors loop-tick.test.sh's fixture pattern), so root +# derivation matches how the script resolves things in production. +repo="$work/repo" +mkdir -p "$repo" +git init -q -b main "$repo" +git -C "$repo" config user.email "test@example.com" +git -C "$repo" config user.name "Test" +echo "seed" > "$repo/README.md" +git -C "$repo" add README.md +git -C "$repo" commit -q -m "seed" + +mkdir -p "$repo/.claude/scripts" +cp "$cleanup_src" "$repo/.claude/scripts/worktree-cleanup.sh" +cp "$resolve_roots_src" "$repo/.claude/scripts/resolve-roots.sh" +chmod +x "$repo/.claude/scripts/worktree-cleanup.sh" + +run_cleanup() { + # $1 = base branch, $@[2:] = branch names to attempt cleanup on. + ( cd "$repo" && bash "$repo/.claude/scripts/worktree-cleanup.sh" "$@" ) +} + +worktree_present() { + git -C "$repo" worktree list --porcelain | grep -qF "worktree $1" +} +branch_present() { + git -C "$repo" branch --list "$1" | grep -q . +} + +# --------------------------------------------------------------------------- +# Scenario 1: merged + clean + matching-name worktree -> removed + branch +# deleted. +# --------------------------------------------------------------------------- +git -C "$repo" checkout -q -b feat/issue-1-x +echo "one" > "$repo/file1.txt" +git -C "$repo" add file1.txt +git -C "$repo" commit -q -m "feat1" +git -C "$repo" checkout -q main +git -C "$repo" merge -q --no-ff feat/issue-1-x -m "merge feat1" +wt1="$repo/.claude/worktrees/agent-test1" +git -C "$repo" worktree add -q "$wt1" feat/issue-1-x + +out1="$(run_cleanup main feat/issue-1-x)" +check "scenario 1: emits a JSON line reporting the removal" bash -c 'printf "%s\n" "$1" | grep -q "worktree_removed"' _ "$out1" +check "scenario 1: worktree_removed path is correct" bash -c 'printf "%s\n" "$1" | grep -qF "\"worktree_removed\":\"$2\""' _ "$out1" "$wt1" +check "scenario 1: branch_deleted reports the branch name" bash -c 'printf "%s\n" "$1" | grep -qF "\"branch_deleted\":\"feat/issue-1-x\""' _ "$out1" +check "scenario 1: worktree is actually gone from git worktree list" bash -c '! git -C "$1" worktree list --porcelain | grep -qF "worktree $2"' _ "$repo" "$wt1" +check "scenario 1: worktree directory removed from disk" bash -c '[ ! -d "$1" ]' _ "$wt1" +check "scenario 1: local branch actually deleted" bash -c '! git -C "$1" branch --list feat/issue-1-x | grep -q .' _ "$repo" + +# --------------------------------------------------------------------------- +# Scenario 2: dirty worktree -> preserved (never removed, branch never +# deleted), even though the branch IS fully merged. +# --------------------------------------------------------------------------- +git -C "$repo" checkout -q -b feat/issue-2-x +echo "two" > "$repo/file2.txt" +git -C "$repo" add file2.txt +git -C "$repo" commit -q -m "feat2" +git -C "$repo" checkout -q main +git -C "$repo" merge -q --no-ff feat/issue-2-x -m "merge feat2" +wt2="$repo/.claude/worktrees/agent-test2" +git -C "$repo" worktree add -q "$wt2" feat/issue-2-x +echo "dirty" >> "$wt2/file2.txt" # make the worktree dirty + +out2="$(run_cleanup main feat/issue-2-x)" +check "scenario 2: emits a skip with reason worktree-dirty" bash -c 'printf "%s\n" "$1" | grep -qF "\"reason\":\"worktree-dirty\""' _ "$out2" +check "scenario 2: worktree still present (untouched)" worktree_present "$wt2" +check "scenario 2: worktree directory still on disk" [ -d "$wt2" ] +check "scenario 2: branch still present (untouched)" branch_present "feat/issue-2-x" + +# --------------------------------------------------------------------------- +# Scenario 3: unmerged branch -> preserved (clean tree, matching path, but +# never merged into base). +# --------------------------------------------------------------------------- +git -C "$repo" checkout -q -b feat/issue-3-x +echo "three" > "$repo/file3.txt" +git -C "$repo" add file3.txt +git -C "$repo" commit -q -m "feat3 (never merged)" +git -C "$repo" checkout -q main +wt3="$repo/.claude/worktrees/agent-test3" +git -C "$repo" worktree add -q "$wt3" feat/issue-3-x + +out3="$(run_cleanup main feat/issue-3-x)" +check "scenario 3: emits a skip with reason branch-not-merged" bash -c 'printf "%s\n" "$1" | grep -qF "\"reason\":\"branch-not-merged\""' _ "$out3" +check "scenario 3: worktree still present (untouched)" worktree_present "$wt3" +check "scenario 3: worktree directory still on disk" [ -d "$wt3" ] +check "scenario 3: branch still present (untouched)" branch_present "feat/issue-3-x" + +# --------------------------------------------------------------------------- +# Scenario 4a: the base/main branch itself (worktree is the MAIN worktree, +# not a worker one) -> never touched, even though "main" trivially satisfies +# "merged into main" and the tree is clean. +# --------------------------------------------------------------------------- +out4a="$(run_cleanup main main)" +check "scenario 4a: emits a skip for the main worktree's own branch" bash -c 'printf "%s\n" "$1" | grep -qF "\"branch\":\"main\""' _ "$out4a" +check "scenario 4a: reason is not-a-worker-worktree-path" bash -c 'printf "%s\n" "$1" | grep -qF "\"reason\":\"not-a-worker-worktree-path\""' _ "$out4a" +check "scenario 4a: main worktree itself is untouched" bash -c '[ -d "$1" ] && git -C "$1" rev-parse --show-toplevel >/dev/null 2>&1' _ "$repo" + +# --------------------------------------------------------------------------- +# Scenario 4b: an arbitrary non-worker-naming worktree path (merged + clean, +# but not .claude/worktrees/agent-* or issue-*) -> never touched. +# --------------------------------------------------------------------------- +git -C "$repo" checkout -q -b feat/issue-4-x +echo "four" > "$repo/file4.txt" +git -C "$repo" add file4.txt +git -C "$repo" commit -q -m "feat4" +git -C "$repo" checkout -q main +git -C "$repo" merge -q --no-ff feat/issue-4-x -m "merge feat4" +wt4="$repo/.claude/worktrees/some-random-dir" +git -C "$repo" worktree add -q "$wt4" feat/issue-4-x + +out4b="$(run_cleanup main feat/issue-4-x)" +check "scenario 4b: emits a skip with reason not-a-worker-worktree-path" bash -c 'printf "%s\n" "$1" | grep -qF "\"reason\":\"not-a-worker-worktree-path\""' _ "$out4b" +check "scenario 4b: worktree still present (untouched)" worktree_present "$wt4" +check "scenario 4b: worktree directory still on disk" [ -d "$wt4" ] +check "scenario 4b: branch still present (untouched)" branch_present "feat/issue-4-x" + +# --------------------------------------------------------------------------- +# Scenario 5: a branch with no worktree at all -> skip:no-worktree, no crash. +# --------------------------------------------------------------------------- +git -C "$repo" branch -q feat/issue-5-x-no-worktree +out5="$(run_cleanup main feat/issue-5-x-no-worktree)" +check "scenario 5: emits a skip with reason no-worktree" bash -c 'printf "%s\n" "$1" | grep -qF "\"reason\":\"no-worktree\""' _ "$out5" + +# --------------------------------------------------------------------------- +# Scenario 6 (issue #91 follow-up, PRODUCTION ordering): merge-ready.sh calls +# worktree-cleanup.sh IMMEDIATELY after `gh pr merge` succeeds, which merges +# on the REMOTE only — local $base isn't fast-forwarded until a block that +# runs AFTER cleanup. Reproduce that ordering hermetically: a real bare repo +# stands in for "origin"; the merge is performed and pushed from a SEPARATE +# clone, so $repo's own local `main` ref is never touched by it. Cleanup must +# still detect the merge (via origin/$base ancestry) and remove the worktree +# + delete the branch — without ever advancing local `main` itself, and +# without ever needing `-D`/`--force`. +# --------------------------------------------------------------------------- +origin_bare="$work/origin.git" +git init -q --bare -b main "$origin_bare" +git -C "$repo" remote add origin "$origin_bare" +git -C "$repo" push -q origin main:main + +git -C "$repo" checkout -q -b feat/issue-6-x +echo "six" > "$repo/file6.txt" +git -C "$repo" add file6.txt +git -C "$repo" commit -q -m "feat6" +git -C "$repo" checkout -q main +git -C "$repo" push -q origin feat/issue-6-x:feat/issue-6-x + +# Perform + push the merge from a THIRD, independent clone (mimics GitHub +# doing the merge server-side) so local `main` in $repo is never advanced. +remote_work="$work/remote-work" +git clone -q "$origin_bare" "$remote_work" +git -C "$remote_work" config user.email "test@example.com" +git -C "$remote_work" config user.name "Test" +git -C "$remote_work" checkout -q main +git -C "$remote_work" merge -q --no-ff origin/feat/issue-6-x -m "merge feat6" +git -C "$remote_work" push -q origin main:main + +local_main_before="$(git -C "$repo" rev-parse main)" + +wt6="$repo/.claude/worktrees/agent-test6" +git -C "$repo" worktree add -q "$wt6" feat/issue-6-x + +out6="$(run_cleanup main feat/issue-6-x)" +check "scenario 6 (sanity): local main is genuinely stale before cleanup runs" bash -c '[ "$1" = "$(git -C "$2" rev-parse main)" ]' _ "$local_main_before" "$repo" +check "scenario 6: emits a JSON line reporting the removal despite stale local main" bash -c 'printf "%s\n" "$1" | grep -q "worktree_removed"' _ "$out6" +check "scenario 6: branch_deleted reports the branch name" bash -c 'printf "%s\n" "$1" | grep -qF "\"branch_deleted\":\"feat/issue-6-x\""' _ "$out6" +check "scenario 6: worktree is actually gone from git worktree list" bash -c '! git -C "$1" worktree list --porcelain | grep -qF "worktree $2"' _ "$repo" "$wt6" +check "scenario 6: worktree directory removed from disk" bash -c '[ ! -d "$1" ]' _ "$wt6" +check "scenario 6: local branch actually deleted" bash -c '! git -C "$1" branch --list feat/issue-6-x | grep -q .' _ "$repo" +check "scenario 6: local main ref itself was never advanced by cleanup" bash -c '[ "$1" = "$(git -C "$2" rev-parse main)" ]' _ "$local_main_before" "$repo" + +echo "" +if [ "$fail" -eq 0 ]; then + echo "worktree-cleanup.test.sh: PASS ($ok checks)" + exit 0 +else + echo "worktree-cleanup.test.sh: FAIL (see FAIL lines above)" + exit 1 +fi