review-depth: key the first-pass Red Team cut to the branch review log (GSTA-167 option a)

A restarted /ship reset fullPasses and last, so it dropped the Red Team on
a branch an earlier run had already reviewed. gstack-review-plan now reads
the branch review log (lib/ship-budget reviewPaths, DIFF_REVIEWS rows) and
keeps the Red Team when a row exists or the log is unreadable.

Repins lib/review-depth.ts under replay V7 (the cut only narrows).
DEPTH_RULES unchanged (e9281c6c3758ea0f).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-31 17:23:58 +00:00
1 parent db1405e8bb
commit 072ef1d5dc
8 files changed
+82 -18

No files matched your search

+12 -3
View File
@@ -25,6 +25,8 @@ import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
import { runScriptSync } from '../lib/script-invocation';
import { collectCommit, collectLive } from '../lib/review-collect';
import { classifyDiff, diffCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, SCOPED_RANGE_LINES, type Depth, type History } from '../lib/review-depth';
import { DIFF_REVIEWS } from '../lib/review-evidence';
import { reviewPaths } from '../lib/ship-budget';
import { classList, targetedBriefSha, targetedClasses, targetedPatchId } from '../lib/review-targeted';
import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check';
@@ -58,7 +60,9 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do
Depth: light (runtime lines <= 50), standard (above 50), full (gate class or any DEPTH_ERROR). 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, stop or done. A full plan
(a full pass on the range: fix-check, Red Team, native adversarial), fix-check, stop or done. Pass 1 at gate class or past
200 runtime lines skips the Red Team only when this branch's review log (gstack-review-log) has no diff-review row from an
earlier run; an unreadable log keeps it (--json: reviewed_before true, false or null). A full plan
from an error exits 0; exit 1 means no plan could be built (git could not list the diff).
${exitTableHelp()}`;
@@ -137,18 +141,23 @@ function main(): number {
: { base: rangeBase, runtime_lines: range.runtimeLines, ...(range.class === 'gate' ? { touched_gate: true as const } : {}) };
}
}
let reviewedBefore: boolean | null;
try {
const rows = fs.readFileSync(reviewPaths(root, process.env).reviewLog, 'utf8').split('\n').filter(Boolean);
reviewedBefore = rows.some(l => { try { return DIFF_REVIEWS.has(JSON.parse(l)?.skill); } catch { return false; } });
} catch (e: any) { reviewedBefore = e?.code === 'ENOENT' ? false : null; }
const targeted = targetedClasses({ ...collected, scope, gateTagged });
const patch = collected.mergeBase ? targetedPatchId(process.cwd(), collected.mergeBase) : 'none';
const brief = targetedBriefSha();
const plan = planReview({
...collected, scope, gateTagged, targeted: { classes: targeted.classes, ran: !['uncommitted', 'none'].includes(patch) && flag('--targeted-ran') === patch }, numbers: scanNumbers(added), force, gated: list('--gated'), floor, tiers: auditEffort !== 'host',
criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'),
criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'), reviewedBefore: reviewedBefore !== false,
history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below') && last !== 'fix-check', blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') || !!fixRange?.touched_gate, pass1Depth, rangeRuntimeLines: fixRange?.runtime_lines },
});
const gateScan = { tagged: gateTagged, head };
if (has('--json')) {
const { runtimeLines, stepDepth, depthReasons, ...rest } = plan;
process.stdout.write(JSON.stringify({ ...rest, 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, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed }) + '\n');
process.stdout.write(JSON.stringify({ ...rest, 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, reviewed_before: reviewedBefore, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed }) + '\n');
return EXIT.ok;
}
const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewDepthLine(plan), `GATE_SCAN: ${gateTagged === null ? 'unavailable' : `tagged=${gateTagged}`} head=${head}`,
+3 -1
View File
@@ -1,7 +1,8 @@
**`/ship` no longer runs the standalone Red Team on the first full review pass of an invocation at gate class or past 200 runtime lines.** The preregistered no-loss replay (GSTA-167 Round 7, handoffs/GSTA-167/noloss7-results.md) removed those Red Team lines from the recorded reviewer outputs. The rest of the stack still found 32 of the 41 panel blockers against 33 with them, a one-sided non-inferiority p of 0.00074 at a margin of 4. The Red Team still runs on a small diff raised to standard, on every post-fix full pass and on the scoped pass. A first pass that logs a CRITICAL gets it only if a post-fix full or scoped pass follows, because `/ship` passes `--critical-found` only to the repeat plan. A new `/ship` after a stop starts counting passes again, so its first full pass has none either.
**`/ship` no longer runs the standalone Red Team on a branch's first full review pass at gate class or past 200 runtime lines.** The preregistered no-loss replay (GSTA-167 Round 7, handoffs/GSTA-167/noloss7-results.md) removed those Red Team lines from the recorded reviewer outputs. The rest of the stack still found 32 of the 41 panel blockers against 33 with them, a one-sided non-inferiority p of 0.00074 at a margin of 4. The Red Team still runs on a small diff raised to standard, on every post-fix full pass and on the scoped pass. A first pass that logs a CRITICAL gets it only if a post-fix full or scoped pass follows, because `/ship` passes `--critical-found` only to the repeat plan. The cut applies only when the branch's review log has no earlier review row. A new `/ship` after a stop, on a branch an earlier run already reviewed, keeps the Red Team on its first full pass. So does a run whose review log can't be read.
### Changed
- **`lib/review-depth.ts`**: on a first full pass (`fullPasses` 0 and no earlier step, so a repeat plan that leaves out `--pass` keeps the Red Team), the gate-class term (a depth error included, since it runs as gate class) and the more-than-`RED_TEAM_LINES` term no longer add the Red Team. The plan prints `SKIP: red-team (first full pass; Round 7 no-loss)`. The CRITICAL and raised small-diff terms are unchanged, and post-fix full passes keep every term. The scoped pass, the fix-check and light depth's batched red-team checklist are unchanged.
- **`bin/gstack-review-plan`**: before the first full pass, it reads this branch's review log through `reviewPaths` (`lib/ship-budget.ts`) and looks for a row from one of the diff-review skills in `lib/review-evidence.ts`. When it finds one, or can't read the log, it passes `reviewedBefore` and the Red Team stays. `--json` adds `reviewed_before`.
- **`RED_TEAM_FIRST_PASS`** (false) joins the `DEPTH_RULES` hash, so the hash moves.
### Docs
@@ -9,6 +10,7 @@
### Tests
- `test/review-depth.test.ts`: separate histories for a first gate pass, a first pass reached through a depth error and a first pass past 200 lines (no Red Team), a first pass with a CRITICAL and a raised small diff (Red Team), post-fix full passes, the scoped pass and the fix-check, light's checklist, and the `DEPTH_RULES` move.
- `test/review-depth.test.ts` and `test/review-plan-cli.test.ts`: a restarted `/ship` on a branch with earlier review records keeps the Red Team on its first full pass. A branch with no records still takes the cut, and an unreadable log keeps the Red Team. Both tests were written first, and both fail at db1405e.
- `test/review-pass.test.ts`: the pass program creates no Red Team on a first full pass, and a failed Red Team on a post-fix pass stops it. `test/review-dispatch.test.ts`: a trimmed Red Team brief is `BRIEF_MISMATCH` and the exact re-send voids it.
### Gate integrity
+18 -9
View File
@@ -208,9 +208,8 @@ next step is a scoped pass, the raise applies to the scoped set only. A fix-chec
is the same at every depth.
<a id="first-pass-red-team"></a>
**The first full pass has no standalone Red Team (GSTA-167 Round 7).** When an
invocation's first full pass (`fullPasses` 0 and no earlier step, so a repeat
plan that leaves out `--pass` still counts as post-fix) is gate class or has more than
**The first full pass has no standalone Red Team (GSTA-167 Round 7).** When a
branch's first full pass is gate class or has more than
`RED_TEAM_LINES` (200) runtime lines, the plan skips the Red Team and says why:
`SKIP: red-team (first full pass; Round 7 no-loss)`. A full plan reached through a
classifier error (`DEPTH_ERROR`) or `--depth-floor full` runs as gate class, so the same rule covers it;
@@ -220,10 +219,19 @@ the scoped pass, and the plan adds it whenever `--critical-found` is given. In
`/ship` that flag reaches only the repeat plan, so a first pass that logs a
CRITICAL gets the Red Team only if a post-fix full or scoped pass follows. When
that CRITICAL is deferred or noted (`done`), fixed below the floor (a fix-check)
or left open (a stop), no Red Team reads the change. `fullPasses`
counts passes in one `/ship` invocation, so a new `/ship` after a stop (a fix made
by hand between runs) starts at 0 and its first full pass has no Red Team either.
A fix-check never had one. `RED_TEAM_FIRST_PASS` (false) records the rule in
or left open (a stop), no Red Team reads the change. A fix-check never had one.
A pass is the branch's first only when three things hold: `fullPasses` is 0, no
step ran earlier in this invocation (so a repeat plan that leaves out `--pass`
still counts as post-fix), and the branch's review log has no diff-review row. The
review log is the `<branch>-reviews.jsonl` file that `gstack-review-log` writes and
`gstack-ship-budget` reads. A diff-review row is any row from the `review`,
`review-fix-check`, `adversarial-review`, `codex-review`, `design-review-lite` or
`ship` skills. So a new `/ship` after a stop, on a branch an earlier run already
reviewed, runs the Red Team on its first full pass, as it did before this rule.
When the log can't be read, the plan keeps the Red Team too. `gstack-review-plan
--json` reports the lookup as `reviewed_before`: true, false, or null when the log
was unreadable. `RED_TEAM_FIRST_PASS` (false) records the rule in
`DEPTH_RULES`.
The evidence is the preregistered no-loss replay
@@ -303,8 +311,9 @@ the specialists, audits and watchers at the host default too
### Behavior changes
- An invocation's first full pass at gate class (a depth error included) or past 200
runtime lines no longer dispatches the standalone Red Team
- A branch's first full pass at gate class (a depth error included) or past 200
runtime lines, when the branch's review log has no earlier review row, no longer
dispatches the standalone Red Team
([first full pass](#first-pass-red-team), GSTA-167 Round 7). This is a gate
edit: the `DEPTH_RULES` hash moves, so receipts written before it carry the old
`rules` value.
+3 -1
View File
@@ -299,6 +299,8 @@ export interface PlanInput extends ClassInput {
tiers?: boolean;
/** The targeted classes (lib/review-targeted.ts) and whether the targeted reader already read this product patch. */
targeted?: { classes: readonly string[]; ran?: boolean };
/** This branch's review log holds a diff-review row from an earlier run, or could not be read: the first full pass keeps the Red Team. */
reviewedBefore?: boolean;
}
/** One reviewer. `exclude` holds git pathspec argv items the reviewer's diff leaves out; `checklists` names what the batched specialist reads. */
export interface Dispatch { role: Role; name: string; effort: Effort; exclude: string[]; checklists?: string[]; reason?: string }
@@ -413,7 +415,7 @@ export function planReview(i: PlanInput): ReviewPlan {
if (checklists.length) add('specialist', 'batched', specialist, full ? [] : docsAndOutputs, { checklists });
const codeLike = dcls === 'code' || dcls === 'gate';
const sized = dcls === 'gate' || (dcls === 'code' && lines > RED_TEAM_LINES);
const firstPassCut = sized && h.fullPasses === 0 && h.last === 'none' && !RED_TEAM_FIRST_PASS;
const firstPassCut = sized && h.fullPasses === 0 && h.last === 'none' && !i.reviewedBefore && !RED_TEAM_FIRST_PASS;
if (codeLike && (!!i.criticalFound || raised || (sized && !firstPassCut))) add('red-team', 'red-team', 'full');
else if (firstPassCut) skipped.push('red-team (first full pass; Round 7 no-loss)');
else skipped.push(`red-team (${dcls} class${dcls === 'code' ? `, ${lines} runtime lines, no critical` : ''})`);
+1 -1
View File
@@ -5,7 +5,7 @@ import { join } from 'node:path';
import { deriveReviewStatus, readJsonl } from './headless-artifacts';
import { resultLine, type ResultCodeName } from './result-codes';
const DIFF_REVIEWS = new Set(['review', 'review-fix-check', 'adversarial-review', 'codex-review', 'design-review-lite', 'ship']);
export const DIFF_REVIEWS: ReadonlySet<string> = new Set(['review', 'review-fix-check', 'adversarial-review', 'codex-review', 'design-review-lite', 'ship']);
const SHARED_LIBS_COVERAGE_VERSION = 1;
const HASH = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/;
+11 -1
View File
@@ -513,7 +513,7 @@ describe('the first full pass drops the standalone Red Team (GSTA-167 Round 7 no
const rt = (p: ReturnType<typeof planReview>) => p.dispatches.some(d => d.role === 'red-team');
const NOTE = 'red-team (first full pass; Round 7 no-loss)';
const postFix = { fullPasses: 1, last: 'full', fixedBlocking: true, rangeRuntimeLines: 300 } as const;
// Value: protects=a change's first full pass at gate class (or full from a depth error) or past RED_TEAM_LINES dispatches no
// Value: protects=the first full pass of a branch with no earlier review record, at gate class (or full from a depth error) or past RED_TEAM_LINES dispatches no
// standalone Red Team and says why in SKIP; fails_when=the Round 7 cut is missing, too broad or unexplained;
// why_new=V7 passed (handoffs/GSTA-167/noloss7-results.md: 32 of 41 against 33, p_NI 0.00074); seam=planReview
test('a first full pass on gate class has no Red Team', () => {
@@ -535,6 +535,16 @@ describe('the first full pass drops the standalone Red Team (GSTA-167 Round 7 no
expect(rt(p)).toBe(false);
expect(p.skipped).toContain(NOTE);
});
// Value: protects=a /ship restarted after a stop on a branch whose review log already has a pass keeps the Red Team on its first
// full pass; fails_when=the cut reads run state (fullPasses, last) instead of the branch's review records;
// why_new=GSTA-167 option a (a restart reset both counters and dropped the Red Team); seam=planReview reviewedBefore
test('a restarted /ship on a branch with earlier review records keeps the Red Team on its first full pass', () => {
for (const o of [{ diffLines: 20, gateTagged: 1 }, { diffLines: 20, scope: { SCOPE_ERROR: 'unmatched path' } }, { diffLines: 201 }]) {
const p = planReview(base({ ...o, reviewedBefore: true }));
expect([p.next.step, rt(p), p.skipped.includes(NOTE)]).toEqual(['full', true, false]);
expect(rt(planReview(base({ ...o, reviewedBefore: false })))).toBe(false);
}
});
test('a first full pass with a CRITICAL keeps the Red Team, on code and gate class', () => {
for (const o of [{ diffLines: 120 }, { diffLines: 201 }, { diffLines: 20, gateTagged: 1 }]) expect(rt(planReview(base({ ...o, criticalFound: true })))).toBe(true);
});
+2 -2
View File
@@ -56,7 +56,7 @@ const NO_LOSS_REPLAYS = {
'G1': "GSTA-167 variant G Arm G1: live no-loss replay of both prose voices with part 1's prompts, 2 passes, normalizer v2, paired rule against pooled C' (parts 1 and 3; specialists at medium as in C and C2)",
'L': "GSTA-167 variant G Arm L: live no-loss replay of one batched reader (testing and security plus scoped checklists) and the Red Team on its findings, 2 passes, scored L∘G1 against G1 on the paired rule (part 2)",
'GST-257': "GST-257 PR 1 dispatch-superset no-loss check (test/review-targeted.test.ts): on every fixture diff the branch dispatches main's reviewers at 8be7f21 unchanged plus only the targeted reader, and over the last 100 non-release commits on main it lost none; adds the targeted role and brief to the G1 and L bytes, whose existing reviewer text is unchanged",
'V7': "GSTA-167 no-loss Round 7 arm V7 (handoffs/GSTA-167/noloss7-results.md): L∘G1 without the standalone Red Team on a change's first full pass found 32 of 41 panel blockers against R's 33, p_NI 0.00074 at delta 4; certifies only the first-full-pass gate-class and >200-line trigger in lib/review-depth.ts",
'V7': "GSTA-167 no-loss Round 7 arm V7 (handoffs/GSTA-167/noloss7-results.md): L∘G1 without the standalone Red Team on the first full pass of a branch with no earlier review record found 32 of 41 panel blockers against R's 33, p_NI 0.00074 at delta 4; certifies only the first-full-pass gate-class and >200-line trigger in lib/review-depth.ts",
} as const;
type Replay = keyof typeof NO_LOSS_REPLAYS;
const REVIEWER_BRIEFS: Record<string, { sha: string; replay: Replay }> = {
@@ -78,7 +78,7 @@ const REVIEWER_BRIEFS: Record<string, { sha: string; replay: Replay }> = {
'review/specialists/testing.md': { sha: 'd6cc2bef7be562387d0c0a22fefe646dff9c295f1d34d8453a6cb496584150e4', replay: '#3638' },
'review/specialists/targeted.md': { sha: '7f92a0d0a8319c3c0e076fb3b1a4689c45120b2be2c14dfc7d4cb03aafa9d6a5', replay: 'GST-257' },
'scripts/resolvers/fix-check.ts': { sha: '712bf5b0c439e96bac32f19cbe25394dabb4c94aa66600100bfde9b100e15a08', replay: '#3638' },
'lib/review-depth.ts': { sha: 'a64130f2f2fd13416994dde01f4ed53349013f7134c41f87d43f59d73c448d0a', replay: 'V7' },
'lib/review-depth.ts': { sha: '709e82486d6845fd98baf1068c791758316e39fbb7291783e16ab430408ff1c3', replay: 'V7' },
};
describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => {
+32
View File
@@ -258,3 +258,35 @@ describe('gstack-review-plan: the scoped second pass (GSTA-152 D6)', () => {
expect(run(r, [...pass2, '--range-base', rangeBase, '--depth-floor', 'full']).stdout).toMatch(/^NEXT: full \(a blocking finding was fixed and the diff is at full depth/m);
});
});
describe('the first-pass Red Team cut reads the branch review log (GSTA-167 option a)', () => {
const LOG = path.join(import.meta.dir, '..', 'bin', 'gstack-review-log');
const NOTE = 'SKIP: red-team (first full pass; Round 7 no-loss)';
const RT = /^DISPATCH: red-team red-team effort=full$/m;
const big = () => repo({ 'src/a.ts': Array.from({ length: 210 }, (_, i) => `export const v${i} = ${i};`).join('\n') + '\n' });
// Value: protects=a /ship restarted after a stop on a branch whose review log already holds a pass plans the Red Team on its
// first full pass, a branch with no records still takes the Round 7 cut, and an unreadable log keeps the Red Team;
// fails_when=the cut reads only --pass/--last (a restart resets both) or a lookup failure drops the Red Team;
// why_new=GSTA-167 option a; seam=bin/gstack-review-plan through lib/ship-budget reviewPaths and the real gstack-review-log
test('a restart on a branch with an earlier review row keeps the Red Team; no row takes the cut; an unreadable log keeps it', () => {
const r = big();
const first = run(r, []);
expect(first.stdout).toContain(NOTE);
expect(first.stdout).not.toMatch(RT);
expect(JSON.parse(run(r, ['--json']).stdout).reviewed_before).toBe(false);
const logged = spawnSync(LOG, ['{"skill":"review","timestamp":"2026-10-31T00:00:00Z","status":"clean","via":"ship","pass":1}'], { cwd: r.dir, encoding: 'utf8', timeout: 30_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } });
expect(logged.status, logged.stderr).toBe(0);
const restarted = run(r, []);
expect(restarted.stdout).toMatch(RT);
expect(restarted.stdout).not.toContain(NOTE);
expect(JSON.parse(run(r, ['--json']).stdout).reviewed_before).toBe(true);
const u = big();
const dir = path.join(u.state, 'projects');
expect(run(u, []).stdout).toContain(NOTE);
fs.rmSync(dir, { recursive: true, force: true });
fs.writeFileSync(dir, 'not a directory');
const unread = run(u, []);
expect(unread.stdout).toMatch(RT);
expect(JSON.parse(run(u, ['--json']).stdout).reviewed_before).toBe(null);
}, 60_000);
});