fix(ship): GST-986 scoped fix-check batch: record binds only when nothing after the read carries a class (gate scan run), a re-raised blocker is not fixed, verify always clears the stale record, carry names #3859

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

bin/gstack-review-dispatch

@@ -357,6 +357,7 @@ function main(): number {
     return r.ok ? EXIT.ok : EXIT.fail;
   }
   if (cmd === 'verify') {
+    if (flag('--targeted-out')) fs.rmSync(flag('--targeted-out')!, { force: true });
     const v = verify(ledger);
     for (const r of v.rows) process.stdout.write(`${r.kind} ${r.reader} ${r.status} ${r.detail}\n`);
     if (!v.briefs) { process.stdout.write('VERIFY: no brief records (this run\'s ledger kept no prompts; dispatched-vs-rendered cannot be checked)\n'); return EXIT.refused; }
@@ -364,7 +365,7 @@ function main(): number {
     if (record) {
       process.stdout.write(`TARGETED_RECORD: classes=${record.classes.join(',')} status=${record.status} findings=${record.findings} brief=${record.brief_sha.slice(0, 12)} rules=${record.rules} read=${record.head?.slice(0, 12) ?? 'none'}\n`);
       if (flag('--targeted-out')) fs.writeFileSync(flag('--targeted-out')!, JSON.stringify(record) + '\n');
-    } else if (flag('--targeted-out')) fs.rmSync(flag('--targeted-out')!, { force: true });
+    }
     process.stdout.write(`VERIFY: ${v.clean ? 'clean' : 'dirty'} briefs=${v.briefs} outputs=${v.rows.filter(r => r.kind === 'OUTPUT').length} voided=${v.voided}${v.unavailable ? ` unavailable=${v.unavailable}` : ''}\n`);
     return v.clean ? EXIT.ok : EXIT.fail;
   }

bin/gstack-ship-receipt

@@ -97,7 +97,7 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate <jso
   unreadable input records full).
   --review-targeted <json|@file> ({classes, rules, brief_sha, status, findings, raised_by, head}: gstack-review-dispatch verify
   --targeted-out); recorded as review_targeted bound to the receipt's product_patch_id, and refused (RECEIPT_INVALID) when the
-  commit the reader read (head) has another product patch and a change since it carries a targeted class. write and carry
+  commit the reader read (head) is not an ancestor or a change since it carries a targeted class (gate scan included). write and carry
   refuse a receipt whose head needs a targeted class (money, gate, auth, secret) that no done pass covers, as the queue's
   targeted verdict does once #3859 lands (RECEIPT_INVALID, RECEIPT_CARRY_REFUSED).
   --approval-run <autoplan run dir> (repeatable; resolves plan-sha citations) (W2: a class declared in origin/<base>'s
@@ -742,7 +742,7 @@ function cmdCarry(rest: string[], json: boolean): number {
   const errors = validateReceipt(carried);
   if (errors.length) return refused('RECEIPT_CARRY_REFUSED', `the carried receipt does not validate: ${errors[0]!.message} at ${errors[0]!.path}`);
   const targetedVerdict = receiptTargetedAtHead(repoRoot, baseRef, carried, head);
-  if (!targetedVerdict.ok) return refused('RECEIPT_CARRY_REFUSED', `${targetedVerdict.line}: ${targetedVerdict.message}, so the queue would refuse the carried receipt; run /ship on ${head.slice(0, 12)}`);
+  if (!targetedVerdict.ok) return refused('RECEIPT_CARRY_REFUSED', `${targetedVerdict.line}: ${targetedVerdict.message}, so the queue refuses it once #3859 lands; run /ship on ${head.slice(0, 12)}`);
   const newBody = placeReceiptFirst(body, carried);
   const line = receiptSummaryLine(carried);
   if (scopedDryRun) return refused('RECEIPT_CARRY_SCOPED_REVIEW', scopedDryRun);

changelog.d/gst-986-targeted-recorder.md

@@ -1,6 +1,6 @@
 ### Fixed
 
-- **The targeted reader's record is honest from verify to the receipt.** `gstack-review-dispatch verify` no longer marks a completed targeted reader `status=failed findings=0` because a Step 11 fix moved HEAD after it read (gstack-evals#140, C5-36 and C6-36). The record now says `done` with its real finding count and names the commit it read. `gstack-ship-receipt write --review-targeted` binds it only while that commit's product patch matches the receipt's or nothing since carries a money, gate, auth or secret change, so a record can no longer vouch for targeted code it never read, and the docs and TODOS.md commits /ship makes after the read still bind.
-- **`write` and `carry` refuse a receipt the queue would refuse (#3854).** When the head touches money, gate, auth or secret code and no done targeted pass covers it, `write` refuses with `RECEIPT_INVALID` and `carry` with `RECEIPT_CARRY_REFUSED`, instead of publishing a receipt `ready` turns away. Legacy receipts still carry.
+- **The targeted reader's record is honest from verify to the receipt.** `gstack-review-dispatch verify` no longer marks a completed targeted reader `status=failed findings=0` because a Step 11 fix moved HEAD after it read (gstack-evals#140, C5-36 and C6-36). The record now says `done` with its real finding count and names the commit it read. `gstack-ship-receipt write --review-targeted` binds it only while nothing since that commit carries a money, gate, auth or secret change (a test edit that skips a case counts as gate), so a record can no longer vouch for targeted code it never read, and the docs and TODOS.md commits /ship makes after the read still bind.
+- **`write` and `carry` refuse a receipt the queue refuses once #3859 lands (#3854).** When the head touches money, gate, auth or secret code and no done targeted pass covers it, `write` refuses with `RECEIPT_INVALID` and `carry` with `RECEIPT_CARRY_REFUSED`, instead of publishing a receipt `ready` turns away. Legacy receipts still carry.
 - **/ship passes the targeted record to the receipt (#3853).** Step 11's verify writes `.gstack/tmp/targeted.json`, and the Step 19, handoff and driver receipt writes pass it with `--review-targeted`.
 - **Fix-check rounds count without hand edits (#3862).** `gstack-review-scope summary --run` counts fix-checks from the dispatch ledgers when no `review-fix-check` row was logged, and counts the blockers those fix-checks followed as fixed.

docs/review-policy.md

@@ -378,13 +378,15 @@ renders its brief; `verify --targeted-out .gstack/tmp/targeted.json` writes the
 (Step 19, the handoff and the driver pass it when the file exists). The record is `done` when
 the reader got its brief verbatim and its reply was collected on a committed diff, whatever
 happened to HEAD after, and it names the commit it read (`head`). `write` binds it only while
-that commit's product patch is the receipt's, or while nothing changed after the read carries a
-targeted class: a money, gate, auth or secret change after the read refuses with
-`RECEIPT_INVALID`, and a test, version, docs or TODOS.md commit (Steps 14 and 14.5) still
-binds. A verify whose ledger has no targeted render removes `--targeted-out`, so an earlier
-run's record never reaches Step 19. `write` and `carry` also
+that commit is an ancestor of HEAD and nothing changed after it carries a targeted class, with
+the gate scan run over that range: a money, gate, auth or secret change after the read refuses
+with `RECEIPT_INVALID`, a test edit the gate scan tags (a skipped case) included, while a
+version, docs, TODOS.md (Steps 14 and 14.5) or untagged test commit still binds. Every verify
+removes `--targeted-out` before it writes a record, so an earlier run's record never reaches
+Step 19. `write` and `carry` (which rescans the gate at the new head) also
 refuse a receipt whose head needs a class no `done` record covers (`RECEIPT_INVALID`,
-`RECEIPT_CARRY_REFUSED`), so /ship never publishes a receipt the queue would refuse.
+`RECEIPT_CARRY_REFUSED`). The queue's own refusal arrives with #3859; until it lands, `ready`
+only reports the targeted line.
 `gstack-ship-queue ready` and `train build` print
 `REVIEW_TARGETED: required=<classes|none> receipt=<classes|none|legacy>
 verdict=ok|missing` and refuse nothing yet; enforcement comes in a later
@@ -416,7 +418,7 @@ puts in the receipt:
 | `review_loop.rounds` | passes plus fix-checks (R51 caps it at 2; the queue's `REVIEW_DEPTH:` line prints it) |
 | `review_loop.dispatches` | reviewer dispatches by role: `specialist`, `red-team`, `adversarial-native`, `outside-voice`, `fix-check`, `targeted` |
 | `review_loop.est_input_tokens` | prompt plus diff bytes the dispatches read, divided by 4: a relative estimate for comparing runs, not a bill |
-| `review_targeted` | the [targeted pass](#targeted-pass-on-money-gate-auth-and-secret-code): the `classes` it covered, `rules`, `brief_sha`, the receipt's own `product_patch_id`, `status` (`done` only with its brief sent verbatim and its output collected on a committed diff, else `failed`; `write` refuses a record whose read commit has another product patch when a change after the read carries a targeted class, and refuses to write without a record when the head needs a class), `findings` and `raised_by`; a carry keeps it while the product patch is unchanged |
+| `review_targeted` | the [targeted pass](#targeted-pass-on-money-gate-auth-and-secret-code): the `classes` it covered, `rules`, `brief_sha`, the receipt's own `product_patch_id`, `status` (`done` only with its brief sent verbatim and its output collected on a committed diff, else `failed`; `write` refuses a record when a change after its read commit carries a targeted class or the read commit is not an ancestor, and refuses to write without a record when the head needs a class), `findings` and `raised_by`; a carry keeps it while the product patch is unchanged |
 
 Each review row may carry `dispatches: [{role, effort, prompt_bytes,
 diff_bytes, status}]`, and each finding `raised_by` (the reviewer that raised it,

docs/troubleshooting.md

@@ -3235,7 +3235,8 @@ same way when the head touches money, gate, auth or secret code and no done
 targeted record covers those classes (`REVIEW_TARGETED: … verdict=missing|failed`),
 or when `--review-targeted` names a read commit whose product patch is not
 the receipt's and a change after it carries a targeted class (money, gate,
-auth or secret code changed after the targeted reader read it).
+auth or secret code changed after the targeted reader read it, a test edit the
+gate scan tags included), or that is not an ancestor of HEAD.
 
 **Fix.** Regenerate it with `gstack-ship-receipt write`; never hand-edit the
 block. For the `--head` refusal, check out that commit and write there. For a

lib/review-scope.ts

@@ -394,7 +394,7 @@ export function ledgerPass(ledger: string): LedgerPass {
  * identity rows and ledgers share until rows carry the ledger id.
  * Fix-checks (GST-986, #3862; `ledgerPasses`, one per ledger): fix_checks is the larger of the review-fix-check rows and the
  * fix-check ledgers, so one logged both ways counts once. When fix-checks ran without their review-fix-check rows (more fix-check
- * ledgers than rows), a blocker no row disposes counts as fixed if a fix-check ledger started after the ledger it came from:
+ * ledgers than rows), a blocker no row disposes counts as fixed if a fix-check ledger started after the last ledger that raised it:
  * decision 3.5 runs a fix-check only after the loop applied that pass's fixes, and a blocker the loop does not fix is deferred
  * or noted by a row. A blocker the last pass found stays unrecorded, and with every fix-check logged the rows decide.
  */
@@ -434,6 +434,7 @@ export function summarizeReviewLoop(rows: readonly Record<string, any>[], since:
   let ledgerOnlyFixed = 0;
   const pairedIds = new Set<string>();
   const followedIds = new Set<string>();
+  const lastPaired = new Map<string, number>();
   const fixCheckStarts = ledgerPasses.filter(p => p.fixCheck && Number.isFinite(p.started)).map(p => p.started);
   const rowFixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length;
   const followed = (n: number) => fixCheckStarts.length > rowFixChecks && fixCheckStarts.some(t => t > (ledgerPasses[n]?.started ?? Infinity));
@@ -441,9 +442,10 @@ export function summarizeReviewLoop(rows: readonly Record<string, any>[], since:
     for (const b of list) {
       const by = b.id.startsWith('fix-check') ? null : raiserOf(b.id);
       const id = b.locus ? [...seen].find(([k, s]) => s.candidate && s.locus === b.locus && (!s.raiser || !by || s.raiser === by) && !paired.has(`${n}\0${k}`))?.[0] : undefined;
-      if (id) { paired.add(`${n}\0${id}`); pairedIds.add(id); if (followed(n)) followedIds.add(id); } else { ledgerOnly++; if (followed(n)) ledgerOnlyFixed++; }
+      if (id) { paired.add(`${n}\0${id}`); pairedIds.add(id); if (!((ledgerPasses[lastPaired.get(id) ?? -1]?.started ?? -Infinity) > (ledgerPasses[n]?.started ?? -Infinity))) lastPaired.set(id, n); } else { ledgerOnly++; if (followed(n)) ledgerOnlyFixed++; }
     }
   }
+  for (const [id, n] of lastPaired) if (followed(n)) followedIds.add(id);
   const blockers = [...seen].filter(([k, s]) => s.blocks || (pairedIds.has(k) && !s.classified));
   for (const [k, s] of blockers) if (!s.disposed && followedIds.has(k)) { s.disposed = true; fixed.add(k); }
   const unrecorded = ledgerOnly - ledgerOnlyFixed + blockers.filter(([, s]) => !s.disposed).length;

lib/ship-receipt-depth.ts

@@ -131,8 +131,9 @@ export type ReviewTargeted = NonNullable<ShipReceipt['review_targeted']>;
 /**
  * `write --review-targeted` input (gstack-review-dispatch verify --targeted-out), bound to the receipt's product patch-id here.
  * `head` is the commit the reader read; `productPatchOf` gives that commit's product patch-id by the receipt's own rules, and the
- * record binds when it equals the receipt's, or when `classesSince` reads no targeted class in what changed after the read (the
- * docs and TODOS.md /ship commits after Step 11), so a record never vouches for targeted-class code it did not read (GST-986).
+ * record binds when `classesSince` reads no targeted class in what changed after the read (the docs and TODOS.md /ship commits
+ * after Step 11; a test change the gate scan tags is gate class though the product patch leaves tests out), so a record never
+ * vouches for targeted-class work it did not read (GST-986); without `classesSince` it binds only on an equal product patch.
  */
 export function parseReviewTargeted(value: unknown, productPatchId: string, productPatchOf: (head: string) => string | undefined, classesSince?: (read: string) => readonly TargetedClass[] | null): { ok: true; targeted: ReviewTargeted } | { ok: false; message: string } {
   const bad = (message: string) => ({ ok: false as const, message });
@@ -146,7 +147,7 @@ export function parseReviewTargeted(value: unknown, productPatchId: string, prod
   if (typeof v.findings !== 'number' || !Number.isInteger(v.findings) || v.findings < 0) return bad('review_targeted.findings must be an integer >= 0');
   if (typeof v.head !== 'string' || !/^[0-9a-f]{40,64}$/.test(v.head)) return bad('review_targeted.head must be the commit the targeted reader read (rerun gstack-review-dispatch verify --targeted-out with this gstack)');
   const read = productPatchOf(v.head);
-  if (read !== productPatchId && classesSince?.(v.head)?.length !== 0) return bad(`the targeted reader read ${v.head.slice(0, 12)}, whose product patch ${read?.slice(0, 12) ?? 'cannot be read'} is not this receipt's ${productPatchId.slice(0, 12)}: the product changed after the read, so run the targeted reader on HEAD and verify again`);
+  if (classesSince ? classesSince(v.head)?.length !== 0 : read !== productPatchId) return bad(`the targeted reader read ${v.head.slice(0, 12)}, whose product patch ${read?.slice(0, 12) ?? 'cannot be read'} is not this receipt's ${productPatchId.slice(0, 12)}: the product changed after the read, so run the targeted reader on HEAD and verify again`);
   const classes = TARGETED_CLASSES.filter(c => (v.classes as string[]).includes(c));
   return { ok: true, targeted: { classes, rules: v.rules as string, brief_sha: v.brief_sha, product_patch_id: productPatchId, status: v.status as ReviewTargeted['status'], findings: v.findings, raised_by: (v.raised_by as string).trim() } };
 }
@@ -231,19 +232,30 @@ export function bindShippedReceipt(root: string, baseRef: string, body: string,
   return { ...b, depthLine: d.line, targetedLine };
 }
 
+/**
+ * The targeted classes of what changed from `from` to the checked-out `head`, with gstack-gate-diff run over that range whenever
+ * a non-docs path changed (a test edit that skips a case is gate class); null when `from` is not an ancestor of `head`, the gate
+ * scan did not run, or a class input is unreadable.
+ */
+export function targetedClassesSince(root: string, from: string, head: string): readonly TargetedClass[] | null {
+  if (!git(root, ['merge-base', '--is-ancestor', from, head]).ok) return null;
+  const at = headInputs(root, from, head);
+  let gateTagged: number | null = 0;
+  if (gateScanNeeded(at.c.paths, at.c.root)) {
+    const g = runScriptSync(path.join(BIN_DIR, 'gstack-gate-diff'), [from], { cwd: root, encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 });
+    const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g.stdout ?? '');
+    if (g.status !== 0 || !m) return null;
+    gateTagged = m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0);
+  }
+  const t = targetedClasses({ ...at.c, scope: at.scope, gateTagged });
+  return t.errors.length ? null : t.classes;
+}
+
 /**
  * receiptTargetedVerdict at the receipt's head, with the classes the queue computes: what write and carry check before they
  * publish (#3854). Classes that failed closed on an input this checkout cannot read (no origin base, no gate scan) are reported,
  * never refused on here: the queue reads its own inputs at ready and refuses there if the head needs the reader.
  */
-/** The targeted classes of what changed from `from` to `head`; null when `from` is not an ancestor of `head` or a class input is unreadable. */
-export function targetedClassesSince(root: string, from: string, head: string): readonly TargetedClass[] | null {
-  if (!git(root, ['merge-base', '--is-ancestor', from, head]).ok) return null;
-  const at = headInputs(root, from, head);
-  const t = targetedClasses({ ...at.c, scope: at.scope, gateTagged: gateScanNeeded(at.c.paths, at.c.root) ? null : 0 });
-  return t.errors.length ? null : t.classes;
-}
-
 export function receiptTargetedAtHead(root: string, baseRef: string, r: ShipReceipt, head: string): DepthVerdict {
   const at = headInputs(root, baseRef, head);
   const gateTagged = boundScan(r, head) ?? (gateScanNeeded(at.c.paths, at.c.root) ? null : 0);