feat: complete isolated-worktree helpers — worktree-review, worktree-clean, tests (#39)
Finishes the isolated-worktree standard begun in #38 (which merged the branches/ gitignore, runbook, and scripts/worktree-start). Adds the two remaining helpers and their tests. - scripts/worktree-review: isolated DETACHED review worktree under branches/review-<branch> (fetch/prune first, refuse to overwrite, print path, --dry-run). Detached so a reviewer cannot accidentally commit and review work never blocks the author's implementation folder. - scripts/worktree-clean: the only deleting helper — removes a branches/ worktree after merge/close, refuses a dirty worktree (no --force), optionally safe-deletes a merged branch (git branch -d), fetch/prune first, --dry-run. Deletes nothing unless explicitly invoked. - tests/test_worktrees.py: path generation + refuse-to-overwrite for all three helpers via --dry-run (no real worktrees/branches/network/deletions). - runbook: reference worktree-review / worktree-clean and the --dry-run flag. Checks: bash -n clean on all three scripts; git diff --check clean; full suite 286 passed, 0 failures. Closes #39. Follow-up to #38. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -205,18 +205,29 @@ git worktree add -b fix/issue-123-example branches/fix-issue-123-example prgs/ma
|
|||||||
cd branches/fix-issue-123-example
|
cd branches/fix-issue-123-example
|
||||||
```
|
```
|
||||||
|
|
||||||
For review work, create a separate review worktree/folder instead of reusing the
|
For review work, create a separate **detached** review worktree instead of
|
||||||
author's implementation folder.
|
reusing the author's implementation folder:
|
||||||
|
|
||||||
Cleanup is explicit and only after merge or close:
|
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
scripts/worktree-review fix/issue-123-example # → branches/review-fix-issue-123-example
|
||||||
|
```
|
||||||
|
|
||||||
|
Cleanup is explicit and only after merge or close. Use the helper (it fetches/
|
||||||
|
prunes first, refuses to remove a dirty worktree, and only safe-deletes a merged
|
||||||
|
branch), or the equivalent manual commands:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
scripts/worktree-clean --delete-branch fix/issue-123-example
|
||||||
|
# equivalent manual commands:
|
||||||
cd <main-repo>
|
cd <main-repo>
|
||||||
git fetch prgs --prune
|
git fetch prgs --prune
|
||||||
git worktree remove branches/fix-issue-123-example
|
git worktree remove branches/fix-issue-123-example
|
||||||
git branch -d fix/issue-123-example
|
git branch -d fix/issue-123-example
|
||||||
```
|
```
|
||||||
|
|
||||||
|
All three helpers accept `--dry-run` to print the exact commands/paths without
|
||||||
|
touching anything.
|
||||||
|
|
||||||
### Create an issue / child issues
|
### Create an issue / child issues
|
||||||
|
|
||||||
- **Profile:** issue-manager or author (any profile allowed to create issues).
|
- **Profile:** issue-manager or author (any profile allowed to create issues).
|
||||||
|
|||||||
Executable
+74
@@ -0,0 +1,74 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
usage() {
|
||||||
|
cat <<'EOF'
|
||||||
|
usage: scripts/worktree-clean [--dry-run] [--delete-branch] <branch-name|folder-name>
|
||||||
|
|
||||||
|
Remove a branch worktree under branches/ AFTER its PR is merged or closed. This
|
||||||
|
is the ONLY helper that deletes anything, and it deletes nothing unless you
|
||||||
|
invoke it explicitly. It refuses to remove a worktree with uncommitted changes
|
||||||
|
(no --force is offered). With --delete-branch it also deletes the local branch,
|
||||||
|
but only with a safe `git branch -d` (fails unless the branch is merged).
|
||||||
|
|
||||||
|
Pass the branch name (with slashes) so --delete-branch can resolve it; the
|
||||||
|
folder is branches/<branch-with-slashes-replaced-by-dashes>.
|
||||||
|
|
||||||
|
Examples:
|
||||||
|
scripts/worktree-clean --dry-run fix/issue-123-example
|
||||||
|
scripts/worktree-clean --delete-branch fix/issue-123-example
|
||||||
|
EOF
|
||||||
|
}
|
||||||
|
|
||||||
|
dry_run=0
|
||||||
|
del_branch=0
|
||||||
|
while [[ "${1:-}" == --* ]]; do
|
||||||
|
case "$1" in
|
||||||
|
--dry-run) dry_run=1 ;;
|
||||||
|
--delete-branch) del_branch=1 ;;
|
||||||
|
--help) usage; exit 0 ;;
|
||||||
|
*) usage >&2; exit 2 ;;
|
||||||
|
esac
|
||||||
|
shift
|
||||||
|
done
|
||||||
|
|
||||||
|
if [[ $# -ne 1 ]]; then
|
||||||
|
usage >&2
|
||||||
|
exit 2
|
||||||
|
fi
|
||||||
|
|
||||||
|
branch="$1"
|
||||||
|
worktree_name="${branch//\//-}"
|
||||||
|
|
||||||
|
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
|
repo_root="$(cd "$script_dir/.." && pwd)"
|
||||||
|
worktree_path="$repo_root/branches/$worktree_name"
|
||||||
|
|
||||||
|
if [[ ! -d "$worktree_path" ]]; then
|
||||||
|
printf 'No such worktree: %s\n' "$worktree_path" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [[ "$dry_run" -eq 1 ]]; then
|
||||||
|
printf 'repo: %s\n' "$repo_root"
|
||||||
|
printf 'worktree: %s\n' "$worktree_path"
|
||||||
|
printf 'delete-branch: %s\n' "$del_branch"
|
||||||
|
printf 'commands:\n'
|
||||||
|
printf ' git -C %q fetch prgs --prune\n' "$repo_root"
|
||||||
|
printf ' git -C %q worktree remove %q\n' "$repo_root" "$worktree_path"
|
||||||
|
if [[ "$del_branch" -eq 1 ]]; then
|
||||||
|
printf ' git -C %q branch -d %q\n' "$repo_root" "$branch"
|
||||||
|
fi
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
git -C "$repo_root" fetch prgs --prune
|
||||||
|
# No --force: `git worktree remove` fails on uncommitted changes, on purpose.
|
||||||
|
git -C "$repo_root" worktree remove "$worktree_path"
|
||||||
|
printf 'removed worktree: %s\n' "$worktree_path"
|
||||||
|
|
||||||
|
if [[ "$del_branch" -eq 1 ]]; then
|
||||||
|
# Safe delete only: refuses to drop an unmerged branch.
|
||||||
|
git -C "$repo_root" branch -d "$branch"
|
||||||
|
printf 'deleted branch: %s\n' "$branch"
|
||||||
|
fi
|
||||||
Executable
+62
@@ -0,0 +1,62 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
usage() {
|
||||||
|
cat <<'EOF'
|
||||||
|
usage: scripts/worktree-review [--dry-run] <branch-name> [start-ref]
|
||||||
|
|
||||||
|
Create an isolated, DETACHED review worktree under branches/review-<branch>.
|
||||||
|
Reviews an existing branch in its own folder without creating a local branch,
|
||||||
|
so review work never blocks or contaminates the author's implementation folder
|
||||||
|
and a reviewer cannot accidentally commit. Default start-ref is prgs/<branch>.
|
||||||
|
|
||||||
|
Examples:
|
||||||
|
scripts/worktree-review fix/issue-123-example
|
||||||
|
scripts/worktree-review --dry-run fix/issue-123-example prgs/fix/issue-123-example
|
||||||
|
EOF
|
||||||
|
}
|
||||||
|
|
||||||
|
dry_run=0
|
||||||
|
if [[ "${1:-}" == "--dry-run" ]]; then
|
||||||
|
dry_run=1
|
||||||
|
shift
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [[ $# -lt 1 || $# -gt 2 ]]; then
|
||||||
|
usage >&2
|
||||||
|
exit 2
|
||||||
|
fi
|
||||||
|
|
||||||
|
branch="$1"
|
||||||
|
start_ref="${2:-prgs/$branch}"
|
||||||
|
|
||||||
|
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
|
repo_root="$(cd "$script_dir/.." && pwd)"
|
||||||
|
worktree_name="review-${branch//\//-}"
|
||||||
|
worktree_path="$repo_root/branches/$worktree_name"
|
||||||
|
|
||||||
|
if [[ -e "$worktree_path" ]]; then
|
||||||
|
cat >&2 <<EOF
|
||||||
|
Refusing to reuse existing review worktree:
|
||||||
|
$worktree_path
|
||||||
|
|
||||||
|
Inspect it manually. Do not overwrite another reviewer's folder.
|
||||||
|
EOF
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [[ "$dry_run" -eq 1 ]]; then
|
||||||
|
printf 'repo: %s\n' "$repo_root"
|
||||||
|
printf 'branch: %s\n' "$branch"
|
||||||
|
printf 'start-ref: %s\n' "$start_ref"
|
||||||
|
printf 'worktree: %s\n' "$worktree_path"
|
||||||
|
printf 'commands:\n'
|
||||||
|
printf ' git -C %q fetch prgs --prune\n' "$repo_root"
|
||||||
|
printf ' git -C %q worktree add --detach %q %q\n' \
|
||||||
|
"$repo_root" "$worktree_path" "$start_ref"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
git -C "$repo_root" fetch prgs --prune
|
||||||
|
git -C "$repo_root" worktree add --detach "$worktree_path" "$start_ref"
|
||||||
|
printf '%s\n' "$worktree_path"
|
||||||
@@ -0,0 +1,104 @@
|
|||||||
|
"""Tests for the branch-worktree helper scripts (#38/#39).
|
||||||
|
|
||||||
|
Exercises path generation and refuse-to-overwrite via the scripts' ``--dry-run``
|
||||||
|
mode, so no real worktrees, branches, network, or deletions are involved.
|
||||||
|
"""
|
||||||
|
import os
|
||||||
|
import subprocess
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
REPO = Path(__file__).resolve().parent.parent
|
||||||
|
SCRIPTS = REPO / "scripts"
|
||||||
|
BRANCHES = REPO / "branches"
|
||||||
|
|
||||||
|
|
||||||
|
def run(script, *args):
|
||||||
|
proc = subprocess.run(
|
||||||
|
["bash", str(SCRIPTS / script), *args],
|
||||||
|
capture_output=True, text=True, cwd=str(REPO),
|
||||||
|
)
|
||||||
|
return proc.returncode, proc.stdout, proc.stderr
|
||||||
|
|
||||||
|
|
||||||
|
class TestWorktreeStart(unittest.TestCase):
|
||||||
|
|
||||||
|
def test_dry_run_path_generation(self):
|
||||||
|
rc, out, _ = run("worktree-start", "--dry-run", "fix/issue-123-example")
|
||||||
|
self.assertEqual(rc, 0)
|
||||||
|
self.assertIn("branches/fix-issue-123-example", out)
|
||||||
|
self.assertIn("fix/issue-123-example", out)
|
||||||
|
self.assertIn("prgs/master", out) # default start-ref
|
||||||
|
|
||||||
|
def test_bad_args_exit_2(self):
|
||||||
|
rc, _, _ = run("worktree-start")
|
||||||
|
self.assertEqual(rc, 2)
|
||||||
|
|
||||||
|
def test_refuses_existing_worktree(self):
|
||||||
|
slug = f"zz-refuse-start-{os.getpid()}"
|
||||||
|
target = BRANCHES / slug
|
||||||
|
target.mkdir(parents=True, exist_ok=True)
|
||||||
|
try:
|
||||||
|
rc, _, err = run("worktree-start", "--dry-run", slug)
|
||||||
|
self.assertEqual(rc, 1)
|
||||||
|
self.assertIn("Refusing to reuse", err)
|
||||||
|
finally:
|
||||||
|
target.rmdir()
|
||||||
|
|
||||||
|
|
||||||
|
class TestWorktreeReview(unittest.TestCase):
|
||||||
|
|
||||||
|
def test_dry_run_detached_review_path(self):
|
||||||
|
rc, out, _ = run("worktree-review", "--dry-run", "fix/issue-9-x")
|
||||||
|
self.assertEqual(rc, 0)
|
||||||
|
self.assertIn("branches/review-fix-issue-9-x", out)
|
||||||
|
self.assertIn("--detach", out)
|
||||||
|
self.assertIn("prgs/fix/issue-9-x", out) # default start-ref
|
||||||
|
|
||||||
|
def test_refuses_existing_review_worktree(self):
|
||||||
|
slug = f"review-zz-refuse-rev-{os.getpid()}"
|
||||||
|
target = BRANCHES / slug
|
||||||
|
target.mkdir(parents=True, exist_ok=True)
|
||||||
|
try:
|
||||||
|
# branch name maps to the same slug: review-<branch>
|
||||||
|
rc, _, err = run("worktree-review", "--dry-run",
|
||||||
|
f"zz-refuse-rev-{os.getpid()}")
|
||||||
|
self.assertEqual(rc, 1)
|
||||||
|
self.assertIn("Refusing to reuse", err)
|
||||||
|
finally:
|
||||||
|
target.rmdir()
|
||||||
|
|
||||||
|
|
||||||
|
class TestWorktreeClean(unittest.TestCase):
|
||||||
|
|
||||||
|
def test_missing_worktree_errors(self):
|
||||||
|
rc, _, err = run("worktree-clean", "--dry-run", "does-not-exist-xyz")
|
||||||
|
self.assertEqual(rc, 1)
|
||||||
|
self.assertIn("No such worktree", err)
|
||||||
|
|
||||||
|
def test_dry_run_does_not_delete(self):
|
||||||
|
slug = f"zz-clean-{os.getpid()}"
|
||||||
|
target = BRANCHES / slug
|
||||||
|
target.mkdir(parents=True, exist_ok=True)
|
||||||
|
try:
|
||||||
|
rc, out, _ = run("worktree-clean", "--dry-run", slug)
|
||||||
|
self.assertEqual(rc, 0)
|
||||||
|
self.assertIn("worktree remove", out)
|
||||||
|
self.assertTrue(target.is_dir()) # nothing removed in dry-run
|
||||||
|
finally:
|
||||||
|
target.rmdir()
|
||||||
|
|
||||||
|
def test_dry_run_delete_branch_lists_branch_command(self):
|
||||||
|
slug = f"zz-clean-b-{os.getpid()}"
|
||||||
|
target = BRANCHES / slug
|
||||||
|
target.mkdir(parents=True, exist_ok=True)
|
||||||
|
try:
|
||||||
|
rc, out, _ = run("worktree-clean", "--dry-run", "--delete-branch", slug)
|
||||||
|
self.assertEqual(rc, 0)
|
||||||
|
self.assertIn("branch -d", out)
|
||||||
|
finally:
|
||||||
|
target.rmdir()
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Reference in New Issue
Block a user