From ded6c8a55ec0b92db22b32c4aa307b29b5e3665d Mon Sep 17 00:00:00 2001 From: Martin conur Date: Tue, 2 Jun 2026 16:00:18 -0400 Subject: [PATCH 1/3] updating claude files [skip ci] --- .claude/settings.json | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.claude/settings.json b/.claude/settings.json index 24008f8..23a9377 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -83,6 +83,8 @@ }, "permissions": { "allow": [ + "Bash(cat *)", + "Bash(find *)", "Bash(gh auth status)", "Bash(gh issue comment *)", "Bash(gh issue list *)", @@ -97,7 +99,12 @@ "Bash(gh project view *)", "Bash(gh repo view *)", "Bash(gh search issues *)", - "Bash(radio *)" + "Bash(ls *)", + "Bash(radio *)", + "Bash(rg *)", + "Glob", + "Grep", + "Read" ] } } From 62e2cdf421ca903dfb9587c4891e9bd2e69ab24c Mon Sep 17 00:00:00 2001 From: Martin conur Date: Tue, 2 Jun 2026 16:17:51 -0400 Subject: [PATCH 2/3] task-done --remove-worktree leaks reviewer branch + state; blocks re-dispatch of task-reviewer: force-delete on PR_NUMBER MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch on the `PR_NUMBER=` marker that `task-reviewer` writes into the worktree's `.info` file. When present, the worktree is a reviewer scaffold whose branch (`task/review-pr`) is forked from the PR's head ref and will never merge into main — use `git branch -D` so a subsequent `task-reviewer ` isn't blocked by its per-PR branch guard. When absent (worker worktree), behavior is unchanged: `git branch -d` (safe-delete) so unmerged work isn't dropped. Lands across the `task-done-std` and `task-done-local` drift groups (7 binaries, byte-identical). New bats coverage exercises the reviewer-branch path for each of the 7 variants plus a worker regression for claude-gh; drift check stays green. Closes #148 Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + claude-gh/bin/task-done | 20 +++++++-- claude-jira/bin/task-done | 20 +++++++-- claude-local/bin/task-done | 20 +++++++-- claude-notion/bin/task-done | 20 +++++++-- kiro-gh/bin/task-done | 20 +++++++-- kiro-local/bin/task-done | 20 +++++++-- kiro-notion/bin/task-done | 20 +++++++-- tests/task_done.bats | 84 +++++++++++++++++++++++++++++++++++++ 9 files changed, 204 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed8041a..092cfea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- **`task-done --remove-worktree` now force-deletes reviewer branches so `task-reviewer` re-dispatch isn't blocked.** The cleanup's `git branch -d` is the safe-delete variant that requires the branch to be fully merged into HEAD — correct for worker branches (don't drop unmerged work), but wrong for reviewer worktrees, whose `task/review-pr` branch is forked from the PR's head ref and is never going to land in main (the PR is). Result: every reviewer cleanup left `task/review-pr` behind as an orphan, and the next `task-reviewer ` refused to dispatch via its per-PR guard with `Error: a review worktree or branch already exists`. The user had to manually `git branch -D task/review-pr` between every re-review round. `task-done` now branches on the `PR_NUMBER=` marker that `task-reviewer` writes into the worktree's `.info` file: when present it does `git branch -D` (scaffold-only, safe to force); when absent worker behavior is unchanged. Lands across the `task-done-std` and `task-done-local` drift groups (7 binaries). (#148) - **`task-reviewer`'s spec-issue cross-check now works on `claude-jira` / `claude-notion` / `claude-local`.** The dispatcher and the `/reviewer` prompt were both GitHub-only: `parse_issue_number` rejected anything that wasn't a bare integer or `github.com/.../issues/N`; PR-body auto-detect only matched `Closes #N`; the URL-synthesis step always emitted a GitHub issues URL; and the prompt hardcoded `gh issue view ` for spec lookup. Net result: in any non-gh loadout, `task-reviewer ` silently degraded to a diff-only review. The dispatcher is now loadout-aware (detects from its own file path) — `claude-gh` keeps numeric / URL parsing and PR-body auto-detect; `claude-jira` / `claude-notion` / `claude-local` accept any non-empty 2nd positional arg as an opaque spec identifier (Jira key, Notion URL, local task slug) and pass it through to `/reviewer` unchanged. The 4 dispatcher bodies stay byte-identical (drift-checked). Each loadout's `commands/reviewer.md` now points the spec-lookup step at the right tool: GH stays on `gh issue view`; `claude-jira` uses `mcp__atlassian__getJiraIssue`; `claude-notion` uses `mcp__notion__notion-fetch`; `claude-local` reads `tasks/-.md` via the `Read` tool. With no 2nd arg in a non-gh loadout, the wrapper emits a clear "PR-body auto-detect is GitHub-only — pass the spec id explicitly" advisory and proceeds diff-only. **Upgrading**: re-run `task-init ` after pulling this change so the updated `/reviewer` prompt lands in `.claude/commands/`. (#144) ### Changed diff --git a/claude-gh/bin/task-done b/claude-gh/bin/task-done index becea68..fb6ca9d 100755 --- a/claude-gh/bin/task-done +++ b/claude-gh/bin/task-done @@ -147,10 +147,24 @@ git worktree remove "$WORKTREE_DIR" --force || { # Clean up info file [[ -f "$INFO_FILE" ]] && rm -f "$INFO_FILE" -if git branch -d "$BRANCH" 2>/dev/null; then - echo "Done. Deleted local branch '$BRANCH'." +# Reviewer worktrees write PR_NUMBER into their info file; they're pure +# scaffolding and their branch never merges into main (the PR does — its +# head ref is what landed). Force-delete in that case so a subsequent +# `task-reviewer ` re-dispatch isn't blocked by the per-PR branch +# guard (#148). Worker branches keep the safe-delete `-d` so unmerged +# work isn't silently dropped. +if [[ -n "${PR_NUMBER:-}" ]]; then + if git branch -D "$BRANCH" 2>/dev/null; then + echo "Done. Deleted reviewer branch '$BRANCH' (-D, scaffold-only)." + else + echo "Done. Branch '$BRANCH' could not be deleted — clean up manually: git branch -D $BRANCH" + fi else - echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + if git branch -d "$BRANCH" 2>/dev/null; then + echo "Done. Deleted local branch '$BRANCH'." + else + echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + fi fi # Close this worker's zellij tab by its stable tab_id, captured before diff --git a/claude-jira/bin/task-done b/claude-jira/bin/task-done index 5bf5bfb..d303634 100755 --- a/claude-jira/bin/task-done +++ b/claude-jira/bin/task-done @@ -153,10 +153,24 @@ git worktree remove "$WORKTREE_DIR" --force || { # Clean up info file [[ -f "$INFO_FILE" ]] && rm -f "$INFO_FILE" -if git branch -d "$BRANCH" 2>/dev/null; then - echo "Done. Deleted local branch '$BRANCH'." +# Reviewer worktrees write PR_NUMBER into their info file; they're pure +# scaffolding and their branch never merges into main (the PR does — its +# head ref is what landed). Force-delete in that case so a subsequent +# `task-reviewer ` re-dispatch isn't blocked by the per-PR branch +# guard (#148). Worker branches keep the safe-delete `-d` so unmerged +# work isn't silently dropped. +if [[ -n "${PR_NUMBER:-}" ]]; then + if git branch -D "$BRANCH" 2>/dev/null; then + echo "Done. Deleted reviewer branch '$BRANCH' (-D, scaffold-only)." + else + echo "Done. Branch '$BRANCH' could not be deleted — clean up manually: git branch -D $BRANCH" + fi else - echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + if git branch -d "$BRANCH" 2>/dev/null; then + echo "Done. Deleted local branch '$BRANCH'." + else + echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + fi fi # Close this worker's zellij tab by its stable tab_id, captured before diff --git a/claude-local/bin/task-done b/claude-local/bin/task-done index 8bbfcf5..8e12549 100755 --- a/claude-local/bin/task-done +++ b/claude-local/bin/task-done @@ -167,10 +167,24 @@ else fi fi -if git branch -d "$BRANCH" 2>/dev/null; then - echo "Done. Deleted local branch '$BRANCH'." +# Reviewer worktrees write PR_NUMBER into their info file; they're pure +# scaffolding and their branch never merges into main (the PR does — its +# head ref is what landed). Force-delete in that case so a subsequent +# `task-reviewer ` re-dispatch isn't blocked by the per-PR branch +# guard (#148). Worker branches keep the safe-delete `-d` so unmerged +# work isn't silently dropped. +if [[ -n "${PR_NUMBER:-}" ]]; then + if git branch -D "$BRANCH" 2>/dev/null; then + echo "Done. Deleted reviewer branch '$BRANCH' (-D, scaffold-only)." + else + echo "Done. Branch '$BRANCH' could not be deleted — clean up manually: git branch -D $BRANCH" + fi else - echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + if git branch -d "$BRANCH" 2>/dev/null; then + echo "Done. Deleted local branch '$BRANCH'." + else + echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + fi fi # Close this worker's zellij tab by its stable tab_id, captured before diff --git a/claude-notion/bin/task-done b/claude-notion/bin/task-done index becea68..fb6ca9d 100755 --- a/claude-notion/bin/task-done +++ b/claude-notion/bin/task-done @@ -147,10 +147,24 @@ git worktree remove "$WORKTREE_DIR" --force || { # Clean up info file [[ -f "$INFO_FILE" ]] && rm -f "$INFO_FILE" -if git branch -d "$BRANCH" 2>/dev/null; then - echo "Done. Deleted local branch '$BRANCH'." +# Reviewer worktrees write PR_NUMBER into their info file; they're pure +# scaffolding and their branch never merges into main (the PR does — its +# head ref is what landed). Force-delete in that case so a subsequent +# `task-reviewer ` re-dispatch isn't blocked by the per-PR branch +# guard (#148). Worker branches keep the safe-delete `-d` so unmerged +# work isn't silently dropped. +if [[ -n "${PR_NUMBER:-}" ]]; then + if git branch -D "$BRANCH" 2>/dev/null; then + echo "Done. Deleted reviewer branch '$BRANCH' (-D, scaffold-only)." + else + echo "Done. Branch '$BRANCH' could not be deleted — clean up manually: git branch -D $BRANCH" + fi else - echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + if git branch -d "$BRANCH" 2>/dev/null; then + echo "Done. Deleted local branch '$BRANCH'." + else + echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + fi fi # Close this worker's zellij tab by its stable tab_id, captured before diff --git a/kiro-gh/bin/task-done b/kiro-gh/bin/task-done index becea68..fb6ca9d 100755 --- a/kiro-gh/bin/task-done +++ b/kiro-gh/bin/task-done @@ -147,10 +147,24 @@ git worktree remove "$WORKTREE_DIR" --force || { # Clean up info file [[ -f "$INFO_FILE" ]] && rm -f "$INFO_FILE" -if git branch -d "$BRANCH" 2>/dev/null; then - echo "Done. Deleted local branch '$BRANCH'." +# Reviewer worktrees write PR_NUMBER into their info file; they're pure +# scaffolding and their branch never merges into main (the PR does — its +# head ref is what landed). Force-delete in that case so a subsequent +# `task-reviewer ` re-dispatch isn't blocked by the per-PR branch +# guard (#148). Worker branches keep the safe-delete `-d` so unmerged +# work isn't silently dropped. +if [[ -n "${PR_NUMBER:-}" ]]; then + if git branch -D "$BRANCH" 2>/dev/null; then + echo "Done. Deleted reviewer branch '$BRANCH' (-D, scaffold-only)." + else + echo "Done. Branch '$BRANCH' could not be deleted — clean up manually: git branch -D $BRANCH" + fi else - echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + if git branch -d "$BRANCH" 2>/dev/null; then + echo "Done. Deleted local branch '$BRANCH'." + else + echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + fi fi # Close this worker's zellij tab by its stable tab_id, captured before diff --git a/kiro-local/bin/task-done b/kiro-local/bin/task-done index 8bbfcf5..8e12549 100755 --- a/kiro-local/bin/task-done +++ b/kiro-local/bin/task-done @@ -167,10 +167,24 @@ else fi fi -if git branch -d "$BRANCH" 2>/dev/null; then - echo "Done. Deleted local branch '$BRANCH'." +# Reviewer worktrees write PR_NUMBER into their info file; they're pure +# scaffolding and their branch never merges into main (the PR does — its +# head ref is what landed). Force-delete in that case so a subsequent +# `task-reviewer ` re-dispatch isn't blocked by the per-PR branch +# guard (#148). Worker branches keep the safe-delete `-d` so unmerged +# work isn't silently dropped. +if [[ -n "${PR_NUMBER:-}" ]]; then + if git branch -D "$BRANCH" 2>/dev/null; then + echo "Done. Deleted reviewer branch '$BRANCH' (-D, scaffold-only)." + else + echo "Done. Branch '$BRANCH' could not be deleted — clean up manually: git branch -D $BRANCH" + fi else - echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + if git branch -d "$BRANCH" 2>/dev/null; then + echo "Done. Deleted local branch '$BRANCH'." + else + echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + fi fi # Close this worker's zellij tab by its stable tab_id, captured before diff --git a/kiro-notion/bin/task-done b/kiro-notion/bin/task-done index becea68..fb6ca9d 100755 --- a/kiro-notion/bin/task-done +++ b/kiro-notion/bin/task-done @@ -147,10 +147,24 @@ git worktree remove "$WORKTREE_DIR" --force || { # Clean up info file [[ -f "$INFO_FILE" ]] && rm -f "$INFO_FILE" -if git branch -d "$BRANCH" 2>/dev/null; then - echo "Done. Deleted local branch '$BRANCH'." +# Reviewer worktrees write PR_NUMBER into their info file; they're pure +# scaffolding and their branch never merges into main (the PR does — its +# head ref is what landed). Force-delete in that case so a subsequent +# `task-reviewer ` re-dispatch isn't blocked by the per-PR branch +# guard (#148). Worker branches keep the safe-delete `-d` so unmerged +# work isn't silently dropped. +if [[ -n "${PR_NUMBER:-}" ]]; then + if git branch -D "$BRANCH" 2>/dev/null; then + echo "Done. Deleted reviewer branch '$BRANCH' (-D, scaffold-only)." + else + echo "Done. Branch '$BRANCH' could not be deleted — clean up manually: git branch -D $BRANCH" + fi else - echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + if git branch -d "$BRANCH" 2>/dev/null; then + echo "Done. Deleted local branch '$BRANCH'." + else + echo "Done. Branch '$BRANCH' still has unmerged commits — delete manually after PR merge: git branch -D $BRANCH" + fi fi # Close this worker's zellij tab by its stable tab_id, captured before diff --git a/tests/task_done.bats b/tests/task_done.bats index 88151c5..12f0419 100644 --- a/tests/task_done.bats +++ b/tests/task_done.bats @@ -473,6 +473,90 @@ assert_task_done_unregisters() { assert_task_done_unregisters "$KIRO_LOCAL_TASK_DONE" } +# --------------------------------------------------------------------------- +# Reviewer-worktree branch cleanup (issue #148) +# A reviewer worktree (created by task-reviewer) writes PR_NUMBER= into its +# $INFO_FILE. The branch (task/review-pr) is pure scaffolding forked from +# the PR's head ref — it's never going to merge into main, so the safe-delete +# `git branch -d` always fails and leaves an orphan that blocks the next +# `task-reviewer ` dispatch. task-done must force-delete (`git branch -D`) +# when PR_NUMBER is set; worker behavior (BASE_BRANCH set, no PR_NUMBER) must +# stay unchanged. +# --------------------------------------------------------------------------- + +# Set up a fake reviewer worktree: branch with a commit not in main, plus a +# PR_NUMBER marker in the .info file. Sets $RSLUG so tests can cd into it. +setup_reviewer_worktree() { + local pr_num="${1:-42}" + RSLUG="review-pr${pr_num}" + + # The reviewer branch carries a commit not in main, so `git branch -d` would + # refuse (this is the production case — branch forks from the PR's head ref). + git -C "$MAIN_REPO" worktree add -q "$WORKTREE_BASE/$RSLUG" -b "task/$RSLUG" + touch "$WORKTREE_BASE/$RSLUG/scaffold.txt" + git -C "$WORKTREE_BASE/$RSLUG" add scaffold.txt + git -C "$WORKTREE_BASE/$RSLUG" commit -q -m "scaffold commit (simulated PR head)" + + printf 'BASE_BRANCH=main\nSLUG=%s\nPR_NUMBER=%s\nISSUE_NUMBER=\n' \ + "$RSLUG" "$pr_num" > "$WORKTREE_BASE/.$RSLUG.info" +} + +assert_reviewer_branch_force_deleted() { + local script="$1" + setup_reviewer_worktree 42 + cd "$WORKTREE_BASE/$RSLUG" + + run "$script" --remove-worktree --force + assert_success + assert_output --partial "Deleted reviewer branch 'task/$RSLUG'" + refute_output --partial "still has unmerged commits" + run git -C "$MAIN_REPO" branch --list "task/$RSLUG" + assert_output "" +} + +@test "claude-gh: reviewer worktree force-deletes branch (PR_NUMBER set)" { + assert_reviewer_branch_force_deleted "$CLAUDE_GH_TASK_DONE" +} + +@test "claude-jira: reviewer worktree force-deletes branch (PR_NUMBER set)" { + assert_reviewer_branch_force_deleted "$JIRA_TASK_DONE" +} + +@test "claude-notion: reviewer worktree force-deletes branch (PR_NUMBER set)" { + assert_reviewer_branch_force_deleted "$CLAUDE_NOTION_TASK_DONE" +} + +@test "claude-local: reviewer worktree force-deletes branch (PR_NUMBER set)" { + assert_reviewer_branch_force_deleted "$CLAUDE_LOCAL_TASK_DONE" +} + +@test "kiro-gh: reviewer worktree force-deletes branch (PR_NUMBER set)" { + assert_reviewer_branch_force_deleted "$KIRO_GH_TASK_DONE" +} + +@test "kiro: (kiro-notion) reviewer worktree force-deletes branch (PR_NUMBER set)" { + assert_reviewer_branch_force_deleted "$KIRO_TASK_DONE" +} + +@test "kiro-local: reviewer worktree force-deletes branch (PR_NUMBER set)" { + assert_reviewer_branch_force_deleted "$KIRO_LOCAL_TASK_DONE" +} + +@test "claude-gh: worker worktree (no PR_NUMBER) still uses safe-delete -d" { + # Regression: BASE_BRANCH set, PR_NUMBER absent → unmerged commits leave the + # branch behind with the manual-cleanup message, unchanged from pre-#148. + touch "$WORKTREE_BASE/$SLUG/unmerged.txt" + git -C "$WORKTREE_BASE/$SLUG" add unmerged.txt + git -C "$WORKTREE_BASE/$SLUG" commit -q -m "unmerged work" + + run_task_done "$CLAUDE_GH_TASK_DONE" --force + assert_success + assert_output --partial "still has unmerged commits" + refute_output --partial "Deleted reviewer branch" + run git -C "$MAIN_REPO" branch --list "task/$SLUG" + assert_output --partial "task/$SLUG" +} + @test "task-done cleanup tolerates radio binary missing from PATH (#94)" { # The `|| true` safety net: if radio isn't installed (or PATH doesn't # include it), cleanup must still succeed. From a1397f994aa9d2981e9e656abc9036a35965432b Mon Sep 17 00:00:00 2001 From: Martin conur Date: Tue, 2 Jun 2026 16:53:24 -0400 Subject: [PATCH 3/3] task-done --remove-worktree leaks reviewer branch + state; blocks re-dispatch of task-reviewer: zero PR_NUMBER before sourcing $INFO_FILE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #153 review blocker: without a pre-source default, an ambient shell export of PR_NUMBER (e.g., left over from an earlier `task-reviewer` session in the same shell) would survive into the new force-delete guard and trigger `git branch -D` on a worker branch with unmerged commits — silently destroying them. Mirror the existing `BASE_BRANCH="main"` default with `PR_NUMBER=` so a worker .info file (which contains no PR_NUMBER= line) always ends up with an empty value regardless of the ambient environment. Lands in the `worktree-context` region across the `task-done-std` and `task-done-local` drift groups (7 binaries, byte-identical). New bats coverage: `claude-gh` and `claude-local` ambient-export regressions export PR_NUMBER=42, run task-done from a worker worktree with an unmerged commit, and assert that: - the safe-delete fallback message ("still has unmerged commits") is emitted, - the reviewer-branch message is NOT emitted, - the branch object still exists at the unmerged commit sha. Cross-drift-group coverage (std + local) keeps a future worktree-context drift from quietly regressing one group. Refs #148 Co-Authored-By: Claude Opus 4.7 (1M context) --- claude-gh/bin/task-done | 5 ++++ claude-jira/bin/task-done | 5 ++++ claude-local/bin/task-done | 5 ++++ claude-notion/bin/task-done | 5 ++++ kiro-gh/bin/task-done | 5 ++++ kiro-local/bin/task-done | 5 ++++ kiro-notion/bin/task-done | 5 ++++ tests/task_done.bats | 50 +++++++++++++++++++++++++++++++++++++ 8 files changed, 85 insertions(+) diff --git a/claude-gh/bin/task-done b/claude-gh/bin/task-done index fb6ca9d..b44dfc6 100755 --- a/claude-gh/bin/task-done +++ b/claude-gh/bin/task-done @@ -50,6 +50,11 @@ INFO_FILE="${WORKTREE_BASE}/.${SLUG}.info" # Load base branch written by task-work; fall back to "main" BASE_BRANCH="main" +# Zero PR_NUMBER before sourcing $INFO_FILE so an ambient shell export +# (e.g., from an earlier task-reviewer in the same shell) can't sneak +# into the reviewer-branch force-delete guard below and -D a worker +# branch with unmerged commits (#148 review). +PR_NUMBER= # shellcheck source=/dev/null [[ -f "$INFO_FILE" ]] && source "$INFO_FILE" # endregion:worktree-context diff --git a/claude-jira/bin/task-done b/claude-jira/bin/task-done index d303634..17af7c1 100755 --- a/claude-jira/bin/task-done +++ b/claude-jira/bin/task-done @@ -50,6 +50,11 @@ INFO_FILE="${WORKTREE_BASE}/.${SLUG}.info" # Load base branch written by task-work; fall back to "main" BASE_BRANCH="main" +# Zero PR_NUMBER before sourcing $INFO_FILE so an ambient shell export +# (e.g., from an earlier task-reviewer in the same shell) can't sneak +# into the reviewer-branch force-delete guard below and -D a worker +# branch with unmerged commits (#148 review). +PR_NUMBER= # shellcheck source=/dev/null [[ -f "$INFO_FILE" ]] && source "$INFO_FILE" # endregion:worktree-context diff --git a/claude-local/bin/task-done b/claude-local/bin/task-done index 8e12549..a5a1b63 100755 --- a/claude-local/bin/task-done +++ b/claude-local/bin/task-done @@ -52,6 +52,11 @@ INFO_FILE="${WORKTREE_BASE}/.${SLUG}.info" # Load base branch written by task-work; fall back to "main" BASE_BRANCH="main" +# Zero PR_NUMBER before sourcing $INFO_FILE so an ambient shell export +# (e.g., from an earlier task-reviewer in the same shell) can't sneak +# into the reviewer-branch force-delete guard below and -D a worker +# branch with unmerged commits (#148 review). +PR_NUMBER= # shellcheck source=/dev/null [[ -f "$INFO_FILE" ]] && source "$INFO_FILE" # endregion:worktree-context diff --git a/claude-notion/bin/task-done b/claude-notion/bin/task-done index fb6ca9d..b44dfc6 100755 --- a/claude-notion/bin/task-done +++ b/claude-notion/bin/task-done @@ -50,6 +50,11 @@ INFO_FILE="${WORKTREE_BASE}/.${SLUG}.info" # Load base branch written by task-work; fall back to "main" BASE_BRANCH="main" +# Zero PR_NUMBER before sourcing $INFO_FILE so an ambient shell export +# (e.g., from an earlier task-reviewer in the same shell) can't sneak +# into the reviewer-branch force-delete guard below and -D a worker +# branch with unmerged commits (#148 review). +PR_NUMBER= # shellcheck source=/dev/null [[ -f "$INFO_FILE" ]] && source "$INFO_FILE" # endregion:worktree-context diff --git a/kiro-gh/bin/task-done b/kiro-gh/bin/task-done index fb6ca9d..b44dfc6 100755 --- a/kiro-gh/bin/task-done +++ b/kiro-gh/bin/task-done @@ -50,6 +50,11 @@ INFO_FILE="${WORKTREE_BASE}/.${SLUG}.info" # Load base branch written by task-work; fall back to "main" BASE_BRANCH="main" +# Zero PR_NUMBER before sourcing $INFO_FILE so an ambient shell export +# (e.g., from an earlier task-reviewer in the same shell) can't sneak +# into the reviewer-branch force-delete guard below and -D a worker +# branch with unmerged commits (#148 review). +PR_NUMBER= # shellcheck source=/dev/null [[ -f "$INFO_FILE" ]] && source "$INFO_FILE" # endregion:worktree-context diff --git a/kiro-local/bin/task-done b/kiro-local/bin/task-done index 8e12549..a5a1b63 100755 --- a/kiro-local/bin/task-done +++ b/kiro-local/bin/task-done @@ -52,6 +52,11 @@ INFO_FILE="${WORKTREE_BASE}/.${SLUG}.info" # Load base branch written by task-work; fall back to "main" BASE_BRANCH="main" +# Zero PR_NUMBER before sourcing $INFO_FILE so an ambient shell export +# (e.g., from an earlier task-reviewer in the same shell) can't sneak +# into the reviewer-branch force-delete guard below and -D a worker +# branch with unmerged commits (#148 review). +PR_NUMBER= # shellcheck source=/dev/null [[ -f "$INFO_FILE" ]] && source "$INFO_FILE" # endregion:worktree-context diff --git a/kiro-notion/bin/task-done b/kiro-notion/bin/task-done index fb6ca9d..b44dfc6 100755 --- a/kiro-notion/bin/task-done +++ b/kiro-notion/bin/task-done @@ -50,6 +50,11 @@ INFO_FILE="${WORKTREE_BASE}/.${SLUG}.info" # Load base branch written by task-work; fall back to "main" BASE_BRANCH="main" +# Zero PR_NUMBER before sourcing $INFO_FILE so an ambient shell export +# (e.g., from an earlier task-reviewer in the same shell) can't sneak +# into the reviewer-branch force-delete guard below and -D a worker +# branch with unmerged commits (#148 review). +PR_NUMBER= # shellcheck source=/dev/null [[ -f "$INFO_FILE" ]] && source "$INFO_FILE" # endregion:worktree-context diff --git a/tests/task_done.bats b/tests/task_done.bats index 12f0419..3cd1972 100644 --- a/tests/task_done.bats +++ b/tests/task_done.bats @@ -557,6 +557,56 @@ assert_reviewer_branch_force_deleted() { assert_output --partial "task/$SLUG" } +# Ambient-export regression — without `PR_NUMBER=` zero-init before sourcing +# the worker's $INFO_FILE, an exported PR_NUMBER from the surrounding shell +# (e.g., left over from a prior task-reviewer session) leaks into the +# force-delete guard and triggers `git branch -D` on a worker branch with +# unmerged commits — silently destroying them. The fix initializes +# `PR_NUMBER=` right before the source call (mirroring `BASE_BRANCH="main"`), +# so a worker .info file that contains no PR_NUMBER= line ends up with +# PR_NUMBER empty regardless of ambient environment. +assert_ambient_pr_number_does_not_force_delete() { + local script="$1" + + touch "$WORKTREE_BASE/$SLUG/unmerged.txt" + git -C "$WORKTREE_BASE/$SLUG" add unmerged.txt + git -C "$WORKTREE_BASE/$SLUG" commit -q -m "unmerged worker commit (must survive)" + + # Capture commit sha so we can confirm the branch object still exists after + # cleanup (i.e., -d refused to drop it). + local sha + sha=$(git -C "$WORKTREE_BASE/$SLUG" rev-parse HEAD) + + # Ambient export — simulates user having run `task-reviewer 42` earlier in + # the same shell with the env still set when they cd'd into the worker. + export PR_NUMBER=42 + + run bash -c "echo y | $script --force" + + unset PR_NUMBER + + assert_success + assert_output --partial "still has unmerged commits" + refute_output --partial "Deleted reviewer branch" + refute_output --partial "(-D, scaffold-only)" + + # Branch must still exist and still point at the unmerged commit. + run git -C "$MAIN_REPO" branch --list "task/$SLUG" + assert_output --partial "task/$SLUG" + run git -C "$MAIN_REPO" rev-parse "task/$SLUG" + assert_output "$sha" +} + +@test "claude-gh: ambient PR_NUMBER export does NOT trigger force-delete on worker" { + assert_ambient_pr_number_does_not_force_delete "$CLAUDE_GH_TASK_DONE" +} + +@test "claude-local: ambient PR_NUMBER export does NOT trigger force-delete on worker" { + # Cross-drift-group coverage — the zero-init lives in worktree-context which + # task-done-std and task-done-local check separately. + assert_ambient_pr_number_does_not_force_delete "$CLAUDE_LOCAL_TASK_DONE" +} + @test "task-done cleanup tolerates radio binary missing from PATH (#94)" { # The `|| true` safety net: if radio isn't installed (or PATH doesn't # include it), cleanup must still succeed.