voyage/tests/lib/critic-severity-rules.test.mjs
Kjell Tore Guttormsen 1be961aab1
fix(gate): part 2 verifier findings — non-object findings, carried backed copy, rule_keys, marks, approval prose, brief_path, engine put-back once
1. plan-review-dedup skips only a null or undefined finding; any other
   entry that is not an object (a string, a number, an array) goes on as
   before E, a finding with no valid severity that the plan gate counts as
   a blocker (it was dropped, so a string blocker recorded PASS).
2. plan-gate --record: a previous finding that rule A carried as counted and
   whose id is backed this round leaves the counted set, the counts, the
   verdict and the stop rule's summary (it was counted and listed in
   known_outside at once: BLOCK, not-fewer, and --accept-open refused).
3. A known_outside entry keeps rule_keys; extractVerdict accepts it (a list
   of strings), and the bare gate re-checks with the same keys the recorder
   used, so a merged finding whose security key is only in rule_keys is a
   WARN, not a STOP.
4. plan-critic, scope-guardian and /trekplan Phase 9's round-2 sentences say
   the Critic Verdict also lists known_outside and each is marked by id in
   previous. Code already accepts such a mark (dedup passes it; countRound
   ignores ids it does not carry; --record removes the entry on true);
   pinned in diff mode and full count.
5. The approval prose names ## Threat Model where it named only ## Intent +
   ## Goal: skills/annotating/SKILL.md (description, step 4, guard list),
   docs/command-modes.md, Handover 1's intent_approved_hash row, README
   (approval, Approve row, intent approval gate) and CLAUDE.md's /trekbrief
   row.
6. threat-model.mjs gains usableInRepo (a brief path relative to the repo
   top level) and repoRelativeBrief; --record stores brief_path relative to
   the top level, never absolute, and both gates read it from there, so the
   gate run from a subdirectory reads the same brief. reviewThreatModel
   uses usableInRepo. Handover 4 and the gate header say so.
7. review-finalize's outside pass puts an entry back only when no finding
   carries its row yet (engine-compare rowOf): an engine finding listed in
   known_outside while its payload is saved went back twice.
8. A test runs scripts/release-check.mjs on a review whose known_outside
   entry the brief does not back: exit 2 (control: backed passes the review
   check).

Existing tests changed: plan-gate-threat-model's entry-keys assertion now
expects rule_keys (item 3), and its planted non-security entry drops the
security key from rule_keys too, since the gate reads rule_keys (item 3).

Part 2 fix round of the E plan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-08 10:50:20 +01:00

118 lines
6.7 KiB
JavaScript

// tests/lib/critic-severity-rules.test.mjs
// The two plan reviewers (plan-critic, scope-guardian) must state rule B word for
// word as lib/review/critic-diff.mjs exports it, keep the 2026-10-08 rule that a
// test which cannot fail on a wrong implementation is major, tag security
// findings with Voyage's own security keys (never the engines' key, decision
// 167, E 2.3), and carry the round-2 fields
// (`previous`, `line_end`, `lowered_from`, `caught_by`) in their json template.
// The name is the brief's.
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_B } from '../../lib/review/critic-diff.mjs';
import { RULE_CATALOGUE } from '../../lib/review/rule-catalogue.mjs';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const read = rel => readFileSync(join(ROOT, rel), 'utf-8');
const norm = s => s.replace(/\s+/g, ' ');
const AGENTS = ['agents/plan-critic.md', 'agents/scope-guardian.md'];
// The one json template an agent file carries: the fenced block after
// "Machine-readable findings block".
function jsonTemplate(text, rel) {
const head = text.indexOf('Machine-readable findings block');
assert.ok(head >= 0, `${rel}: machine-readable findings block heading missing`);
const m = /```json\n([\s\S]*?)\n```/.exec(text.slice(head));
assert.ok(m, `${rel}: no fenced json template after the heading`);
return JSON.parse(m[1]);
}
// The engines' key (/security-review, Claude Security): never the critic's (decision 167).
const ENGINES_KEY = 'SECURITY_REVIEW_VULNERABILITY';
test('the catalogue has exactly three security keys: SECURITY_INJECTION, SECURITY_REVIEW_VULNERABILITY and SECURITY_RISK', () => {
const keys = RULE_CATALOGUE.filter(r => r.category === 'security').map(r => r.rule_key).sort();
assert.deepEqual(keys, ['SECURITY_INJECTION', 'SECURITY_REVIEW_VULNERABILITY', 'SECURITY_RISK']);
});
for (const rel of AGENTS) {
test(`${rel}: states rule B verbatim (never a blocker, never a security finding)`, () => {
const t = norm(read(rel));
assert.ok(t.includes(norm(RULE_B)), `${rel} must contain the RULE_B sentence from critic-diff.mjs`);
assert.ok(t.includes('never a blocker, never a security finding'));
assert.ok(
t.includes('The 2026-10-08 rule still holds for rule B: a test that cannot fail on a wrong implementation catches nothing, so the finding stays major.'),
`${rel} must say the 2026-10-08 rule still holds for rule B`,
);
assert.ok(t.includes('Give `lowered_from` and `caught_by` when you lower a finding this way.'));
assert.ok(!/\brule A\b/i.test(t), `${rel} must not state rule A (plan Design § Gotchas)`);
});
test(`${rel}: keeps the 2026-10-08 rule (cannot fail on a wrong implementation is major; catalogue tier)`, () => {
const raw = read(rel);
assert.match(raw, /cannot fail on a wrong\s+implementation[\s\S]{0,200}\*\*major\*\*/);
assert.ok(norm(raw).includes("Whenever a finding is the same class as a catalogue rule, use that rule's tier"));
});
test(`${rel}: a security finding uses the catalogue's security rule_key`, () => {
const t = norm(read(rel));
const keys = RULE_CATALOGUE.filter(r => r.category === 'security' && r.rule_key !== ENGINES_KEY).map(r => r.rule_key);
for (const k of keys) assert.ok(t.includes(`\`${k}\``), `${rel} must name \`${k}\``);
assert.ok(t.includes("A security finding uses the catalogue's security `rule_key`"));
});
test(`${rel}: the critic never names the engines' key (decision 167)`, () => {
assert.ok(!read(rel).includes(ENGINES_KEY), `${rel} must not name ${ENGINES_KEY}: it is the engines' key only`);
});
test(`${rel}: a security finding needs a concrete attack path and high confidence, and quotes the Outside: line in threat_model_ref (E 2.4)`, () => {
const t = norm(read(rel));
assert.ok(t.includes('concrete attack path'), `${rel} must ask for a concrete attack path`);
assert.match(t, /high confidence[^.]*Claude Code's (own )?threshold/, `${rel} must name the high-confidence threshold (167)`);
assert.match(t, /`Outside:`[^.]*`threat_model_ref`|`threat_model_ref`[^.]*`Outside:`/, `${rel} must say the Outside: line goes in threat_model_ref`);
assert.match(t, /exactly/, `${rel} must say the line is quoted exactly`);
const tpl = jsonTemplate(read(rel), rel);
assert.ok('threat_model_ref' in tpl.findings[0], `${rel}: the json template carries threat_model_ref`);
});
test(`${rel}: [STP] rule_key names the class, never the instance (the stop rule's class, decision 217)`, () => {
const t = norm(read(rel));
assert.ok(t.includes('`rule_key` names the class, never the instance'), `${rel} must say rule_key names the class, never the instance`);
});
test(`${rel}: round 2+ marks each known_outside entry by id in previous too (E fix round)`, () => {
const t = norm(read(rel));
assert.match(t, /`## Critic Verdict` also lists `known_outside`[^.]*\. Mark each of them by id in `previous` the same way/);
assert.match(t, /`previous` \(from round 2; omit it in round 1\) holds one `\{id, resolved\}` per finding the current `## Critic Verdict` lists, its `known_outside` entries included/);
});
test(`${rel}: round 2+ lists every previous finding by id and cites the exact line`, () => {
const t = norm(read(rel));
assert.ok(t.includes('From round 2 the prompt gives you the diff since the last reviewed text'), `${rel}: round-2 diff sentence missing`);
assert.ok(t.includes('List every previous finding by id in `previous`, with `resolved: true` only when the plan now fixes it.'));
assert.ok(t.includes('Cite the exact line, with `line_end` for a range.'));
});
test(`${rel}: the json template carries previous, line_end, lowered_from and caught_by beside the old fields`, () => {
const tpl = jsonTemplate(read(rel), rel);
assert.equal(typeof tpl.agent, 'string');
assert.ok(Array.isArray(tpl.findings) && tpl.findings.length >= 1, 'findings array with one example');
assert.ok(Array.isArray(tpl.previous) && tpl.previous.length >= 1, 'previous array with one example');
assert.ok('id' in tpl.previous[0] && 'resolved' in tpl.previous[0], 'previous entries carry id and resolved');
const f = tpl.findings[0];
for (const k of ['file', 'line', 'rule_key', 'severity', 'text', 'line_end', 'lowered_from', 'caught_by']) {
assert.ok(k in f, `finding template lacks "${k}"`);
}
});
}
test('plan-critic no longer tells the critic to judge only the plan in front of it', () => {
assert.ok(
!norm(read('agents/plan-critic.md')).includes('judge the plan in front of you'),
'the round-2 paragraph replaces "judge the plan in front of you"',
);
});