voyage/tests/lib/review-prose-pins.test.mjs
Kjell Tore Guttormsen ed6699da58
refactor(simplify): one source per rule, shared test helpers, a faster isolation test [skip-docs]
/simplify over a97efb6..HEAD (the plan_version 1.9 / trial-4 delivery).

- coordinator-contract: engine names come from engine-findings ENGINES;
  one markedMajorCodeReview predicate under both lowering checks;
  isEngineFinding everywhere; the unreachable branch (c) of
  illegalBriefLowering folded into (e).
- armB workflow: RULE_KEYS = Object.keys(RULE_TIERS); the engine map is a
  literal pinned equal to ENGINES by the workflow test.
- plan-schema: usesFilesGrammar() is the one 1.9 check (validator, sweep,
  testFirstFiles, stepTestsOnlyFromFields); opensEntry built on opensChunk.
- plan-validator: one issue builder for the Files and Test first grammar.
- plan-scope-sweep: the 1.9 and the 6.2.0 reader are one if/else.
- artifact-commit: TRAILER_RE/trailerProblem no longer exported (no
  importer); the order comment says the CLI finds the repo root first.
- tests: the lock drivers in session-id-isolation (d) run side by side per
  mode (~100 s -> ~30 s); a shared artifact-repo helper; severityRank and
  extractFindingsBlock reused; childEnv(env) without a spread; one copy of
  the PLAN_VERSION_CURRENT check.

Plan readers: validator errors/warnings, plannedPaths, fieldPaths,
testFirstFiles and stepTestsOnlyFromFields compared byte for byte against
HEAD over 510 plans / 1692 steps plus 210 synthetic 1.7-1.9 cases: same.
Suite 2384 (2382/0/2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-12 21:06:47 +02:00

194 lines
8.3 KiB
JavaScript

// tests/lib/review-prose-pins.test.mjs
// Prose pins for the review process (ordre 5, bølge 1: P11 + P5). Counted as
// doc-pins by lib/util/test-census.mjs. Each pin names the behaviour it guards
// so a failing pin says what broke, not just which string moved.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { readFileSync } from 'node:fs';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
import { RULE_KEYS } from '../../lib/review/rule-catalogue.mjs';
import { UNVERIFIED_REASONS } from '../../lib/review/coordinator-contract.mjs';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const read = (rel) => readFileSync(join(ROOT, rel), 'utf-8');
function section(text, heading) {
const start = text.indexOf(heading);
assert.ok(start >= 0, `heading missing: ${heading}`);
const next = text.indexOf('\n## ', start + heading.length);
return text.slice(start, next === -1 ? undefined : next);
}
const CMD = read('commands/trekreview.md');
const COORD = read('agents/review-coordinator.md');
// ---- P11: Claude Code's review as the engine --------------------------------
test('trekreview — frontmatter allows the Skill tool (the engine is started through it)', () => {
const fm = CMD.slice(0, CMD.indexOf('\n---', 4));
assert.match(fm, /allowed-tools:.*\bSkill\b/);
});
test('trekreview — Phase 4.7 starts /code-review and /security-review through the Skill tool', () => {
const p = section(CMD, '## Phase 4.7');
assert.ok(p.includes('Skill("code-review"'), 'code-review via Skill');
assert.ok(p.includes('Skill("security-review")'), 'security-review via Skill');
assert.ok(p.includes('lib/review/engine-findings.mjs'), 'engine findings go through the adapter');
});
test('trekreview — the engine never edits code, posts or uploads (no --fix/--comment/--post/ultra)', () => {
const p = section(CMD, '## Phase 4.7');
assert.match(p, /\*\*Never\*\* pass `--fix`, `--comment`, `--post` or `ultra`/);
const call = p.match(/Skill\("code-review", args: "([^"]*)"\)/);
assert.ok(call, 'the code-review call is written out');
for (const banned of ['--fix', '--comment', '--post', 'ultra']) {
assert.ok(!call[1].includes(banned), `the engine call must not carry ${banned}`);
}
});
test('trekreview — an engine that did not run forbids ALLOW (fail-closed), and engine payloads stay out of the reviewers', () => {
const p = section(CMD, '## Phase 4.7');
assert.match(p, /forbids `ALLOW`/);
assert.match(p, /NOT shown to the Phase 5\s+reviewers/);
});
test('trekreview — the engine claims cite the live docs they were verified in', () => {
const p = section(CMD, '## Phase 4.7');
assert.ok(p.includes('v2.1.246'), 'model-invocation floor for /code-review');
assert.ok(p.includes("origin's"), '/security-review base is origin default branch');
});
// ---- P11: no finding struck for length; declined-to-judge ----------------------
test('coordinator — succinctness never removes a finding; only MINOR/SUGGESTION text is trimmed', () => {
assert.match(COORD, /\| Succinctness \|[^\n]*never removed/);
assert.ok(!UNVERIFIED_REASONS.includes('succinctness:detail'), 'length is no longer an unverified reason');
assert.ok(!UNVERIFIED_REASONS.includes('succinctness:title'));
});
test('coordinator — writes ## Declined to Judge with the trim count', () => {
assert.ok(COORD.includes('`## Declined to Judge` — REQUIRED'));
assert.ok(COORD.includes('Trimmed: N'));
});
// ---- K7: the verdict in the file, and the gate -------------------------------
test('coordinator — review_version 1.1 frontmatter carries verdict', () => {
assert.ok(COORD.includes('review_version: "1.1"'));
assert.match(COORD, /\nverdict: \{BLOCK \| WARN \| ALLOW\}\n/);
});
test('trekreview — Phase 8 decides with lib/review/verdict-gate.mjs and calls exit 2 not green', () => {
const p = section(CMD, '## Phase 8 —');
assert.ok(p.includes('lib/review/verdict-gate.mjs'));
assert.match(p, /never\s+green/);
});
test('gate — a control that did not run stops a merge (operator decision 2026-10-06) is written down', () => {
assert.match(section(CMD, '## Phase 8 —'), /a review not performed in full stops a\s+merge/);
assert.match(section(read('docs/HANDOVER-CONTRACTS.md'), '## Handover 6'), /did not run/);
});
test('trekreview — a BLOCKER is written as a numbered Step R<n> with Source finding (Handover 6)', () => {
assert.ok(CMD.includes('### Step R<n>'));
assert.ok(COORD.includes('**Source finding:**'));
const h6 = section(read('docs/HANDOVER-CONTRACTS.md'), '## Handover 6');
assert.ok(h6.includes('Remediation Steps'), 'Handover 6 documents the remediation steps');
assert.match(h6, /\| `verdict` \| string \| yes/, 'Handover 6 lists verdict as required');
assert.ok(h6.includes('verdict-gate.mjs'), 'Handover 6 names the gate');
});
test('template — templates/trekreview-template.md validates strictly and the gate reads it as BLOCK', async () => {
const { validateReview } = await import('../../lib/validators/review-validator.mjs');
const { decideVerdictFile } = await import('../../lib/review/verdict-gate.mjs');
const path = join(ROOT, 'templates', 'trekreview-template.md');
const v = validateReview(path, { strict: true });
assert.equal(v.valid, true, JSON.stringify(v.errors));
assert.equal(decideVerdictFile(path).exit, 1);
});
test('trekreview — the zero-diff review the prose prescribes passes the gate as ALLOW', async () => {
const { decideVerdict } = await import('../../lib/review/verdict-gate.mjs');
const p3 = section(CMD, '## Phase 3');
assert.ok(p3.includes('findings: []') && p3.includes('"findings": []') && p3.includes('"allow_blocked_by": []'),
'Phase 3 prescribes the empty list, the JSON block and allow_blocked_by');
const text = `---
type: trekreview
review_version: "1.1"
task: "t"
slug: s
project_dir: p/
brief_path: p/brief.md
scope_sha_end: abc
reviewed_files_count: 0
verdict: ALLOW
findings: []
---
## Executive Summary
No diff.
## Coverage
No diff.
## Declined to Judge
None.
Trimmed: 0
## Remediation Summary
No diff.
\`\`\`json
{"verdict": "ALLOW", "counts": {"BLOCKER": 0, "MAJOR": 0, "MINOR": 0, "SUGGESTION": 0}, "allow_blocked_by": [], "findings": []}
\`\`\`
`;
assert.equal(decideVerdict(text).exit, 0, decideVerdict(text).reason);
});
test('trekreview — Phase 8.7 points recurrence at the session directory, not the git toplevel', () => {
const p = section(CMD, '## Phase 8.7');
assert.ok(p.includes('--repo "$PWD"'));
assert.ok(!p.includes('--repo "$(git rev-parse --show-toplevel)"'));
});
// ---- P5: project memory --------------------------------------------------------
test('trekreview — Phase 8.7 asks the operator per line (AskUserQuestion) before recurrence.mjs apply', () => {
const p = section(CMD, '## Phase 8.7');
const ask = p.indexOf('AskUserQuestion');
const apply = p.indexOf('recurrence.mjs apply');
assert.ok(ask >= 0 && apply > ask, 'approval comes before the write');
assert.match(p, /over-budget/);
assert.match(p, /belong in a skill/);
});
test('trekreview — the write-outside-project_dir exception names only the marked block and the pointer', () => {
const rules = section(CMD, '## Hard rules');
assert.ok(rules.includes('.claude/rules/voyage-review-lessons.md'));
assert.ok(rules.includes('only after the operator approved that exact line'));
});
// ---- the Workflow substrate stays in step -----------------------------------------
test('armB workflow — its inlined rule-key enum equals the catalogue (no drift)', () => {
const src = read('scripts/trekreview-armB.workflow.mjs');
assert.ok(src.includes('const RULE_KEYS = Object.keys(RULE_TIERS);'), 'the enum is the keys of the inlined RULE_TIERS');
const start = src.indexOf('const RULE_TIERS = {');
assert.ok(start !== -1, 'the script carries a RULE_TIERS literal block');
const block = src.slice(start, src.indexOf('};', start));
const keys = [...block.matchAll(/([A-Z_]+):\s*'[A-Z]+'/g)].map((m) => m[1]).sort();
assert.deepEqual(keys, [...RULE_KEYS].sort());
});
test('armB workflow — a missing reviewer or engine forbids ALLOW on the Workflow path too', () => {
const src = read('scripts/trekreview-armB.workflow.mjs');
assert.ok(src.includes("if (verdict === 'ALLOW' && missingReviewers > 0) verdict = 'WARN';"));
assert.ok(src.includes('enginePayloads'));
});