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

// tests/lib/engine-findings.test.mjs
// Claude Code's /code-review and /security-review as the engine (ordre 5, P11):
// their transcribed findings become a coordinator payload, and meet Voyage's
// brief control in the same dedup + verdict.

import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { readFileSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { toReviewerPayload, ENGINES } from '../../lib/review/engine-findings.mjs';
import { runContract } from '../../lib/review/coordinator-contract.mjs';
import { validateFindings } from '../../lib/review/findings-schema.mjs';
import { getRule } from '../../lib/review/rule-catalogue.mjs';

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

test('engine-findings — a /code-review finding becomes a MAJOR CODE_REVIEW_DEFECT', () => {
  const r = toReviewerPayload({ engine: 'code-review', findings: [
    { file: './lib/a.mjs', line: 12, summary: 'Off-by-one drops the last page', failure_scenario: '10 items, page size 5 → page 2 empty', category: 'correctness' },
  ] });
  assert.equal(r.ok, true, r.errors.join('\n'));
  assert.equal(r.payload.reviewer, ENGINES['code-review'].reviewer);
  const [f] = r.payload.findings;
  assert.deepEqual([f.rule_key, f.severity, f.file, f.line], ['CODE_REVIEW_DEFECT', 'MAJOR', 'lib/a.mjs', 12]);
  assert.match(f.detail, /page 2 empty/);
  assert.equal(validateFindings(r.payload).valid, true);
});

test('engine-findings — a /security-review finding becomes a BLOCKER SECURITY_REVIEW_VULNERABILITY', () => {
  const r = toReviewerPayload({ engine: 'security-review', findings: [
    { file: 'lib/q.mjs', line: 4, summary: 'SQL built from request body' },
  ] });
  assert.equal(r.ok, true);
  assert.equal(r.payload.findings[0].severity, 'BLOCKER');
});

test('engine-findings — a narrower catalogue key given by the orchestrator is kept; an unknown one falls back', () => {
  const r = toReviewerPayload({ engine: 'code-review', findings: [
    { file: 'a.mjs', line: 1, summary: 's', rule_key: 'MISSING_TEST' },
    { file: 'b.mjs', line: 1, summary: 's', rule_key: 'MADE_UP' },
  ] });
  assert.deepEqual(r.payload.findings.map((f) => f.rule_key), ['MISSING_TEST', 'CODE_REVIEW_DEFECT']);
});

test('engine-findings — a file outside the review scope is listed, never dropped silently', () => {
  const r = toReviewerPayload({ engine: 'security-review', findings: [
    { file: 'in.mjs', line: 1, summary: 'a' },
    { file: 'old/out.mjs', line: 1, summary: 'b' },
  ] }, { scopeFiles: ['in.mjs'] });
  assert.equal(r.payload.findings.length, 1);
  assert.equal(r.out_of_scope.length, 1);
  assert.equal(r.out_of_scope[0].file, 'old/out.mjs');
});

test('engine-findings — empty file or summary is an input error, unknown engine too', () => {
  assert.equal(toReviewerPayload({ engine: 'code-review', findings: [{ file: '', summary: 'x' }] }).ok, false);
  assert.equal(toReviewerPayload({ engine: 'code-review', findings: [{ file: 'a', summary: '' }] }).ok, false);
  assert.equal(toReviewerPayload({ engine: 'simplify', findings: [] }).ok, false);
});

test('engine-findings — engine and brief control meet in one verdict; engine + Voyage on the same triplet collapse', () => {
  const engine = toReviewerPayload({ engine: 'code-review', findings: [
    { file: 'lib/a.mjs', line: 12, summary: 'No test covers the new branch', rule_key: 'MISSING_TEST' },
  ] }).payload;
  const conformance = { reviewer: 'brief-conformance-reviewer', findings: [
    { file: 'lib/a.mjs', line: 12, rule_key: 'MISSING_TEST', severity: 'MAJOR', title: 'SC2 untested' },
    { file: 'lib/b.mjs', line: 3, rule_key: 'UNIMPLEMENTED_CRITERION', severity: 'BLOCKER', title: 'SC3 missing' },
  ] };
  const result = runContract([engine, conformance],
    { expectedReviewers: [engine.reviewer, 'brief-conformance-reviewer'] });
  assert.equal(result.verdict, 'BLOCK', 'the brief control stops');
  assert.equal(result.findings.length, 2);
  const a = result.findings.find((f) => f.file === 'lib/a.mjs');
  assert.deepEqual([...a.raised_by].sort(), ['brief-conformance-reviewer', 'claude-code-review']);
});

test('engine-findings — a "narrower" key can never lower a security BLOCKER', () => {
  const r = toReviewerPayload({ engine: 'security-review', findings: [
    { file: 'a.mjs', line: 1, summary: 's', rule_key: 'MISSING_ERROR_HANDLING' }, // MINOR → refused
    { file: 'b.mjs', line: 1, summary: 's', rule_key: 'SECURITY_INJECTION' },     // BLOCKER → kept
  ] });
  assert.deepEqual(r.payload.findings.map((f) => [f.rule_key, f.severity]),
    [['SECURITY_REVIEW_VULNERABILITY', 'BLOCKER'], ['SECURITY_INJECTION', 'BLOCKER']]);
});

test('engine-findings — absolute paths under the repo root match the scope; a numeric-string line is kept', () => {
  const r = toReviewerPayload({ engine: 'code-review', findings: [
    { file: '/abs/repo/lib/a.mjs', line: '12', summary: 's' },
  ] }, { scopeFiles: ['lib/a.mjs'], repoRoot: '/abs/repo' });
  assert.equal(r.payload.findings.length, 1);
  assert.equal(r.payload.findings[0].file, 'lib/a.mjs');
  assert.equal(r.payload.findings[0].line, 12);
});

test('engine-findings — an empty scope list is an error, never "everything out of scope"', () => {
  const r = toReviewerPayload({ engine: 'security-review', findings: [{ file: 'a.mjs', summary: 's' }] }, { scopeFiles: [] });
  assert.equal(r.ok, false);
});

test('engine-findings — an out-of-scope engine finding forbids ALLOW and is declined to judge', () => {
  const r = toReviewerPayload({ engine: 'security-review', findings: [{ file: 'old/x.mjs', line: 2, summary: 'leak' }] },
    { scopeFiles: ['in.mjs'] });
  const result = runContract(
    [r.payload, { reviewer: 'brief-conformance-reviewer', findings: [] }],
    { expectedReviewers: [r.payload.reviewer, 'brief-conformance-reviewer'], engineOutOfScope: r.out_of_scope },
  );
  assert.equal(result.verdict, 'WARN');
  assert.deepEqual(result.declined_to_judge.map((d) => d.reason), ['engine:out-of-scope']);
});

test('engine-findings — an engine that was expected but never reported forbids ALLOW', () => {
  const result = runContract([{ reviewer: 'brief-conformance-reviewer', findings: [] }],
    { expectedReviewers: ['brief-conformance-reviewer', ENGINES['security-review'].reviewer] });
  assert.equal(result.verdict, 'WARN');
  assert.deepEqual(result.missing_reviewers, ['claude-security-review']);
});

const engineFinding = (extra = {}) => ({
  file: 'src/slug.mjs', line: 9,
  summary: 'A two-step detour where one split/filter/join would do',
  failure_scenario: 'slug("a--b") walks the string twice',
  ...extra,
});

// ---- an improvement from /code-review is not a MAJOR defect -------------------

test('engine-findings — a /code-review simplification or efficiency finding is MINOR, a correctness finding stays MAJOR', () => {
  const r = toReviewerPayload({ engine: 'code-review', findings: [
    engineFinding({ category: 'simplification' }),
    engineFinding({ line: 10, category: 'efficiency' }),
    engineFinding({ line: 11, category: 'correctness' }),
    engineFinding({ line: 12 }),
  ] });
  assert.equal(r.ok, true, r.errors.join('; '));
  const sev = r.payload.findings.map((f) => f.severity);
  assert.deepEqual(sev, ['MINOR', 'MINOR', 'MAJOR', 'MAJOR']);
  for (const f of r.payload.findings) assert.equal(f.severity, getRule(f.rule_key).severity, 'severity is fixed by rule_key');
});

test('engine-findings — /security-review keeps its tier whatever the category', () => {
  const r = toReviewerPayload({ engine: 'security-review', findings: [engineFinding({ category: 'simplification' })] });
  assert.equal(r.payload.findings[0].severity, 'BLOCKER');
});

test('engine-findings — a `reuse` finding is an improvement too', () => {
  const r = toReviewerPayload({ engine: 'code-review', findings: [engineFinding({ category: 'Reuse ' })] });
  assert.equal(r.payload.findings[0].severity, 'MINOR');
});

// ---- recommended_action is a remedy, not the failure scenario ----------------

test('engine-findings — the engine\'s own fix text becomes the recommended action', () => {
  const r = toReviewerPayload({ engine: 'code-review', findings: [engineFinding({ fix: 'Use one split/filter/join pass' })] });
  assert.equal(r.payload.findings[0].recommended_action, 'Use one split/filter/join pass');
});

test('engine-findings — without a fix text the action is a step: reproduce the scenario in a test, then change the location', () => {
  const r = toReviewerPayload({ engine: 'code-review', findings: [engineFinding()] });
  const a = r.payload.findings[0].recommended_action;
  assert.doesNotMatch(a, /so this no longer happens/);
  assert.match(a, /^Write a test that reproduces/);
  assert.match(a, /src\/slug\.mjs:9/);
});

test('engine-findings — /trekreview asks the transcription for the engine\'s fix text and forbids inventing one', () => {
  const md = readFileSync(join(ROOT, 'commands', 'trekreview.md'), 'utf8');
  const p = md.slice(md.indexOf('## Phase 4.7'), md.indexOf('## Phase 5'));
  assert.match(p, /the\s+fix\s+the\s+engine\s+proposes\s+\(`fix`\)/);
  assert.match(p, /never\s+write\s+one\s+the\s+engine\s+did\s+not\s+give/);
});