voyage/tests/lib/review-recurrence.test.mjs
Kjell Tore Guttormsen 40f6ae1177
fix(review): close the gate's fail-open paths an independent review found
An adversarial read of d88245c reproduced 15 problems; all fixed here, each
with a test that fails on the old code.

- verdict-gate: a greener verdict must be backed by the LAST fence being a
  `json` block of the right shape, with no BLOCKER in the JSON (any case),
  in `## Findings (BLOCKER)`, or a differing finding-id list; two verdict
  lines and more than one file argument are exit 2. ({}, null, lowercase
  "blocker", a trailing ```jsonc and a later json fence all gave exit 0.)
- engine-findings: a transcribed "narrower" key can no longer lower an
  engine tier; absolute paths under --repo-root match the scope; "12" lines
  parse; an empty scope is an error; out-of-scope engine findings are
  unverified (`engine:out-of-scope`) and forbid ALLOW.
- recurrence: the catch-all engine keys and keys normalised from unknown
  ones are never proposed; the same unfixed finding in two iterations is not
  a recurrence; proposals are validated before they are offered; \r/U+2028
  and damaged markers are refused; writes go through a symlinked CLAUDE.md.
  Prose points --repo at the session directory, not the git toplevel.
- Workflow substrate: engines are counted from what arrived, and the
  verdict's floor is recomputed from the coordinator's findings; now tested
  by running the script with stubbed Workflow globals. Two literal NUL bytes
  written as \u0000 (git saw the file as binary).
- Trimming keeps Pass 1's "Also flagged by" provenance; `## Declined to
  Judge` and remediation steps are required from review_version 1.1 only
  (N-1); the zero-diff review now carries the JSON block the gate needs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-06 20:03:16 +02:00

309 lines
14 KiB
JavaScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

// tests/lib/review-recurrence.test.mjs
// P5 — the project's memory (ordre 5). A finding class (the same rule_key) that
// shows up in two review.md files in the target repo becomes ONE proposal; one
// sighting is no proposal. Proposals are read from the review.md files the repo
// already has (no new store), and an approved one is written only between
// Voyage's own markers, under a size budget, with CLAUDE.md getting a pointer.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { spawnSync } from 'node:child_process';
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, existsSync, symlinkSync, lstatSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const CLI = join(ROOT, 'lib', 'review', 'recurrence.mjs');
const mod = existsSync(CLI) ? await import('../../lib/review/recurrence.mjs') : null;
function reviewText({ slug, sha, findings }) {
const json = JSON.stringify({
verdict: 'WARN',
counts: { BLOCKER: 0, MAJOR: findings.length, MINOR: 0, SUGGESTION: 0 },
findings,
}, null, 2);
return `---
type: trekreview
review_version: "1.1"
task: "${slug}"
slug: ${slug}
project_dir: .claude/projects/2026-10-0${sha.length % 9}-${slug}/
brief_path: .claude/projects/x/brief.md
scope_sha_end: ${sha}
reviewed_files_count: 1
verdict: WARN
findings:
${findings.map((f) => ` - ${f.id}`).join('\n')}
---
# Review: ${slug}
## Executive Summary
x
## Coverage
x
## Remediation Summary
x
\`\`\`json
${json}
\`\`\`
`;
}
function finding(id, rule_key, extra = {}) {
return {
id: id.padEnd(40, '0'), severity: 'MAJOR', rule_key, file: 'lib/a.mjs', line: 3,
title: `${rule_key} title ${id}`, detail: 'd', recommended_action: `Fix it (${id}).`, ...extra,
};
}
function repoWith(reviews) {
const dir = mkdtempSync(join(tmpdir(), 'recurrence-'));
reviews.forEach((r, i) => {
const p = join(dir, '.claude', 'projects', `2026-10-0${i + 1}-${r.slug}`);
mkdirSync(p, { recursive: true });
writeFileSync(join(p, 'review.md'), reviewText(r));
});
return dir;
}
test('recurrence — the module exists', () => {
assert.ok(mod, 'lib/review/recurrence.mjs must exist (P5)');
});
test('recurrence — two review.md with the same finding class → exactly one proposal', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST', { file: 'lib/b.mjs' })] },
]);
try {
const { proposals } = mod.proposeLessons(repo);
assert.equal(proposals.length, 1);
assert.equal(proposals[0].rule_key, 'MISSING_TEST');
assert.equal(proposals[0].evidence.length, 2, 'the proposal carries both findings as its evidence');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — one review.md with the class → no proposal', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'PLACEHOLDER_IN_CODE')] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — the same class twice inside ONE review is still one sighting → no proposal', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST'), finding('a2', 'MISSING_TEST', { line: 9 })] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — routing: mechanical → a check, judgement → a .claude/rules/ file', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'PLACEHOLDER_IN_CODE'), finding('a2', 'PLAN_EXECUTE_DRIFT')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'PLACEHOLDER_IN_CODE'), finding('b2', 'PLAN_EXECUTE_DRIFT')] },
]);
try {
const byKey = Object.fromEntries(mod.proposeLessons(repo).proposals.map((p) => [p.rule_key, p]));
assert.equal(byKey.PLACEHOLDER_IN_CODE.route, 'check');
assert.equal(byKey.PLAN_EXECUTE_DRIFT.route, 'rule');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — the proposed line has the three parts: mistake · fix · how to check', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
const [p] = mod.proposeLessons(repo).proposals;
assert.match(p.line, /Mistake:/);
assert.match(p.line, /Fix:/);
assert.match(p.line, /How to check:/);
assert.equal(mod.validateLessonLine(p.line).valid, true);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — review-hygiene classes never become target-repo lessons', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_BRIEF_REF'), finding('a2', 'COVERAGE_SILENT_SKIP')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_BRIEF_REF'), finding('b2', 'COVERAGE_SILENT_SKIP')] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — apply writes only between Voyage markers and gives CLAUDE.md a pointer only', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
writeFileSync(join(repo, 'CLAUDE.md'), '# Project\n\nOperator text stays.\n');
mkdirSync(join(repo, '.claude', 'rules'), { recursive: true });
writeFileSync(join(repo, '.claude', 'rules', mod.LESSONS_FILE_NAME), 'Operator preface.\n');
const r = mod.applyLesson(repo, 'MISSING_TEST');
assert.equal(r.written, true, JSON.stringify(r));
const rules = readFileSync(join(repo, '.claude', 'rules', mod.LESSONS_FILE_NAME), 'utf-8');
assert.ok(rules.startsWith('Operator preface.\n'), 'text outside the markers is untouched');
const start = rules.indexOf(mod.MARK_START);
const end = rules.indexOf(mod.MARK_END);
assert.ok(start >= 0 && end > start);
assert.match(rules.slice(start, end), /MISSING_TEST/);
const claude = readFileSync(join(repo, 'CLAUDE.md'), 'utf-8');
assert.ok(claude.startsWith('# Project\n\nOperator text stays.\n'));
assert.ok(claude.includes(mod.LESSONS_FILE_NAME), 'CLAUDE.md gets a pointer to the rules file');
assert.ok(!claude.includes('Mistake:'), 'CLAUDE.md gets ONLY the pointer, never the lesson');
assert.deepEqual(mod.proposeLessons(repo).proposals, [], 'an applied class is not proposed again');
const again = mod.applyLesson(repo, 'MISSING_TEST');
assert.equal(again.written, false, 'applying twice writes nothing');
const claude2 = readFileSync(join(repo, 'CLAUDE.md'), 'utf-8');
assert.equal(claude2, claude, 'the pointer is written once');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — apply refuses past the size budget (the lesson belongs in a skill then)', () => {
const keys = ['MISSING_TEST', 'PLACEHOLDER_IN_CODE', 'PLAN_EXECUTE_DRIFT', 'SCOPE_CREEP_BUILT'];
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: keys.map((k, i) => finding(`a${i}`, k)) },
{ slug: 'two', sha: 'bbb', findings: keys.map((k, i) => finding(`b${i}`, k)) },
]);
try {
const r = mod.applyLesson(repo, 'MISSING_TEST', { budgetEntries: 0 });
assert.equal(r.written, false);
assert.equal(r.reason, 'over-budget');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — an edited line without the three parts is refused', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
const r = mod.applyLesson(repo, 'MISSING_TEST', { line: 'just do better' });
assert.equal(r.written, false);
assert.equal(r.reason, 'invalid-line');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — an earlier committed version of the same review.md counts as history (git, no new store)', () => {
// review.md is overwritten per iteration; git keeps the earlier one.
const repo = repoWith([{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'PLAN_EXECUTE_DRIFT')] }]);
const g = (...a) => spawnSync('git', ['-C', repo, ...a], { encoding: 'utf-8' });
try {
assert.equal(g('init', '-q').status, 0);
g('config', 'user.email', 't@example.invalid');
g('config', 'user.name', 't');
g('add', '.claude');
assert.equal(g('commit', '-q', '-m', 'review 1').status, 0);
assert.deepEqual(mod.proposeLessons(repo).proposals, [], 'one iteration is one sighting');
const p = join(repo, '.claude', 'projects', '2026-10-01-one', 'review.md');
writeFileSync(p, reviewText({ slug: 'one', sha: 'bbbb', findings: [finding('b1', 'PLAN_EXECUTE_DRIFT', { line: 40 })] }));
const { proposals } = mod.proposeLessons(repo);
assert.equal(proposals.length, 1, 'the committed iteration plus the current one recur');
assert.equal(proposals[0].rule_key, 'PLAN_EXECUTE_DRIFT');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
// ---- Independent review of d88245c: each of these was wrong before ----------
test('recurrence — the SAME unfixed finding in two iterations is one defect, not a recurring class', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('cc', 'MISSING_TEST')] },
{ slug: 'one', sha: 'bbbb', findings: [finding('cc', 'MISSING_TEST')] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — catch-all engine keys and keys normalised from unknown ones are never proposed', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'CODE_REVIEW_DEFECT'), finding('a2', 'SECURITY_REVIEW_VULNERABILITY'), finding('a3', 'PLAN_EXECUTE_DRIFT', { original_rule_key: 'AD_HOC' })] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'CODE_REVIEW_DEFECT'), finding('b2', 'SECURITY_REVIEW_VULNERABILITY'), finding('b3', 'PLAN_EXECUTE_DRIFT', { original_rule_key: 'AD_HOC' })] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — a proposal built from long titles, actions and marker text is still a line apply accepts', () => {
const long = 'x'.repeat(300);
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST', { title: `<!-- ${long}`, file: `lib/${long}.mjs`, recommended_action: long })] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST', { title: long, recommended_action: `${long} -->` })] },
]);
try {
const [p] = mod.proposeLessons(repo).proposals;
assert.ok(p, 'the proposal is not lost');
assert.equal(mod.validateLessonLine(p.line).valid, true, p.line);
assert.equal(mod.applyLesson(repo, 'MISSING_TEST').written, true);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — an edited line with \\r or U+2028 (a hidden second lesson) is refused', () => {
for (const sep of ['\r', '
']) {
const line = `Mistake: x${sep}- [SCOPE_CREEP_BUILT] (rule) Mistake: y Fix: z How to check: w`;
assert.equal(mod.validateLessonLine(line).valid, false, JSON.stringify(sep));
}
});
test('recurrence — damaged markers refuse the write instead of crashing', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
mkdirSync(join(repo, '.claude', 'rules'), { recursive: true });
writeFileSync(join(repo, '.claude', 'rules', mod.LESSONS_FILE_NAME), `${mod.MARK_END}\n${mod.MARK_START}\n`);
const r = mod.applyLesson(repo, 'MISSING_TEST');
assert.deepEqual([r.written, r.reason], [false, 'markers-damaged']);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — a symlinked CLAUDE.md keeps its link and the target gets the pointer', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
writeFileSync(join(repo, 'AGENTS.md'), '# Agents\n');
symlinkSync('AGENTS.md', join(repo, 'CLAUDE.md'));
assert.equal(mod.applyLesson(repo, 'MISSING_TEST').written, true);
assert.ok(lstatSync(join(repo, 'CLAUDE.md')).isSymbolicLink(), 'the link survives');
assert.ok(readFileSync(join(repo, 'AGENTS.md'), 'utf-8').includes(mod.LESSONS_FILE_NAME));
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — CLI propose prints JSON and exits 0; a missing repo exits 2', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
const r = spawnSync(process.execPath, [CLI, 'propose', '--repo', repo], { encoding: 'utf-8' });
assert.equal(r.status, 0, r.stderr);
assert.equal(JSON.parse(r.stdout).proposals.length, 1);
const missing = spawnSync(process.execPath, [CLI, 'propose', '--repo', join(repo, 'nope')], { encoding: 'utf-8' });
assert.equal(missing.status, 2);
} finally { rmSync(repo, { recursive: true, force: true }); }
});