diff --git a/review/SKILL.md.tmpl b/review/SKILL.md.tmpl index 330ae5c04..afbdcc229 100644 --- a/review/SKILL.md.tmpl +++ b/review/SKILL.md.tmpl @@ -42,16 +42,7 @@ Review the branch diff against the base for structural issues tests miss. 1. Run `git branch --show-current` to get the current branch. 2. If on the base branch, output: **"Nothing to review — you're on the base branch or have no changes against it."** and stop. -3. Refresh the base, then check for a diff: - -```bash -git fetch origin --quiet && echo "BASE_REFRESH: fresh" || echo "BASE_REFRESH: stale (fetch failed; local origin/ at $(git rev-parse --short origin/))" -DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" --stat -``` - - If no diff, output the same message and stop. A failed fetch (for example a - read-only `.git` in a sandbox) is not an empty diff: continue, and state - `Base coverage: stale — reviewed against origin/ at ` in the review output. +3. Run `git fetch origin --quiet && echo "BASE_REFRESH: fresh" || echo "BASE_REFRESH: stale $(git rev-parse --short origin/)"`, then `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" --stat`. If no diff, output the same message and stop. `stale` is not an empty diff: continue; report `Base coverage: stale at `. --- @@ -86,7 +77,7 @@ Each pass has one direction: collect findings in Steps 3–4.8, approve and appl fixes in Step 5, then choose repeat or final persistence in Step 5.8. Do not edit reviewed source until Step 5. All readers examine the same candidate. -Step 1 already refreshed the base; keep its `BASE_REFRESH` result. Compute the merge base, then diff the working tree against that point: +Diff the working tree against the merge base: ```bash DIFF_BASE=$(git merge-base origin/ HEAD) diff --git a/test/review-base-refresh.test.ts b/test/review-base-refresh.test.ts index 36eba54d4..c79836aee 100644 --- a/test/review-base-refresh.test.ts +++ b/test/review-base-refresh.test.ts @@ -14,13 +14,13 @@ function git(cwd: string, ...args: string[]) { return r.stdout.trim(); } -/** Step 1's base-refresh block from the /review template, with `` bound. */ +/** Step 1 item 3's commands from the /review template, run in order, with `` bound. */ function stepOneBlock(base: string): string { const body = fs.readFileSync(path.join(ROOT, 'review/SKILL.md.tmpl'), 'utf8'); - const step = body.slice(body.indexOf('## Step 1: Check branch'), body.indexOf('{{SCOPE_DRIFT}}')); - const block = /```bash\n([\s\S]*?)\n```/.exec(step); - if (!block) throw new Error('Step 1 has no bash block'); - return block[1]!.replaceAll('', base); + const item = body.slice(body.indexOf('3. Run `git fetch origin '), body.indexOf('{{SCOPE_DRIFT}}')); + const commands = [...item.matchAll(/`(git fetch [^`]+|DIFF_BASE=[^`]+)`/g)].map(m => m[1]!); + if (commands.length !== 2) throw new Error('Step 1 item 3 commands moved'); + return commands.join('\n').replaceAll('', base); } function clone() { @@ -50,7 +50,7 @@ describe('/review Step 1 base refresh', () => { git(repo, 'remote', 'set-url', 'origin', path.join(repo, 'no-such-remote')); const result = run(repo, stepOneBlock('main')); const rev = git(repo, 'rev-parse', '--short', 'origin/main'); - expect(result.stdout).toContain(`BASE_REFRESH: stale (fetch failed; local origin/main at ${rev})`); + expect(result.stdout).toContain(`BASE_REFRESH: stale ${rev}`); expect(result.stdout).toMatch(/app\.ts \| 2 \+-/); }); @@ -63,7 +63,7 @@ describe('/review Step 1 base refresh', () => { test('the review reports stale coverage instead of stopping', () => { const body = fs.readFileSync(path.join(ROOT, 'review/SKILL.md.tmpl'), 'utf8').replace(/\s+/g, ' '); - expect(body).toContain('A failed fetch (for example a read-only `.git` in a sandbox) is not an empty diff: continue, and state `Base coverage: stale — reviewed against origin/ at ` in the review output.'); + expect(body).toContain('`stale` is not an empty diff: continue; report `Base coverage: stale at `.'); expect(body).not.toContain('git fetch origin --quiet && DIFF_BASE='); }); });