voyage/lib/verification/test-integrity.mjs
Kjell Tore Guttormsen 29b4ac91b3
fix: round 7 of the release review of 6.3.0, for 6.3.1
Round 7 (516dffa..45aa9e1, WARN: 12 MAJOR, 0 BLOCKER) and the fence defect
found while writing its review.md. Each fix began with a test that failed on
0f400d3 (v6.3.0); suite 2208 (2206/0/2), 14 tests more.

- Tests-only (operator 2026-10-11): the plan's Files: alone decides, and the
  text says so (red-proof, trekexecute Sub-step B, step-implementer,
  HANDOVER-CONTRACTS). red-proof no longer reads the step's diff, so a change
  already in the tree when the step began no longer counts against it, and a
  plan placed outside .claude/ reads the same. The round-6 test that had the
  diff overrule the plan is rewritten to the new rule.
- The Files: reader keeps a plain file beside a backticked path
  (", Dockerfile", "the CLI entry src/cli.mjs", "and Makefile"). Measured on
  the 520 Files: fields in 182 plans: no field reads differently.
- Red proof reports a command its timeout killed as killed after the timeout,
  not "could not start"; the release check tells a timeout from output past
  its buffer and from another signal. One reading of a stop
  (criteria-runner stoppedBy) for red proof, the criteria runner and the
  release check.
- A trailing `# comment` is dropped in a backticked check too, and in a
  command passed as written, so the words screened are the words run.
- The release check reads a relative --review in --repo.
- The 15-minute timeout is defined once (criteria-runner.mjs).
- The findings check reads each cited file once per validation.
- One isMain helper (lib/util/is-main.mjs) for every CLI: the 31 copies of
  the realpath guard and the 20 modules still on the file:// guard, which
  exited 0 having done nothing under a symlink or a path with a space.
- The Bash guard writes its no-one-to-ask refusal once, on a direct condition.
- The findings check ends a ```json block only at a fence on its own line, so
  three backticks inside a JSON string no longer end it (also in 6.2.0).
- Brief-conformance notes: a test for test-lock green on exit 127 (it
  passed at once; it fails when 127 is read as could-not-run); the prose now
  names the git revision `@~1` as the one leading-@ exception.

Not changed: the example plan's steps 1-2 keep their tests-only warning
(operator decision 2026-10-09; the r5 test records it). Brief note 4 (a
Regression: whose runner is not installed goes to the fix rounds) is a
routing question for the operator.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 09:11:57 +02:00

278 lines
13 KiB
JavaScript

// lib/verification/test-integrity.mjs
// The floor guard (P9 (2)): read a step's diff and report every change that
// makes the tests weaker instead of the code better.
//
// removed-assert a test file loses more assertions than it gains
// deleted-test a test file loses more test declarations than it gains
// skipped-test a test is skipped, marked todo/xfail, or focused (.only)
// muted-check a check is silenced: `|| true`, continue-on-error,
// --passWithNoTests, an inline lint/type suppression ...
// stub non-test code gains a "not implemented" body
// loosened-threshold a numeric bound in a test or a check config moves in the
// permissive direction (`ms < 200` -> `ms < 2000`)
//
// Exit 0 = clean, 1 = findings, 2 = could not run (no input, unreadable input,
// an unresolvable --base). 2 is never green: an absent measurement is reported
// as absent, not as a pass.
//
// A finding is not a verdict. /trekexecute hands every finding to its
// controller, which either quotes the plan line that asked for the change or
// fails the step; nothing is dropped silently.
//
// Borrowed: the floor guard of addyosmani/agent-skills@9d0c60d (MIT),
// constraint-driven development - mechanized here as a diff reader so a
// command, not the model's word, says whether a test was weakened.
import { readFileSync } from 'node:fs';
import { spawnSync } from 'node:child_process';
import { resolve } from 'node:path';
import { isTestPath } from '../util/test-path.mjs';
import { isMain } from '../util/is-main.mjs';
export { isTestPath };
// --- what counts as a test / a check config ----------------------------------
// A test path is lib/util/test-path.mjs (shared with the plan reader).
const CHECK_CONFIG_RE =
/(^|\/)(\.github|\.forgejo|\.gitea|\.woodpecker)\/|(^|\/)(\.gitlab-ci\.yml|Makefile|package\.json|pyproject\.toml|setup\.cfg|tox\.ini|\.pre-commit-config\.yaml|tsconfig[^/]*\.json|\.eslintrc[^/]*|eslint\.config\.[a-z]+|(jest|vitest|playwright)\.config\.[a-z]+|\.c8rc[^/]*|\.nycrc[^/]*|codecov\.ya?ml)$/;
const isCheckPath = (p) => isTestPath(p) || CHECK_CONFIG_RE.test(p);
const PROSE_RE = /\.(md|markdown|mdx|txt|rst|adoc)$/i;
// --- the patterns -------------------------------------------------------------
const ASSERT_RE =
/\bassert\w*\b|\bexpect\s*\(|\.should\b|\bt\.(equal|is|ok|true|false|deepEqual|throws|notThrows|match)\b|\bself\.assert\w+|\brequire\.\w+\(|\bt\.(Error|Fatal)f?\(/g;
const TEST_DECL_RE =
/^\s*(test|it|describe|suite)(\.\w+)?\s*\(|^\s*(async\s+)?def\s+test_\w*|^\s*func\s+Test\w*|^\s*#\[test\]|@Test\b/;
const SKIP_RE =
/\b(test|it|describe|suite)\.(skip|skipIf|todo|only)\s*\(|\bt\.(skip|todo)\s*\(|\bx(it|describe|test)\s*\(|\bf(it|describe)\s*\(|\{\s*(skip|todo)\s*:\s*(true|['"`])|@pytest\.mark\.(skip|skipif|xfail)\b|\bpytest\.skip\s*\(|@unittest\.skip|\bt\.Skip(Now|f)?\s*\(|#\[ignore\]|@(Disabled|Ignore)\b/;
// Inline suppressions silence a check wherever they appear.
const SUPPRESS_RE =
/@ts-ignore|@ts-nocheck|eslint-disable|#\s*type:\s*ignore|#\s*noqa\b|\/\/\s*nolint\b|#\s*pragma:\s*no\s*cover|istanbul\s+ignore|c8\s+ignore/;
// Exit-code mutes count only where a check runs: tests, scripts under tests/, CI
// and check configs. In ordinary code `|| true` is a normal shell idiom.
const MUTE_RE =
/\|\|\s*true\b|\|\|\s*:\s*($|[;)])|\|\|\s*exit\s+0\b|continue-on-error\s*:\s*true|--passWithNoTests|--pass-with-no-tests|\bset\s+\+e\b|--no-verify\b|allow_failure\s*:\s*true/;
const STUB_RE =
/throw\s+new\s+Error\s*\(\s*['"`]\s*(not\s+implemented|unimplemented|todo)\b|\braise\s+NotImplementedError\b|\b(unimplemented|todo)!\s*\(|panic\s*\(\s*"(not\s+implemented|todo)|(\/\/|#)\s*stub\b|TODO:?\s*implement/i;
// Number after a "smaller is stricter" bound: bigger = looser.
const UPPER_BOUND_RE = /(<=?|toBeLessThan(OrEqual)?|lessThan|\bmax\w*|timeout\w*|tolerance|epsilon|delta|within|retries)\W*$/i;
// Number after a "bigger is stricter" bound: smaller = looser.
const LOWER_BOUND_RE = /(>=?|toBeGreaterThan(OrEqual)?|greaterThan|\bmin\w*|threshold|coverage|branches|lines|functions|statements)\W*$/i;
const NUM_RE = /\d+(?:\.\d+)?/g;
// --- diff parsing ---------------------------------------------------------------
/**
* Parse a unified diff into files with hunks of {type: ' '|'-'|'+', text, line}.
* Returns null when non-empty input carries no diff header at all.
*/
export function parseDiff(text) {
const src = String(text ?? '');
if (src.trim() === '') return [];
if (!/^(diff --git |--- |\+\+\+ )/m.test(src)) return null;
const files = [];
let file = null;
let hunk = null;
let oldLine = 0;
let newLine = 0;
// Lines still owed to the current hunk, from its @@ header. While a hunk is
// owed lines, every line is content - even one that reads "--- x" or
// "+++ y" (a removed "-- x" / an added "++ y"). Headers only between hunks.
let oldLeft = 0;
let newLeft = 0;
const lines = src.split('\n');
for (const l of lines) {
if (hunk && (oldLeft > 0 || newLeft > 0)) {
if (l.startsWith('\\')) continue; // "\ No newline at end of file"
if (l.startsWith('-')) { hunk.push({ type: '-', text: l.slice(1), line: oldLine++ }); oldLeft--; continue; }
if (l.startsWith('+')) { hunk.push({ type: '+', text: l.slice(1), line: newLine++ }); newLeft--; continue; }
if (l.startsWith(' ') || l === '') {
hunk.push({ type: ' ', text: l.slice(1), line: newLine });
oldLine++; newLine++; oldLeft--; newLeft--;
continue;
}
// Anything else ends a malformed hunk early; fall through to headers.
oldLeft = 0; newLeft = 0;
}
if (l.startsWith('diff --git ')) {
const m = l.match(/^diff --git a\/(.+) b\/(.+)$/);
file = { path: m ? m[2] : l.slice(11), deleted: false, hunks: [] };
files.push(file);
hunk = null;
continue;
}
if (l.startsWith('--- ')) {
if (!file || file.hunks.length > 0) { file = { path: '', deleted: false, hunks: [] }; files.push(file); }
const oldPath = l.slice(4).replace(/^a\//, '');
if (!file.path && oldPath !== '/dev/null') file.path = oldPath;
hunk = null;
continue;
}
if (l.startsWith('+++ ') && file) {
const newPath = l.slice(4).replace(/^b\//, '');
if (newPath === '/dev/null') file.deleted = true;
else file.path = newPath;
continue;
}
const h = l.match(/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/);
if (h && file) {
oldLine = Number(h[1]);
newLine = Number(h[3]);
oldLeft = h[2] === undefined ? 1 : Number(h[2]);
newLeft = h[4] === undefined ? 1 : Number(h[4]);
hunk = [];
file.hunks.push(hunk);
}
}
return files;
}
// --- detectors ------------------------------------------------------------------
const count = (lines, re) => lines.reduce((n, l) => n + (l.text.match(new RegExp(re.source, 'g')) || []).length, 0);
const norm = (s) => s.trim().replace(/\s+/g, ' ');
function addedNotRemoved(removed, added, re) {
const before = new Set(removed.filter((l) => re.test(l.text)).map((l) => norm(l.text)));
return added.filter((l) => re.test(l.text) && !before.has(norm(l.text)));
}
/** A paired -/+ line whose only change is numbers; returns the looser numbers. */
function loosened(oldText, newText) {
const shape = (s) => norm(s).replace(NUM_RE, '#');
if (shape(oldText) !== shape(newText)) return null;
const a = [...norm(oldText).matchAll(NUM_RE)];
const b = [...norm(newText).matchAll(NUM_RE)];
for (let i = 0; i < a.length; i++) {
const before = Number(a[i][0]);
const after = Number(b[i][0]);
if (before === after) continue;
const lead = norm(newText).slice(Math.max(0, b[i].index - 30), b[i].index);
if (UPPER_BOUND_RE.test(lead) && after > before) return `${before} -> ${after}`;
if (LOWER_BOUND_RE.test(lead) && after < before) return `${before} -> ${after}`;
}
return null;
}
function finding(kind, file, l, detail) {
return { kind, file, line: l ? l.line : null, text: l ? l.text.trim() : '', ...(detail ? { detail } : {}) };
}
export function analyzeDiff(text) {
const files = parseDiff(text);
if (files === null) return { status: 'unreadable', findings: [], reason: 'the input is not a unified diff' };
const findings = [];
for (const f of files) {
const all = f.hunks.flat();
const removed = all.filter((l) => l.type === '-');
const added = all.filter((l) => l.type === '+');
const test = isTestPath(f.path);
// Prose (docs, notes) describes code; a "not implemented" or an
// eslint-disable written in it is text, not a stub or a muted check.
const prose = PROSE_RE.test(f.path);
if (test) {
const lostAsserts = count(removed, ASSERT_RE) - count(added, ASSERT_RE);
if (lostAsserts > 0) {
findings.push(finding('removed-assert', f.path, removed.find((l) => new RegExp(ASSERT_RE.source).test(l.text)), `${lostAsserts} fewer`));
}
const lostTests = removed.filter((l) => TEST_DECL_RE.test(l.text)).length - added.filter((l) => TEST_DECL_RE.test(l.text)).length;
if (lostTests > 0 || (f.deleted && removed.some((l) => TEST_DECL_RE.test(l.text)))) {
findings.push(finding('deleted-test', f.path, removed.find((l) => TEST_DECL_RE.test(l.text)), f.deleted ? 'test file deleted' : `${lostTests} fewer`));
}
for (const l of addedNotRemoved(removed, added, SKIP_RE)) findings.push(finding('skipped-test', f.path, l));
}
if (!prose) for (const l of addedNotRemoved(removed, added, SUPPRESS_RE)) findings.push(finding('muted-check', f.path, l));
if (isCheckPath(f.path)) {
for (const l of addedNotRemoved(removed, added, MUTE_RE)) findings.push(finding('muted-check', f.path, l));
for (const h of f.hunks) {
const minus = h.filter((l) => l.type === '-');
const plus = h.filter((l) => l.type === '+');
for (const m of minus) {
for (const p of plus) {
const d = loosened(m.text, p.text);
if (d) { findings.push(finding('loosened-threshold', f.path, p, d)); break; }
}
}
}
}
if (!test && !prose) for (const l of addedNotRemoved(removed, added, STUB_RE)) findings.push(finding('stub', f.path, l));
}
return { status: findings.length ? 'findings' : 'clean', findings };
}
// --- CLI ------------------------------------------------------------------------
const USAGE = 'usage: test-integrity.mjs (--diff <file|-> | --base <ref> [--head <ref>]) [--cwd <dir>] [--json]';
function diffFromGit(cwd, base, head) {
const ok = spawnSync('git', ['rev-parse', '--verify', '--quiet', `${base}^{commit}`], { cwd, encoding: 'utf8' });
if (ok.status !== 0) return { error: `cannot resolve --base ${base} in ${cwd}` };
if (head) {
const okHead = spawnSync('git', ['rev-parse', '--verify', '--quiet', `${head}^{commit}`], { cwd, encoding: 'utf8' });
if (okHead.status !== 0) return { error: `cannot resolve --head ${head} in ${cwd}` };
}
// --no-renames: a rename is read as a deletion plus an addition. Paired as a
// rename, `foo.test.js -> foo.test.js.bak` showed no removed line at all and
// switched a test off silently (review, 2026-10-06).
const args = ['diff', '--no-color', '--no-ext-diff', '--no-renames', base, ...(head ? [head] : [])];
const r = spawnSync('git', args, { cwd, encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 });
if (r.status !== 0) return { error: `git diff failed: ${String(r.stderr).trim()}` };
return { text: r.stdout };
}
export function main(argv) {
let diffPath = null;
let base = null;
let head = null;
let cwd = process.cwd();
let json = false;
for (let i = 0; i < argv.length; i++) {
const a = argv[i];
if (a === '--diff' && argv[i + 1] !== undefined) diffPath = argv[++i];
else if (a === '--base' && argv[i + 1]) base = argv[++i];
else if (a === '--head' && argv[i + 1]) head = argv[++i];
else if (a === '--cwd' && argv[i + 1]) cwd = resolve(argv[++i]);
else if (a === '--json') json = true;
else { process.stderr.write(`test-integrity: unknown argument ${a}\n${USAGE}\n`); return 2; }
}
if ((diffPath === null) === (base === null)) {
process.stderr.write(`test-integrity: give exactly one of --diff or --base\n${USAGE}\n`);
return 2;
}
let text;
if (diffPath !== null) {
try { text = readFileSync(diffPath === '-' ? 0 : diffPath, 'utf8'); }
catch (e) { return out({ status: 'unreadable', findings: [], reason: `cannot read ${diffPath}: ${e.message}` }, json); }
} else {
const g = diffFromGit(cwd, base, head);
if (g.error) return out({ status: 'unreadable', findings: [], reason: g.error }, json);
text = g.text;
}
return out(analyzeDiff(text), json);
}
function out(r, json) {
if (json) process.stdout.write(JSON.stringify(r, null, 2) + '\n');
else if (r.status === 'unreadable') process.stdout.write(`floor guard: NOT RUN - ${r.reason} (exit 2, never green)\n`);
else if (r.status === 'clean') process.stdout.write('floor guard: clean\n');
else {
process.stdout.write(`floor guard: ${r.findings.length} finding(s)\n`);
for (const f of r.findings) process.stdout.write(` ${f.kind} ${f.file}:${f.line ?? '-'} ${f.text}${f.detail ? ` (${f.detail})` : ''}\n`);
}
return r.status === 'clean' ? 0 : r.status === 'findings' ? 1 : 2;
}
if (isMain(import.meta.url)) process.exitCode = main(process.argv.slice(2));