feat(plan): the plan gate counts critic rounds 2+ by rule A and records rule B [skip-docs]
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
31fa6f8e43
commit
7b46cdd788
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
3 changed files with 1018 additions and 20 deletions
|
|
@ -21,12 +21,56 @@
|
|||
// recurse between them; a gap with no unique line uses an LCS table, ties
|
||||
// resolved toward matching the earliest old line.
|
||||
//
|
||||
// countRound({round, previous, base, baseNote, current, planPaths, findings, marks})
|
||||
// -> { findings, resolved, diffBase, diffNote, lowered }
|
||||
// Decides each finding's counted severity for one critic round (RULE_A,
|
||||
// RULE_B below). `current` = {text, sha256, fullToOp, opToFull}, built by the
|
||||
// caller from the plan gate's operativeLines and planHash; `base` = {sha256,
|
||||
// text} is the operative text the previous round reviewed, or null.
|
||||
// Rule A is on only from round 2, with a base whose sha256 is the previous
|
||||
// verdict's plan_sha256 and differs from the plan now. Otherwise the round is
|
||||
// counted as before (each finding at its reported severity, plus the rule-B
|
||||
// record) and `diffNote` says why in one line. With rule A on, a current
|
||||
// finding outside the diff is matched to a previous one (same id, its mapped
|
||||
// line, or the same wording in the same file class); a previous finding stays
|
||||
// counted until every reviewer that raised it marks it resolved, no reviewer
|
||||
// says it is not, and nobody re-reports it.
|
||||
// Carried findings come first (previous order), then new ones (dedup order).
|
||||
//
|
||||
// Trust limit: anything that can write the plan can rewrite the previous
|
||||
// verdict's plan_sha256 and plant a matching base snapshot. That was already
|
||||
// the stated limit of the verdict block; rule A raises what such a forgery
|
||||
// buys. An unchanged base lowers nothing.
|
||||
//
|
||||
// Pure: no I/O, and it never reads the plan gate (one dependency direction).
|
||||
//
|
||||
// Credit: patience-style anchoring (unique-line anchors kept by their longest
|
||||
// increasing subsequence), the idea behind Bram Cohen's patience diff; the code
|
||||
// here is written fresh.
|
||||
|
||||
import { resolve } from 'node:path';
|
||||
import {
|
||||
normalizeSeverity, contentOverlap, contentTokens, DEFAULT_THRESHOLD, ANCHOR_MIN_CONTENT_WORDS,
|
||||
} from './plan-review-dedup.mjs';
|
||||
import { getRule } from './rule-catalogue.mjs';
|
||||
|
||||
export const RULE_A = 'Rule A: from round 2 on, a new major finding that cites a plan line outside the diff since the last reviewed text counts as minor; a blocker, a security finding and a finding with no plan line count at full severity, and a previous finding stays counted until it is marked resolved.';
|
||||
export const RULE_B = 'Rule B: a major that a named test, `Regression:` command or gate in the plan will fail on during execution is minor — never a blocker, never a security finding.';
|
||||
|
||||
// The optional voyage.plan-verdict/1 fields of decision 43; absent means round-1 behaviour.
|
||||
export const DIFF_FIELDS = Object.freeze([
|
||||
// top level
|
||||
'diff_base', 'diff_note', 'resolved',
|
||||
// per finding
|
||||
'op_line', 'op_line_end', 'line_end', 'rule_keys', 'original_severity', 'lowered_by', 'caught_by',
|
||||
'lowering_refused', 'carried_from',
|
||||
]);
|
||||
|
||||
/** A rule_key whose catalogue entry is in the security category (case and spaces ignored). */
|
||||
export function isSecurityKey(key) {
|
||||
return getRule(String(key).trim().toUpperCase())?.category === 'security';
|
||||
}
|
||||
|
||||
const toLines = (text) => {
|
||||
const t = String(text ?? '').replace(/\r\n?/g, '\n');
|
||||
if (t === '') return [];
|
||||
|
|
@ -167,3 +211,274 @@ export function lineDiff(oldText, newText) {
|
|||
}
|
||||
return { inside, oldToNew, hunks };
|
||||
}
|
||||
|
||||
// ---- counting (rule A, rule B) ----------------------------------------------------
|
||||
|
||||
// Lower index = more severe; an unrated severity counts as blocker.
|
||||
const SEV_ORDER = ['blocker', 'major', 'minor'];
|
||||
const rankOf = s => SEV_ORDER.indexOf(s);
|
||||
const countedOf = s => normalizeSeverity(s) ?? 'blocker';
|
||||
const higher = (a, b) => (rankOf(a) <= rankOf(b) ? a : b);
|
||||
const hasOwn = (o, k) => Object.prototype.hasOwnProperty.call(o, k);
|
||||
|
||||
/** The rule keys of a finding: `rule_keys` when it is a list, else its one `rule_key`. */
|
||||
function keysOf(f) {
|
||||
const keys = Array.isArray(f.rule_keys) ? f.rule_keys : [f.rule_key];
|
||||
return keys.filter(k => typeof k === 'string' && k !== '');
|
||||
}
|
||||
|
||||
const isSecurity = f => keysOf(f).some(isSecurityKey);
|
||||
|
||||
function union(...lists) {
|
||||
const out = [];
|
||||
for (const list of lists) {
|
||||
for (const x of Array.isArray(list) ? list : []) if (!out.includes(x)) out.push(x);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* The rule-B record of one finding at its rated severity.
|
||||
* A null or absent `lowered_from` is no record at all.
|
||||
* @returns {{severity: string, record: object}}
|
||||
*/
|
||||
function ruleB(f, rated) {
|
||||
if (f.lowered_from === undefined || f.lowered_from === null) return { severity: rated, record: {} };
|
||||
const from = normalizeSeverity(f.lowered_from);
|
||||
const caught = typeof f.caught_by === 'string' && f.caught_by.trim() !== '' ? f.caught_by : null;
|
||||
const security = isSecurity(f);
|
||||
if (from === 'major' && rated === 'minor' && caught && !security) {
|
||||
return { severity: 'minor', record: { original_severity: 'major', lowered_by: `rule B: ${caught}`, caught_by: caught } };
|
||||
}
|
||||
let why;
|
||||
if (from === null) why = 'rule B: lowered_from is not a severity; counted as blocker';
|
||||
else if (from === 'blocker') why = 'rule B never lowers a blocker';
|
||||
else if (from !== 'major') why = `rule B lowers only major to minor, not ${from}`;
|
||||
else if (rated !== 'minor') why = `rule B lowers major to minor only; the finding is rated ${rated}`;
|
||||
else if (security) why = 'rule B never lowers a security finding';
|
||||
else why = 'rule B needs caught_by: the named test, Regression: command or gate that will fail on it';
|
||||
const severity = higher(from ?? 'blocker', rated);
|
||||
const record = { lowering_refused: why };
|
||||
if (caught) record.caught_by = caught;
|
||||
if (severity !== rated) record.original_severity = rated;
|
||||
return { severity, record };
|
||||
}
|
||||
|
||||
/** A current finding in today's verdict shape, plus the pass-through fields. */
|
||||
function project(f) {
|
||||
const rated = normalizeSeverity(f.severity);
|
||||
const out = {
|
||||
id: f.id ?? null,
|
||||
severity: rated ?? 'blocker',
|
||||
rule_key: f.rule_key ?? null,
|
||||
file: f.file ?? null,
|
||||
line: f.line ?? null,
|
||||
text: typeof f.text === 'string' ? f.text : '',
|
||||
raised_by: Array.isArray(f.raised_by) ? [...f.raised_by] : [],
|
||||
};
|
||||
if (rated === null) out.severity_defaulted = true;
|
||||
if (Number.isInteger(f.line_end)) out.line_end = f.line_end;
|
||||
if (Array.isArray(f.rule_keys)) out.rule_keys = [...f.rule_keys];
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Where a current finding sits in the operative text, or null when it has no
|
||||
* plan line (another file, no line, a line in the review log, past the end,
|
||||
* or a `line_end` that does not map).
|
||||
*/
|
||||
function placeOf(f, fullToOp, planSet) {
|
||||
if (typeof f.file !== 'string' || f.file === '' || !planSet.has(resolve(f.file))) return null;
|
||||
const at = l => (Number.isInteger(l) && l >= 1 && l < fullToOp.length ? fullToOp[l] ?? null : null);
|
||||
const start = at(f.line);
|
||||
if (start === null) return null;
|
||||
const hasEnd = f.line_end !== undefined && f.line_end !== null;
|
||||
let end = start;
|
||||
if (hasEnd) {
|
||||
const e = Number.isInteger(f.line_end) && f.line_end >= f.line ? at(f.line_end) : null;
|
||||
if (e === null) return null;
|
||||
end = Math.max(start, e);
|
||||
}
|
||||
return { start, end, hasEnd };
|
||||
}
|
||||
|
||||
const contentSize = t => new Set(contentTokens(typeof t === 'string' ? t : '')).size;
|
||||
|
||||
/** Does current finding `f` (placed at `pl`, outside the diff) re-report previous finding `p`? */
|
||||
function sameFinding(f, pl, p, d) {
|
||||
if (typeof f.id === 'string' && f.id === p.id) return true;
|
||||
const pCites = hasOwn(p, 'op_line');
|
||||
if (pl && pCites && Number.isInteger(p.op_line)) {
|
||||
const n = d.oldToNew[p.op_line] ?? null;
|
||||
if (n !== null && n >= pl.start && n <= pl.end) return true;
|
||||
}
|
||||
const sameClass = (pl !== null && pCites) ||
|
||||
(typeof f.file === 'string' && f.file !== '' && f.file === p.file);
|
||||
if (!sameClass) return false;
|
||||
if (contentSize(f.text) < ANCHOR_MIN_CONTENT_WORDS || contentSize(p.text) < ANCHOR_MIN_CONTENT_WORDS) return false;
|
||||
return contentOverlap(f.text, p.text) >= DEFAULT_THRESHOLD;
|
||||
}
|
||||
|
||||
/** A previous finding leaves the count only on its raisers' true, nobody's false. */
|
||||
function isResolved(p, marks) {
|
||||
const ms = marks.filter(m => m && typeof m.id === 'string' && m.id === p.id);
|
||||
if (!ms.some(m => m.resolved === true)) return false;
|
||||
if (ms.some(m => m.resolved === false)) return false;
|
||||
const raisers = Array.isArray(p.raised_by) ? p.raised_by : [];
|
||||
return raisers.every(r => ms.some(m => m.agent === r && m.resolved === true));
|
||||
}
|
||||
|
||||
/** A previous finding carried into this round, with its matches' severities and keys. */
|
||||
function carry(p, matched, d, current, prevRound) {
|
||||
const pSev = countedOf(p.severity);
|
||||
const orig = normalizeSeverity(p.original_severity);
|
||||
const wasLowered = orig !== null && rankOf(orig) < rankOf(pSev);
|
||||
const keys = union(keysOf(p), ...matched.map(m => keysOf(m.f)));
|
||||
let sev;
|
||||
if (wasLowered) {
|
||||
if (keys.some(isSecurityKey)) sev = matched.reduce((s, m) => higher(s, m.sev), orig);
|
||||
else if (matched.some(m => m.sev === 'blocker')) sev = 'blocker';
|
||||
else sev = pSev;
|
||||
} else {
|
||||
sev = matched.reduce((s, m) => higher(s, m.sev), pSev);
|
||||
}
|
||||
const out = {
|
||||
id: p.id ?? null,
|
||||
severity: sev,
|
||||
rule_key: p.rule_key ?? null,
|
||||
file: p.file ?? null,
|
||||
line: p.line ?? null,
|
||||
text: typeof p.text === 'string' ? p.text : '',
|
||||
raised_by: union(p.raised_by, ...matched.map(m => m.f.raised_by)),
|
||||
};
|
||||
if (p.severity_defaulted === true) out.severity_defaulted = true;
|
||||
if (Number.isInteger(p.line_end)) out.line_end = p.line_end;
|
||||
if (keys.length > 0) out.rule_keys = keys;
|
||||
if (hasOwn(p, 'op_line')) {
|
||||
// A plan line: follow the diff. Its own line edited (or no op_line already):
|
||||
// keep the old line, op_line null, never re-mapped from a stale position.
|
||||
const n = Number.isInteger(p.op_line) ? (d.oldToNew[p.op_line] ?? null) : null;
|
||||
out.op_line = n;
|
||||
if (n !== null) {
|
||||
out.line = current.opToFull[n] ?? out.line;
|
||||
const e = Number.isInteger(p.op_line_end) ? (d.oldToNew[p.op_line_end] ?? null) : null;
|
||||
const eFull = e !== null ? (current.opToFull[e] ?? null) : null;
|
||||
if (e !== null && eFull !== null) {
|
||||
out.line_end = eFull;
|
||||
out.op_line_end = e;
|
||||
} else {
|
||||
delete out.line_end;
|
||||
}
|
||||
} else if (hasOwn(p, 'op_line_end')) {
|
||||
out.op_line_end = null;
|
||||
}
|
||||
}
|
||||
out.carried_from = Number.isInteger(p.carried_from) ? p.carried_from : prevRound;
|
||||
if (wasLowered) {
|
||||
if (rankOf(sev) > rankOf(orig)) {
|
||||
out.original_severity = orig;
|
||||
if (typeof p.lowered_by === 'string') out.lowered_by = p.lowered_by;
|
||||
if (typeof p.caught_by === 'string') out.caught_by = p.caught_by;
|
||||
}
|
||||
} else {
|
||||
if (orig !== null && orig !== sev) out.original_severity = orig;
|
||||
if (typeof p.lowering_refused === 'string') out.lowering_refused = p.lowering_refused;
|
||||
if (typeof p.caught_by === 'string') out.caught_by = p.caught_by;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** One line: the reason a round is counted fully. */
|
||||
const oneLine = s => String(s).replace(/\s*[\r\n]+\s*/g, ' ').trim();
|
||||
|
||||
/**
|
||||
* Count one critic round by rule A (from round 2, against the diff since the
|
||||
* text the previous round reviewed) and record rule B (every round).
|
||||
* Pure and deterministic.
|
||||
* @param {{round: number, previous: object|null, base: {sha256: string, text: string}|null,
|
||||
* baseNote?: string|null, current: {text: string, sha256: string, fullToOp: Array<number|null>, opToFull: Array<number|null>},
|
||||
* planPaths: string[], findings: object[], marks: Array<{agent: string, id: string, resolved: boolean}>}} input
|
||||
* @returns {{findings: object[], resolved: string[]|null, diffBase: string|null, diffNote: string|null, lowered: number}}
|
||||
*/
|
||||
export function countRound({ round, previous, base, baseNote, current, planPaths, findings, marks }) {
|
||||
const prevRound = previous && Number.isInteger(previous.round) ? previous.round : null;
|
||||
let diffNote = null;
|
||||
let d = null;
|
||||
if (Number.isInteger(round) && round >= 2 && prevRound !== null) {
|
||||
if (!base || typeof base !== 'object') {
|
||||
diffNote = typeof baseNote === 'string' && baseNote.trim() !== ''
|
||||
? oneLine(baseNote)
|
||||
: `no base snapshot of round ${prevRound}'s reviewed text; full counting`;
|
||||
} else if (typeof base.sha256 !== 'string' || base.sha256 !== previous.plan_sha256) {
|
||||
diffNote = `the base snapshot is not the text round ${prevRound} reviewed; full counting`;
|
||||
} else if (base.sha256 === current.sha256) {
|
||||
diffNote = `the plan has not changed since round ${prevRound}; full counting`;
|
||||
} else if (typeof base.text !== 'string') {
|
||||
diffNote = `the base snapshot of round ${prevRound} has no text; full counting`;
|
||||
} else {
|
||||
d = lineDiff(base.text, current.text);
|
||||
}
|
||||
}
|
||||
const ruleA = d !== null;
|
||||
const planSet = new Set(Array.isArray(planPaths) ? planPaths : []);
|
||||
const fullToOp = Array.isArray(current?.fullToOp) ? current.fullToOp : [];
|
||||
const prevFindings = ruleA && Array.isArray(previous.findings)
|
||||
? previous.findings.filter(p => p && typeof p === 'object')
|
||||
: [];
|
||||
const markList = Array.isArray(marks) ? marks : [];
|
||||
|
||||
const matchedTo = new Map(prevFindings.map(p => [p, []]));
|
||||
const fresh = [];
|
||||
for (const f of Array.isArray(findings) ? findings : []) {
|
||||
const o = project(f);
|
||||
const b = ruleB(f, o.severity);
|
||||
o.severity = b.severity;
|
||||
Object.assign(o, b.record);
|
||||
const pl = placeOf(f, fullToOp, planSet);
|
||||
let inside = false;
|
||||
if (ruleA && pl) {
|
||||
for (let n = pl.start; n <= pl.end && !inside; n++) inside = d.inside.has(n);
|
||||
}
|
||||
if (ruleA && !inside) {
|
||||
const p = prevFindings.find(q => sameFinding(f, pl, q, d));
|
||||
if (p) {
|
||||
matchedTo.get(p).push({ f, sev: o.severity });
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if (pl) {
|
||||
o.op_line = pl.start;
|
||||
if (pl.hasEnd) o.op_line_end = pl.end;
|
||||
}
|
||||
if (ruleA && o.severity === 'major' && pl && !inside && !isSecurity(f)) {
|
||||
o.severity = 'minor';
|
||||
o.original_severity = 'major';
|
||||
o.lowered_by = `rule A: outside the diff since round ${prevRound}`;
|
||||
}
|
||||
fresh.push(o);
|
||||
}
|
||||
|
||||
const carried = [];
|
||||
const resolved = [];
|
||||
for (const p of prevFindings) {
|
||||
const matched = matchedTo.get(p);
|
||||
if (matched.length === 0 && isResolved(p, markList)) {
|
||||
resolved.push(p.id);
|
||||
continue;
|
||||
}
|
||||
carried.push(carry(p, matched, d, current, prevRound));
|
||||
}
|
||||
|
||||
const out = [...carried, ...fresh];
|
||||
const lowered = out.filter((x) => {
|
||||
const o = normalizeSeverity(x.original_severity);
|
||||
return o !== null && rankOf(o) < rankOf(x.severity);
|
||||
}).length;
|
||||
return {
|
||||
findings: out,
|
||||
resolved: ruleA ? resolved : null,
|
||||
diffBase: ruleA ? base.sha256 : null,
|
||||
diffNote,
|
||||
lowered,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -53,12 +53,30 @@
|
|||
// then stop and ask; never silently on. Unrated findings count as blockers.
|
||||
// The verdict is a trace of a process, not a signature: anything that can
|
||||
// write the plan can write the block.
|
||||
//
|
||||
// Rule A (decision 43): from critic round 2 on, `recordVerdict` counts a round
|
||||
// against the diff between the operative text the previous round reviewed (the
|
||||
// base, `opts.base`) and the plan now, through `countRound` in
|
||||
// lib/review/critic-diff.mjs. A new MAJOR that cites a plan line outside that
|
||||
// diff counts as minor; a blocker, a security finding and a finding with no
|
||||
// plan line count at full severity; a previous finding stays counted (carried,
|
||||
// with its id) until every reviewer that raised it marks it resolved and nobody
|
||||
// re-reports it. Rule B (a major a named test, `Regression:` command or gate
|
||||
// will fail on is minor) is recorded from round 1 and never lowers a blocker or
|
||||
// a security finding. Without a usable base (none, not the previous round's
|
||||
// text, or the plan unchanged since it) the round is counted as before, and
|
||||
// `diff_note` says why in one line. Every new verdict field is optional
|
||||
// (DIFF_FIELDS); its absence means round-1 behaviour, so an older block gives
|
||||
// the same exits. Trust limit: anything that can write the plan can also
|
||||
// rewrite the previous verdict's plan_sha256 and plant a matching base; rule A
|
||||
// raises what such a forgery buys, and an unchanged base lowers nothing.
|
||||
|
||||
import { readFileSync, existsSync, writeFileSync, renameSync, rmSync } from 'node:fs';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { validatePlanContent } from './plan-validator.mjs';
|
||||
import { splitSections, fenceTracker, extractPlanVersion, versionAtLeast } from '../parsers/plan-schema.mjs';
|
||||
import { normalizeSeverity } from '../review/plan-review-dedup.mjs';
|
||||
import { countRound } from '../review/critic-diff.mjs';
|
||||
import { isMain } from '../util/is-main.mjs';
|
||||
|
||||
export const MAX_ROUNDS = 3;
|
||||
|
|
@ -218,9 +236,15 @@ function missingReviewers(dedup) {
|
|||
* @param {string} text - the plan
|
||||
* @param {{findings: Array<object>, sources: Array<{agent: string, present: boolean}>}} dedup
|
||||
* plan-review-dedup.mjs output
|
||||
* @param {{reviewed?: string, extraRound?: boolean, maxRounds?: number, now?: string}} [opts]
|
||||
* reviewed = planHash of the text the reviewers read (from --hash)
|
||||
* @returns {{code: 0|1|2, text?: string, verdict?: object, message: string}}
|
||||
* @param {{reviewed?: string, extraRound?: boolean, maxRounds?: number, now?: string,
|
||||
* base?: {sha256: string, text: string}|null, baseNote?: string, planPaths?: string[]}} [opts]
|
||||
* reviewed = planHash of the text the reviewers read (from --hash);
|
||||
* base = the operative text the previous round reviewed, with its sha256;
|
||||
* baseNote = why there is no base (one line, written as diff_note);
|
||||
* planPaths = the absolute forms of the plan path (a finding whose
|
||||
* resolve(file) is one of them cites the plan). Without base, baseNote and
|
||||
* planPaths the round is recorded as before, plus the rule-B record.
|
||||
* @returns {{code: 0|1|2, text?: string, verdict?: object, lowered?: number, message: string}}
|
||||
*/
|
||||
export function recordVerdict(text, dedup, opts = {}) {
|
||||
if (!dedup || typeof dedup !== 'object' || !Array.isArray(dedup.findings)) {
|
||||
|
|
@ -257,20 +281,19 @@ export function recordVerdict(text, dedup, opts = {}) {
|
|||
};
|
||||
}
|
||||
const extra = prevExtra + (capped ? 1 : 0);
|
||||
const findings = dedup.findings.map((f) => {
|
||||
const sev = normalizeSeverity(f.severity);
|
||||
const out = {
|
||||
id: f.id ?? null,
|
||||
severity: sev ?? 'blocker',
|
||||
rule_key: f.rule_key ?? null,
|
||||
file: f.file ?? null,
|
||||
line: f.line ?? null,
|
||||
text: typeof f.text === 'string' ? f.text : '',
|
||||
raised_by: Array.isArray(f.raised_by) ? f.raised_by : [],
|
||||
};
|
||||
if (sev === null) out.severity_defaulted = true;
|
||||
return out;
|
||||
const diffMode = opts.base !== undefined || opts.baseNote !== undefined || opts.planPaths !== undefined;
|
||||
const lines = operativeLines(text);
|
||||
const c = countRound({
|
||||
round,
|
||||
previous: prev.verdict ?? null,
|
||||
base: opts.base ?? null,
|
||||
baseNote: opts.baseNote ?? null,
|
||||
current: { text: lines.text, sha256: current, fullToOp: lines.fullToOp, opToFull: lines.opToFull },
|
||||
planPaths: Array.isArray(opts.planPaths) ? opts.planPaths : [],
|
||||
findings: dedup.findings,
|
||||
marks: Array.isArray(dedup.marks) ? dedup.marks : [],
|
||||
});
|
||||
const findings = c.findings;
|
||||
const counts = { blocker: 0, major: 0, minor: 0 };
|
||||
for (const f of findings) counts[f.severity] += 1;
|
||||
const verdict = {
|
||||
|
|
@ -280,11 +303,17 @@ export function recordVerdict(text, dedup, opts = {}) {
|
|||
extra_rounds: extra,
|
||||
recorded_at: opts.now ?? new Date().toISOString(),
|
||||
plan_sha256: current,
|
||||
verdict: verdictFor(counts),
|
||||
counts,
|
||||
findings,
|
||||
};
|
||||
return { code: 0, text: placeSection(text, renderSection(verdict)), verdict, message: `round ${round} recorded: ${verdict.verdict}` };
|
||||
if (c.diffBase !== null) verdict.diff_base = c.diffBase;
|
||||
if (diffMode && c.diffNote !== null) verdict.diff_note = c.diffNote;
|
||||
verdict.verdict = verdictFor(counts);
|
||||
verdict.counts = counts;
|
||||
verdict.findings = findings;
|
||||
if (c.resolved !== null) verdict.resolved = c.resolved;
|
||||
return {
|
||||
code: 0, text: placeSection(text, renderSection(verdict)), verdict, lowered: c.lowered,
|
||||
message: `round ${round} recorded: ${verdict.verdict}`,
|
||||
};
|
||||
}
|
||||
|
||||
const sevOf = f => normalizeSeverity(f.severity);
|
||||
|
|
|
|||
654
tests/validators/plan-gate-diff-mode.test.mjs
Normal file
654
tests/validators/plan-gate-diff-mode.test.mjs
Normal file
|
|
@ -0,0 +1,654 @@
|
|||
// tests/validators/plan-gate-diff-mode.test.mjs
|
||||
// Decision 43, Step 4 of the converging-review-loops plan: the plan gate counts
|
||||
// critic rounds 2+ by rule A and records rule B (lib/review/critic-diff.mjs
|
||||
// `countRound`, called by plan-gate.mjs `recordVerdict`).
|
||||
//
|
||||
// Every case runs through recordVerdict with a `base` (the operative text the
|
||||
// previous round reviewed), on reviewer payloads put through the real dedup.
|
||||
// Each guard case also records a new major outside the diff in the same round,
|
||||
// so every test is red on code that does not count by rule A.
|
||||
//
|
||||
// Expected line numbers come from findIndex over the full plan text, never
|
||||
// from the gate's own line map; expected severities are literals.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { resolve } from 'node:path';
|
||||
import { gatePlan, recordVerdict, planHash, operativeText, acceptOpen } from '../../lib/validators/plan-gate.mjs';
|
||||
import * as diffMod from '../../lib/review/critic-diff.mjs';
|
||||
import { dedupFindings, contentOverlap } from '../../lib/review/plan-review-dedup.mjs';
|
||||
import { buildPlan } from '../helpers/plan-v18-builder.mjs';
|
||||
|
||||
const NOW = '2026-10-16T00:00:00.000Z';
|
||||
const PLAN = resolve('plan.md');
|
||||
const PATHS = [PLAN];
|
||||
const BOTH = [{ agent: 'plan-critic', present: true }, { agent: 'scope-guardian', present: true }];
|
||||
|
||||
/** The 1-based full-file line holding `s` (findIndex over the whole text). */
|
||||
function at(text, s) {
|
||||
const n = text.split('\n').findIndex(l => l.includes(s)) + 1;
|
||||
assert.ok(n > 0, `the plan holds ${JSON.stringify(s)}`);
|
||||
return n;
|
||||
}
|
||||
|
||||
/** The base a round diffs against: the operative text the previous round reviewed. */
|
||||
const baseOf = t => ({ sha256: planHash(t), text: operativeText(t) });
|
||||
|
||||
/** One round as Phase 9 runs it, with the plan path known to the gate. */
|
||||
function rec(text, d, opts = {}) {
|
||||
const r = recordVerdict(text, d, { reviewed: planHash(text), planPaths: PATHS, now: NOW, ...opts });
|
||||
assert.equal(r.code, 0, r.message);
|
||||
return r;
|
||||
}
|
||||
|
||||
/** Reviewer payloads through the real dedup. */
|
||||
function dd({ pc = [], sg = [], pcPrev, sgPrev } = {}) {
|
||||
return dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { findings: pc, ...(pcPrev ? { previous: pcPrev } : {}) } },
|
||||
{ agent: 'scope-guardian', payload: { findings: sg, ...(sgPrev ? { previous: sgPrev } : {}) } },
|
||||
]);
|
||||
}
|
||||
|
||||
const f = (severity, line, text, extra = {}) => ({ file: 'plan.md', line, rule_key: 'step-quality', severity, text, ...extra });
|
||||
const byText = (v, s) => {
|
||||
const hit = v.findings.filter(x => x.text === s);
|
||||
assert.equal(hit.length, 1, `exactly one finding reads ${JSON.stringify(s)}: ${JSON.stringify(v.findings)}`);
|
||||
return hit[0];
|
||||
};
|
||||
const byId = (v, id) => v.findings.filter(x => x.id === id);
|
||||
|
||||
// ---- the edits ---------------------------------------------------------------
|
||||
|
||||
const R2_CHANGES = '- **Changes:** Add `run2()` to `src/mod2.mjs`. (new file)';
|
||||
const R2_EDITED = '- **Changes:** Add `run2()` and its error path to `src/mod2.mjs`. (new file)\n - `run2()` throws on empty input.';
|
||||
const WHY = 'Why: the brief asks for a parser CLI.';
|
||||
|
||||
/** Edit A: Step 2's Changes line becomes two lines (a two-line hunk, nothing above it moves). */
|
||||
function editA(t) {
|
||||
assert.ok(t.includes(R2_CHANGES));
|
||||
return t.replace(R2_CHANGES, R2_EDITED);
|
||||
}
|
||||
/** Edit B: the Context line is reworded (a one-line hunk high in the plan, nothing moves). */
|
||||
function editB(t) {
|
||||
assert.ok(t.includes(WHY));
|
||||
return t.replace(WHY, 'Why: the brief asks for a parser CLI with clear errors.');
|
||||
}
|
||||
/** Edit C: a line is inserted below the Context line (every line below it shifts by one). */
|
||||
function editC(t) {
|
||||
assert.ok(t.includes(WHY));
|
||||
return t.replace(WHY, `${WHY}\nScope: the parser and its CLI only.`);
|
||||
}
|
||||
|
||||
// Fixed places in buildPlan(), all outside every edit above.
|
||||
const S1_CHANGES = 'Add `run1()`';
|
||||
const S1_VERIFIES = 'Verifies: run1()';
|
||||
const S1_ROLLBACK = 'checkout -- src/mod1.mjs';
|
||||
const S1_VERIFY = 'node --test tests/mod1.test.mjs';
|
||||
|
||||
const GUARD = 'The rollback named for this step gives no check that the revert actually worked';
|
||||
/** The guard: a new major outside the diff, which rule A counts as minor. */
|
||||
const guard = (t, text = GUARD, line = S1_ROLLBACK) => f('major', at(t, line), text);
|
||||
function assertGuardLowered(v, round, text = GUARD) {
|
||||
const g = byText(v, text);
|
||||
assert.equal(g.severity, 'minor', 'a new major outside the diff counts as minor');
|
||||
assert.equal(g.original_severity, 'major');
|
||||
assert.equal(g.lowered_by, `rule A: outside the diff since round ${round}`);
|
||||
}
|
||||
|
||||
/** Round 1 on buildPlan() with the given payloads; returns its result. */
|
||||
const round1 = (payload = {}) => rec(buildPlan(), dd(payload));
|
||||
|
||||
// ---- exports ---------------------------------------------------------------------
|
||||
|
||||
test('critic-diff exports the canonical rule sentences, the new verdict fields and the security check', () => {
|
||||
assert.equal(diffMod.RULE_A, 'Rule A: from round 2 on, a new major finding that cites a plan line outside the diff since the last reviewed text counts as minor; a blocker, a security finding and a finding with no plan line count at full severity, and a previous finding stays counted until it is marked resolved.');
|
||||
assert.equal(diffMod.RULE_B, 'Rule B: a major that a named test, `Regression:` command or gate in the plan will fail on during execution is minor — never a blocker, never a security finding.');
|
||||
assert.ok(Array.isArray(diffMod.DIFF_FIELDS), 'DIFF_FIELDS is a list');
|
||||
for (const k of ['diff_base', 'diff_note', 'resolved', 'op_line', 'op_line_end', 'line_end', 'rule_keys',
|
||||
'original_severity', 'lowered_by', 'caught_by', 'lowering_refused', 'carried_from']) {
|
||||
assert.ok(diffMod.DIFF_FIELDS.includes(k), `DIFF_FIELDS names ${k}`);
|
||||
}
|
||||
assert.equal(typeof diffMod.isSecurityKey, 'function', 'isSecurityKey is exported');
|
||||
assert.equal(diffMod.isSecurityKey('SECURITY_INJECTION'), true);
|
||||
assert.equal(diffMod.isSecurityKey(' security_review_vulnerability '), true);
|
||||
assert.equal(diffMod.isSecurityKey('MISSING_TEST'), false);
|
||||
assert.equal(diffMod.isSecurityKey(null), false);
|
||||
assert.equal(typeof diffMod.countRound, 'function', 'countRound is exported');
|
||||
});
|
||||
|
||||
// ---- 1 -------------------------------------------------------------------------
|
||||
|
||||
test('1: a new major outside the diff counts as minor, round 2 passes and the gate exits 0', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const r2 = rec(t2, dd({ pc: [guard(t2)] }), { base: baseOf(r1.text) });
|
||||
assertGuardLowered(r2.verdict, 1);
|
||||
assert.equal(r2.verdict.verdict, 'PASS');
|
||||
assert.deepEqual(r2.verdict.counts, { blocker: 0, major: 0, minor: 1 });
|
||||
assert.equal(r2.verdict.diff_base, planHash(r1.text));
|
||||
assert.equal(gatePlan(r2.text).code, 0, JSON.stringify(gatePlan(r2.text).reasons));
|
||||
});
|
||||
|
||||
// ---- 2 -------------------------------------------------------------------------
|
||||
|
||||
test('2: a major on the hunk\'s first and last line stays major; the lines right before and after are outside', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const first = at(t2, 'its error path');
|
||||
const last = at(t2, 'throws on empty input');
|
||||
assert.equal(last, first + 1);
|
||||
const pc = [
|
||||
f('major', first, 'Error path wording leaves the thrown type unnamed for callers'),
|
||||
f('major', last, 'Empty input behaviour conflicts with the Data Models section'),
|
||||
f('major', first - 1, 'Files list omits a fixture directory the tests will need'),
|
||||
f('major', last + 1, 'Test first block does not name a failing assertion message'),
|
||||
];
|
||||
const r2 = rec(t2, dd({ pc }), { base: baseOf(r1.text) });
|
||||
const v = r2.verdict;
|
||||
assert.equal(byText(v, 'Error path wording leaves the thrown type unnamed for callers').severity, 'major');
|
||||
assert.equal(byText(v, 'Empty input behaviour conflicts with the Data Models section').severity, 'major');
|
||||
assert.equal(byText(v, 'Files list omits a fixture directory the tests will need').severity, 'minor');
|
||||
assert.equal(byText(v, 'Test first block does not name a failing assertion message').severity, 'minor');
|
||||
assert.equal(v.verdict, 'REVISE');
|
||||
assert.deepEqual(v.counts, { blocker: 0, major: 2, minor: 2 });
|
||||
});
|
||||
|
||||
// ---- 3 -------------------------------------------------------------------------
|
||||
|
||||
test('3: a line..line_end range crossing either diff edge is inside; a range wholly outside is outside', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const first = at(t2, 'its error path');
|
||||
const last = at(t2, 'throws on empty input');
|
||||
const pc = [
|
||||
f('major', first - 1, 'Range over the top edge of the hunk names a stale file', { line_end: first }),
|
||||
f('major', last, 'Range over the bottom edge of the hunk drops the test file', { line_end: last + 1 }),
|
||||
f('major', last + 1, 'Range below the hunk repeats an unchecked verification claim', { line_end: last + 2 }),
|
||||
];
|
||||
const r2 = rec(t2, dd({ pc }), { base: baseOf(r1.text) });
|
||||
const v = r2.verdict;
|
||||
assert.equal(byText(v, 'Range over the top edge of the hunk names a stale file').severity, 'major');
|
||||
assert.equal(byText(v, 'Range over the bottom edge of the hunk drops the test file').severity, 'major');
|
||||
const out = byText(v, 'Range below the hunk repeats an unchecked verification claim');
|
||||
assert.equal(out.severity, 'minor');
|
||||
assert.equal(out.lowered_by, 'rule A: outside the diff since round 1');
|
||||
assert.equal(out.line_end, last + 2);
|
||||
});
|
||||
|
||||
// ---- 4 -------------------------------------------------------------------------
|
||||
|
||||
test('4: a new security major outside the diff stays major (catalogue key in any case, and merged into another key)', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const out = at(t2, S1_CHANGES);
|
||||
const merged = 'Step 1 builds a shell command from the plan text without quoting it first';
|
||||
const r2 = rec(t2, dd({
|
||||
pc: [
|
||||
guard(t2),
|
||||
f('major', out, 'Run one passes the input path straight into a spawned shell line', { rule_key: 'SECURITY_INJECTION' }),
|
||||
f('major', at(t2, S1_VERIFIES), 'Verification command interpolates an untrusted argument into exec', { rule_key: 'security_injection' }),
|
||||
f('major', at(t2, S1_VERIFY), merged),
|
||||
],
|
||||
sg: [f('major', at(t2, S1_VERIFY), merged, { rule_key: 'SECURITY_INJECTION' })],
|
||||
}), { base: baseOf(r1.text) });
|
||||
const v = r2.verdict;
|
||||
assertGuardLowered(v, 1);
|
||||
assert.equal(byText(v, 'Run one passes the input path straight into a spawned shell line').severity, 'major');
|
||||
assert.equal(byText(v, 'Verification command interpolates an untrusted argument into exec').severity, 'major');
|
||||
const m = byText(v, merged);
|
||||
assert.equal(m.rule_key, 'step-quality', 'the survivor keeps its own key');
|
||||
assert.equal(m.severity, 'major', 'a security key merged into a non-security survivor still counts');
|
||||
assert.equal(v.verdict, 'REVISE');
|
||||
});
|
||||
|
||||
// ---- 5 -------------------------------------------------------------------------
|
||||
|
||||
test('5: a new blocker outside the diff stays blocker (BLOCK, PLAN_GATE_OPEN_BLOCKER)', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const r2 = rec(t2, dd({ pc: [guard(t2), f('blocker', at(t2, S1_CHANGES), 'Step 1 reads a config value that no step ever writes')] }), { base: baseOf(r1.text) });
|
||||
const v = r2.verdict;
|
||||
assertGuardLowered(v, 1);
|
||||
assert.equal(byText(v, 'Step 1 reads a config value that no step ever writes').severity, 'blocker');
|
||||
assert.equal(v.verdict, 'BLOCK');
|
||||
const g = gatePlan(r2.text);
|
||||
assert.equal(g.code, 1);
|
||||
assert.ok(g.reasons.some(x => x.code === 'PLAN_GATE_OPEN_BLOCKER'), JSON.stringify(g.reasons));
|
||||
});
|
||||
|
||||
// ---- 6 -------------------------------------------------------------------------
|
||||
|
||||
test('6: a major with no plan line stays major (null line, line 0, another file, a line in ## Revisions)', () => {
|
||||
const r1 = round1();
|
||||
const t2 = `${editA(r1.text).replace(/\s*$/, '\n')}\n## Revisions\n\n| # | Finding | Fix |\n|---|---------|-----|\n| 1 | an old note on the parser | reworded |\n`;
|
||||
const rev = at(t2, 'an old note on the parser');
|
||||
const r2 = rec(t2, dd({
|
||||
pc: [
|
||||
guard(t2),
|
||||
f('major', null, 'The plan as a whole never names its rollback owner'),
|
||||
f('major', 0, 'The whole file lacks a stated performance budget for parsing'),
|
||||
{ file: 'src/mod1.mjs', line: 12, rule_key: 'step-quality', severity: 'major', text: 'Source module already exports a conflicting run helper' },
|
||||
f('major', rev, 'Revision log row contradicts the current Step 2 wording'),
|
||||
],
|
||||
}), { base: baseOf(r1.text) });
|
||||
const v = r2.verdict;
|
||||
assertGuardLowered(v, 1);
|
||||
for (const s of ['The plan as a whole never names its rollback owner', 'The whole file lacks a stated performance budget for parsing',
|
||||
'Source module already exports a conflicting run helper', 'Revision log row contradicts the current Step 2 wording']) {
|
||||
const x = byText(v, s);
|
||||
assert.equal(x.severity, 'major', s);
|
||||
assert.equal(x.lowered_by, undefined, s);
|
||||
}
|
||||
assert.equal(v.verdict, 'REVISE');
|
||||
});
|
||||
|
||||
// ---- 7 -------------------------------------------------------------------------
|
||||
|
||||
test('7: a round-1 major marked resolved by both reviewers leaves the count', () => {
|
||||
const r1 = round1({ pc: [f('major', at(buildPlan(), S1_VERIFIES), 'Step 1 verification never states the expected return value')] });
|
||||
const id = r1.verdict.findings[0].id;
|
||||
assert.match(id, /^[0-9a-f]{40}$/);
|
||||
const t2 = editA(r1.text);
|
||||
const r2 = rec(t2, dd({ pc: [guard(t2)], pcPrev: [{ id, resolved: true }], sgPrev: [{ id, resolved: true }] }), { base: baseOf(r1.text) });
|
||||
const v = r2.verdict;
|
||||
assertGuardLowered(v, 1);
|
||||
assert.deepEqual(v.resolved, [id]);
|
||||
assert.equal(byId(v, id).length, 0, 'the resolved finding is not counted');
|
||||
assert.equal(v.verdict, 'PASS');
|
||||
});
|
||||
|
||||
// ---- 8 -------------------------------------------------------------------------
|
||||
|
||||
test('8: a round-1 major that is not resolved keeps its severity (raiser silent; reworded re-report at a shifted line; resolved yet re-reported)', () => {
|
||||
const p0 = buildPlan();
|
||||
const M1 = 'Step 1 never says what run1 returns on an empty input file';
|
||||
const r1 = round1({ pc: [f('major', at(p0, S1_CHANGES), M1)] });
|
||||
const id = r1.verdict.findings[0].id;
|
||||
|
||||
// a) left out by its raiser: the other reviewer's true is not enough.
|
||||
const ta = editA(r1.text);
|
||||
const va = rec(ta, dd({ pc: [guard(ta)], sgPrev: [{ id, resolved: true }] }), { base: baseOf(r1.text) }).verdict;
|
||||
assertGuardLowered(va, 1);
|
||||
const ca = byId(va, id);
|
||||
assert.equal(ca.length, 1);
|
||||
assert.equal(ca[0].severity, 'major');
|
||||
assert.equal(ca[0].carried_from, 1);
|
||||
assert.deepEqual(va.resolved, []);
|
||||
assert.equal(va.verdict, 'REVISE');
|
||||
|
||||
// b) re-reported reworded at the line an edit above it shifted it to, outside the diff.
|
||||
const tb = editC(r1.text);
|
||||
const shifted = at(tb, S1_CHANGES);
|
||||
assert.equal(shifted, at(p0, S1_CHANGES) + 1);
|
||||
const vb = rec(tb, dd({ pc: [guard(tb), f('major', shifted, 'For an empty input file the first step leaves the run1 result unstated')] }), { base: baseOf(r1.text) }).verdict;
|
||||
assertGuardLowered(vb, 1);
|
||||
const cb = byId(vb, id);
|
||||
assert.equal(cb.length, 1, 'the reworded re-report is the old finding, not a new one');
|
||||
assert.equal(cb[0].severity, 'major');
|
||||
assert.equal(cb[0].text, M1);
|
||||
assert.equal(cb[0].line, shifted);
|
||||
assert.equal(vb.findings.filter(x => x.severity === 'major').length, 1);
|
||||
|
||||
// c) marked resolved by both, and re-reported: the re-report wins.
|
||||
const tc = editA(r1.text);
|
||||
const vc = rec(tc, dd({
|
||||
pc: [guard(tc), f('major', at(tc, S1_CHANGES), M1)],
|
||||
pcPrev: [{ id, resolved: true }], sgPrev: [{ id, resolved: true }],
|
||||
}), { base: baseOf(r1.text) }).verdict;
|
||||
assertGuardLowered(vc, 1);
|
||||
assert.equal(byId(vc, id).length, 1);
|
||||
assert.equal(byId(vc, id)[0].severity, 'major');
|
||||
assert.ok(!vc.resolved.includes(id));
|
||||
});
|
||||
|
||||
// ---- 9 -------------------------------------------------------------------------
|
||||
|
||||
test('9: a round-1 minor re-reported as major is carried at major', () => {
|
||||
const p0 = buildPlan();
|
||||
const r1 = round1({ pc: [f('minor', at(p0, S1_CHANGES), 'Step 1 change line names no return type for run1')] });
|
||||
const id = r1.verdict.findings[0].id;
|
||||
const t2 = editA(r1.text);
|
||||
const v = rec(t2, dd({ pc: [guard(t2), f('major', at(t2, S1_CHANGES), 'Step 1 change line names no return type for run1')] }), { base: baseOf(r1.text) }).verdict;
|
||||
assertGuardLowered(v, 1);
|
||||
const c = byId(v, id);
|
||||
assert.equal(c.length, 1);
|
||||
assert.equal(c[0].severity, 'major');
|
||||
assert.equal(c[0].carried_from, 1);
|
||||
assert.equal(v.verdict, 'REVISE');
|
||||
});
|
||||
|
||||
// ---- 10 ------------------------------------------------------------------------
|
||||
|
||||
test('10: one reviewer\'s true and the other\'s false keeps it; the non-raiser\'s silence is neutral', () => {
|
||||
const p0 = buildPlan();
|
||||
const r1 = round1({
|
||||
pc: [
|
||||
f('major', at(p0, S1_CHANGES), 'Step 1 never says what run1 returns on an empty input file'),
|
||||
f('major', at(p0, S1_VERIFIES), 'Verification for the first module omits the error exit code'),
|
||||
],
|
||||
});
|
||||
const [kept, gone] = r1.verdict.findings.map(x => x.id);
|
||||
const t2 = editA(r1.text);
|
||||
const v = rec(t2, dd({
|
||||
pc: [guard(t2)],
|
||||
pcPrev: [{ id: kept, resolved: true }, { id: gone, resolved: true }],
|
||||
sgPrev: [{ id: kept, resolved: false }],
|
||||
}), { base: baseOf(r1.text) }).verdict;
|
||||
assertGuardLowered(v, 1);
|
||||
assert.equal(byId(v, kept).length, 1);
|
||||
assert.equal(byId(v, kept)[0].severity, 'major');
|
||||
assert.equal(byId(v, gone).length, 0);
|
||||
assert.deepEqual(v.resolved, [gone]);
|
||||
});
|
||||
|
||||
// ---- 11 ------------------------------------------------------------------------
|
||||
|
||||
test('11: a major lowered in round 2 stays minor in round 3; a major re-report keeps it minor, a blocker re-report makes it blocker', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const L = 'Step 1 change gives run1 no documented failure mode at all';
|
||||
const r2 = rec(t2, dd({ pc: [f('major', at(t2, S1_CHANGES), L)] }), { base: baseOf(r1.text) });
|
||||
const l2 = byText(r2.verdict, L);
|
||||
assert.equal(l2.severity, 'minor');
|
||||
const id = l2.id;
|
||||
const t3 = editB(r2.text);
|
||||
const base = baseOf(r2.text);
|
||||
const NEW3 = 'The verify command for step one names no expected failing output';
|
||||
|
||||
// not resolved, not re-reported
|
||||
const v3 = rec(t3, dd({ pc: [guard(t3, NEW3, S1_VERIFY)] }), { base }).verdict;
|
||||
assert.equal(v3.round, 3);
|
||||
assertGuardLowered(v3, 2, NEW3);
|
||||
const c = byId(v3, id);
|
||||
assert.equal(c.length, 1);
|
||||
assert.equal(c[0].severity, 'minor');
|
||||
assert.equal(c[0].original_severity, 'major');
|
||||
assert.equal(c[0].lowered_by, 'rule A: outside the diff since round 1');
|
||||
assert.equal(c[0].carried_from, 2);
|
||||
assert.equal(v3.verdict, 'PASS');
|
||||
|
||||
// re-reported as major: stays minor
|
||||
const vm = rec(t3, dd({ pc: [guard(t3, NEW3, S1_VERIFY), f('major', at(t3, S1_CHANGES), L)] }), { base }).verdict;
|
||||
assert.equal(byId(vm, id).length, 1);
|
||||
assert.equal(byId(vm, id)[0].severity, 'minor');
|
||||
assert.equal(byId(vm, id)[0].original_severity, 'major');
|
||||
|
||||
// re-reported as blocker: blocker
|
||||
const vb = rec(t3, dd({ pc: [guard(t3, NEW3, S1_VERIFY), f('blocker', at(t3, S1_CHANGES), L)] }), { base }).verdict;
|
||||
assert.equal(byId(vb, id).length, 1);
|
||||
assert.equal(byId(vb, id)[0].severity, 'blocker');
|
||||
assert.equal(vb.verdict, 'BLOCK');
|
||||
});
|
||||
|
||||
// ---- 12 ------------------------------------------------------------------------
|
||||
|
||||
test('12: rule B lowers only a non-security major with caught_by; a security key or a blocker is refused', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const first = at(t2, 'its error path');
|
||||
const last = at(t2, 'throws on empty input');
|
||||
const r2 = rec(t2, dd({
|
||||
pc: [
|
||||
guard(t2),
|
||||
f('minor', at(t2, S1_CHANGES), 'Run one shells out with the raw path and nothing escapes it',
|
||||
{ rule_key: 'SECURITY_INJECTION', lowered_from: 'major', caught_by: 'tests/mod1.test.mjs' }),
|
||||
f('minor', first, 'The error path change writes partial output before it throws',
|
||||
{ lowered_from: 'blocker', caught_by: 'tests/mod2.test.mjs' }),
|
||||
f('minor', last, 'Empty input throw has no message the caller can match on',
|
||||
{ lowered_from: 'major' }),
|
||||
],
|
||||
sg: [
|
||||
f('minor', first, 'Second module wording drifts from the interface named in Design',
|
||||
{ rule_key: 'scope-drift', lowered_from: 'major', caught_by: 'node --test tests/mod2.test.mjs' }),
|
||||
],
|
||||
}), { base: baseOf(r1.text) });
|
||||
const v = r2.verdict;
|
||||
assertGuardLowered(v, 1);
|
||||
|
||||
const sec = byText(v, 'Run one shells out with the raw path and nothing escapes it');
|
||||
assert.equal(sec.severity, 'major', 'rule B never lowers a security finding');
|
||||
assert.equal(typeof sec.lowering_refused, 'string');
|
||||
assert.ok(sec.lowering_refused.length > 0);
|
||||
|
||||
const blk = byText(v, 'The error path change writes partial output before it throws');
|
||||
assert.equal(blk.severity, 'blocker', 'rule B never lowers a blocker');
|
||||
assert.equal(typeof blk.lowering_refused, 'string');
|
||||
|
||||
const noCatch = byText(v, 'Empty input throw has no message the caller can match on');
|
||||
assert.equal(noCatch.severity, 'major', 'rule B needs caught_by');
|
||||
assert.equal(typeof noCatch.lowering_refused, 'string');
|
||||
|
||||
const ok = byText(v, 'Second module wording drifts from the interface named in Design');
|
||||
assert.equal(ok.severity, 'minor');
|
||||
assert.equal(ok.original_severity, 'major');
|
||||
assert.equal(ok.lowered_by, 'rule B: node --test tests/mod2.test.mjs');
|
||||
assert.equal(ok.caught_by, 'node --test tests/mod2.test.mjs');
|
||||
assert.equal(ok.lowering_refused, undefined);
|
||||
assert.equal(v.verdict, 'BLOCK');
|
||||
});
|
||||
|
||||
// ---- 13 ------------------------------------------------------------------------
|
||||
|
||||
test('13: the same round recorded twice gives deep-equal findings, counts and verdict', () => {
|
||||
const p0 = buildPlan();
|
||||
const r1 = round1({
|
||||
pc: [
|
||||
f('major', at(p0, S1_CHANGES), 'Step 1 never says what run1 returns on an empty input file'),
|
||||
f('minor', at(p0, S1_VERIFIES), 'Verification for the first module omits the error exit code'),
|
||||
],
|
||||
});
|
||||
const [, m2] = r1.verdict.findings.map(x => x.id);
|
||||
const t2 = editA(r1.text);
|
||||
const d = dd({
|
||||
pc: [guard(t2), f('major', at(t2, 'its error path'), 'Error path wording leaves the thrown type unnamed for callers')],
|
||||
pcPrev: [{ id: m2, resolved: true }],
|
||||
});
|
||||
const a = rec(t2, d, { base: baseOf(r1.text) });
|
||||
const b = rec(t2, d, { base: baseOf(r1.text) });
|
||||
assertGuardLowered(a.verdict, 1);
|
||||
assert.deepEqual(a.verdict.findings, b.verdict.findings);
|
||||
assert.deepEqual(a.verdict.counts, b.verdict.counts);
|
||||
assert.equal(a.verdict.verdict, b.verdict.verdict);
|
||||
assert.deepEqual(a.verdict, b.verdict);
|
||||
assert.equal(a.text, b.text);
|
||||
});
|
||||
|
||||
// ---- 14 ------------------------------------------------------------------------
|
||||
|
||||
test('14: no base, a base that is not the previous round\'s text, or an unchanged plan: the round is counted as today, with one diff_note', () => {
|
||||
const p0 = buildPlan();
|
||||
const r1 = round1({ pc: [f('major', at(p0, S1_VERIFIES), 'Verification for the first module omits the error exit code')] });
|
||||
const prevId = r1.verdict.findings[0].id;
|
||||
const t2 = editA(r1.text);
|
||||
const cases = [
|
||||
{ name: 'no base', text: t2, opts: { base: null } },
|
||||
{ name: 'no base, with a reason', text: t2, opts: { base: null, baseNote: 'snapshot missing: no file for round 1' }, note: /snapshot missing/ },
|
||||
{ name: 'base is not round 1\'s text', text: t2, opts: { base: baseOf(editB(r1.text)) } },
|
||||
{ name: 'plan unchanged since round 1', text: r1.text, opts: { base: baseOf(r1.text) }, note: /has not changed since round 1/ },
|
||||
];
|
||||
for (const c of cases) {
|
||||
const v = rec(c.text, dd({ pc: [guard(c.text)] }), c.opts).verdict;
|
||||
const g = byText(v, GUARD);
|
||||
assert.equal(g.severity, 'major', `${c.name}: an outside major stays major`);
|
||||
assert.equal(g.lowered_by, undefined, c.name);
|
||||
assert.equal(byId(v, prevId).length, 0, `${c.name}: a previous finding left out is not carried`);
|
||||
assert.ok(!('resolved' in v), `${c.name}: resolved is absent`);
|
||||
assert.ok(!('diff_base' in v), `${c.name}: diff_base is absent`);
|
||||
assert.equal(typeof v.diff_note, 'string', `${c.name}: one diff_note`);
|
||||
assert.ok(v.diff_note.length > 0 && !v.diff_note.includes('\n'), `${c.name}: diff_note is one line`);
|
||||
if (c.note) assert.match(v.diff_note, c.note, c.name);
|
||||
assert.equal(v.verdict, 'REVISE', c.name);
|
||||
}
|
||||
});
|
||||
|
||||
// ---- 15 ------------------------------------------------------------------------
|
||||
|
||||
/** A round's verdict block exactly as plan-gate wrote it before decision 43 (no new fields). */
|
||||
function frozenBlock(plan, { round = 1, findings = [], accepted = false } = {}) {
|
||||
const sha = planHash(plan);
|
||||
const counts = { blocker: 0, major: 0, minor: 0 };
|
||||
for (const x of findings) counts[x.severity] += 1;
|
||||
const v = {
|
||||
schema: 'voyage.plan-verdict/1',
|
||||
round,
|
||||
max_rounds: 3,
|
||||
extra_rounds: 0,
|
||||
recorded_at: '2026-10-01T08:00:00.000Z',
|
||||
plan_sha256: sha,
|
||||
verdict: counts.blocker ? 'BLOCK' : counts.major ? 'REVISE' : 'PASS',
|
||||
counts,
|
||||
findings,
|
||||
};
|
||||
if (accepted) {
|
||||
v.accepted_open = {
|
||||
round, plan_sha256: sha, accepted_at: '2026-10-01T09:00:00.000Z',
|
||||
findings: findings.filter(x => x.severity === 'major').map(x => ({ id: x.id, text: x.text })),
|
||||
};
|
||||
}
|
||||
return `${plan.replace(/\s*$/, '\n')}\n## Critic Verdict\n\n<!-- Written by lib/validators/plan-gate.mjs --record (round ${round} of 3). Do not edit by hand: the gate re-hashes the plan and reads this block. -->\n\n\`\`\`json\n${JSON.stringify(v, null, 2)}\n\`\`\`\n`;
|
||||
}
|
||||
const old = (severity, text) => ({ id: '1'.repeat(40), severity, rule_key: 'step-quality', file: 'plan.md', line: 49, text, raised_by: ['plan-critic'] });
|
||||
|
||||
test('15: a frozen round-1 block with no new fields gives today\'s exits; a round recorded on top of it counts fully', () => {
|
||||
const p0 = buildPlan();
|
||||
assert.equal(gatePlan(frozenBlock(p0, { findings: [old('minor', 'a typo in the context')] })).code, 0, 'minors → 0');
|
||||
assert.equal(gatePlan(frozenBlock(p0, { findings: [old('major', 'Step 1 lacks an error path')] })).code, 1, 'major, rounds left → 1');
|
||||
assert.equal(gatePlan(frozenBlock(p0, { findings: [old('blocker', 'Step 2 reads nothing')] })).code, 1, 'blocker → 1');
|
||||
const final = frozenBlock(p0, { round: 3, findings: [old('major', 'Step 1 lacks an error path')] });
|
||||
assert.equal(gatePlan(final).code, 1, 'major after round 3, not accepted → 1');
|
||||
assert.equal(gatePlan(frozenBlock(p0, { round: 3, findings: [old('major', 'Step 1 lacks an error path')], accepted: true })).code, 0, 'after round 3 with accepted_open → 0');
|
||||
const a = acceptOpen(final, { now: NOW });
|
||||
assert.equal(a.code, 0, a.message);
|
||||
assert.equal(gatePlan(a.text).code, 0);
|
||||
|
||||
// A round on top of the frozen block: no snapshot was ever kept for it, so it counts fully.
|
||||
const t2 = editA(frozenBlock(p0, { findings: [old('major', 'Step 1 lacks an error path')] }));
|
||||
const v = rec(t2, dd({ pc: [guard(t2)] }), { base: null }).verdict;
|
||||
assert.equal(v.round, 2);
|
||||
assert.equal(byText(v, GUARD).severity, 'major');
|
||||
assert.equal(byId(v, '1'.repeat(40)).length, 0);
|
||||
assert.equal(typeof v.diff_note, 'string');
|
||||
assert.equal(v.verdict, 'REVISE');
|
||||
});
|
||||
|
||||
// ---- 16 ------------------------------------------------------------------------
|
||||
|
||||
test('16: security on carry — a lowered finding re-reported with a security key counts at major; keys and raisers are the union', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const L = 'Step 1 change gives run1 no documented failure mode at all';
|
||||
const r2 = rec(t2, dd({ pc: [f('major', at(t2, S1_CHANGES), L)] }), { base: baseOf(r1.text) });
|
||||
const id = byText(r2.verdict, L).id;
|
||||
assert.equal(byText(r2.verdict, L).severity, 'minor');
|
||||
const t3 = editB(r2.text);
|
||||
const NEW3 = 'The verify command for step one names no expected failing output';
|
||||
const v3 = rec(t3, dd({
|
||||
pc: [guard(t3, NEW3, S1_VERIFY)],
|
||||
sg: [f('major', at(t3, S1_CHANGES), 'Run one feeds the unescaped input path into a shell command line', { rule_key: 'SECURITY_INJECTION' })],
|
||||
}), { base: baseOf(r2.text) }).verdict;
|
||||
assertGuardLowered(v3, 2, NEW3);
|
||||
const c = byId(v3, id);
|
||||
assert.equal(c.length, 1, JSON.stringify(v3.findings));
|
||||
assert.equal(c[0].severity, 'major');
|
||||
assert.deepEqual([...c[0].rule_keys].sort(), ['SECURITY_INJECTION', 'step-quality']);
|
||||
assert.deepEqual([...c[0].raised_by].sort(), ['plan-critic', 'scope-guardian']);
|
||||
assert.equal(v3.verdict, 'REVISE');
|
||||
|
||||
// A hand-built dedup finding with a security rule_key and no rule_keys.
|
||||
const t2b = editA(r1.text);
|
||||
const hand = {
|
||||
findings: [
|
||||
{ id: 'a'.repeat(40), file: 'plan.md', line: at(t2b, S1_CHANGES), rule_key: 'SECURITY_INJECTION', severity: 'major', text: 'Hand built: the path reaches exec unquoted', raised_by: ['plan-critic'] },
|
||||
{ id: 'b'.repeat(40), file: 'plan.md', line: at(t2b, S1_ROLLBACK), rule_key: 'step-quality', severity: 'major', text: GUARD, raised_by: ['plan-critic'] },
|
||||
],
|
||||
marks: [],
|
||||
sources: BOTH,
|
||||
};
|
||||
const vh = rec(t2b, hand, { base: baseOf(r1.text) }).verdict;
|
||||
assertGuardLowered(vh, 1);
|
||||
assert.equal(byText(vh, 'Hand built: the path reaches exec unquoted').severity, 'major');
|
||||
});
|
||||
|
||||
// ---- 17 ------------------------------------------------------------------------
|
||||
|
||||
test('17: a finding inside the diff is never matched — worded like a previous lowered finding, it counts at major', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const L = 'Step 1 change gives run1 no documented failure mode at all for callers';
|
||||
const r2 = rec(t2, dd({ pc: [f('major', at(t2, S1_CHANGES), L)] }), { base: baseOf(r1.text) });
|
||||
const id = byText(r2.verdict, L).id;
|
||||
const t3 = editB(r2.text);
|
||||
const LIKE = 'Context gives run1 no documented failure mode at all for callers';
|
||||
assert.ok(contentOverlap(L, LIKE) >= 0.7, 'the wording overlaps at least 0.7');
|
||||
const v3 = rec(t3, dd({ pc: [f('major', at(t3, 'with clear errors'), LIKE)] }), { base: baseOf(r2.text) }).verdict;
|
||||
const n = byText(v3, LIKE);
|
||||
assert.equal(n.severity, 'major');
|
||||
assert.equal(n.lowered_by, undefined);
|
||||
assert.notEqual(n.id, id);
|
||||
const c = byId(v3, id);
|
||||
assert.equal(c.length, 1, 'the previous finding is carried on its own');
|
||||
assert.equal(c[0].severity, 'minor');
|
||||
assert.equal(v3.verdict, 'REVISE');
|
||||
});
|
||||
|
||||
// ---- 18 ------------------------------------------------------------------------
|
||||
|
||||
test('18: carried positions — another file keeps its line; an edited own line keeps the old line with op_line null; a shifted line moves', () => {
|
||||
const p0 = buildPlan();
|
||||
const ownOld = at(p0, R2_CHANGES);
|
||||
const shiftedOld = at(p0, S1_CHANGES);
|
||||
const r1 = round1({
|
||||
pc: [
|
||||
{ file: 'src/mod1.mjs', line: 12, rule_key: 'step-quality', severity: 'major', text: 'Source module already exports a conflicting run helper' },
|
||||
f('major', ownOld, 'Second step change names no return value for run2'),
|
||||
f('major', shiftedOld, 'Step 1 never says what run1 returns on an empty input file'),
|
||||
],
|
||||
});
|
||||
const ids = Object.fromEntries(r1.verdict.findings.map(x => [x.text, x.id]));
|
||||
const t2 = editA(editC(r1.text));
|
||||
const r2 = rec(t2, dd({ pc: [guard(t2)] }), { base: baseOf(r1.text) });
|
||||
const v2 = r2.verdict;
|
||||
assertGuardLowered(v2, 1);
|
||||
|
||||
const src = byId(v2, ids['Source module already exports a conflicting run helper']);
|
||||
assert.equal(src.length, 1);
|
||||
assert.equal(src[0].file, 'src/mod1.mjs');
|
||||
assert.equal(src[0].line, 12, 'a finding on another file keeps its line');
|
||||
|
||||
const own = byId(v2, ids['Second step change names no return value for run2']);
|
||||
assert.equal(own.length, 1);
|
||||
assert.equal(own[0].line, ownOld, 'its own line was edited: it keeps the old line');
|
||||
assert.equal(own[0].op_line, null);
|
||||
|
||||
const moved = byId(v2, ids['Step 1 never says what run1 returns on an empty input file']);
|
||||
assert.equal(moved.length, 1);
|
||||
assert.equal(moved[0].line, at(t2, S1_CHANGES));
|
||||
assert.equal(moved[0].line, shiftedOld + 1);
|
||||
assert.ok(Number.isInteger(moved[0].op_line));
|
||||
|
||||
// A finding whose op_line went null keeps its line and op_line null on the next carry.
|
||||
const t3 = editB(r2.text);
|
||||
const v3 = rec(t3, dd(), { base: baseOf(r2.text) }).verdict;
|
||||
const own3 = byId(v3, ids['Second step change names no return value for run2']);
|
||||
assert.equal(own3.length, 1);
|
||||
assert.equal(own3[0].line, ownOld);
|
||||
assert.equal(own3[0].op_line, null);
|
||||
assert.equal(own3[0].carried_from, 1);
|
||||
});
|
||||
|
||||
// ---- acceptOpen after round 3 ------------------------------------------------------
|
||||
|
||||
test('acceptOpen after round 3 lists only the real majors, never a lowered one', () => {
|
||||
const r1 = round1();
|
||||
const t2 = editA(r1.text);
|
||||
const r2 = rec(t2, dd({ pc: [guard(t2)] }), { base: baseOf(r1.text) });
|
||||
const t3 = editB(r2.text);
|
||||
const REAL = 'Context rewording drops the stated reason for the parser CLI';
|
||||
const r3 = rec(t3, dd({ pc: [f('major', at(t3, 'with clear errors'), REAL), guard(t3, 'The verify command for step one names no expected failing output', S1_VERIFY)] }), { base: baseOf(r2.text) });
|
||||
assert.equal(r3.verdict.round, 3);
|
||||
const a = acceptOpen(r3.text, { now: NOW });
|
||||
assert.equal(a.code, 0, a.message);
|
||||
const v = JSON.parse(a.text.match(/```json\n([\s\S]*?)\n```/)[1]);
|
||||
assert.deepEqual(v.accepted_open.findings.map(x => x.text), [REAL]);
|
||||
assert.equal(gatePlan(a.text).code, 0);
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue