v1.91.198.0 fix(outside-review): a reviewer's own "not ready" verdict floors advisory-only findings (#3329, port of #3364) (#3376)

* honor not-ready review verdicts with advisory findings

The classifier now honors the last recognized not-ready verdict
regardless of parsed severity, returning findings and exit code 3 while
retaining the highest severity in the summary. Advisory findings with a
ready verdict or no verdict keep their existing behavior. Regression
coverage and troubleshooting documentation describe this behavior across
the execution, structured, and review gates.

Fixes #3329

* docs(changelog): outside reviewer not-ready verdict floors advisory findings (#3329, port of #3364)

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

---------

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

CHANGELOG.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## [1.91.198.0] - 2026-10-25
+
+**A second-opinion reviewer that says "not ready" is no longer reported as clean.** When the outside reviewer (Codex or Claude Code) ended its reply with `Verdict: not ready` but listed only advisory P2 or P3 findings, `lib/outside-review-result.ts` printed `VERDICT: clean` and exited 0, so /ship and /codex carried on as if the reviewer had approved. The not-ready check only ran when no severity parsed at all, so a parsed P2 or P3 switched it off (#3329). Ported from #3364 by @mvanhorn.
+
+### Fixed
+- **`lib/outside-review-result.ts`**: when the reply's last recognized `Verdict:` line says `not ready`, the result is `findings` (exit 3) whatever severities parsed, and `FINDINGS:` still names the highest one. Advisory P2/P3 findings with a ready verdict, or with no verdict, stay `clean`. A `Verdict:` line quoted in a code fence or inline code still does not count, and the proposal and spec gates ignore it as before.
+- **`docs/troubleshooting.md`**: the "Every review gate" rule under *How the verdict is read from a review* now says a not-ready reply blocks even when P2 or P3 parsed.
+
+### Tests
+- `test/outside-review-unparsed-severity.test.ts` covers the issue's reply (numbered bold P2 plus P3, then `Verdict: not ready`) on the execution, structured and review gates, P3-only and unlabeled not-ready replies, the last verdict winning in both directions, bold, bulleted and heading verdict lines, and an unparsed label beside a parsed P2. Five of its tests fail on main's classifier and pass with the fix. Controls that stay `clean`: P2/P3 with `Verdict: ready` or no verdict, a not-ready verdict inside a code fence or inline code, and `ready-one-p3.txt`. Missing review markers, a failed execution, and the proposal and spec gates keep their old results.
+
+### Gate integrity
+- One gate edit, and it strengthens the gate: the pinned `bare-dash-p2.txt` fixture (two P2 lines, then `Verdict: not ready (2 P2)`) moves from `VERDICT: clean`, exit 0 to `VERDICT: findings`, exit 3. Garry approved this reversal on 2026-10-24. No threshold, budget, timeout, trial count or case kind changed.
+
 ## [1.91.197.0] - 2026-10-25
 
 **Pregate gives touched Rust files a test lane, and runs a nested pnpm or vitest package with its own runner (#3344).** In a mixed Cargo and JavaScript repo, every touched `.rs` file failed `lanes` with "has no test lane", so /ship could not get a receipt, and a nested package's vitest tests ran under `bun test` from the repo root (about 98 false failures in the reporter's repo). Ported from #3346 by vilano93, whose two commits carry the feature; our commits on top close four findings from a security-focused review of the port and make the new fixtures hermetic.

VERSION

@@ -1 +1 @@
-1.91.197.0
+1.91.198.0

agents-digest/gstack-AGENTS.md

@@ -1,4 +1,4 @@
-# gstack digest v1.91.197.0 — regenerate/re-copy after upgrading gstack
+# gstack digest v1.91.198.0 — regenerate/re-copy after upgrading gstack
 
 Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed
 for agent hosts without a full skill install. The full skills add workflows,

docs/troubleshooting.md

@@ -170,8 +170,10 @@ the readiness dashboard and the PR body, and never count it as a pass.
   findings.
 - **Every review gate.** These rules apply to every gate that summarises a
   review, including the `execution` gate behind `/codex` consult and challenge.
-  A reply whose last `Verdict:` line says `not ready` but carries no readable
-  severity prints `FINDINGS: unspecified` and exits 3 like a blocking finding.
+  A reply whose last `Verdict:` line says `not ready` is `findings` and exits
+  3 even when readable advisory severities (P2 or P3) were parsed; `FINDINGS:`
+  names the highest one. A not-ready reply with no readable severity still
+  prints `FINDINGS: unspecified` and exits 3.
 - **No findings.** A review with no tag and no label is `clean` only when its
   conclusion says so: the last sentence (or its last ", and" clause), or the
   `because` clause of its `Recommendation:` line, opens with `NO_FINDINGS`,

lib/outside-review-result.ts

@@ -263,7 +263,8 @@ export function classifyOutsideReview(input: OutsideReviewInput): OutsideReviewC
   }
   if (!reviewed) return result(blocking ? 'findings' : 'clean');
   const unparsed = firstUnparsed(code);
-  const notReady = !findings.highest && verdictNotReady(plainCode);
+  // A last explicit not-ready blocks even when the only parsed severities are advisory.
+  const notReady = verdictNotReady(plainCode);
   if (blocking || notReady) return { ...result('findings'), ...(unparsed ? { unparsed } : {}) };
   if (unparsed) return { ...result('unverified', 'unparsed_severity', unparsed), unparsed };
   if ((input.gate === 'structured' || input.gate === 'review') && !findings.highest && !concludesNoFindings(code)) {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "gstack",
-  "version": "1.91.197",
+  "version": "1.91.198",
   "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
   "license": "MIT",
   "type": "module",

test/outside-review-unparsed-severity.test.ts

@@ -5,12 +5,14 @@
  * FINDINGS: none` for all of them. The fixtures in
  * test/fixtures/outside-review-severity/codex-3105/ are synthesized from the
  * forms the issue documents (it describes 13 replies but does not include them).
+ * #3329: that same gate then printed `clean` for a parsed advisory finding
+ * whose last `Verdict:` line says `not ready`. `bare-dash-p2.txt` is unchanged.
  */
 import { describe, expect, test } from 'bun:test';
 import { spawnSync } from 'node:child_process';
 import * as fs from 'node:fs';
 import * as path from 'node:path';
-import { classifyOutsideReview, type OutsideGate } from '../lib/outside-review-result';
+import { classifyOutsideReview, validateOutsideReview, type OutsideGate } from '../lib/outside-review-result';
 import { GATE_OUTCOMES } from '../lib/gate-outcomes';
 
 const ROOT = path.resolve(import.meta.dir, '..');
@@ -26,7 +28,7 @@ describe('#3105 Codex reply shapes under the execution gate', () => {
   for (const [name, status, stdout] of [
     ['plan-bold-dash.txt', 3, 'VERDICT: findings\nFINDINGS: P1\n'],
     ['diff-numbered-dash.txt', 3, 'VERDICT: findings\nFINDINGS: P1\n'],
-    ['bare-dash-p2.txt', 0, 'VERDICT: clean\nFINDINGS: P2\n'],
+    ['bare-dash-p2.txt', 3, 'VERDICT: findings\nFINDINGS: P2\n'],
     ['ready-one-p3.txt', 0, 'VERDICT: clean\nFINDINGS: P3\n'],
     ['table-p1.txt', 3, 'VERDICT: findings\nFINDINGS: P1\n'],
     ['not-ready-unlabeled.txt', 3, 'VERDICT: findings\nFINDINGS: unspecified\n'],
@@ -112,3 +114,73 @@ describe('#3105 severity forms are read by every gate that summarises a review',
     expect(classifyOutsideReview({ text: proposal, gate: 'proposal' }).verdict).toBe('clean');
   });
 });
+
+describe('#3329 a last not-ready Verdict blocks even with advisory findings', () => {
+  const ISSUE = '1. **P2 — The docs lag the new flag.**\n2. **P3 — Rename the helper.**\nVerdict: not ready (1 P1/P2)';
+  const P2 = '1. **P2 — The docs lag the new flag.**';
+  const textFor = (gate: OutsideGate, text: string) => gate === 'review' ? `${text}\n${REC}` : text;
+  const see = (text: string, gate: OutsideGate) => {
+    const r = classifyOutsideReview({ text: textFor(gate, text), gate });
+    return { verdict: r.verdict as string, highest: r.findings.highest as string | null, reason: (r.reason ?? null) as string | null, unparsed: r.unparsed ?? null };
+  };
+
+  test('numbered bold P2 plus P3 with not ready is findings/P2 on execution, structured and review', () => {
+    for (const gate of ['execution', 'structured', 'review'] as const)
+      expect({ gate, ...see(ISSUE, gate) }).toEqual({ gate, verdict: 'findings', highest: 'P2', reason: null, unparsed: null });
+    expect(validateOutsideReview(ISSUE, 'structured')).toEqual({ completed: true, gate: 'fail' });
+  });
+
+  test('P3-only and unlabeled not-ready replies block; P0/P1 block even when the last Verdict says ready', () => {
+    for (const gate of ['execution', 'structured', 'review'] as const) {
+      expect({ gate, ...see('1. **P3 — Rename the helper.**\nVerdict: not ready', gate) }).toEqual({ gate, verdict: 'findings', highest: 'P3', reason: null, unparsed: null });
+      expect({ gate, ...see('The retry path can double-send.\nVerdict: not ready', gate) }).toEqual({ gate, verdict: 'findings', highest: null, reason: null, unparsed: null });
+    }
+    expect(see('1. **P1 — The cap is ignored.**\nVerdict: ready', 'review')).toMatchObject({ verdict: 'findings', highest: 'P1' });
+    expect(see('**P0 — Secrets reach the log.**\nVerdict: ready', 'execution')).toMatchObject({ verdict: 'findings', highest: 'P0' });
+  });
+
+  test('advisory P2/P3 with ready or no Verdict stay clean', () => {
+    for (const text of [
+      `${P2}\n2. **P3 — Rename the helper.**\nVerdict: ready`,
+      `${P2}\n2. **P3 — Rename the helper.**`,
+      '1. **P3 — Rename the helper.**\nVerdict: ready (1 P3)',
+    ]) {
+      const r = see(text, 'execution');
+      expect({ text, verdict: r.verdict, highest: r.highest }).toEqual({ text, verdict: 'clean', highest: text.includes('P2') ? 'P2' : 'P3' });
+    }
+    expect(see(`${P2}\nVerdict: ready`, 'review').verdict).toBe('clean');
+    expect(validateOutsideReview(`${P2}\nVerdict: ready`, 'structured')).toEqual({ completed: true, gate: 'pass' });
+  });
+
+  test('the last recognized Verdict wins, including supported formatting; quotes do not', () => {
+    expect(see(`${P2}\nVerdict: ready\nVerdict: not ready (1 P1/P2)`, 'execution')).toMatchObject({ verdict: 'findings', highest: 'P2' });
+    expect(see(`${P2}\nVerdict: not ready (1 P1/P2)\nVerdict: ready`, 'execution').verdict).toBe('clean');
+    for (const verdictLine of ['**Verdict:** not ready', '- Verdict: not ready', '### Verdict: not ready'])
+      expect({ verdictLine, ...see(`${P2}\n${verdictLine}`, 'execution') }).toMatchObject({ verdict: 'findings', highest: 'P2' });
+    expect(see(`${P2}\nVerdict: not ready\n### Verdict: ready`, 'execution').verdict).toBe('clean');
+    const fence = ['```', 'Verdict: not ready (2 P2)', '```'].join('\n');
+    expect(see(`${P2}\nVerdict: ready\n${fence}`, 'execution').verdict).toBe('clean');
+    expect(see(`${fence}\n${P2}\nVerdict: not ready`, 'execution')).toMatchObject({ verdict: 'findings', highest: 'P2' });
+    expect(see(`${P2}\nVerdict: ready\nThe rubric says \`Verdict: not ready\`.`, 'execution').verdict).toBe('clean');
+    expect(see(`${P2}\nThe rubric says \`Verdict: not ready\`.`, 'execution').verdict).toBe('clean');
+    expect(see(`No issues found.\n${fence}`, 'review').verdict).toBe('clean');
+  });
+
+  test('parsed P2 plus an unparsed label and not-ready still blocks and keeps the diagnostic', () => {
+    const text = `${P2}\n## P1/P2 findings\nVerdict: not ready`;
+    expect(see(text, 'execution')).toEqual({ verdict: 'findings', highest: 'P2', reason: null, unparsed: 'line 2: "## P1/P2 findings"' });
+    expect(see(`${P2}\n## P1/P2 findings`, 'structured')).toMatchObject({ verdict: 'unverified', reason: 'unparsed_severity', unparsed: 'line 2: "## P1/P2 findings"' });
+  });
+
+  test('missing markers and failed execution stay unavailable; proposal and spec ignore the verdict', () => {
+    expect(classifyOutsideReview({ text: `${P2}\nVerdict: not ready`, gate: 'review' }).reason).toBe('missing_markers');
+    const failed = classifyOutsideReview({ text: textFor('review', `${P2}\nVerdict: not ready`), gate: 'review', exit: 1, stderr: 'fatal: provider exploded' });
+    expect(failed.verdict).toBe('unavailable');
+    expect(failed.reason).toBe('execution_failed');
+    expect(classifyOutsideReview({ text: `${P2}\nVerdict: not ready\n${REC}`, gate: 'proposal' }).verdict).toBe('clean');
+    const passing = classifyOutsideReview({ text: `SCORE: 8\nAMBIGUITIES: NONE\n${P2}\nVerdict: not ready`, gate: 'spec' });
+    expect([passing.verdict, passing.score]).toEqual(['clean', 8]);
+    const failing = classifyOutsideReview({ text: 'SCORE: 6\nAMBIGUITIES: NONE\nVerdict: ready', gate: 'spec' });
+    expect([failing.verdict, failing.score]).toEqual(['findings', 6]);
+  });
+});