review-plan: a malformed review-log row keeps the first-pass Red Team (GSTA-167 fail closed)

A readable log whose rows do not parse counted as no record, so it took
the first-pass cut. It now keeps the Red Team like an unreadable log;
a missing or empty log still takes the cut. lib/review-depth.ts and
DEPTH_RULES (e9281c6c3758ea0f) unchanged, so no repin.

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:53:46 +00:00
1 parent 072ef1d5dc
commit 7d8eda991c
4 files changed
+28 -5

No files matched your search

+1 -1
View File
@@ -144,7 +144,7 @@ function main(): number {
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; } });
reviewedBefore = rows.some(l => { try { return DIFF_REVIEWS.has(JSON.parse(l)?.skill); } catch { return true; } });
} 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';
+3 -3
View File
@@ -1,8 +1,8 @@
**`/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.
**`/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 or holds a line that isn't valid JSON.
### 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`.
- **`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, a line that isn't valid JSON, or a log it can't read, 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
@@ -10,7 +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-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. A log of only malformed rows keeps the Red Team and an empty log still takes the cut (written first, fails at 072ef1d).
- `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
+1 -1
View File
@@ -229,7 +229,7 @@ review log is the `<branch>-reviews.jsonl` file that `gstack-review-log` writes
`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
When the log can't be read, or holds a line that isn't valid JSON, 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`.
+23
View File
@@ -290,3 +290,26 @@ describe('the first-pass Red Team cut reads the branch review log (GSTA-167 opti
expect(JSON.parse(run(u, ['--json']).stdout).reviewed_before).toBe(null);
}, 60_000);
});
describe('a review log of only malformed rows keeps the Red Team (GSTA-167 fail closed)', () => {
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;
// Value: protects=a readable review log whose rows do not parse counts as unreadable, so the first full pass keeps the Red Team,
// while an empty log still takes the cut; fails_when=a malformed row is skipped as absent; why_new=GSTA-167 Captain ruling
// (fail closed on malformed rows); seam=bin/gstack-review-plan's review-log lookup
test('a log holding only malformed rows plans the Red Team; an empty log still takes the cut', () => {
const r = repo({ 'src/a.ts': Array.from({ length: 210 }, (_, i) => `export const v${i} = ${i};`).join('\n') + '\n' });
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 projects = path.join(r.state, 'projects');
const file = path.join(projects, fs.readdirSync(projects)[0]!, 'feat-reviews.jsonl');
expect(fs.existsSync(file)).toBe(true);
fs.writeFileSync(file, '');
expect(run(r, []).stdout).toContain(NOTE);
fs.writeFileSync(file, '{"skill":"review","timestamp":\nnot json at all\n');
const torn = run(r, []);
expect(torn.stdout).toMatch(RT);
expect(torn.stdout).not.toContain(NOTE);
}, 60_000);
});