voyage/tests/lib/trial2-minor.test.mjs

// tests/lib/trial2-minor.test.mjs
// The minor points of the second voyage-lekerepo trial (2026-10-08, report
// parts B and C) that are behaviour, not prose. Each test was red before its
// fix.
//   B9  K10 — optional step field `Regression:`: one runnable command, or the
//       validator refuses it.
//   B10 K11 — `On failure:` opens with revert | retry | skip | escalate, or the
//       validator warns.
//   B11 two findings on the same file:line with the same content merge (the
//       trial's pair measured Jaccard 0.44 against a 0.7 threshold).
//   C14 `--evidence` shows the first output line on a PASS row, not "—".
//   C16 a file:line cited in a reviewer's prose is checked against the file.

import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { validatePlanContent } from '../../lib/validators/plan-validator.mjs';
import { onFailureAction, ON_FAILURE_ACTIONS } from '../../lib/parsers/plan-schema.mjs';
import { dedupFindings, DEFAULT_THRESHOLD } from '../../lib/review/plan-review-dedup.mjs';
import { formatCriteriaEvidence } from '../../lib/verification/criteria-runner.mjs';
import { validateFindings, validateReviewerOutput } from '../../lib/review/findings-schema.mjs';
import { buildPlan } from '../helpers/plan-v18-builder.mjs';

const codes = (list) => list.map((e) => e.code);

// Step 1's Verify line in the builder's plan; the new field goes right under it.
const STEP1_VERIFY = '- **Verify:** `node --test tests/mod1.test.mjs` → expected: pass';
const withRegression = (line, plan = buildPlan()) => plan.replace(STEP1_VERIFY, `${STEP1_VERIFY}\n${line}`);
const withOnFailure = (text, plan = buildPlan()) =>
  plan.replace(/^- \*\*On failure:\*\* .*$/m, `- **On failure:** ${text}`);

// --- B9: K10 `Regression:` ----------------------------------------------------

test('K10: `Regression: npm test | tee x` is refused — one command, unpiped', () => {
  const r = validatePlanContent(withRegression('- **Regression:** `npm test | tee x` → expected: exit 0'));
  assert.equal(r.valid, false);
  const e = r.errors.find((x) => x.code === 'PLAN_STEP_REGRESSION_NOT_RUNNABLE');
  assert.ok(e, codes(r.errors).join(','));
  assert.match(e.message, /Step 1/);
  assert.match(e.message, /shell operator/);
});

test('K10: `Regression: npm test` is accepted', () => {
  const r = validatePlanContent(withRegression('- **Regression:** `npm test` → expected: exit 0'));
  assert.deepEqual(codes(r.errors), []);
  assert.ok(!codes(r.warnings).includes('PLAN_STEP_REGRESSION_NOT_RUNNABLE'));
});

test('K10: without the field the plan validates exactly as before', () => {
  const r = validatePlanContent(buildPlan());
  assert.equal(r.valid, true, codes(r.errors).join(','));
  assert.deepEqual(codes(r.warnings), []);
});

test('K10: `npm test && bash tests/e2e.sh`, a bare word, or no backticks is refused', () => {
  for (const line of [
    '- **Regression:** `npm test && bash tests/e2e.sh`',
    '- **Regression:** `./run-all-tests`',
    '- **Regression:** npm test',
    '- **Regression:**',
  ]) {
    const r = validatePlanContent(withRegression(line));
    assert.ok(codes(r.errors).includes('PLAN_STEP_REGRESSION_NOT_RUNNABLE'), `${line}: ${codes(r.errors).join(',')}`);
  }
});

test('K10: --soft reports an unrunnable Regression as a warning, not an error', () => {
  const r = validatePlanContent(withRegression('- **Regression:** `npm test | tee x`'), { strict: false });
  assert.equal(r.valid, true, codes(r.errors).join(','));
  assert.ok(codes(r.warnings).includes('PLAN_STEP_REGRESSION_NOT_RUNNABLE'));
});

// --- B10: K11 `On failure:` ---------------------------------------------------

test('K11: `On failure: try again` warns, and the plan stays valid', () => {
  const r = validatePlanContent(withOnFailure('try again'));
  assert.equal(r.valid, true, codes(r.errors).join(','));
  const w = r.warnings.find((x) => x.code === 'PLAN_STEP_ON_FAILURE_UNKNOWN');
  assert.ok(w, codes(r.warnings).join(','));
  assert.match(w.message, /Step 1/);
  for (const word of ON_FAILURE_ACTIONS) assert.match(w.hint, new RegExp(word));
});

test('K11: `On failure: retry once, then escalate` gives no warning', () => {
  const r = validatePlanContent(withOnFailure('retry once, then escalate'));
  assert.ok(!codes(r.warnings).includes('PLAN_STEP_ON_FAILURE_UNKNOWN'), codes(r.warnings).join(','));
});

test('K11: the four words, in any case and with markdown around them, are read; a longer word is not', () => {
  assert.deepEqual([...ON_FAILURE_ACTIONS], ['revert', 'retry', 'skip', 'escalate']);
  assert.equal(onFailureAction('revert — `git checkout -- x`'), 'revert');
  assert.equal(onFailureAction('Escalate: stop the run'), 'escalate');
  assert.equal(onFailureAction('**skip** this step'), 'skip');
  assert.equal(onFailureAction('`retry` once'), 'retry');
  assert.equal(onFailureAction('retrying is fine'), null);
  assert.equal(onFailureAction('try again'), null);
  assert.equal(onFailureAction(''), null);
});

// --- B11: same file:line, same content ---------------------------------------

// The two findings from trial 2's round 1, verbatim (plan.md:248).
const PLAN_MD = '.claude/projects/2026-10-08-slug-sep-option/plan.md';
const CRITIC_248 = {
  severity: 'minor', rule_key: 'verification/contract-brief-form-missing', file: PLAN_MD, line: 248,
  text: "Verification runs `bash tests/e2e.sh` in the brief's own form but not `bash tests/contract.sh`, the form brief SC4 names.",
};
const GUARDIAN_248 = {
  severity: 'minor', rule_key: 'gap-verification-command-form', file: PLAN_MD, line: 248,
  text: "Brief SC4 runs 'bash tests/contract.sh'; Verification lists only '/bin/bash tests/contract.sh' (unlike e2e, where both forms are listed) - add the brief's form",
};
const sources = (a, b) => [
  { agent: 'plan-critic', payload: { findings: a } },
  { agent: 'scope-guardian', payload: { findings: b } },
];

test('B11: the two trial-2 findings on plan.md:248 merge into one, raised by both', () => {
  const r = dedupFindings(sources([CRITIC_248], [GUARDIAN_248]));
  assert.equal(r.findings.length, 1, JSON.stringify(r.findings, null, 2));
  assert.deepEqual(r.findings[0].raised_by, ['plan-critic', 'scope-guardian']);
  assert.equal(r.dedup_stats.anchor_dups, 1);
});

test('B11: two different findings on the same line stay two', () => {
  const other = {
    severity: 'minor', rule_key: 'verification/no-negative-case', file: PLAN_MD, line: 248,
    text: 'No criterion feeds an invalid separator such as a letter and checks the exit code.',
  };
  const r = dedupFindings(sources([CRITIC_248], [other]));
  assert.equal(r.findings.length, 2);
});

test('B11: a short finding held by a longer one on the same line stays its own (independent review)', () => {
  const at = (text, severity = 'minor') => ({ severity, rule_key: `k-${text.length}`, file: PLAN_MD, line: 40, text });
  for (const [a, b] of [
    ['The token is checked.', 'The token is not checked before the write.'],
    ['Step 3 test is missing.', 'Step 3 is missing a checkpoint; the test file already exists.'],
    ['Verify runs npm test.', 'Verify must not run npm test, it is green before the step.'],
  ]) {
    assert.equal(dedupFindings(sources([at(a)], [at(b)])).findings.length, 2, `${a} | ${b}`);
  }
});

test('B11: one reviewer\'s two findings on one line are never merged by the anchor rule', () => {
  const r = dedupFindings([{ agent: 'plan-critic', payload: { findings: [CRITIC_248, { ...GUARDIAN_248, rule_key: 'other' }] } }]);
  assert.equal(r.findings.length, 2);
  assert.equal(r.dedup_stats.anchor_dups, 0);
});

test('B11: the same content on another line is not merged by the anchor rule; the threshold elsewhere is unchanged', () => {
  assert.equal(DEFAULT_THRESHOLD, 0.7);
  const r = dedupFindings(sources([CRITIC_248], [{ ...GUARDIAN_248, line: 249 }]));
  assert.equal(r.findings.length, 2);
  const noLine = dedupFindings(sources([{ ...CRITIC_248, line: 0 }], [{ ...GUARDIAN_248, line: 0 }]));
  assert.equal(noLine.findings.length, 2, 'line 0 is file-scoped, not an anchor');
});

// --- C14: --evidence on a PASS row ---------------------------------------------

const evidenceRep = (results) => ({
  source: 'brief.md',
  heading: '## Success Criteria',
  error: null,
  results,
  summary: { passed: 1, failed: 0, blocked: 0, unrunnable: 0, total: 1, ok: true },
});

test('C14: --evidence shows the first line of output on a passed criterion', () => {
  const block = formatCriteriaEvidence(evidenceRep([
    { label: 'SC1', text: 't', command: 'npm test', status: 'passed', exitCode: 0, output: '\n# tests 12\n# pass 12\n' },
  ]));
  assert.match(block, /\| SC1 \| `npm test` \| PASS \| exit 0 \| # tests 12 \|/);
});

test('C14: a passed criterion with no output still says "—"', () => {
  const block = formatCriteriaEvidence(evidenceRep([
    { label: 'SC1', text: 't', command: 'npm test', status: 'passed', exitCode: 0, output: '' },
  ]));
  assert.match(block, /\| SC1 \| `npm test` \| PASS \| exit 0 \| — \|/);
});

// --- C16: file:line in prose --------------------------------------------------

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 finding = (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('C16: 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: [finding({ 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('C16: the same citation in prose outside the JSON block is a warning, never silent (a JSON-only re-ask cannot fix prose)', () => {
  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('C16: 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: [finding({ 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 });
  }
});