voyage/tests/lib/findings-schema.test.mjs

import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { spawnSync } from 'node:child_process';
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import {
  validateFindings,
  extractFindingsBlock,
  validateReviewerOutput,
  FINDING_REQUIRED_FIELDS,
} from '../../lib/review/findings-schema.mjs';

const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');

// ---- 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');
});

// ---- a file:line cited in a reviewer's prose is checked against the file ------

function repoWith33() {
  const dir = mkdtempSync(join(tmpdir(), 'voyage-prose-cite-'));
  mkdirSync(join(dir, 'tests'));
  writeFileSync(join(dir, 'tests', 'x.sh'), Array.from({ length: 33 }, (_, i) => `echo ${i + 1}`).join('\n') + '\n');
  return dir;
}

const proseFinding = (over = {}) => ({
  id: '0123456789abcdef0123456789abcdef01234567',
  severity: 'MAJOR', rule_key: 'UNIMPLEMENTED_CRITERION', file: 'tests/x.sh', line: 3,
  title: 'A title', detail: 'A detail.', recommended_action: 'Do one thing.', ...over,
});

test('validateFindings — a finding whose prose cites `tests/x.sh:119` in a 33-line file is refused', () => {
  const dir = repoWith33();
  try {
    const r = validateFindings(
      { reviewer: 'code-correctness-reviewer', findings: [proseFinding({ detail: 'The check at `tests/x.sh:119` relies on git.' })] },
      { repoRoot: dir },
    );
    assert.equal(r.valid, false);
    const e = r.errors.find((x) => x.code === 'FINDING_PROSE_LINE_OUT_OF_RANGE');
    assert.ok(e, JSON.stringify(r.errors));
    assert.match(e.message, /tests\/x\.sh:119/);
    assert.match(e.message, /33 lines/);
  } finally {
    rmSync(dir, { recursive: true, force: true });
  }
});

test('validateReviewerOutput — the same citation in prose outside the JSON block is a warning, never silent', () => {
  const dir = repoWith33();
  try {
    const raw = [
      '### Risk noted but not raised as a finding',
      '',
      '`tests/x.sh:119` relies on `git` and on a fixed commit.',
      '',
      '```json',
      JSON.stringify({ reviewer: 'code-correctness-reviewer', findings: [] }),
      '```',
    ].join('\n');
    const r = validateReviewerOutput(raw, { repoRoot: dir });
    assert.equal(r.valid, true, JSON.stringify(r.errors));
    const w = r.warnings.find((x) => x.code === 'FINDING_PROSE_LINE_OUT_OF_RANGE');
    assert.ok(w, JSON.stringify(r.warnings));
    assert.match(w.message, /reviewer prose cites `?tests\/x\.sh:119/);
  } finally {
    rmSync(dir, { recursive: true, force: true });
  }
});

test('validateFindings — a line that exists, a range ending in the file, a file absent from the tree and no repo root all pass', () => {
  const dir = repoWith33();
  try {
    const ok = (detail, opts = { repoRoot: dir }) =>
      validateFindings({ reviewer: 'r', findings: [proseFinding({ detail })] }, opts).valid;
    assert.equal(ok('See tests/x.sh:33 and tests/x.sh:10-12.'), true);
    assert.equal(ok('See src/absent.mjs:900.'), true);
    assert.equal(ok('See tests/x.sh:119.', {}), true);
    assert.equal(ok('See tests/x.sh:30-40.'), false, 'a range that runs past the end is out of range');
    assert.equal(ok('Served on http://localhost:8080 and example.com:443.'), true);
    assert.equal(ok('On Windows: C:\\work\\tests\\x.sh:119 and tests\\x.sh:119.'), true, 'a Windows path is not the root\'s file');
    assert.equal(ok('See tests/x.sh:30\u201440.'), false, 'an em-dash range is read as a range');
  } finally {
    rmSync(dir, { recursive: true, force: true });
  }
});

test('findings-schema CLI — what the check is given decides: the whole answer warns on a prose citation, the block alone cannot', () => {
  const dir = mkdtempSync(join(tmpdir(), 'findings-schema-cli-'));
  try {
    mkdirSync(join(dir, 'src'));
    writeFileSync(join(dir, 'src', 'a.mjs'), 'export const a = 1;\n');
    const block = '```json\n{"reviewer":"code-correctness-reviewer","findings":[]}\n```\n';
    writeFileSync(join(dir, 'whole.md'), `The bug is at \`src/a.mjs:40\`.\n\n${block}`);
    writeFileSync(join(dir, 'block.md'), block);
    const cli = (f) => spawnSync(process.execPath,
      [join(ROOT, 'lib', 'review', 'findings-schema.mjs'), '--json', '--repo-root', dir, join(dir, f)], { encoding: 'utf8' });
    const whole = JSON.parse(cli('whole.md').stdout);
    assert.deepEqual(whole.warnings.map((w) => w.code), ['FINDING_PROSE_LINE_OUT_OF_RANGE']);
    assert.deepEqual(JSON.parse(cli('block.md').stdout).warnings, []);
  } finally { rmSync(dir, { recursive: true, force: true }); }
});