From 4d38c926fdee40ef6e5a88c46e582bdccde8c536 Mon Sep 17 00:00:00 2001 From: Ahmet Taspinar Date: Sat, 3 Oct 2026 01:25:51 +0200 Subject: [PATCH] Clean up merged worktrees and branches with one command cleanup-worktree.sh , planning/, or a path removes a worktree and its local branch once GitHub reports its pull request as merged, which also works after a squash merge, and fast-forwards main. It runs only from the primary checkout and refuses unmerged, dirty, and ahead-of-PR worktrees; ignored review files do not count. --merged cleans every merged worktree, and --discard removes an unmerged one after confirmation. Failed removals are reported as errors. Closes #56 Co-Authored-By: Claude Opus 5.5 --- README.md | 5 +- docs/development.md | 16 ++- docs/example.md | 4 +- docs/project-map.md | 2 +- docs/workflow.md | 6 +- scripts/cleanup-worktree.sh | 193 +++++++++++++++++++++++++++- scripts/finish-feature.sh | 1 + scripts/finish-planning.sh | 3 +- scripts/verify.conf | 1 + tests/cleanup-worktree-test.sh | 226 +++++++++++++++++++++++++++++++++ 10 files changed, 442 insertions(+), 15 deletions(-) create mode 100755 tests/cleanup-worktree-test.sh diff --git a/README.md b/README.md index c8494b9..26292f8 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ flowchart LR Commit, push, and merge the planning PR as `finish-planning.sh` shows, then remove the planning worktree with - `./scripts/cleanup-worktree.sh ../-planning-project-bootstrap`. + `./scripts/cleanup-worktree.sh planning/project-bootstrap`. 4. For each roadmap feature, from the primary checkout and then the feature worktree: @@ -54,6 +54,7 @@ flowchart LR Push, open a PR containing `Closes #12`, merge it after CI, and remove the worktree from the primary checkout with - `./scripts/cleanup-worktree.sh ../-12-recipes`. + `./scripts/cleanup-worktree.sh 12`, or all merged worktrees at once with + `./scripts/cleanup-worktree.sh --merged`. Rules for agents are in `AGENTS.md`; boundaries are in `.agents/policies/`. diff --git a/docs/development.md b/docs/development.md index a750a4c..4c54d54 100644 --- a/docs/development.md +++ b/docs/development.md @@ -522,11 +522,21 @@ git push -u origin feature/12-player-movement Open a PR containing `Closes #12`. After CI and required human gates pass, merge it; GitHub then closes the linked Issue. From the primary checkout, -remove the merged worktree: +remove the merged worktree and its branch: ```bash -./scripts/cleanup-worktree.sh ../project-12-player-movement -``` +./scripts/cleanup-worktree.sh 12 +``` + +The script asks GitHub whether the branch's pull request is merged, so it also +works after a squash merge. It refuses an unmerged worktree, a branch with +commits after its merged pull request, and a worktree with uncommitted changes; +ignored review and triage files do not count. After removing the worktree and +the local branch it fast-forwards `main` when the primary checkout is a clean +checkout of `main`. A planning worktree is cleaned with +`./scripts/cleanup-worktree.sh planning/`, every merged worktree at once +with `--merged`, and an abandoned, unmerged one with `--discard` after +confirmation. ## Linking a feature plan diff --git a/docs/example.md b/docs/example.md index 0e96bec..42b7971 100644 --- a/docs/example.md +++ b/docs/example.md @@ -139,7 +139,7 @@ its description, and merge it after CI. Remove the planning worktree from the primary checkout: ```bash -./scripts/cleanup-worktree.sh ../recipe-box-planning-project-bootstrap +./scripts/cleanup-worktree.sh planning/project-bootstrap ``` ## 4. Start the first feature @@ -241,7 +241,7 @@ Open a pull request containing `Closes #12` and merge it after CI. Remove the worktree from the primary checkout: ```bash -./scripts/cleanup-worktree.sh ../recipe-box-12-recipes +./scripts/cleanup-worktree.sh 12 ``` The next feature starts again at step 4 with `create-feature-issue.sh F02`. diff --git a/docs/project-map.md b/docs/project-map.md index 18a6912..4abf216 100644 --- a/docs/project-map.md +++ b/docs/project-map.md @@ -51,7 +51,7 @@ What each part of the template is for. The workflow that connects them is in | `scripts/finish-feature.sh` | Checks the feature and creates the commit | | `scripts/check-review.sh` | Reports whether a review still matches what it covers | | `scripts/update-issue-with-plan.sh` | Links an optional feature plan to its Issue | -| `scripts/cleanup-worktree.sh` | Removes a worktree after its merge | +| `scripts/cleanup-worktree.sh` | Removes a merged worktree and its branch and updates `main` | ## Script libraries diff --git a/docs/workflow.md b/docs/workflow.md index cf155dc..be7bfde 100644 --- a/docs/workflow.md +++ b/docs/workflow.md @@ -102,8 +102,8 @@ flowchart TD - When the triage has no `FIX_NOW` findings, only deferred and accepted ones, you can finish directly. - After `finish-feature.sh`, push the branch, open a pull request containing - `Closes #`, and merge it after CI. Then remove the worktree with - `cleanup-worktree.sh`. + `Closes #`, and merge it after CI. Then remove the worktree and its + branch with `cleanup-worktree.sh `. ## Artifacts @@ -215,4 +215,4 @@ tools. | `finish-planning.sh --check` | To see whether the planning approval still matches the planning documents | | `triage-review.sh --publish ` | To repeat a failed publication of the review and triage reports | | `update-issue-with-plan.sh ` | To link an optional feature plan in `.agents/plans/` to its Issue | -| `cleanup-worktree.sh ` | After a merge, from the primary checkout, to remove the worktree | +| `cleanup-worktree.sh `, `planning/`, or `--merged` | After a merge, from the primary checkout: removes the worktree and its branch once GitHub reports the pull request as merged, and updates `main` | diff --git a/scripts/cleanup-worktree.sh b/scripts/cleanup-worktree.sh index d67f8ba..b71dcfa 100755 --- a/scripts/cleanup-worktree.sh +++ b/scripts/cleanup-worktree.sh @@ -1,6 +1,193 @@ #!/usr/bin/env bash + set -euo pipefail -if [[ $# -ne 1 ]]; then echo "Usage: $0 "; exit 1; fi -git worktree remove "$1" + +usage() { + echo "Usage:" + echo " $0 Remove the merged worktree of feature/-*" + echo " $0 planning/ Remove the merged planning worktree" + echo " $0 Remove the merged worktree at a path" + echo " $0 --merged Remove every worktree whose pull request is merged" + echo + echo "Add --discard to remove an unmerged worktree and its branch after confirmation." + echo "Run from the primary checkout." + exit 1 +} + +fail() { + echo "Error: $*" >&2 + exit 1 +} + +discard=0 +merged_mode=0 +target="" +for argument in "$@"; do + case "$argument" in + --discard) discard=1 ;; + --merged) merged_mode=1 ;; + -*) usage ;; + *) + [[ -z "$target" ]] || usage + target="$argument" + ;; + esac +done +[[ "$merged_mode" -eq 1 && -z "$target" && "$discard" -eq 0 ]] || [[ "$merged_mode" -eq 0 && -n "$target" ]] || usage + +command -v gh >/dev/null 2>&1 || fail "GitHub CLI 'gh' is not installed." +command -v jq >/dev/null 2>&1 || fail "jq is required." + +# The primary checkout is the first entry of the worktree list. +primary="$(git worktree list --porcelain | sed -n '1s/^worktree //p')" +current="$(git rev-parse --show-toplevel)" +current="$(cd "$current" && pwd -P)" +primary="$(cd "$primary" && pwd -P)" +[[ "$current" == "$primary" ]] || + fail "run cleanup-worktree.sh from the primary checkout ($primary), not from a linked worktree." + +# linked_worktrees [all]: prints "\t" for every linked worktree +# on a feature or planning branch, or on any branch with "all". Paths are +# compared resolved, because Git may print them unresolved. +linked_worktrees() { + local scope="${1:-workflow}" + local path + local branch + + git worktree list --porcelain | awk -v scope="$scope" ' + /^worktree / { path = substr($0, 10) } + /^branch refs\/heads\// { + branch = substr($0, 19) + if (scope == "all" || branch ~ /^feature\// || branch ~ /^planning\//) { + print path "\t" branch + } + } + ' | while IFS=$'\t' read -r path branch; do + [[ -d "$path" && "$(cd "$path" && pwd -P)" != "$primary" ]] || continue + printf '%s\t%s\n' "$path" "$branch" + done +} + +# merged_head : prints the head commit of the merged pull request of +# a branch, or nothing when no pull request of it is merged. +merged_head() { + gh pr list --head "$1" --state merged --limit 1 --json headRefOid --jq '.[0].headRefOid // empty' : removes one worktree and its branch when that is +# safe. Returns 0 when removed, 1 when kept, and 2 when a removal failed; +# prints the reason. Callers run it in a condition, so every destructive step +# checks its own status. +cleanup() { + local path="$1" + local branch="$2" + local head + local tip + local answer="" + + path="$(cd "$path" && pwd -P)" + if [[ "$path" == "$current" ]]; then + echo "Kept $branch: this is the checkout you run the script in. Run it from the primary checkout." + return 1 + fi + + # Ignored files, such as review and triage results, do not count. + if [[ -n "$(git -C "$path" status --porcelain)" ]]; then + echo "Kept $branch: $path has uncommitted changes." + return 1 + fi + + if ! head="$(merged_head "$branch")"; then + echo "Kept $branch: could not ask GitHub about its pull request." + return 1 + fi + tip="$(git -C "$path" rev-parse HEAD)" + + if [[ -z "$head" || "$head" != "$tip" ]]; then + if [[ "$discard" -eq 1 ]]; then + printf "Discard %s and its worktree %s, although it is not merged? [y/N] " "$branch" "$path" + read -r answer || true + case "$answer" in + y | Y | yes | YES) ;; + *) + echo "Kept $branch." + return 1 + ;; + esac + elif [[ -z "$head" ]]; then + echo "Kept $branch: its pull request is not merged." + return 1 + else + echo "Kept $branch: it has commits after its merged pull request." + return 1 + fi + fi + + if ! git worktree remove "$path"; then + echo "Error: could not remove the worktree of $branch at $path; the branch was kept." >&2 + return 2 + fi + if ! git branch -D "$branch" >/dev/null; then + echo "Error: removed the worktree of $branch, but could not delete the branch." >&2 + return 2 + fi + echo "Removed $branch and its worktree $path." + return 0 +} + +removed=0 +errors=0 +if [[ "$merged_mode" -eq 1 ]]; then + while IFS=$'\t' read -r path branch; do + [[ -n "$path" ]] || continue + status=0 + cleanup "$path" "$branch" ." +echo "feature Issues with ./scripts/create-feature-issue.sh , and remove" +echo "this worktree from the primary checkout with ./scripts/cleanup-worktree.sh $branch." diff --git a/scripts/verify.conf b/scripts/verify.conf index 0e0c109..3497a2e 100644 --- a/scripts/verify.conf +++ b/scripts/verify.conf @@ -35,6 +35,7 @@ review-planning: ./tests/review-planning-test.sh revise-planning: ./tests/revise-planning-test.sh finish-planning: ./tests/finish-planning-test.sh finish-feature: ./tests/finish-feature-test.sh +cleanup-worktree: ./tests/cleanup-worktree-test.sh triage-review: ./tests/triage-review-test.sh apply-triage: ./tests/apply-triage-test.sh start-planning: ./tests/start-planning-test.sh diff --git a/tests/cleanup-worktree-test.sh b/tests/cleanup-worktree-test.sh new file mode 100755 index 0000000..9936cba --- /dev/null +++ b/tests/cleanup-worktree-test.sh @@ -0,0 +1,226 @@ +#!/usr/bin/env bash + +set -euo pipefail + +# Run as on CI: without the user's global or system Git configuration, so a +# test cannot depend on a local Git identity or setting. +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 + +source_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)" +tmp="$(mktemp -d "${TMPDIR:-/tmp}/cleanup-worktree-test.XXXXXX")" +tmp="$(cd "$tmp" && pwd -P)" + +cleanup() { + rm -rf "$tmp" +} +trap cleanup EXIT + +fail() { + echo "cleanup-worktree test failed: $*" >&2 + exit 1 +} + +mkdir -p "$tmp/bin" +ln -sf "$(command -v jq)" "$tmp/bin/jq" + +# The fake GitHub CLI reports a merged pull request for each " " +# line in MOCK_MERGED_FILE. +cat >"$tmp/bin/gh" <<'GH' +#!/usr/bin/env bash +set -euo pipefail +if [[ "${1:-} ${2:-}" == "pr list" ]]; then + [[ -z "${MOCK_GH_FAIL:-}" ]] || exit 1 + head="" + while [[ $# -gt 0 ]]; do + [[ "$1" != "--head" ]] || head="$2" + shift + done + awk -v branch="$head" '$1 == branch { print $2 }' "$MOCK_MERGED_FILE" 2>/dev/null || true + exit 0 +fi +echo "Unexpected gh invocation: $*" >&2 +exit 1 +GH +chmod +x "$tmp/bin/gh" + +# Creates a primary checkout with a remote, and returns its path. +setup_repo() { + local label="$1" + local seed="$tmp/$label-seed" + local remote="$tmp/$label-remote.git" + local repo="$tmp/$label" + + mkdir -p "$seed/scripts" + cp "$source_root/scripts/cleanup-worktree.sh" "$seed/scripts/" + cp "$source_root/.gitignore" "$seed/.gitignore" + printf '# Project\n' >"$seed/README.md" + git -C "$seed" init -q -b main + git -C "$seed" config user.name "Cleanup Test" + git -C "$seed" config user.email "cleanup-test@example.com" + git -C "$seed" add . + git -C "$seed" commit -qm "Seed" + git init -q --bare -b main "$remote" + git -C "$seed" push -q "$remote" main + git clone -q "$remote" "$repo" + git -C "$repo" config user.name "Cleanup Test" + git -C "$repo" config user.email "cleanup-test@example.com" + : >"$repo.merged" + printf '%s\n' "$repo" +} + +# add_worktree : a worktree with one commit. +add_worktree() { + git -C "$1" worktree add -q "$tmp/$3" -b "$2" origin/main + printf '%s\n' "$2" >"$tmp/$3/work.txt" + git -C "$tmp/$3" add work.txt + git -C "$tmp/$3" commit -qm "Work on $2" +} + +# merge : squash-merges the branch on the remote +# and records its head as the merged pull request head. +merge() { + local other="$tmp/merge-$RANDOM" + git clone -q "$(git -C "$1" remote get-url origin)" "$other" + git -C "$other" config user.name "Cleanup Test" + git -C "$other" config user.email "cleanup-test@example.com" + git -C "$other" checkout -q "$(git -C "$tmp/$3" rev-parse HEAD)" -- . 2>/dev/null || + cp "$tmp/$3/work.txt" "$other/work.txt" + git -C "$other" add -A + git -C "$other" commit -qm "Squash merge $2" + git -C "$other" push -q origin main + printf '%s %s\n' "$2" "$(git -C "$tmp/$3" rev-parse HEAD)" >>"$1.merged" +} + +run_cleanup() { + local repo="$1" + shift + ( + cd "$repo" + PATH="$tmp/bin:/usr/bin:/bin" MOCK_MERGED_FILE="$repo.merged" ./scripts/cleanup-worktree.sh "$@" + ) >"$repo.out" 2>&1 +} + +branch_exists() { + git -C "$1" show-ref --verify --quiet "refs/heads/$2" +} + +# A merged feature worktree is removed with its branch, also with ignored +# review files in it, and main is updated. +repo="$(setup_repo merged)" +add_worktree "$repo" feature/12-recipes merged-12-recipes +mkdir -p "$tmp/merged-12-recipes/.agents/reviews" +printf '{}\n' >"$tmp/merged-12-recipes/.agents/reviews/feature-12-recipes-review-01.json" +merge "$repo" feature/12-recipes merged-12-recipes +run_cleanup "$repo" 12 || { + cat "$repo.out" >&2 + fail "cleaning a merged feature worktree failed" +} +[[ ! -e "$tmp/merged-12-recipes" ]] || fail "the merged worktree was not removed" +! branch_exists "$repo" feature/12-recipes || fail "the merged branch was not deleted" +[[ -f "$repo/work.txt" ]] || fail "main was not updated with the merged work" + +# A planning worktree is found by its branch name, and a path still works. +repo="$(setup_repo planning)" +add_worktree "$repo" planning/project-bootstrap planning-wt +merge "$repo" planning/project-bootstrap planning-wt +run_cleanup "$repo" planning/project-bootstrap || { + cat "$repo.out" >&2 + fail "cleaning a merged planning worktree failed" +} +[[ ! -e "$tmp/planning-wt" ]] || fail "the merged planning worktree was not removed" + +repo="$(setup_repo by-path)" +add_worktree "$repo" feature/13-plan by-path-13 +merge "$repo" feature/13-plan by-path-13 +run_cleanup "$repo" "$tmp/by-path-13" || { + cat "$repo.out" >&2 + fail "cleaning by path failed" +} +[[ ! -e "$tmp/by-path-13" ]] || fail "the worktree given by path was not removed" + +# Unmerged, ahead-of-PR, and dirty worktrees are kept. +repo="$(setup_repo kept)" +add_worktree "$repo" feature/20-unmerged kept-20 +if run_cleanup "$repo" 20; then fail "an unmerged worktree was removed"; fi +[[ -d "$tmp/kept-20" ]] && branch_exists "$repo" feature/20-unmerged || fail "an unmerged worktree or branch is gone" +grep -Fq "not merged" "$repo.out" || fail "an unmerged worktree was not reported" + +add_worktree "$repo" feature/21-ahead kept-21 +merge "$repo" feature/21-ahead kept-21 +printf 'later\n' >>"$tmp/kept-21/work.txt" +git -C "$tmp/kept-21" commit -qam "Commit after the merge" +if run_cleanup "$repo" 21; then fail "a worktree with commits after its merge was removed"; fi +[[ -d "$tmp/kept-21" ]] || fail "a worktree with commits after its merge is gone" + +add_worktree "$repo" feature/22-dirty kept-22 +merge "$repo" feature/22-dirty kept-22 +printf 'uncommitted\n' >>"$tmp/kept-22/work.txt" +if run_cleanup "$repo" 22; then fail "a dirty worktree was removed"; fi +[[ -d "$tmp/kept-22" ]] || fail "a dirty worktree is gone" + +if MOCK_GH_FAIL=1 run_cleanup "$repo" 20; then fail "a failed GitHub lookup removed a worktree"; fi + +# --merged removes exactly the merged worktrees. +repo="$(setup_repo all-merged)" +add_worktree "$repo" feature/30-done all-30 +add_worktree "$repo" feature/31-open all-31 +merge "$repo" feature/30-done all-30 +run_cleanup "$repo" --merged || { + cat "$repo.out" >&2 + fail "--merged failed" +} +[[ ! -e "$tmp/all-30" ]] || fail "--merged kept a merged worktree" +[[ -d "$tmp/all-31" ]] || fail "--merged removed an unmerged worktree" +grep -Fq "Kept feature/31-open" "$repo.out" || fail "--merged did not report the kept worktree" + +# --discard removes an unmerged worktree only after confirmation. +repo="$(setup_repo discard)" +add_worktree "$repo" feature/40-abandoned discard-40 +( cd "$repo" && printf 'n\n' | PATH="$tmp/bin:/usr/bin:/bin" MOCK_MERGED_FILE="$repo.merged" ./scripts/cleanup-worktree.sh 40 --discard ) >/dev/null 2>&1 || true +[[ -d "$tmp/discard-40" ]] || fail "--discard removed a worktree without confirmation" +( cd "$repo" && printf 'y\n' | PATH="$tmp/bin:/usr/bin:/bin" MOCK_MERGED_FILE="$repo.merged" ./scripts/cleanup-worktree.sh 40 --discard ) >"$repo.out" 2>&1 || { + cat "$repo.out" >&2 + fail "--discard with confirmation failed" +} +[[ ! -e "$tmp/discard-40" ]] && ! branch_exists "$repo" feature/40-abandoned || fail "--discard did not remove the worktree and branch" + +# The script runs only from the primary checkout, so it never removes the +# worktree it runs in or one next to it. +repo="$(setup_repo self)" +add_worktree "$repo" feature/50-self self-50 +add_worktree "$repo" feature/51-other self-51 +merge "$repo" feature/50-self self-50 +merge "$repo" feature/51-other self-51 +for target in 50 51; do + if ( cd "$tmp/self-50" && PATH="$tmp/bin:/usr/bin:/bin" MOCK_MERGED_FILE="$repo.merged" ./scripts/cleanup-worktree.sh "$target" ) >"$repo.out" 2>&1; then + fail "cleanup ran from a linked worktree (target $target)" + fi +done +[[ -d "$tmp/self-50" && -d "$tmp/self-51" ]] || fail "a worktree was removed when run from a linked worktree" +grep -Fq "primary checkout" "$repo.out" || fail "running from a linked worktree was not explained" + +# A worktree that Git refuses to remove is reported as an error and its branch +# is kept. +repo="$(setup_repo locked)" +add_worktree "$repo" feature/60-locked locked-60 +merge "$repo" feature/60-locked locked-60 +git -C "$repo" worktree lock "$tmp/locked-60" +if run_cleanup "$repo" 60; then fail "a failed removal reported success"; fi +grep -Fq "could not remove the worktree" "$repo.out" || fail "a failed removal was not reported" +branch_exists "$repo" feature/60-locked || fail "the branch was deleted although its worktree remained" +git -C "$repo" worktree unlock "$tmp/locked-60" + +# An explicit path also works for a worktree on another kind of branch. +repo="$(setup_repo other-branch)" +add_worktree "$repo" fix/70-typo fix-70 +merge "$repo" fix/70-typo fix-70 +run_cleanup "$repo" "$tmp/fix-70" || { + cat "$repo.out" >&2 + fail "cleaning a merged fix/ worktree by path failed" +} +[[ ! -e "$tmp/fix-70" ]] || fail "the merged fix/ worktree was not removed" + +# An unknown target fails. +if run_cleanup "$repo" 99; then fail "an unknown target succeeded"; fi + +echo "cleanup-worktree tests passed"