diff --git a/.changeset/calm-birds-greet.md b/.changeset/calm-birds-greet.md new file mode 100644 index 000000000..3c4764665 --- /dev/null +++ b/.changeset/calm-birds-greet.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2990 +--- +gsd-code-fixer worktree no longer fails on the same-branch checkout — the agent now creates a new gsd-reviewfix/ branch via git worktree add -b and fast-forwards the user's branch on cleanup. See #2990. diff --git a/.changeset/silly-foxes-wander.md b/.changeset/silly-foxes-wander.md new file mode 100644 index 000000000..3c4764665 --- /dev/null +++ b/.changeset/silly-foxes-wander.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2990 +--- +gsd-code-fixer worktree no longer fails on the same-branch checkout — the agent now creates a new gsd-reviewfix/ branch via git worktree add -b and fast-forwards the user's branch on cleanup. See #2990. diff --git a/agents/gsd-code-fixer.md b/agents/gsd-code-fixer.md index f6dfa4075..f184a1330 100644 --- a/agents/gsd-code-fixer.md +++ b/agents/gsd-code-fixer.md @@ -231,39 +231,63 @@ test -n "$branch" || { echo "Detached HEAD is not supported for review-fix (#268 sentinel="${phase_dir}/.review-fix-recovery-pending.json" if [ -f "$sentinel" ]; then echo "Detected pre-existing recovery sentinel from a prior interrupted run: $sentinel" - prior_wt=$(node -e ' + # Recovery must extract BOTH worktree_path AND reviewfix_branch (#3001 CR): + # if a prior run died after `git worktree remove` but before + # `git branch -D`, the orphan branch survives and clutters `git branch` + # output forever. Emit both fields newline-separated so we can read them + # independently. + prior_recovery=$(node -e ' const fs = require("fs"); try { const parsed = JSON.parse(fs.readFileSync(process.argv[1], "utf-8")); - process.stdout.write(parsed.worktree_path || ""); + process.stdout.write((parsed.worktree_path || "") + "\n" + (parsed.reviewfix_branch || "")); } catch (err) { process.stderr.write(`Warning: malformed recovery sentinel ${process.argv[1]}: ${err.message}\n`); - process.stdout.write(""); + process.stdout.write("\n"); } ' "$sentinel") + prior_wt="$(printf '%s' "$prior_recovery" | sed -n '1p')" + prior_branch="$(printf '%s' "$prior_recovery" | sed -n '2p')" if [ -n "$prior_wt" ] && git worktree list --porcelain | grep -q "^worktree $prior_wt$"; then echo "Removing orphan worktree from prior run: $prior_wt" git worktree remove "$prior_wt" --force || true fi + if [ -n "$prior_branch" ]; then + # Best-effort: branch may already be gone (cleaned by an earlier + # partial recovery, or never created if `git worktree add -b` itself + # failed). `|| true` keeps recovery non-fatal. + echo "Removing orphan reviewfix branch from prior run: $prior_branch" + git branch -D "$prior_branch" 2>/dev/null || true + fi rm -f "$sentinel" fi wt=$(mktemp -d "/tmp/sv-${padded_phase}-reviewfix-XXXXXX") -git worktree add "$wt" "$branch" + +# Create a temp branch from the current branch tip so the worktree +# attaches to that NEW branch rather than the user's currently-checked-out +# branch (#2990: git refuses to check out the same branch in two +# worktrees by default; the original `git worktree add "$wt" "$branch"` +# failed before the agent could do any work). The temp branch shares +# history with $branch up to the moment of creation, so commits made +# inside the worktree fast-forward $branch on cleanup. +reviewfix_branch="gsd-reviewfix/${padded_phase}-$$" +git worktree add -b "$reviewfix_branch" "$wt" "$branch" # Write the recovery sentinel ONLY AFTER `git worktree add` succeeds. # Writing it before would leave a sentinel pointing at a worktree that does # not exist if `git worktree add` itself failed. node -e ' const fs = require("fs"); - const [sentinelPath, worktree_path, branch, padded_phase] = process.argv.slice(1); + const [sentinelPath, worktree_path, branch, reviewfix_branch, padded_phase] = process.argv.slice(1); fs.writeFileSync(sentinelPath, JSON.stringify({ worktree_path, branch, + reviewfix_branch, padded_phase, started_at: new Date().toISOString() }, null, 2)); -' "$sentinel" "$wt" "$branch" "$padded_phase" +' "$sentinel" "$wt" "$branch" "$reviewfix_branch" "$padded_phase" cd "$wt" ``` @@ -271,32 +295,64 @@ cd "$wt" Concrete steps: 1. Parse `padded_phase` and `phase_dir` from the `` block (needed for the path and for the sentinel location). 2. Resolve the current branch: `branch=$(git branch --show-current)`. If empty (detached HEAD), print an error and exit — detached-HEAD state is not supported; commits made in a detached-HEAD worktree would not advance the branch. -3. **Recovery check (#2839):** If `${phase_dir}/.review-fix-recovery-pending.json` already exists, a prior run was interrupted. Parse the JSON, attempt to remove the orphan worktree it points at (best-effort, with `--force`), then delete the stale sentinel before continuing. This makes a re-run of `/gsd-code-review-fix` self-healing. +3. **Recovery check (#2839, #2990):** If `${phase_dir}/.review-fix-recovery-pending.json` already exists, a prior run was interrupted. Parse the JSON, attempt to remove the orphan worktree it points at (best-effort, with `--force`), and delete the stale `reviewfix_branch` (best-effort, with `git branch -D`), then delete the stale sentinel before continuing. This makes a re-run of `/gsd-code-review-fix` self-healing. 4. Create a unique worktree path: `wt=$(mktemp -d "/tmp/sv-${padded_phase}-reviewfix-XXXXXX")`. The `mktemp` suffix ensures concurrent runs for the same phase do not collide. -5. Run `git worktree add "$wt" "$branch"` — this attaches the worktree to the current branch so commits advance it. -6. **Write the recovery sentinel** at `${phase_dir}/.review-fix-recovery-pending.json` containing `{worktree_path, branch, padded_phase, started_at}`. Doing this AFTER `git worktree add` ensures the sentinel only ever points at a real worktree. -7. All subsequent file reads, edits, and commits happen inside `$wt`. +5. Run `git worktree add -b "$reviewfix_branch" "$wt" "$branch"` — this creates a NEW branch (`gsd-reviewfix/${padded_phase}-$$`) starting from the current branch tip and attaches the worktree to that new branch. Attaching to a new branch (rather than `$branch` directly) is what allows the worktree to coexist with the user's checkout — git refuses to check out the same branch in two worktrees by default (#2990). Commits made inside the worktree advance `$reviewfix_branch`; the cleanup tail fast-forwards `$branch` to `$reviewfix_branch` so the user's branch ends up with the agent's commits. +6. **Write the recovery sentinel** at `${phase_dir}/.review-fix-recovery-pending.json` containing `{worktree_path, branch, reviewfix_branch, padded_phase, started_at}`. Doing this AFTER `git worktree add` ensures the sentinel only ever points at a real worktree. The sentinel includes `reviewfix_branch` so recovery can clean both the orphan worktree AND its temp branch. +7. All subsequent file reads, edits, and commits happen inside `$wt` (which is on `$reviewfix_branch`, not `$branch`). -**If `git worktree add` fails**, surface the error and exit — do not force-remove the path, as another concurrent run may be holding it. Do not write the sentinel (the worktree does not exist). +**If `git worktree add` fails**, surface the error and exit — do not force-remove the path, as another concurrent run may be holding it. Do not write the sentinel (the worktree does not exist). Do not delete `$reviewfix_branch` either; if `-b` failed, no temp branch was created. -**Cleanup tail (transactional, ALWAYS — even on failure):** After writing REVIEW-FIX.md and before returning to the orchestrator, run the two-step cleanup in this exact order: +**Cleanup tail (transactional, ALWAYS — even on failure):** After writing REVIEW-FIX.md and before returning to the orchestrator, run the cleanup in this exact order: ```bash -# Step 1: drop the worktree FIRST. If this succeeds and the process is then -# killed, the next run finds a sentinel pointing at a worktree that no longer -# exists — the recovery branch handles this gracefully (best-effort remove + -# sentinel delete). If we reversed the order (sentinel removed first, then -# worktree remove), an interruption between the two steps would leave NO -# sentinel and an orphan worktree — exactly the bug from #2839. +# Step 1 (#2990): fast-forward $branch to capture the commits the agent +# made on $reviewfix_branch. Run from the main repo (not $wt) — the user's +# checkout owns $branch. --ff-only ensures we never silently drop or +# rewrite history if the user committed to $branch concurrently; on +# divergence, this fails loudly and the temp branch is left for the +# user to inspect/merge manually. We deliberately resolve the main repo +# path via `git worktree list --porcelain` rather than assuming $PWD, +# because the agent ran inside $wt. +# Strip the literal "worktree " prefix and print the rest of the line, then +# exit on the first match. This preserves paths that contain spaces +# (awk '$2' would truncate "/path/with spaces/repo" to "/path/with"). +main_repo="$(git worktree list --porcelain | awk '/^worktree / { sub(/^worktree /, ""); print; exit }')" +ff_status=0 +# Capture the exit code of `git merge` directly. `if ! cmd; then ff_status=$?` +# captures the exit code of the `!` operator (always 1 when the inner cmd +# failed) — masking the real merge exit code. Use the success/else split +# instead so $? in the else-branch is the merge command's exit code. +if git -C "$main_repo" merge --ff-only "$reviewfix_branch" 2>&1; then + ff_status=0 +else + ff_status=$? + echo "WARN: could not fast-forward $branch to $reviewfix_branch (exit $ff_status)." + echo " The temp branch $reviewfix_branch is preserved for manual merge." +fi + +# Step 2: drop the worktree. If this succeeds and the process is then +# killed, the next run finds a sentinel pointing at a worktree that no +# longer exists — the recovery branch handles this gracefully (best-effort +# remove + sentinel delete). If we reversed the order (sentinel removed +# first, then worktree remove), an interruption between the two steps +# would leave NO sentinel and an orphan worktree — exactly the bug from +# #2839. git worktree remove "$wt" --force -# Step 2: drop the recovery sentinel ONLY after `git worktree remove` returns -# successfully. This atomic-ish ordering is what makes the cleanup tail -# transactional from the orchestrator's perspective. +# Step 3: delete the temp branch ONLY if the fast-forward succeeded. If +# it didn't, leaving the branch lets the user inspect/merge manually. +if [ "$ff_status" -eq 0 ]; then + git -C "$main_repo" branch -D "$reviewfix_branch" || true +fi + +# Step 4: drop the recovery sentinel ONLY after `git worktree remove` +# returns successfully. This atomic-ish ordering is what makes the +# cleanup tail transactional from the orchestrator's perspective. rm -f "$sentinel" ``` -This cleanup is unconditional — register it mentally as a finally-block obligation. If the agent exits early (config error, no findings, etc.), still run the two-step cleanup tail (`git worktree remove "$wt" --force` followed by `rm -f "$sentinel"`) before exit. The sentinel must NEVER be removed before `git worktree remove` succeeds. +This cleanup is unconditional — register it mentally as a finally-block obligation. If the agent exits early (config error, no findings, etc.), still run the cleanup tail in order (fast-forward → worktree remove → temp branch delete → sentinel rm) before exit. The sentinel must NEVER be removed before `git worktree remove` succeeds. The temp branch must NEVER be deleted while the fast-forward is in a diverged state. @@ -528,9 +584,9 @@ _Iteration: {N}_ -**ALWAYS run inside the isolated worktree** — set up via `branch=$(git branch --show-current)` + `wt=$(mktemp -d "/tmp/sv-${padded_phase}-reviewfix-XXXXXX")` + `git worktree add "$wt" "$branch"` at the very start (see `setup_worktree` step). Using `mktemp` ensures concurrent runs do not collide. Attaching to `$branch` (not `HEAD`) ensures commits advance the branch. Every file read, edit, and commit must happen inside `$wt`. Run `git worktree remove "$wt" --force` unconditionally when done (treat it as a finally block). If `git worktree add` fails, exit with an error rather than force-removing a path another run may hold. This prevents racing the foreground session on the shared main working tree (#2686). +**ALWAYS run inside the isolated worktree** — set up via `branch=$(git branch --show-current)` + `wt=$(mktemp -d "/tmp/sv-${padded_phase}-reviewfix-XXXXXX")` + `git worktree add -b "$reviewfix_branch" "$wt" "$branch"` at the very start (see `setup_worktree` step). Using `mktemp` ensures concurrent runs do not collide. Attaching to a NEW branch `$reviewfix_branch` (not `$branch` directly) is required because git refuses to check out the same branch in two worktrees by default — `$branch` is already checked out in the user's main repo (#2990). Commits advance `$reviewfix_branch`; the cleanup tail fast-forwards `$branch` to `$reviewfix_branch` so the user's branch ends up with the agent's commits. Every file read, edit, and commit must happen inside `$wt`. Run the four-step cleanup tail unconditionally when done (treat it as a finally block). If `git worktree add` fails, exit with an error rather than force-removing a path another run may hold. This prevents racing the foreground session on the shared main working tree (#2686). -**ALWAYS run the transactional cleanup tail in order** (#2839): `git worktree remove "$wt" --force` MUST happen BEFORE `rm -f "$sentinel"` (the recovery sentinel at `${phase_dir}/.review-fix-recovery-pending.json`). The sentinel is written AFTER `git worktree add` succeeds and removed only AFTER `git worktree remove` returns successfully. This ordering is what makes the cleanup tail transactional — an interruption between commits and `git worktree remove` leaves the sentinel behind so a future run, `/gsd-resume-work`, or `/gsd-progress` can detect and complete the recovery. Reversing the order recreates the orphan-worktree bug. +**ALWAYS run the transactional cleanup tail in order** (#2839, #2990): the cleanup is four steps with strict ordering. (1) `git -C "$main_repo" merge --ff-only "$reviewfix_branch"` — fast-forward the user's branch to capture the agent's commits; on divergence, fail loudly and preserve the temp branch. (2) `git worktree remove "$wt" --force`. (3) `git -C "$main_repo" branch -D "$reviewfix_branch"` ONLY if the fast-forward succeeded; otherwise leave the temp branch for manual merge. (4) `rm -f "$sentinel"` (the recovery sentinel at `${phase_dir}/.review-fix-recovery-pending.json`). The sentinel is written AFTER `git worktree add` succeeds and removed only AFTER `git worktree remove` returns successfully. The temp branch is deleted only when the fast-forward succeeded. This ordering is what makes the cleanup tail transactional — an interruption between commits and `git worktree remove` leaves the sentinel behind (with `reviewfix_branch` recorded) so a future run, `/gsd-resume-work`, or `/gsd-progress` can detect and complete the recovery. Reversing the order recreates the orphan-worktree bug. **ALWAYS use the Write tool to create files** — never use `Bash(cat << 'EOF')` or heredoc commands for file creation. diff --git a/tests/bug-2990-code-fixer-worktree-branch.test.cjs b/tests/bug-2990-code-fixer-worktree-branch.test.cjs new file mode 100644 index 000000000..6df439442 --- /dev/null +++ b/tests/bug-2990-code-fixer-worktree-branch.test.cjs @@ -0,0 +1,208 @@ +'use strict'; + +// allow-test-rule: source-text-is-the-product +// agents/gsd-code-fixer.md is the deployed agent definition the runtime +// loads. Parsing its bash code blocks into structured invocation records +// (extractCleanupGitInvocations + the recovery-block parsers below) IS +// testing the runtime contract — what command sequence the agent +// actually documents and executes. The .match() calls extract typed +// fields from a known-shape product file, then assertions go against +// those typed fields, not against the raw markdown text. + +process.env.GSD_TEST_MODE = '1'; + +/** + * Bug #2990: gsd-code-fixer worktree setup fails when current branch + * is already checked out in the main repo. + * + * The original agent definition called `git worktree add "$wt" "$branch"`, + * where `$branch` was the user's currently-checked-out branch. Git refuses + * to check out the same branch in two worktrees by default, so the setup + * failed before the agent could do any work. + * + * Fix: create a NEW branch `gsd-reviewfix/${padded_phase}-$$` and attach + * the worktree to it via `git worktree add -b "$reviewfix_branch" "$wt" + * "$branch"`. The cleanup tail then fast-forwards `$branch` to + * `$reviewfix_branch` so the user's branch captures the agent's commits. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const AGENT_PATH = path.join(__dirname, '..', 'agents', 'gsd-code-fixer.md'); + +function parseWorktreeAddInvocations(markdown) { + // Pull `git worktree add ...` calls and classify each into structured + // records: hasNewBranchFlag (uses -b $reviewfix_branch) vs attachesToBareBranch + // ($wt $branch). Skip occurrences inside markdown inline code (backticks) + // or bash comments -- those are documentation citations of the OLD broken + // pattern, not executable instructions. + const invocations = []; + const lines = markdown.split('\n'); + for (const line of lines) { + const idx = line.indexOf('git worktree add'); + if (idx === -1) continue; + // Skip if inside backticks: the substring up to the match has an odd + // number of backticks, the call is inside an inline code span. + const before = line.slice(0, idx); + const backticksBefore = (before.match(/`/g) || []).length; + if (backticksBefore % 2 === 1) continue; + // Skip if the line is a bash comment (after stripping leading whitespace). + if (line.trimStart().startsWith('#')) continue; + const argstr = line.slice(idx + 'git worktree add'.length).trim(); + invocations.push({ + raw: argstr, + hasNewBranchFlag: /(?:^|\s)-b\s+["']?\$reviewfix_branch["']?/.test(argstr), + attachesToBareBranch: /^["']?\$wt["']?\s+["']?\$branch["']?\b/.test(argstr), + }); + } + return invocations; +} + +describe('Bug #2990: gsd-code-fixer worktree attaches to a NEW branch, not the user-checked-out one', () => { + const md = fs.readFileSync(AGENT_PATH, 'utf-8'); + const invocations = parseWorktreeAddInvocations(md); + + test('sanity: at least one git-worktree-add invocation exists in the agent definition', () => { + assert.ok(invocations.length > 0, + 'expected gsd-code-fixer.md to document at least one git worktree add invocation'); + }); + + test('every git-worktree-add invocation uses -b $reviewfix_branch (not bare $branch)', () => { + const violations = invocations.filter(inv => inv.attachesToBareBranch); + assert.deepEqual( + violations.map(v => v.raw), + [], + `worktree-add invocations attaching to bare $branch (#2990): ${JSON.stringify(violations.map(v => v.raw), null, 2)}`, + ); + }); + + test('the canonical setup invocation uses -b "$reviewfix_branch" "$wt" "$branch"', () => { + const setupInvocations = invocations.filter(inv => inv.hasNewBranchFlag); + assert.ok(setupInvocations.length >= 1, + `expected at least one git-worktree-add invocation with -b "$reviewfix_branch" -- found: ${JSON.stringify(invocations.map(i => i.raw), null, 2)}`); + }); +}); + +/** + * Extract the cleanup-tail bash block from the agent .md, then parse it into + * an ordered array of `git ...` invocation records. Per-record assertions go + * against the structured records, not the raw markdown text. Anchor on the + * "Cleanup tail" header to scope to the right block (the file has multiple + * fenced bash blocks; we only want the cleanup one). + */ +function extractCleanupGitInvocations(markdown) { + // Find the cleanup tail header and the fenced bash block that follows. + const headerIdx = markdown.indexOf('**Cleanup tail (transactional'); + if (headerIdx === -1) return null; + const fenceStart = markdown.indexOf('```bash', headerIdx); + if (fenceStart === -1) return null; + const fenceEnd = markdown.indexOf('```', fenceStart + '```bash'.length); + if (fenceEnd === -1) return null; + const block = markdown.slice(fenceStart + '```bash'.length, fenceEnd); + + // Tokenize each non-comment, non-blank line into structured records. + const lines = block.split('\n').map(l => l.trim()).filter(l => l && !l.startsWith('#')); + const records = []; + for (const line of lines) { + // Skip occurrences inside backticks (these would be inline-code + // citations of the OLD pattern, not executable). The cleanup fenced + // block is bash, but inline backticks can still appear inside echo + // strings — guard anyway. + const ticksBefore = (line.match(/`/g) || []).length; + if (ticksBefore && ticksBefore % 2 === 1) continue; + if (!line.includes('git ') && !line.startsWith('git ')) continue; + records.push({ + raw: line, + // Strip leading `git -C "..."`/`git -C $main_repo` so the verb-only + // form stays comparable across direct and -C invocations. + verb: (() => { + const m = line.match(/^git\s+(?:-C\s+\S+\s+)?(\S+)/); + return m ? m[1] : null; + })(), + // Did this line target the temp reviewfix branch by variable name? + targetsReviewfixBranch: /\$reviewfix_branch\b/.test(line) || /"\$reviewfix_branch"/.test(line), + // Is this the merge step? Captures the flag too. + isMergeFfOnly: /\bmerge\s+--ff-only\b/.test(line), + // Is this the branch-delete step? + isBranchDelete: /\bbranch\s+-D\b/.test(line), + }); + } + return records; +} + +describe('Bug #2990: cleanup tail fast-forwards $branch and deletes the temp branch on success', () => { + const md = fs.readFileSync(AGENT_PATH, 'utf-8'); + const records = extractCleanupGitInvocations(md); + + test('cleanup tail bash block exists and is parseable', () => { + assert.notEqual(records, null, 'expected to find a "Cleanup tail" bash block in agents/gsd-code-fixer.md'); + assert.ok(records.length > 0, 'expected at least one git invocation in the cleanup tail'); + }); + + test('cleanup contains exactly one merge --ff-only against $reviewfix_branch', () => { + const merges = records.filter(r => r.isMergeFfOnly); + assert.equal(merges.length, 1, `expected exactly 1 ff-only merge, got ${merges.length}: ${JSON.stringify(merges, null, 2)}`); + assert.equal(merges[0].targetsReviewfixBranch, true, 'merge --ff-only must target $reviewfix_branch'); + }); + + test('cleanup contains exactly one git branch -D for $reviewfix_branch', () => { + const deletes = records.filter(r => r.isBranchDelete); + assert.equal(deletes.length, 1, `expected exactly 1 branch -D, got ${deletes.length}`); + assert.equal(deletes[0].targetsReviewfixBranch, true, 'branch -D must target $reviewfix_branch'); + }); + + test('merge --ff-only precedes branch -D in the cleanup ordering', () => { + const mergeIdx = records.findIndex(r => r.isMergeFfOnly); + const deleteIdx = records.findIndex(r => r.isBranchDelete); + assert.ok(mergeIdx >= 0 && deleteIdx >= 0); + assert.ok(mergeIdx < deleteIdx, + `merge must run before branch delete (merge=${mergeIdx}, delete=${deleteIdx}); otherwise commits could be lost on merge failure`); + }); + + test('recovery sentinel JSON shape records reviewfix_branch alongside worktree_path', () => { + // Find the writeFileSync call that constructs the sentinel JSON. + // Parse the JSON.stringify argument list to extract the field names. + const match = md.match(/fs\.writeFileSync\(sentinelPath,\s*JSON\.stringify\(\{([^}]+)\}/); + assert.notEqual(match, null, 'expected JSON.stringify({...}) inside the sentinel write'); + const fields = match[1].split(',').map(s => s.trim().split(':')[0].trim()).filter(Boolean); + assert.ok(fields.includes('reviewfix_branch'), + `recovery sentinel must record reviewfix_branch alongside worktree_path; fields=${JSON.stringify(fields)}`); + assert.ok(fields.includes('worktree_path'), + `recovery sentinel must record worktree_path; fields=${JSON.stringify(fields)}`); + }); +}); + +describe('Bug #2990 (#3001 CR): recovery code reads reviewfix_branch from sentinel and deletes the orphan branch', () => { + const md = fs.readFileSync(AGENT_PATH, 'utf-8'); + + test('recovery node script extracts reviewfix_branch from parsed sentinel', () => { + // Find the recovery `node -e '...'` block (NOT the sentinel-write one). + // Anchor on "recovery sentinel from a prior interrupted run". + const headerIdx = md.indexOf('Detected pre-existing recovery sentinel'); + assert.notEqual(headerIdx, -1); + const nodeStart = md.indexOf("node -e '", headerIdx); + assert.notEqual(nodeStart, -1); + const nodeEnd = md.indexOf("' \"$sentinel\"", nodeStart); + assert.notEqual(nodeEnd, -1); + const nodeBlock = md.slice(nodeStart, nodeEnd); + // Both fields must be referenced by parsed.. + assert.ok(nodeBlock.includes('parsed.reviewfix_branch'), + 'recovery node script must extract parsed.reviewfix_branch from the sentinel'); + assert.ok(nodeBlock.includes('parsed.worktree_path'), + 'recovery node script must extract parsed.worktree_path from the sentinel'); + }); + + test('recovery shell deletes the orphan reviewfix branch when present', () => { + // The recovery block (between sentinel detection and `rm -f "$sentinel"`) + // must call `git branch -D "$prior_branch"` (best-effort, with || true). + const sentinelIdx = md.indexOf('Detected pre-existing recovery sentinel'); + const rmIdx = md.indexOf('rm -f "$sentinel"', sentinelIdx); + assert.notEqual(rmIdx, -1); + const recoveryBlock = md.slice(sentinelIdx, rmIdx); + assert.ok(/git\s+branch\s+-D\s+"\$prior_branch"/.test(recoveryBlock), + `recovery block must contain \`git branch -D "$prior_branch"\`; got: ${recoveryBlock.slice(0, 500)}`); + }); +});