review-plan: a granted fix round after the targeted read plans one targeted-only reader (GST-1607) (#4064)

* review-plan: a granted fix round after the targeted read plans one targeted-only reader (GST-1607)

After the owner granted a fix round past the loop cap, the granted round's
fix moved the product patch the targeted record names, so the receipt asked
for a fresh targeted read while the plan printed done (harness-evals #271).
With that --grant (checked against the approval record as before), any
change since the read except plain docs now plans NEXT: targeted on the
GST-1598 route: one targeted reader on HEAD for the head classes. A head
with no targeted class adds no reader; no grant keeps done.

The Capy pass program now runs a targeted manifest's voices stage, so a
NEXT: targeted pass driven through it creates its reader (it activated
voices only for fix-check).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: changelog fragment for GST-1607

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: review-policy names the granted case in the targeted-only skip list (docs audit)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-review-plan

@@ -73,7 +73,8 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do
                             reader last read it: the same patch skips it; any other id, none, or uncommitted edits dispatch it again.
                             With --range-base naming the commit that read happened on, a loop that is done plans NEXT: targeted
                             (one targeted reader on HEAD, TARGETED_DELTA: names the range) when the only change since is docs the
-                            receipt cannot carry the record across (a money, security or numeric claim; GST-1598)
+                            receipt cannot carry the record across (a money, security or numeric claim; GST-1598), or, with a
+                            --grant this invocation checks, any change but plain docs (the granted fix round; GST-1607)
   --grant <approval id>     the owner's grant of one scoped fix-check after round 2, recorded with gstack-approvals add (approved,
                             its ask or scope naming review_loop_grant and the stop head's sha, at or after --range-base and at or
                             before HEAD; needs --range-base): the stop after a round-2 full pass with a fixed blocking finding or a
@@ -234,9 +235,10 @@ function main(): number {
   });
   if (postReview && pass > 0) plan.next.reason = `edits after the review bound (Step 16 stage 2.1) plan as a fix to a gate-class path, never a note: ${plan.next.reason}`;
   const ranArg = flag('--targeted-ran');
-  const delta = plan.next.step === 'done' && rangeBase && ranArg && targeted.classes.length && !ran ? targetedDelta(root, ref, rangeBase, ranArg) : null;
+  const delta = plan.next.step === 'done' && rangeBase && ranArg && targeted.classes.length && !ran ? targetedDelta(root, ref, rangeBase, ranArg, !!grant) : null;
   if (delta?.state === 'dispatch') {
-    plan.next = { step: 'targeted', reason: `the loop is done and the only change since the targeted read at ${delta.range_base.slice(0, 12)} is docs whose claim the record cannot carry (${delta.paths.join(', ')}): one targeted reader reads HEAD for ${classList(targeted.classes)}; it is not a review round` };
+    const why = grant ? `the owner's grant ${grant} covers the fix round that changed ${delta.paths.join(', ')}` : `the only change since is docs whose claim the record cannot carry (${delta.paths.join(', ')})`;
+    plan.next = { step: 'targeted', reason: `the loop is done and ${why} after the targeted read at ${delta.range_base.slice(0, 12)}: one targeted reader reads HEAD for ${classList(targeted.classes)}; it is not a review round` };
     plan.dispatches.push({ role: 'targeted', name: 'targeted', effort: ROLE_EFFORT.targeted, exclude: active ? [...active.specs] : [], classes: [...targeted.classes], ...(active?.keep.length ? { keep: [...active.keep] } : {}) });
   } else if (delta) plan.skipped.push(`targeted-only read (${delta.reason})`);
   const targetedDeltaOut = delta?.state === 'dispatch' ? { range_base: delta.range_base, paths: delta.paths, classes: targeted.classes } : null;

changelog.d/gst-1607-granted-targeted.md

@@ -0,0 +1 @@
+- **A granted fix round no longer needs a second grant to refresh the targeted read.** When the owner grants a fix round past the review-loop cap, that fix changes the product patch the targeted record names, so the receipt asks for a new targeted read. The plan used to print `done` once the granted fix-check came back clean, which left the lane needing another manual grant. `gstack-review-plan` now takes the same `--grant` (checked as before) and plans `NEXT: targeted`: one targeted reader on the head for its targeted classes. Without a grant, a code change still plans `done`. The Capy pass program now runs a targeted manifest's voices stage too, so that reader actually runs there. A granted fix that edits a gate-class path still stops for the owner. (GST-1607)

docs/review-policy.md

@@ -540,13 +540,24 @@ among its plan lines. After `verify --targeted-out`, pass the new
 `targeted_patch_id` and the plan is `done`.
 
 Every other case stays `done` and prints `SKIP: targeted-only read
-(<reason>)`. That covers a code or binary change, which takes the review loop
-as before; plain docs, which the record carries; a range_base the
+(<reason>)`. That covers a code or binary change with no `--grant` (the granted
+case is below), which takes the review loop as before; plain docs, which the record carries; a range_base the
 `--targeted-ran` patch doesn't match; and uncommitted edits. `--force
 targeted` refuses (exit 2) and names these flags, because the targeted reader
 is no specialist (`lib/review-targeted-delta.ts`,
 `test/review-plan-targeted-delta.test.ts`).
 
+A fix round the owner granted past the cap has the same problem in code
+(harness-evals #271, GST-1607). The granted round's fix moves the product
+patch the targeted record names, so the receipt asks for a fresh read once
+the loop is done. With that grant passed as `--grant` (checked as above:
+an approved `review_loop_grant` row naming a stop head inside
+range_base..HEAD), any change since the read except plain docs plans the
+same `NEXT: targeted`, and the reason names the grant. A head with no
+targeted class adds no reader, and the same change with no grant keeps
+`done` (`test/review-plan-granted-targeted.test.ts`). The Capy pass program
+runs a targeted manifest's voices stage, so this one reader runs there too.
+
 <a id="review-exclude-paths"></a>
 ## What readers read under `review_exclude_paths`
 

lib/capy-review-pass.js

@@ -146,7 +146,7 @@ const beforeKey = input.stage === 'voices' ? 'before_voices' : 'before';
 
 try {
   let m = await load();
-  if (input.stage !== 'voices' && m.next === 'fix-check') active.push('voices');
+  if (input.stage !== 'voices' && (m.next === 'fix-check' || m.next === 'targeted')) active.push('voices');
   if (active.includes('voices')) {
     const v = await sh('cat ' + q(voiceFile) + ' 2>/dev/null || true');
     if (v.out.trim()) voices = JSON.parse(v.out);

lib/review-targeted-delta.ts

@@ -7,7 +7,9 @@
  * asks this module when the loop is done and the head has targeted classes the passed --targeted-ran no longer covers.
  * It dispatches one targeted reader on HEAD only when range_base is the commit that read happened on (its targeted
  * patch-id is --targeted-ran), every path whose branch patch changed since is docs class and not binary, and the change
- * is not plain docs. Anything else keeps `done` with the reason.
+ * is not plain docs. With the owner's review_loop_grant for a stop head inside range_base..HEAD (`granted`, checked by the
+ * plan before it gets here), a code change since the read takes the same one reader (GST-1607, harness-evals #271: the
+ * granted fix round moved the product patch the record names). Anything else keeps `done` with the reason.
  */
 import { spawnSync } from 'node:child_process';
 import { classifyPath } from './review-depth';
@@ -17,7 +19,7 @@ import { plainDocsSince } from './ship-receipt-depth';
 export type TargetedDelta = { state: 'dispatch'; range_base: string; paths: string[] } | { state: 'skip'; reason: string };
 
 /** Whether the change since `rangeBase` (where the targeted reader read the patch `ran`) earns the targeted-only read. */
-export function targetedDelta(root: string, base: string, rangeBase: string, ran: string): TargetedDelta {
+export function targetedDelta(root: string, base: string, rangeBase: string, ran: string, granted = false): TargetedDelta {
   const git = (args: string[], input?: string) => {
     const r = spawnSync('git', ['-c', 'core.quotePath=false', ...args], { cwd: root, input, encoding: 'utf8', timeout: 60_000, maxBuffer: 256 * 1024 * 1024 });
     return r.status === 0 ? r.stdout : null;
@@ -41,7 +43,7 @@ export function targetedDelta(root: string, base: string, rangeBase: string, ran
   if (!paths.length) return skip(`the branch patch is unchanged since range_base ${read.slice(0, 12)}`);
   const binary = new Set((git(['diff', '--numstat', '-z', '--no-renames', headBase, head]) ?? '').split('\0').filter(l => l.startsWith('-\t-\t')).map(l => l.slice(4)));
   const other = paths.filter(p => classifyPath(p, root) !== 'docs' || binary.has(p));
-  if (other.length) return skip(`the change since range_base ${read.slice(0, 12)} is not docs-only (${other.join(', ')}), so it takes the review loop`);
+  if (other.length && !granted) return skip(`the change since range_base ${read.slice(0, 12)} is not docs-only (${other.join(', ')}), so it takes the review loop`);
   if (plainDocsSince(root, read, head, base)) return skip(`the change since range_base ${read.slice(0, 12)} is plain docs, which the targeted record carries (GST-1195)`);
   return { state: 'dispatch', range_base: read, paths };
 }

ship/sections/pr-body.md

@@ -227,7 +227,7 @@ NS=$(~/.claude/skills/gstack/bin/gstack-no-stop list | sed -n 's/^NO_STOP_FILE:
 ~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr <pr-number> --base <base> --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json $([ -s .gstack/tmp/targeted.json ] && echo --review-targeted @.gstack/tmp/targeted.json) ${NS:+--no-stop "@$NS"} --body "$PR_BODY_FILE"
 ```
 
-If write refuses `review_targeted` for a docs claim added after the read, rerun Step 9's plan with `--range-base <its head>
+If write refuses `review_targeted` for a docs claim added after the read, or after a granted fix round (add its `--grant`), rerun Step 9's plan with `--range-base <its head>
 --targeted-ran <its targeted_patch_id>`; on `NEXT: targeted`, run that one reader (`pass`, its voices stage,
 `verify --targeted-out .gstack/tmp/targeted.json`) and write again.
 A Step 5 red-base pause adds `--base-red <pause file>` (a conditional receipt the queue

ship/sections/pr-body.md.tmpl

@@ -210,7 +210,7 @@ NS=$(~/.claude/skills/gstack/bin/gstack-no-stop list | sed -n 's/^NO_STOP_FILE:
 ~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr <pr-number> --base <base> --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json $([ -s .gstack/tmp/targeted.json ] && echo --review-targeted @.gstack/tmp/targeted.json) ${NS:+--no-stop "@$NS"} --body "$PR_BODY_FILE"
 ```
 
-If write refuses `review_targeted` for a docs claim added after the read, rerun Step 9's plan with `--range-base <its head>
+If write refuses `review_targeted` for a docs claim added after the read, or after a granted fix round (add its `--grant`), rerun Step 9's plan with `--range-base <its head>
 --targeted-ran <its targeted_patch_id>`; on `NEXT: targeted`, run that one reader (`pass`, its voices stage,
 `verify --targeted-out .gstack/tmp/targeted.json`) and write again.
 A Step 5 red-base pause adds `--base-red <pause file>` (a conditional receipt the queue

test/review-pass.test.ts

@@ -239,6 +239,27 @@ describe('the targeted reader in a pass (GST-257 on main)', () => {
   }, 180_000);
 });
 
+describe('the targeted-only read in the Capy pass program (GST-1598, GST-1607)', () => {
+  // Value: protects=a NEXT: targeted manifest driven through the checked-in pass program runs its one targeted reader (voices stage)
+  //   and ends on NEXT: judge; fails_when=the program runs only first, specialists and red-team for a targeted manifest, so it creates
+  //   no reader (lib/capy-review-pass.js activated voices only for fix-check); seam=lib/capy-review-pass.js stage selection;
+  //   no_claim=what the reader finds
+  test('a targeted-only manifest creates the one targeted reader', async () => {
+    const { dir, env } = repo();
+    const sh = (c: string) => { const r = spawnSync('bash', ['-c', c], { cwd: dir, env, encoding: 'utf8', timeout: 60_000 }); if (r.status !== 0) throw new Error(r.stderr); return r.stdout; };
+    sh("printf 'export const spendCapUsd = 25;\\n' > src/billing.ts && git add -A && git commit -qm money");
+    const read = sh('git rev-parse HEAD').trim();
+    const plan = spawnSync(path.join(BIN, 'gstack-review-plan'), ['--base', 'main', '--gate-tagged', '0', '--json'], { cwd: dir, env, encoding: 'utf8', timeout: 60_000, maxBuffer: 64 * 1024 * 1024 });
+    const ran = JSON.parse(plan.stdout).targeted.targeted_patch_id as string;
+    sh("mkdir -p changelog.d && echo '- Each PR eval run now costs about $4.20.' > changelog.d/cost.md && git add -A && git commit -qm claim");
+    const h = host(dir, env);
+    const res = await runPass(h, { base: 'main', out: path.join(dir, '.state/run/p1'), pass: 'td1p1', planFlags: ['--gate-tagged', '0', '--pass', '1', '--last', 'fix-check', '--fixed-below', '--range-base', read, '--targeted-ran', ran] });
+    expect(res).toContain('NEXT: targeted');
+    expect(h.creates.map(c => c.title)).toEqual(['ship td1p1 targeted']);
+    expect(res).toContain('NEXT: judge');
+  }, 180_000);
+});
+
 describe('a large diff (GST-507)', () => {
   // Value: protects=a pass on a results PR with thousands of files reads gstack-review-plan's whole JSON plan through the pipe and stages its readers; fails_when=either bin exits before its stdout drains, so the plan JSON arrives cut short and pass dies on JSON.parse;
   //   why_new=HARN-1 hit it on a 4,000-file receipt tree (a 1.39 MB plan); seam=none; no_claim=what reviewers read on a large diff (GST-508)

test/review-plan-granted-targeted.test.ts

@@ -0,0 +1,99 @@
+import { afterEach, describe, expect, test } from 'bun:test';
+import { spawnSync } from 'node:child_process';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { parseReviewTargeted, plainDocsSince, targetedClassesSince } from '../lib/ship-receipt-depth';
+
+const BIN = path.join(import.meta.dir, '..', 'bin');
+const dirs: string[] = [];
+afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); });
+
+const money = (cap: number) => `export const spendCapUsd = ${cap};\nexport function charge(amountUsd: number) { if (amountUsd > spendCapUsd) throw new Error("over cap"); return amountUsd; }\n`;
+
+/**
+ * The harness-evals #271 shape: the targeted reader read R, the loop stopped at the cap on S (a fix after R), the owner
+ * granted one scoped fix-check naming S, and the granted round's fix commit moved HEAD past S.
+ */
+function granted(file: string, body: (n: number) => string) {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'granted-targeted-'));
+  const state = fs.mkdtempSync(path.join(os.tmpdir(), 'granted-targeted-state-'));
+  dirs.push(dir, state);
+  const env: NodeJS.ProcessEnv = { ...process.env, GSTACK_STATE_ROOT: state, GSTACK_APPROVALS_MIRROR: '0' };
+  delete env.GSTACK_APPROVALS_DIR;
+  const git = (...a: string[]) => spawnSync('git', a, { cwd: dir, encoding: 'utf8', timeout: 10_000 }).stdout.trim();
+  const commit = (files: Record<string, string>, msg: string) => {
+    for (const [p, text] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(dir, p)), { recursive: true }); fs.writeFileSync(path.join(dir, p), text); }
+    git('add', '.'); git('commit', '-qm', msg);
+    return git('rev-parse', 'HEAD');
+  };
+  git('init', '-q', '-b', 'main'); git('config', 'user.email', 't@t'); git('config', 'user.name', 't'); git('config', 'commit.gpgsign', 'false');
+  commit({ 'README.md': '# x\n' }, 'base');
+  git('checkout', '-qb', 'feat');
+  const read = commit({ [file]: body(25) }, 'feature');
+  const plan = (args: string[]) => spawnSync(path.join(BIN, 'gstack-review-plan'), ['--base', 'main', '--gate-tagged', '0', ...args], { cwd: dir, encoding: 'utf8', timeout: 60_000, env });
+  const patchOf = () => JSON.parse(plan(['--json']).stdout).targeted.targeted_patch_id as string;
+  const readPatch = patchOf();
+  const stop = commit({ [file]: body(30) }, 'round 2 fix');
+  const add = spawnSync(path.join(BIN, 'gstack-approvals'), ['add', '--ask', `Owner grant: one scoped fix-check after round 2 (review_loop_grant ${stop.slice(0, 12)})`, '--words', 'do it', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { cwd: dir, encoding: 'utf8', timeout: 60_000, env });
+  expect(add.status, add.stderr).toBe(0);
+  const grant = JSON.parse(add.stdout).added[0] as string;
+  const head = commit({ [file]: body(35) }, 'granted round fix');
+  const doneArgs = (ran: string) => ['--pass', '2', '--last', 'fix-check', '--fix-checks', '1', '--range-base', read, '--targeted-ran', ran];
+  return { dir, read, readPatch, head, grant, plan, patchOf, doneArgs };
+}
+const dispatches = (stdout: string) => stdout.split('\n').filter(l => l.startsWith('DISPATCH: '));
+
+describe('GST-1607: a granted fix round after the targeted read gets one targeted-only reader', () => {
+  // Value: protects=the #271 shape has an exit: after the granted fix-check the loop is done, the granted round's fix moved the
+  //   product patch the targeted record names, and with that grant the plan dispatches exactly one targeted reader for the head's
+  //   classes, then plans done once its id comes back; the receipt refuses the record read before the fix and binds the one read
+  //   at HEAD; fails_when=the plan prints NEXT: done with no reader (harness-evals #271), or keeps dispatching after the read;
+  //   seam=bin/gstack-review-plan, parseReviewTargeted with the receipt's own carry predicates; no_claim=what the reader finds
+  test('the #271 shape: one targeted reader under the grant, then the receipt binds its record', () => {
+    const r = granted('src/billing.ts', money);
+    const p = r.plan([...r.doneArgs(r.readPatch), '--grant', r.grant]);
+    expect(p.status, p.stderr).toBe(0);
+    expect(p.stdout).toContain(`NEXT: targeted (the loop is done and the owner's grant ${r.grant}`);
+    expect(dispatches(p.stdout)).toEqual(['DISPATCH: targeted targeted effort=full classes=money']);
+    expect(p.stdout).toContain(`TARGETED_DELTA: range_base=${r.read} paths=src/billing.ts classes=money`);
+    const after = r.plan([...r.doneArgs(r.patchOf()), '--grant', r.grant]);
+    expect(after.stdout).toContain('NEXT: done');
+    expect(dispatches(after.stdout)).toEqual([]);
+    const record = (at: string) => ({ classes: ['money'], rules: 'r', brief_sha: 'a'.repeat(64), status: 'done', findings: 0, raised_by: 'targeted', head: at });
+    const productOf = (c: string) => spawnSync('git', ['patch-id', '--verbatim'], { input: spawnSync('git', ['diff', 'main', c], { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }).stdout, cwd: r.dir, encoding: 'utf8', timeout: 10_000 }).stdout.split(' ')[0];
+    const bind = (at: string) => parseReviewTargeted(record(at), productOf(r.head)!, productOf, from => targetedClassesSince(r.dir, from, r.head, 'main'), from => plainDocsSince(r.dir, from, r.head, 'main'));
+    expect(bind(r.read).ok).toBe(false);
+    expect(bind(r.head)).toMatchObject({ ok: true, targeted: { classes: ['money'], status: 'done' } });
+  });
+
+  // Value: protects=a granted round on a head with no targeted class adds no reader; fails_when=the grant alone dispatches the targeted
+  //   reader; seam=bin/gstack-review-plan; no_claim=the fix-check itself
+  test('a granted fix on a head with no targeted class adds no reader', () => {
+    const r = granted('src/format.ts', n => `export const width = ${n};\nexport const pad = (s: string) => s.padEnd(width);\n`);
+    const p = r.plan([...r.doneArgs(r.readPatch), '--grant', r.grant]);
+    expect(p.status, p.stderr).toBe(0);
+    expect(p.stdout).toContain('TARGETED: classes=none');
+    expect(p.stdout).toContain('NEXT: done');
+    expect(dispatches(p.stdout)).toEqual([]);
+  });
+
+  // Value: protects=without the owner's grant a code change after the read keeps today's route; fails_when=a code change alone
+  //   dispatches the targeted-only reader; seam=bin/gstack-review-plan; no_claim=how finish routes it
+  test('the same code change with no grant keeps done', () => {
+    const r = granted('src/billing.ts', money);
+    const p = r.plan(r.doneArgs(r.readPatch));
+    expect(p.stdout).toContain('NEXT: done');
+    expect(dispatches(p.stdout)).toEqual([]);
+    expect(p.stdout).toContain('SKIP: targeted-only read (the change since range_base');
+  });
+
+  // Value: protects=a grant the approval record does not hold refuses before any plan; fails_when=a fabricated --grant prints a plan or
+  //   dispatches the reader; seam=bin/gstack-review-plan's grant check; no_claim=who may grant
+  test('a fabricated grant refuses', () => {
+    const r = granted('src/billing.ts', money);
+    const p = r.plan([...r.doneArgs(r.readPatch), '--grant', 'a-000000000000']);
+    expect([p.status, p.stdout]).toEqual([3, '']);
+    expect(p.stderr).toContain('--grant a-000000000000');
+  });
+});

test/size-ledger/ship/gst-1607-granted-targeted.json

@@ -0,0 +1,4 @@
+{
+  "bytes": 50,
+  "why": "GST-1607: Step 19 names the --grant for the targeted-only read after a granted fix round"
+}