voyage/tests/fixtures/bakeoff/delivered.diff

diff --git a/commands/trekreview.md b/commands/trekreview.md
index 4fd24b3..288072d 100644
--- a/commands/trekreview.md
+++ b/commands/trekreview.md
@@ -199,9 +199,25 @@ Each reviewer prompt includes:
 - **Brief path** — `{brief_path}` (read on demand; do not inline).
 - **Rule catalogue** — reference to `lib/review/rule-catalogue.mjs`.
 
-Collect each reviewer's trailing JSON block (last fenced `json` block in
-their output). Parse with `JSON.parse()`. On parse error, ask the agent
-to re-emit the JSON only.
+Collect each reviewer's trailing JSON block and **validate it against the
+reviewer-output schema** rather than merely parsing it. Run:
+
+```bash
+node ${CLAUDE_PLUGIN_ROOT}/lib/review/findings-schema.mjs --json <reviewer-output-file>
+```
+
+`validateReviewerOutput` in `lib/review/findings-schema.mjs` extracts the
+last fenced `json` block, parses it, and schema-checks every finding
+(load-bearing fields: `file`, `rule_key` ∈ catalogue, `severity` ∈ enum,
+`line` integer ≥ 0). Parse failure and schema failure surface through the
+same stable error codes (`FINDINGS_NO_JSON_BLOCK`, `FINDINGS_PARSE_ERROR`,
+`FINDING_*`).
+
+On any failure, re-ask **that reviewer** to re-emit a conforming JSON
+block only — quote the reported error codes/locations so the fix is
+targeted. **Bounded retries: N=2.** If the output still fails after 2
+re-asks, stop and report which reviewer produced non-conforming output;
+do not feed unvalidated findings to the coordinator.
 
 In `quick` mode, launch only `code-correctness-reviewer`. The Executive
 Summary will note the brief-conformance pass was skipped.
diff --git a/lib/review/findings-schema.mjs b/lib/review/findings-schema.mjs
new file mode 100644
index 0000000..fb09c72
--- /dev/null
+++ b/lib/review/findings-schema.mjs
@@ -0,0 +1,175 @@
+// lib/review/findings-schema.mjs
+// Reviewer-output JSON schema contract for /trekreview Phase 5 (NW1).
+//
+// brief-conformance-reviewer and code-correctness-reviewer each emit a trailing
+// fenced `json` block of shape:
+//
+//   { "reviewer": "<name>", "findings": [ { id, severity, rule_key, file, line,
+//                                           brief_ref, title, detail,
+//                                           recommended_action }, ... ] }
+//
+// This module codifies that contract so main can VALIDATE each reviewer's JSON
+// (not merely JSON.parse it) and re-ask on *schema* failure as well as parse
+// failure, replacing the fragile "parse the last json block" contract that used
+// to live in commands/trekreview.md (the :202–204 prose).
+//
+// Load-bearing fields (the downstream dedup triplet + verdict severity) are hard
+// errors: file, rule_key, severity, line. Unknown rule_keys are errors too — the
+// catalogue is the contract. Descriptive fields (title/detail/recommended_action/
+// brief_ref) and unknown top-level keys are tolerated (forward-compat, mirroring
+// review-validator.mjs).
+//
+// 3-layer pattern (Content → Raw-text → CLI shim) mirroring the other validators.
+
+import { readFileSync, existsSync } from 'node:fs';
+import { issue, fail } from '../util/result.mjs';
+import { RULE_KEYS, SEVERITY_VALUES } from './rule-catalogue.mjs';
+
+// The fields main + the coordinator depend on. Descriptive fields are not here
+// on purpose: a missing recommended_action should not trigger a re-ask.
+export const FINDING_REQUIRED_FIELDS = Object.freeze([
+  'severity',
+  'rule_key',
+  'file',
+  'line',
+]);
+
+// Last fenced ```json … ``` block in a reviewer's output. The contract pins the
+// JSON block as the LAST fence so prose above it never confuses the parser.
+const JSON_FENCE_GLOBAL = /```json[ \t]*\r?\n([\s\S]*?)```/gi;
+
+/**
+ * Extract the inner body of the LAST fenced `json` block in `text`.
+ * @param {string} text
+ * @returns {string|null} the JSON source, or null if no json fence is present.
+ */
+export function extractFindingsBlock(text) {
+  if (typeof text !== 'string') return null;
+  JSON_FENCE_GLOBAL.lastIndex = 0;
+  let last = null;
+  let m;
+  while ((m = JSON_FENCE_GLOBAL.exec(text)) !== null) {
+    last = m[1];
+  }
+  return last;
+}
+
+function validateFinding(finding, index, errors) {
+  const loc = `findings[${index}]`;
+  if (finding === null || typeof finding !== 'object' || Array.isArray(finding)) {
+    errors.push(issue('FINDING_NOT_OBJECT', `${loc} is not an object`, undefined, loc));
+    return;
+  }
+
+  if (typeof finding.file !== 'string' || finding.file.length === 0) {
+    errors.push(issue('FINDING_MISSING_FILE', `${loc}.file must be a non-empty string`, undefined, loc));
+  }
+
+  if (typeof finding.rule_key !== 'string' || finding.rule_key.length === 0) {
+    errors.push(issue('FINDING_MISSING_RULE_KEY', `${loc}.rule_key must be a non-empty string`, undefined, loc));
+  } else if (!RULE_KEYS.has(finding.rule_key)) {
+    errors.push(issue(
+      'FINDING_UNKNOWN_RULE_KEY',
+      `${loc}.rule_key "${finding.rule_key}" is not in the rule catalogue`,
+      'Use a rule_key from lib/review/rule-catalogue.mjs',
+      loc,
+    ));
+  }
+
+  if (typeof finding.severity !== 'string' || !SEVERITY_VALUES.includes(finding.severity)) {
+    errors.push(issue(
+      'FINDING_BAD_SEVERITY',
+      `${loc}.severity must be one of ${SEVERITY_VALUES.join('|')}, got ${JSON.stringify(finding.severity)}`,
+      undefined,
+      loc,
+    ));
+  }
+
+  if (typeof finding.line !== 'number' || !Number.isInteger(finding.line) || finding.line < 0) {
+    errors.push(issue(
+      'FINDING_BAD_LINE',
+      `${loc}.line must be an integer ≥ 0, got ${JSON.stringify(finding.line)}`,
+      'Use 0 for file-scoped findings without a specific line.',
+      loc,
+    ));
+  }
+}
+
+/**
+ * Validate an already-parsed reviewer-output payload against the schema.
+ * Accumulates every error (so a re-ask can name all problems at once).
+ * @param {unknown} payload
+ * @returns {import('../util/result.mjs').Result}
+ */
+export function validateFindings(payload) {
+  if (payload === null || typeof payload !== 'object' || Array.isArray(payload)) {
+    return fail(issue('FINDINGS_NOT_OBJECT', `Reviewer output must be a JSON object, got ${Array.isArray(payload) ? 'array' : typeof payload}`));
+  }
+
+  const errors = [];
+  const warnings = [];
+
+  if (typeof payload.reviewer !== 'string' || payload.reviewer.length === 0) {
+    warnings.push(issue('FINDINGS_MISSING_REVIEWER', 'Reviewer output should carry a non-empty "reviewer" name'));
+  }
+
+  if (!Array.isArray(payload.findings)) {
+    errors.push(issue('FINDINGS_NOT_ARRAY', `Field "findings" must be an array, got ${typeof payload.findings}`));
+    return { valid: false, errors, warnings, parsed: payload };
+  }
+
+  for (let i = 0; i < payload.findings.length; i++) {
+    validateFinding(payload.findings[i], i, errors);
+  }
+
+  return { valid: errors.length === 0, errors, warnings, parsed: payload };
+}
+
+/**
+ * Validate a reviewer's raw output text: extract the last json fence, parse it,
+ * then schema-validate. Parse-stage failures get stable codes so they flow
+ * through the same bounded re-ask path as schema failures.
+ * @param {string} rawText
+ * @returns {import('../util/result.mjs').Result}
+ */
+export function validateReviewerOutput(rawText) {
+  const block = extractFindingsBlock(rawText);
+  if (block === null) {
+    return fail(issue(
+      'FINDINGS_NO_JSON_BLOCK',
+      'No trailing fenced ```json block found in reviewer output',
+      'Reviewers must end their output with a single ```json findings block.',
+    ));
+  }
+  let parsed;
+  try {
+    parsed = JSON.parse(block);
+  } catch (e) {
+    return fail(issue('FINDINGS_PARSE_ERROR', `Reviewer JSON block did not parse: ${e.message}`));
+  }
+  return validateFindings(parsed);
+}
+
+// ---- CLI shim ----------------------------------------------------------------
+
+if (import.meta.url === `file://${process.argv[1]}`) {
+  const args = process.argv.slice(2);
+  const filePath = args.find((a) => !a.startsWith('--'));
+  if (!filePath) {
+    process.stderr.write('Usage: findings-schema.mjs [--json] <reviewer-output.txt|.md>\n');
+    process.exit(2);
+  }
+  if (!existsSync(filePath)) {
+    process.stderr.write(`findings-schema: file not found: ${filePath}\n`);
+    process.exit(2);
+  }
+  const r = validateReviewerOutput(readFileSync(filePath, 'utf-8'));
+  if (args.includes('--json')) {
+    process.stdout.write(JSON.stringify({ valid: r.valid, errors: r.errors, warnings: r.warnings }, null, 2) + '\n');
+  } else {
+    process.stdout.write(`findings-schema: ${r.valid ? 'PASS' : 'FAIL'} ${filePath}\n`);
+    for (const e of r.errors) process.stderr.write(`  ERROR  [${e.code}] ${e.message}\n`);
+    for (const w of r.warnings) process.stderr.write(`  WARN   [${w.code}] ${w.message}\n`);
+  }
+  process.exit(r.valid ? 0 : 1);
+}
diff --git a/tests/lib/findings-schema.test.mjs b/tests/lib/findings-schema.test.mjs
new file mode 100644
index 0000000..e94a7c6
--- /dev/null
+++ b/tests/lib/findings-schema.test.mjs
@@ -0,0 +1,244 @@
+import { test } from 'node:test';
+import { strict as assert } from 'node:assert';
+import {
+  validateFindings,
+  extractFindingsBlock,
+  validateReviewerOutput,
+  FINDING_REQUIRED_FIELDS,
+} from '../../lib/review/findings-schema.mjs';
+
+// ---- helpers ----------------------------------------------------------------
+
+function validFinding(over = {}) {
+  return {
+    id: '0123456789abcdef0123456789abcdef01234567',
+    severity: 'BLOCKER',
+    rule_key: 'UNIMPLEMENTED_CRITERION',
+    file: 'lib/foo.mjs',
+    line: 0,
+    brief_ref: 'SC2 — exact quoted criterion text',
+    title: 'Short imperative title',
+    detail: 'Multi-sentence explanation citing concrete diff evidence',
+    recommended_action: 'Imperative, single-step recommendation',
+    ...over,
+  };
+}
+
+function validPayload(findings = [validFinding()]) {
+  return { reviewer: 'brief-conformance-reviewer', findings };
+}
+
+function codes(result) {
+  return result.errors.map((e) => e.code);
+}
+
+// ---- exports ----------------------------------------------------------------
+
+test('module exports the validator surface', () => {
+  assert.equal(typeof validateFindings, 'function');
+  assert.equal(typeof extractFindingsBlock, 'function');
+  assert.equal(typeof validateReviewerOutput, 'function');
+  assert.ok(Array.isArray(FINDING_REQUIRED_FIELDS));
+  for (const f of ['severity', 'rule_key', 'file', 'line']) {
+    assert.ok(FINDING_REQUIRED_FIELDS.includes(f), `${f} should be a required field`);
+  }
+});
+
+// ---- validateFindings: happy path ------------------------------------------
+
+test('validateFindings — accepts a well-formed findings array', () => {
+  const r = validateFindings(validPayload());
+  assert.equal(r.valid, true, JSON.stringify(r.errors));
+  assert.deepEqual(r.errors, []);
+});
+
+test('validateFindings — accepts both reviewer example shapes', () => {
+  const conformance = validateFindings(validPayload([validFinding()]));
+  const correctness = validateFindings({
+    reviewer: 'code-correctness-reviewer',
+    findings: [validFinding({ rule_key: 'SECURITY_INJECTION', file: 'lib/exec.mjs', line: 23 })],
+  });
+  assert.equal(conformance.valid, true);
+  assert.equal(correctness.valid, true);
+});
+
+test('validateFindings — accepts an empty findings array', () => {
+  const r = validateFindings(validPayload([]));
+  assert.equal(r.valid, true);
+});
+
+test('validateFindings — accepts line: 0 (file-scoped finding)', () => {
+  const r = validateFindings(validPayload([validFinding({ line: 0 })]));
+  assert.equal(r.valid, true);
+});
+
+test('validateFindings — tolerates unknown top-level keys (forward-compat)', () => {
+  const payload = { ...validPayload(), future_field: 'whatever' };
+  const r = validateFindings(payload);
+  assert.equal(r.valid, true);
+});
+
+// ---- validateFindings: the four named malformations ------------------------
+
+test('validateFindings — rejects missing rule_key → FINDING_MISSING_RULE_KEY', () => {
+  const f = validFinding();
+  delete f.rule_key;
+  const r = validateFindings(validPayload([f]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_MISSING_RULE_KEY'), codes(r).join(','));
+});
+
+test('validateFindings — rejects unknown rule_key → FINDING_UNKNOWN_RULE_KEY', () => {
+  const r = validateFindings(validPayload([validFinding({ rule_key: 'NOT_A_REAL_RULE' })]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_UNKNOWN_RULE_KEY'), codes(r).join(','));
+});
+
+test('validateFindings — rejects bad severity enum → FINDING_BAD_SEVERITY', () => {
+  const r = validateFindings(validPayload([validFinding({ severity: 'CRITICAL' })]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_BAD_SEVERITY'), codes(r).join(','));
+});
+
+test('validateFindings — rejects missing severity → FINDING_BAD_SEVERITY', () => {
+  const f = validFinding();
+  delete f.severity;
+  const r = validateFindings(validPayload([f]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_BAD_SEVERITY'));
+});
+
+test('validateFindings — rejects non-numeric line → FINDING_BAD_LINE', () => {
+  const r = validateFindings(validPayload([validFinding({ line: '23' })]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_BAD_LINE'), codes(r).join(','));
+});
+
+test('validateFindings — rejects non-integer line → FINDING_BAD_LINE', () => {
+  const r = validateFindings(validPayload([validFinding({ line: 23.5 })]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_BAD_LINE'));
+});
+
+test('validateFindings — rejects negative line → FINDING_BAD_LINE', () => {
+  const r = validateFindings(validPayload([validFinding({ line: -1 })]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_BAD_LINE'));
+});
+
+test('validateFindings — rejects missing file → FINDING_MISSING_FILE', () => {
+  const f = validFinding();
+  delete f.file;
+  const r = validateFindings(validPayload([f]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_MISSING_FILE'), codes(r).join(','));
+});
+
+test('validateFindings — rejects empty-string file → FINDING_MISSING_FILE', () => {
+  const r = validateFindings(validPayload([validFinding({ file: '' })]));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_MISSING_FILE'));
+});
+
+// ---- validateFindings: top-level shape -------------------------------------
+
+test('validateFindings — rejects non-object payload → FINDINGS_NOT_OBJECT', () => {
+  for (const bad of [null, undefined, 42, 'x', []]) {
+    const r = validateFindings(bad);
+    assert.equal(r.valid, false);
+    assert.ok(codes(r).includes('FINDINGS_NOT_OBJECT'), `${JSON.stringify(bad)} → ${codes(r)}`);
+  }
+});
+
+test('validateFindings — rejects missing/non-array findings → FINDINGS_NOT_ARRAY', () => {
+  assert.ok(codes(validateFindings({ reviewer: 'x' })).includes('FINDINGS_NOT_ARRAY'));
+  assert.ok(codes(validateFindings({ reviewer: 'x', findings: {} })).includes('FINDINGS_NOT_ARRAY'));
+});
+
+test('validateFindings — rejects non-object finding element → FINDING_NOT_OBJECT', () => {
+  const r = validateFindings(validPayload(['not-an-object']));
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_NOT_OBJECT'));
+});
+
+test('validateFindings — missing reviewer is a non-blocking warning', () => {
+  const r = validateFindings({ findings: [] });
+  assert.equal(r.valid, true);
+  assert.ok(r.warnings.some((w) => w.code === 'FINDINGS_MISSING_REVIEWER'));
+});
+
+// ---- validateFindings: error shape + accumulation --------------------------
+
+test('validateFindings — every error has stable {code, message} + per-finding location', () => {
+  const r = validateFindings(validPayload([validFinding({ file: '', severity: 'NOPE' })]));
+  assert.equal(r.valid, false);
+  for (const e of r.errors) {
+    assert.ok(typeof e.code === 'string' && e.code.length > 0);
+    assert.ok(typeof e.message === 'string' && e.message.length > 0);
+    assert.ok(typeof e.location === 'string' && e.location.includes('findings[0]'));
+  }
+});
+
+test('validateFindings — accumulates errors across multiple bad findings', () => {
+  const r = validateFindings(validPayload([
+    validFinding({ rule_key: undefined }),
+    validFinding({ line: 'x' }),
+  ]));
+  assert.equal(r.valid, false);
+  assert.ok(r.errors.some((e) => e.code === 'FINDING_MISSING_RULE_KEY' && e.location.includes('[0]')));
+  assert.ok(r.errors.some((e) => e.code === 'FINDING_BAD_LINE' && e.location.includes('[1]')));
+});
+
+// ---- extractFindingsBlock ---------------------------------------------------
+
+test('extractFindingsBlock — extracts the LAST json fence', () => {
+  const text = [
+    '## Prose',
+    '```json',
+    '{"reviewer":"early","findings":[]}',
+    '```',
+    'more prose',
+    '```json',
+    '{"reviewer":"last","findings":[]}',
+    '```',
+  ].join('\n');
+  const block = extractFindingsBlock(text);
+  assert.ok(block.includes('"last"'));
+  assert.ok(!block.includes('"early"'));
+});
+
+test('extractFindingsBlock — returns null when no json fence present', () => {
+  assert.equal(extractFindingsBlock('just prose, no fence'), null);
+  assert.equal(extractFindingsBlock(''), null);
+  assert.equal(extractFindingsBlock(123), null);
+});
+
+// ---- validateReviewerOutput (raw text → extract → parse → schema) -----------
+
+test('validateReviewerOutput — valid raw output round-trips to valid', () => {
+  const raw = `## Review\n\nprose here\n\n\`\`\`json\n${JSON.stringify(validPayload(), null, 2)}\n\`\`\`\n`;
+  const r = validateReviewerOutput(raw);
+  assert.equal(r.valid, true, JSON.stringify(r.errors));
+});
+
+test('validateReviewerOutput — no json block → FINDINGS_NO_JSON_BLOCK', () => {
+  const r = validateReviewerOutput('prose with no fenced json');
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDINGS_NO_JSON_BLOCK'));
+});
+
+test('validateReviewerOutput — malformed JSON → FINDINGS_PARSE_ERROR', () => {
+  const raw = '```json\n{"reviewer":"x","findings":[],}\n```'; // trailing comma
+  const r = validateReviewerOutput(raw);
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDINGS_PARSE_ERROR'));
+});
+
+test('validateReviewerOutput — well-formed JSON but schema-invalid surfaces schema code', () => {
+  const bad = { reviewer: 'x', findings: [{ severity: 'BLOCKER', file: 'a.mjs', line: 1 }] }; // no rule_key
+  const raw = `\`\`\`json\n${JSON.stringify(bad)}\n\`\`\``;
+  const r = validateReviewerOutput(raw);
+  assert.equal(r.valid, false);
+  assert.ok(codes(r).includes('FINDING_MISSING_RULE_KEY'),
+    'parse and schema validation must flow through the same result');
+});