review-plan: a docs claim added after the loop plans one targeted-only reader (GST-1598) (#4062)

* test: failing-first repro for GST-1598 (post-review docs money delta has no targeted-only reader)

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

* review-plan: a docs claim added after the loop plans one targeted-only reader (GST-1598)

After a finished review loop, a docs change with a money, security or numeric
claim left the receipt unable to carry the targeted record and the plan at
done, so nothing could dispatch the reader the receipt asks for (harness-evals
#225). With --range-base naming the commit the targeted reader read and
--targeted-ran its patch, a docs-only change since then now plans NEXT:
targeted: one targeted reader on HEAD for the head classes, with
TARGETED_DELTA printed. Code changes keep done; --force targeted refuses and
names the route; gstack-review-dispatch pass builds the one-reader manifest.

Repins lib/review-depth.ts under replay GST-1598 (one Step union member;
the GST-967 and GST-972 grids hash unchanged).

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

* review-targeted-delta: pass the base to plainDocsSince (GST-1598 after #4058)

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

* fix: pre-landing review fixes

targetedDelta counts a path whose branch hunks are unchanged and only the base moved lines beside them (comparePath context) as unchanged, so a main merge next to a reviewed hunk no longer hides the targeted-only read (GST-1598).

Review-Finding: p1/outside-voice#1 reader=outside-voice
Review-Finding: p1/adversarial-native#2 reader=adversarial-native
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: GST-1598 targeted-only read fragment

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>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-11-06 11:27:58 +00:00
1 parent f7015fa72c
commit 937b360fed
13 files changed
+290 -11

No files matched your search

+1 -1
View File
@@ -403,7 +403,7 @@ function pass(): number {
const step = p.next.step;
const fixChecks = step === 'fix-check' ? [] : p.dispatches.filter(d => d.role === 'fix-check');
const planLines = [...text.stdout.trimEnd().split('\n'), ...fixChecks.map(d => `FIX_CHECK: planned effort=${d.effort} (Step 3.5 runs it on its own; not in this manifest)`)];
if (step !== 'full' && step !== 'scoped' && step !== 'fix-check') { process.stdout.write([...planLines, `PASS: none next=${step} (no reviewer pass; follow NEXT)`].join('\n') + '\n'); return EXIT.ok; }
if (step !== 'full' && step !== 'scoped' && step !== 'fix-check' && step !== 'targeted') { process.stdout.write([...planLines, `PASS: none next=${step} (no reviewer pass; follow NEXT)`].join('\n') + '\n'); return EXIT.ok; }
const rangeBase = step === 'scoped' ? git(['rev-parse', '--verify', '--quiet', `${p.fix_range?.base ?? ''}^{commit}`])?.trim() : undefined;
if (step === 'scoped' && !rangeBase) throw new BriefError('a scoped plan names no readable fix range base');
const mergeBase = mergeBaseWith(base);
+21 -6
View File
@@ -38,8 +38,9 @@ import { loadPolicyAt } from '../lib/ship-policy-at';
import { loadReaders, readersLine } from '../lib/review-fetch';
import { policySelection, selectFor } from '../lib/review-selection';
import { effectiveReaders, READERS_POLICY_DEFAULT } from '../lib/ship-policy-readers';
import { classifyDiff, diffCommand, keepCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, SCOPED_RANGE_LINES, type Depth, type History } from '../lib/review-depth';
import { classifyDiff, diffCommand, keepCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, ROLE_EFFORT, SCOPED_RANGE_LINES, type Depth, type History } from '../lib/review-depth';
import { classList, targetedBriefSha, targetedClasses, targetedPatchId } from '../lib/review-targeted';
import { targetedDelta } from '../lib/review-targeted-delta';
import { RISK_CLASSES, type RiskClass } from '../lib/review-risk';
import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check';
@@ -59,7 +60,8 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do
--gated a,b specialists the hit-rate stats tag GATE_CANDIDATE
--force a,b|all specialists forced by prompt flags (--all-specialists is all): at standard and full depth they join the
batched specialist's checklists, where maintainability, performance and simplification are read only when
forced; light runtime-0 and scoped passes dispatch them one reader each
forced; light runtime-0 and scoped passes dispatch them one reader each; targeted is no specialist and
refuses (exit 2): the plan dispatches it itself
--reviewed-numbers <fp> the numeric-claim fingerprint the outside voice already reviewed
--depth-floor standard|full raise the depth to at least this (never lowers it); /ship passes its prompt flag on every run
--range-base <sha> the fix range's base (this pass's PASS_HEAD): after a fixed blocker on a diff below full depth, a
@@ -68,7 +70,10 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do
--json the plan as JSON (depth, runtime_lines, rules, errors with input and repair, gate_scan, per-dispatch exclude)
--pass1-depth <d> pass 1's REVIEW_DEPTH depth: full, or absent, keeps pass 2 full
--targeted-ran <id> the TARGETED line's targeted_patch_id (the whole committed diff, tests included) when the targeted
reader last read it: the same patch skips it; any other id, none, or uncommitted edits dispatch it again
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)
--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
@@ -94,7 +99,7 @@ Depth (R51, risk not size): full on gate class, a risk class (money, auth, secre
standard only by --depth-floor or a raise; otherwise light. TARGETED: the money,
gate, auth and secret classes the diff touches; any class dispatches the targeted reader at every depth. NEXT: full, scoped
(a full pass on the range: fix-check, Red Team, native adversarial), fix-check, voices (gate class: Step 11 before any stop,
then ask again with --adversarial-ran), stop or done. A full plan
then ask again with --adversarial-ran), targeted (the targeted-only read after the loop), stop or done. A full plan
from an error exits 0; exit 1 means no plan could be built (git could not list the diff).
${exitTableHelp()}`;
@@ -134,6 +139,7 @@ function main(): number {
const rangeBase = flag('--range-base');
if (has('--range-base') && (!rangeBase || rangeBase.startsWith('-'))) { process.stderr.write('gstack-review-plan: --range-base needs the fix range\'s base commit (this pass\'s PASS_HEAD)\n'); return EXIT.usage; }
if (grant && !rangeBase) { process.stderr.write('gstack-review-plan: --grant needs --range-base (round 2\'s PASS_HEAD), which binds the grant to the stop it answers\n'); return EXIT.usage; }
if (list('--force').includes('targeted')) { process.stderr.write('gstack-review-plan: --force targeted: the targeted reader is no specialist; the plan dispatches it whenever the diff has a targeted class, and after a finished loop for a docs claim added since its read: pass --range-base <the commit it read> --targeted-ran <the targeted_patch_id it read> (docs/review-policy.md#targeted-only-read-after-the-loop)\n'); return EXIT.usage; }
const declared = list('--declare-risk');
if (has('--declare-risk') && (!declared.length || declared.some(c => !(RISK_CLASSES as readonly string[]).includes(c)))) { process.stderr.write(`gstack-review-plan: --declare-risk ${flag('--declare-risk') ?? ''}: each class is one of ${RISK_CLASSES.join(', ')} (a declaration only raises the depth)\n`); return EXIT.usage; }
const root = git(['rev-parse', '--show-toplevel'])?.trim();
@@ -219,19 +225,27 @@ function main(): number {
const targeted = targetedClasses({ ...collected, ...(active ? { paths: paths.filter(p => !active.excluded.has(p)), added: kept(added), removed: kept(collected.removed) } : {}), scope, gateTagged });
const patch = collected.mergeBase ? targetedPatchId(process.cwd(), collected.mergeBase) : 'none';
const brief = targetedBriefSha();
const ran = !['uncommitted', 'none'].includes(patch) && flag('--targeted-ran') === patch;
const postReview = has('--post-review');
const plan = planReview({
...collected, ...exclusionInput, ...riskPaths, declared: declared as RiskClass[], shape: { panel: review.panel, readerModel: review.reader_model, platform: review.platform, head, platformReview: platform?.ok ? platform.review : null }, scope, gateTagged, targeted: { classes: targeted.classes, ran: !['uncommitted', 'none'].includes(patch) && flag('--targeted-ran') === patch }, numbers: scanNumbers(kept(added)), force, gated: list('--gated'), floor, tiers: auditEffort !== 'host',
...collected, ...exclusionInput, ...riskPaths, declared: declared as RiskClass[], shape: { panel: review.panel, readerModel: review.reader_model, platform: review.platform, head, platformReview: platform?.ok ? platform.review : null }, scope, gateTagged, targeted: { classes: targeted.classes, ran }, numbers: scanNumbers(kept(added)), force, gated: list('--gated'), floor, tiers: auditEffort !== 'host',
criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'),
history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: (has('--fixed-below') || (postReview && pass > 0)) && last !== 'fix-check', blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') || !!fixRange?.touched_gate || (postReview && pass > 0), pass1Depth, rangeRuntimeLines: fixRange?.runtime_lines, granted: grant, fixChecks: has('--fix-checks') ? fixChecks : undefined },
});
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;
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` };
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;
const selection = policySelection(active && { state: 'active', specs: active.specs, keep: active.keep, excerpt: active.excerpt }, readers);
plan.dispatches.forEach((d, i) => { if (d.role !== 'fix-check') plan.dispatches[i] = selectFor(d, selection); });
const gateScan = { tagged: gateTagged, head };
if (has('--json')) {
const { runtimeLines, stepDepth, depthReasons, ...rest } = plan;
process.stdout.write(JSON.stringify({ ...rest, review_policy: review, platform_review: platform ? (platform.ok ? platform.review : { none: platform.reason }) : null, readers, targeted: { ...targeted, brief, targeted_patch_id: patch }, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, fix_range: fixRange ?? null, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, base: collected.base, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed, drawings: collected.drawings, executables: collected.executables,
process.stdout.write(JSON.stringify({ ...rest, review_policy: review, platform_review: platform ? (platform.ok ? platform.review : { none: platform.reason }) : null, readers, targeted: { ...targeted, brief, targeted_patch_id: patch }, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, fix_range: fixRange ?? null, targeted_delta: targetedDeltaOut, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, base: collected.base, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed, drawings: collected.drawings, executables: collected.executables,
...(exclusion.state === 'head-only' ? { review_exclude: exclusion } : active ? { review_exclude: { state: 'active', ...active.record, specs: active.specs, keep: active.keep, excerpt: active.excerpt, kept_files: active.kept, content: active.content, references: active.references } } : {}) }) + '\n');
return EXIT.ok;
}
@@ -242,6 +256,7 @@ function main(): number {
if (declared.length) out.push(`RISK_DECLARED: classes=${plan.declared.join(',')} detected=${plan.detected.join(',') || 'none'}`);
if (platform) out.push(platform.ok ? `PLATFORM_REVIEW: ${platform.review.id} reviewer=${platform.review.reviewer} head=${platform.review.head}` : `PLATFORM_REVIEW: none (${platform.reason})`);
if (fixRange) out.push(`FIX_RANGE: ${fixRange.runtime_lines === null ? `unread (${fixRange.error})` : `runtime_lines=${fixRange.runtime_lines}`} base=${fixRange.base}`);
if (targetedDeltaOut) out.push(`TARGETED_DELTA: range_base=${targetedDeltaOut.range_base} paths=${targetedDeltaOut.paths.join(',')} classes=${classList(targetedDeltaOut.classes)}`);
out.push(reviewPlanLine(plan, pass + (plan.next.step === 'full' || plan.next.step === 'scoped' ? 1 : 0)), `NEXT: ${plan.next.step} (${plan.next.reason})`);
for (const e of plan.errors) out.push(`DEPTH_ERROR: ${e.code} input=${e.input} detail=${e.detail}; fix: ${e.repair}${e.result ? ` (${e.result})` : ''}`);
for (const d of plan.dispatches) {
+1
View File
@@ -0,0 +1 @@
- **A docs claim added after the review no longer leaves /ship stuck.** When the review loop had finished and a later commit added only docs with a money, security or numeric claim (such as a changelog line with dollar figures), `gstack-review-plan` printed `NEXT: done`, so nothing ran the targeted reader the receipt then asked for. Given `--range-base <the commit the targeted reader read>` and `--targeted-ran <its patch id>`, the plan now prints `NEXT: targeted`, a `TARGETED_DELTA:` line and one targeted reader on HEAD. It is not a review round. A code change, plain docs, a range base the reader did not read, or uncommitted edits keep `done` and print the reason. A main merge that only edits lines next to a reviewed hunk does not count as a branch change. `--force targeted` now refuses and names this route. (GST-1598)
+38 -1
View File
@@ -512,6 +512,41 @@ Over the last 100 non-release commits on main (8663c3d2..8be7f213), money hit
which hit 43 of 100; those paths already get full depth, so the gate class was
narrowed to the gate scan's tagged hunks.
### Targeted-only read after the loop (GST-1598)
A docs claim added after the review loop finished, such as a changelog
fragment with dollar figures, used to have no way through. The targeted record
names the commit its reader read, and `write` carries it across a later docs
change only when that change is plain docs, so the receipt asked for the
targeted reader on HEAD. But the plan printed `done`, so nothing could
dispatch it (harness-evals #225). Now, when the loop is done, pass
`--range-base <the commit the reader read>` and `--targeted-ran <the
targeted_patch_id it read>`. If every path whose branch patch changed since
then is docs class and not binary, and the change is not plain docs, the plan
prints `NEXT: targeted`, one `DISPATCH: targeted` for the head's classes and
`TARGETED_DELTA: range_base=<sha> paths=<paths> classes=<classes>`
(`targeted_delta` in `--json`). The patch-id check proves that range_base
is what the reader read. The branch patch is compared on each side's own
merge base, so a base merge alone adds no path.
The reader reads the whole merge-base diff at HEAD, like every targeted
reader. It isn't scoped to the docs change, because `render` reads no other
base and a record that names HEAD is the one `write` binds. It is read-only
and applies no fix, so it isn't a review round under R51 and needs no grant.
A `--grant` is still checked against the approval record first, and a row
that isn't there refuses (exit 3). `gstack-review-dispatch pass` builds the
manifest: the one reader in the voices stage, with the `TARGETED_DELTA` line
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
`--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 id="review-exclude-paths"></a>
## What readers read under `review_exclude_paths`
@@ -690,7 +725,8 @@ and finish prints `ROUTE_BOUNDED_REFUSED: <path> <reason> (fix: …)`
([troubleshooting](troubleshooting.md#route-bounded-refused)). A lane
change whose other top-level keys differ from the reviewed copy stops too. A
targeted record read before the edits does not carry to the new head:
receipt write still asks for the targeted reader on HEAD. A test file whose only removed lines are swapped
receipt write still asks for the targeted reader on HEAD (a docs-only change
plans it on its own: [targeted-only read](#targeted-only-read-after-the-loop)). A test file whose only removed lines are swapped
one for one with lines that differ inside plain quoted strings, in
statements with no assertion or test call, takes the plain test route (the
scoped reader) instead of behavior. A changed expected value in the
@@ -938,6 +974,7 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
- `scripts/resolvers/review-army.ts`: the floor and `NO FINDINGS` in every specialist and Red Team prompt (rendered into `review/sections/review-army.md` and `ship/sections/review-army.md`), and the lead judgment's "On record" line that turns a [ruled class](#rulings-below-the-floor) into a note; in `/ship` every specialist and Red Team prompt it renders carries its diff base as a literal sha the parent writes in (`DIFF_BASE=<diff-base>`: the merge base on pass 1, `RANGE_BASE` from pass 2 on) beside its plan's `DIFF_CMD` excludes, because a reviewer is its own process and never sees the parent's shell variables; the fan-out batches above.
- `ship/sections/review-army.md` Step 9: the plan's `REVIEW_DEPTH`, `GATE_SCAN` and `--depth-floor` on every run, the depth fields of the review-log row, passes, `RANGE_BASE`, classify, defer, render, the cap check before any edit, the Step 11 late-defer row, scratch files under `.gstack/tmp/`, the cap decisions; `ship/sections/adversarial.md` Step 11: the outside voice reads the same range and scopes queued fixes the same way.
- `lib/review-exclude.ts`: the base policy's `review_exclude_paths` (matched set, kept reasons, the kept-file argument bound, the digest the receipt, carry and the queue compare) and the 2,000,000-byte reader ceiling (`test/review-exclude.test.ts`).
- `lib/review-targeted-delta.ts`: the [targeted-only read after the loop](#targeted-only-read-after-the-loop) (`test/review-plan-targeted-delta.test.ts`).
- `lib/review-depth.ts`, `bin/gstack-review-plan`: the diff class, the depth (`depthFor`, `DEPTH_RULES`, `depthOfCommitInput`), each dispatch's `exclude` pathspecs and `DIFF_CMD`, the transition table, the numeric-claim scan and each role's effort tier (`test/review-depth.test.ts`, `test/review-plan-cli.test.ts`).
- `lib/review-collect.ts`: `collectCommit` (git objects only, from any checkout) and `collectLive` (the working tree), per-path lines and the base commit's registry (`test/review-collect.test.ts`).
- `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary` (with `fix_checks`, `dispatches`, `est_input_tokens`, `unrecorded`, `ledgers_read`); refusals are `REVIEW_SCOPE_INPUT`, `REVIEW_SCOPE_RANGE` and `REVIEW_SCOPE_LEDGER` ([troubleshooting.md](troubleshooting.md#review-scope-input)).
+1 -1
View File
@@ -20,7 +20,7 @@ export type PathClass = 'docs' | 'tests' | 'code';
export type Depth = 'light' | 'standard' | 'full';
export type Effort = 'light' | 'medium' | 'full' | 'host';
export type Role = 'specialist' | 'red-team' | 'adversarial-native' | 'outside-voice' | 'fix-check' | 'targeted';
export type Step = 'full' | 'scoped' | 'fix-check' | 'voices' | 'platform' | 'done' | 'stop';
export type Step = 'full' | 'scoped' | 'fix-check' | 'voices' | 'platform' | 'targeted' | 'done' | 'stop';
export const SPECIALISTS = ['testing', 'maintainability', 'security', 'performance', 'data-migration', 'api-contract', 'design', 'simplification', 'state-machines'] as const;
export type Specialist = (typeof SPECIALISTS)[number];
+47
View File
@@ -0,0 +1,47 @@
/**
* review-targeted-delta — the targeted-only read after a finished review loop (GST-1598,
* docs/review-policy.md#targeted-only-read-after-the-loop). A targeted record names the commit its reader read, and
* gstack-ship-receipt write carries it across a later docs change only when that change is plain docs (plainDocsSince,
* GST-1195). A docs claim added after the loop finished (a changelog fragment with dollar figures, harness-evals #225)
* left no exit: the plan printed `done`, so nothing dispatched the reader the receipt asks for. bin/gstack-review-plan
* 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.
*/
import { spawnSync } from 'node:child_process';
import { classifyPath } from './review-depth';
import { comparePath, touchedPaths } from './ship-patch';
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 {
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;
};
const skip = (reason: string): TargetedDelta => ({ state: 'skip', reason });
const read = git(['rev-parse', '--verify', '--quiet', `${rangeBase}^{commit}`])?.trim();
const head = git(['rev-parse', '--verify', '--quiet', 'HEAD^{commit}'])?.trim();
if (!read || !head || git(['merge-base', '--is-ancestor', read, head]) === null) return skip(`range_base ${rangeBase} is not a commit on this branch`);
if (git(['diff', '--quiet', 'HEAD', '--']) === null || git(['ls-files', '--others', '--exclude-standard'])?.trim()) return skip('uncommitted or untracked edits: commit them first');
const readBase = git(['merge-base', base, read])?.trim();
const headBase = git(['merge-base', base, head])?.trim();
if (!readBase || !headBase) return skip(`no merge base with ${base}`);
const diff = git(['diff', '--no-color', '--no-ext-diff', readBase, read]);
const readPatch = diff?.trim() ? git(['patch-id', '--verbatim'], diff)?.trim().split(/\s+/)[0] : undefined;
if (!readPatch || readPatch !== ran) return skip(`--targeted-ran ${ran} is not the targeted patch at range_base ${read.slice(0, 12)} (${readPatch ?? 'unreadable'}), so it does not say what the reader read`);
const before = touchedPaths(root, readBase, read);
const now = touchedPaths(root, headBase, head);
if (!before || !now) return skip('git could not list the branch patch at range_base or at HEAD');
const sides = { reviewedBase: readBase, reviewed: read, currentBase: headBase, current: head };
const paths = [...new Set([...before, ...now])].filter(p => !['same', 'context'].includes(comparePath(root, sides, p))).sort();
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 (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 };
}
+1 -1
View File
@@ -81,7 +81,7 @@ function newShip(f: Fixture, sc: Scenario): { calls: Call[]; ending: string } {
let adversarialRan = false;
for (let i = 0; i < 6; i++) {
const plan = planReview({ ...base(f), history: h, criticalFound: sc === 'blocker' && i === 0, adversarialRan });
if (plan.next.step === 'done' || plan.next.step === 'stop' || plan.next.step === 'platform') { ending = plan.next.step; break; }
if (plan.next.step === 'done' || plan.next.step === 'stop' || plan.next.step === 'platform' || plan.next.step === 'targeted') { ending = plan.next.step; break; }
const diffBytes = plan.next.step === 'fix-check' ? f.diff_bytes * FIX_CHECK_FRACTION : h.fullPasses === 0 ? f.diff_bytes : f.diff_bytes * RANGE_FRACTION;
for (const d of plan.dispatches) calls.push({ role: d.role, name: d.name, effort: d.effort, diffBytes: d.role === 'outside-voice' ? f.diff_bytes : diffBytes });
adversarialRan ||= plan.dispatches.some(d => d.role === 'adversarial-native');
+3
View File
@@ -227,6 +227,9 @@ 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>
--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
refuses, `QUEUE_RECEIPT_CONDITIONAL`). `--docs` reads the record Step 14.5 saved (`{status, scope[, exception]}`): the
receipt carries it as `docs`, and a `blocked` status without the owner's exception,
+3
View File
@@ -210,6 +210,9 @@ 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>
--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
refuses, `QUEUE_RECEIPT_CONDITIONAL`). `--docs` reads the record Step 14.5 saved (`{status, scope[, exception]}`): the
receipt carries it as `docs`, and a `blocked` status without the owner's exception,
+1 -1
View File
@@ -1 +1 @@
{"sha":"713ff6413608f1ab75f2c5306286cd1f275d2434ba84bc07b3b68876f4c81a3b","replay":"GST-1276"}
{"sha":"76da578bca44e918a22014b68deb9664a4bdc04c3f4ac6b2a1360cf811f8c820","replay":"GST-1598"}
+1
View File
@@ -0,0 +1 @@
{"replay":"GST-1598","justification":"GST-1598 (targeted-only read after the loop): lib/review-depth.ts is main's GST-1276 bytes plus one member, 'targeted', in the Step type union. planReview and nextStep never return it; only bin/gstack-review-plan sets it, after planReview printed done, through lib/review-targeted-delta.ts. Deterministic no-loss replay, no paid run: the GST-967 grid (MAIN_PLANS) and the GST-972 grid (MAIN_972_PLANS) in test/review-depth.test.ts hash unchanged, so no plan main builds drops, renames or re-efforts a reader, and no brief body moves (MAIN_SECTIONS unchanged)."}
+168
View File
@@ -0,0 +1,168 @@
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_CODE = 'export const spendCapUsd = 25;\nexport function charge(amountUsd: number) { if (amountUsd > spendCapUsd) throw new Error("over cap"); return amountUsd; }\n';
/** A branch whose reviewed head adds money code (the targeted reader read it), as on harness-evals #225 at 563f858. */
function reviewed(baseFiles: Record<string, string> = {}, branchFiles: Record<string, string> = { 'src/billing.ts': MONEY_CODE }): { dir: string; env: NodeJS.ProcessEnv; git: (...a: string[]) => string; commit: (files: Record<string, string>, msg: string) => string } {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'targeted-delta-'));
const state = fs.mkdtempSync(path.join(os.tmpdir(), 'targeted-delta-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', ...baseFiles }, 'base');
git('checkout', '-qb', 'feat');
commit(branchFiles, 'money');
return { dir, env, git, commit };
}
const plan = (r: { dir: string; env: NodeJS.ProcessEnv }, args: string[]) => spawnSync(path.join(BIN, 'gstack-review-plan'), ['--base', 'main', '--gate-tagged', '0', ...args], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env: r.env });
const patchOf = (r: { dir: string; env: NodeJS.ProcessEnv }) => JSON.parse(plan(r, ['--json']).stdout).targeted.targeted_patch_id as string;
const dispatches = (stdout: string) => stdout.split('\n').filter(l => l.startsWith('DISPATCH: '));
/** The loop finished clean: pass 1, a clean fix-check, nothing open. */
const done = (rangeBase: string, ran: string) => ['--pass', '1', '--last', 'fix-check', '--fixed-below', '--range-base', rangeBase, '--targeted-ran', ran];
describe('GST-1598: a post-review docs delta that trips a targeted claim gets one targeted-only reader', () => {
// Value: protects=the #225 shape has an exit: when the loop is done and the only change since the targeted read is a docs file
// with a money claim, the plan dispatches exactly one targeted reader for the head's classes, prints the delta's range_base,
// and once that reader's id is passed back the plan is done with no reader; the receipt binds a record read at HEAD and refuses
// the one read before the claim; fails_when=the plan prints NEXT: done with no dispatch (harness-evals #225), dispatches any
// other reader, 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 #225 shape: one targeted reader on the money claim, then the receipt binds its record', () => {
const r = reviewed();
const read = r.git('rev-parse', 'HEAD');
const readPatch = patchOf(r);
expect(dispatches(plan(r, ['--pass', '1', '--last', 'fix-check', '--fixed-below', '--targeted-ran', readPatch]).stdout)).toEqual([]);
const head = r.commit({ 'changelog.d/eval-cost.md': '- Each PR eval run now costs about $4.20 instead of $11.\n' }, 'changelog');
const p = plan(r, done(read, readPatch));
expect(p.status, p.stderr).toBe(0);
expect(p.stdout).toContain('NEXT: targeted');
expect(dispatches(p.stdout)).toEqual(['DISPATCH: targeted targeted effort=full classes=money']);
expect(p.stdout).toContain(`TARGETED_DELTA: range_base=${read} paths=changelog.d/eval-cost.md classes=money`);
const j = JSON.parse(plan(r, [...done(read, readPatch), '--json']).stdout);
expect([j.next.step, j.dispatches.map((d: any) => d.role), j.targeted_delta.range_base]).toEqual(['targeted', ['targeted'], read]);
const headPatch = patchOf(r);
const after = plan(r, done(read, headPatch));
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(head)!, productOf, from => targetedClassesSince(r.dir, from, head, 'main'), from => plainDocsSince(r.dir, from, head, 'main'));
expect(bind(read).ok).toBe(false);
expect(bind(head)).toMatchObject({ ok: true, targeted: { classes: ['money'], status: 'done' } });
});
// Value: protects=a code change after the loop keeps today's route (the review loop, not a targeted-only read);
// fails_when=the targeted-only step admits a code delta; seam=bin/gstack-review-plan; no_claim=how finish routes it
test('a code delta after the read keeps today\'s behavior: no targeted-only reader', () => {
const r = reviewed();
const read = r.git('rev-parse', 'HEAD');
const readPatch = patchOf(r);
r.commit({ 'src/billing.ts': MONEY_CODE.replace('25', '50'), 'changelog.d/eval-cost.md': '- Cap raised to $50.\n' }, 'code');
const p = plan(r, done(read, readPatch));
expect(p.status, p.stderr).toBe(0);
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 plain docs change still carries the record, so no reader is spent on it (GST-1195), and the range_base must be
// the commit the targeted reader read; fails_when=the plan dispatches on plain docs or on a range_base whose patch the passed
// --targeted-ran does not name; seam=bin/gstack-review-plan; no_claim=receipt carry itself (parseReviewTargeted's tests)
test('plain docs, or a range_base the targeted read did not read, dispatch nothing', () => {
const r = reviewed();
const read = r.git('rev-parse', 'HEAD');
const readPatch = patchOf(r);
r.commit({ 'docs/howto.md': '# How to\nRun the setup script first.\n' }, 'plain docs');
expect(dispatches(plan(r, done(read, readPatch)).stdout)).toEqual([]);
const plainHead = r.git('rev-parse', 'HEAD');
r.commit({ 'changelog.d/eval-cost.md': '- Each PR eval run now costs about $4.20.\n' }, 'claim');
const wrong = plan(r, done(plainHead, readPatch));
expect(dispatches(wrong.stdout)).toEqual([]);
expect(wrong.stdout).toContain('SKIP: targeted-only read (--targeted-ran');
});
// Value: protects=a main merge that only edits lines next to a reviewed branch hunk (comparePath's `context`) is no branch change,
// so the docs claim after it still plans the targeted-only read; fails_when=the delta counts a context-only path as changed and
// keeps NEXT: done with a not-docs-only skip (the GST-1598 dead end after a base merge); seam=bin/gstack-review-plan;
// no_claim=a base merge that edits the reviewed lines themselves
test('a base merge that only moves context next to a branch hunk still plans the targeted-only read', () => {
const billing = (cap: string, note: string) => ['// billing', '// rates', `// ${note}`, '', `export const spendCapUsd = ${cap};`, 'export function charge(amountUsd: number) { if (amountUsd > spendCapUsd) throw new Error("over cap"); return amountUsd; }', ''].join('\n');
const r = reviewed({ 'src/billing.ts': billing('25', 'old note') }, { 'src/billing.ts': billing('30', 'old note') });
const read = r.git('rev-parse', 'HEAD');
const readPatch = patchOf(r);
r.git('checkout', '-q', 'main');
r.commit({ 'src/billing.ts': billing('25', 'new note') }, 'base comment');
r.git('checkout', '-q', 'feat');
r.git('merge', '-q', '--no-edit', 'main');
r.commit({ 'changelog.d/eval-cost.md': '- The spend cap is now $30.\n' }, 'claim');
const p = plan(r, done(read, readPatch));
expect(p.status, p.stderr).toBe(0);
expect(p.stdout).toContain('NEXT: targeted');
expect(p.stdout).toContain(`TARGETED_DELTA: range_base=${read} paths=changelog.d/eval-cost.md classes=money`);
});
// Value: protects=a grant is not what opens this read, and a grant the approval record does not hold still refuses before any plan;
// fails_when=a fabricated --grant prints a plan; seam=bin/gstack-review-plan's grant check; no_claim=who may grant
test('a fabricated grant refuses', () => {
const r = reviewed();
const read = r.git('rev-parse', 'HEAD');
const readPatch = patchOf(r);
const head = r.commit({ 'changelog.d/eval-cost.md': '- Each PR eval run now costs about $4.20.\n' }, 'changelog');
const p = plan(r, [...done(read, readPatch), '--grant', 'a-000000000000']);
expect([p.status, p.stdout]).toEqual([3, '']);
expect(p.stderr).toContain('--grant a-000000000000');
expect(p.stderr).toContain('review_loop_grant');
expect(head).toMatch(/^[0-9a-f]{40}$/);
});
// Value: protects=--force targeted no longer dispatches zero readers in silence: targeted is not a specialist, so the flag refuses and
// names the route; fails_when=--force targeted exits 0; seam=bin/gstack-review-plan; no_claim=other --force names
test('--force targeted refuses and names the route', () => {
const r = reviewed();
const p = plan(r, ['--force', 'targeted']);
expect(p.status).toBe(2);
expect(p.stderr).toContain('--range-base');
expect(p.stderr).toContain('--targeted-ran');
});
// Value: protects=the pass command builds the targeted-only manifest (the brief renders only inside a pass manifest when an exclusion
// is active): one targeted reader in the voices stage on the merge base, the TARGETED_DELTA line in its plan lines, and its voices
// stage renders the brief on HEAD; fails_when=pass prints PASS: none for NEXT: targeted, or the stage renders on another base;
// seam=gstack-review-dispatch pass and stage; no_claim=the reader's reply
test('gstack-review-dispatch pass builds a manifest with the one targeted reader', () => {
const r = reviewed();
const read = r.git('rev-parse', 'HEAD');
const readPatch = patchOf(r);
r.commit({ 'changelog.d/eval-cost.md': '- Each PR eval run now costs about $4.20.\n' }, 'changelog');
const out = fs.mkdtempSync(path.join(os.tmpdir(), 'targeted-delta-out-'));
dirs.push(out);
const p = spawnSync(path.join(BIN, 'gstack-review-dispatch'), ['pass', '--base', 'main', '--out', out, '--pass', 'td1', '--', '--gate-tagged', '0', ...done(read, readPatch)], { cwd: r.dir, encoding: 'utf8', timeout: 120_000, env: r.env });
expect(p.status, p.stderr).toBe(0);
expect(p.stdout).toMatch(/MANIFEST: file=\S+ pass=td1 readers=1 first=0 specialists=0 red-team=0 voices=1/);
const file = p.stdout.match(/MANIFEST: file=(\S+)/)![1]!;
const m = JSON.parse(fs.readFileSync(file, 'utf8'));
const mergeBase = r.git('merge-base', 'main', 'HEAD');
expect([m.next, m.readers.map((x: any) => [x.role, x.stage, x.classes, x.diff_base])]).toEqual(['targeted', [['targeted', 'voices', ['money'], mergeBase]]]);
expect(m.plan_lines.filter((l: string) => l.startsWith('TARGETED_DELTA: '))).toEqual([`TARGETED_DELTA: range_base=${read} paths=changelog.d/eval-cost.md classes=money`]);
const stage = spawnSync(path.join(BIN, 'gstack-review-dispatch'), ['stage', '--manifest', file, '--stage', 'voices'], { cwd: r.dir, encoding: 'utf8', timeout: 120_000, env: r.env });
expect(stage.status, stage.stderr).toBe(0);
const rendered = fs.readFileSync(m.ledger, 'utf8').split('\n').filter(Boolean).map(l => JSON.parse(l)).filter(e => e.event === 'rendered');
expect(rendered.map(e => [e.reader, e.diff_base, e.head, e.classes])).toEqual([['targeted', mergeBase, r.git('rev-parse', 'HEAD'), ['money']]]);
});
});
@@ -0,0 +1,4 @@
{
"bytes": 301,
"why": "GST-1598: /ship Step 19 names the targeted-only read when write refuses review_targeted after a post-review docs claim"
}