fix(verification): red proof prints the list it ran and refuses a plan it cannot read

Operator decision 30 (14.10); review findings 793fc1aa, 09ae46d8, d01df508, 11ee78c5 and 289e59cd.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-14 11:50:37 +02:00
commit 73e53f3134
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 141 additions and 28 deletions

View file

@ -22,15 +22,20 @@
// With `--plan <plan.md> --step <N>` the test list is also checked against the
// step's `Test first:` File line (plan-schema `testFirstFiles`: the leading
// backticked paths, never a path in the description - operator decision 20).
// No `--tests`: the plan's list is used. A `--tests` whose set differs from the
// plan's is `not-run` with reason `tests-mismatch` and the plan's list in
// `plan_tests` (review cf241526). A step whose plan list is empty (a 1.7 step
// without `Test first:`) or cannot be read changes nothing.
// No `--tests`: the plan's list is used, and that is the documented call. A
// `--tests` whose set differs from the plan's is `not-run` with reason
// `tests-mismatch` and the plan's list in `plan_tests` (review cf241526). A plan
// or step that cannot be read is `not-run` with a reason that starts
// `plan-unreadable`, whether or not `--tests` is given (operator decision 30,
// 2026-10-14). An empty plan list (a 1.7 step without `Test first:`) changes
// nothing. Every result from the point the test list is settled on carries it in
// `tests` (the list it ran); the text output prints it on a ` tests:` line, and
// `plan_tests` on a ` plan_tests:` line, comma-joined as `--tests` takes it.
//
// Exit 0 = proven, 1 = rejected (`not-red-at-base` or `not-green-at-result`),
// 2 = could not run (`not-run`: no test files, an unknown start point, a command
// outside the test-runner allowlist, a command that cannot start, a control run
// that stays red). 2 is never a pass and never "proven".
// 2 = could not run (`not-run`: no test files, a plan or step that cannot be
// read, an unknown start point, a command outside the test-runner allowlist, a
// command that cannot start, a control run that stays red). 2 is never a pass and never "proven".
//
// The start point is a temp `git clone --shared` of the repo checked out at
// start_sha - a real git checkout, so a git-reading command behaves as it does
@ -119,8 +124,9 @@ const notRun = (reason, extra = {}) => ({ status: 'not-run', reason, ...extra })
// PLAN_STEP_TESTS_ONLY). Such a step can never be red: the behaviour its tests
// cover already exists at its start point. Rewriting the test cannot fix that, so
// the executor sends the step up to the plan instead of into fix rounds
// (voyage-lekerepo trial 3, 2026-10-09). `null` = the plan or the step could
// not be read; without `--plan` the field is absent and nothing new happens.
// (voyage-lekerepo trial 3, 2026-10-09). Without `--plan` the field is absent
// and nothing new happens; a plan or step that cannot be read never reaches here
// (`proveRed` returns `plan-unreadable` first).
//
// The plan's `Files:` alone decides (operator 2026-10-11): the step's diff is
// not read. In the documented flow the helper writes only the test before this
@ -131,7 +137,6 @@ const notRun = (reason, extra = {}) => ({ status: 'not-run', reason, ...extra })
// controller's scope finding (trekexecute Sub-step C, Hard rule 2), not this check's.
function testsOnly(step) {
if (!step) return {};
if (step.error) return { tests_only: null, tests_only_note: step.error };
return { tests_only: stepTestsOnlyFromFields(step.fields, step.version).testsOnly };
}
@ -202,8 +207,14 @@ function applyResult(root, tree, base) {
}
/**
* With `plan` and `step`: a plan or step that cannot be read is `not-run` with a
* reason that starts `plan-unreadable`; no `tests` means the step's `Test first:`
* File line is the list; a `tests` that is another set is `tests-mismatch` (the
* plan's list in `plan_tests`); an empty plan list (a 1.7 step without
* `Test first:`) changes nothing. Every result once the list is settled on
* carries it in `tests` (the list it ran).
* @param {{cwd: string, base: string, cmd: string, tests?: string[], link?: string[], redOnly?: boolean, timeoutMs?: number, plan?: string, step?: number}} o
* @returns {{status: 'proven'|'rejected'|'not-run', reason: string, plan_tests?: string[], base?: object, result?: object, control?: object}}
* @returns {{status: 'proven'|'rejected'|'not-run', reason: string, tests?: string[], plan_tests?: string[], base?: object, result?: object, control?: object}}
*/
export function proveRed(o) {
const given = resolve(o.cwd ?? process.cwd());
@ -214,9 +225,11 @@ export function proveRed(o) {
if (!cwd) return notRun(`--cwd ${given} is not inside a git repository`);
// The step's own test list (review cf241526): the step's `Test first:` File
// line as testFirstFiles reads it (operator decision 20), taken when none is
// given; a given list that is another set is refused. An empty or unreadable
// plan list changes nothing.
// given; a given list that is another set is refused. A plan or step that
// cannot be read is refused with or without a given list (operator decision
// 30); an empty plan list changes nothing.
const step = planStep(o);
if (step?.error) return notRun(`plan-unreadable: ${step.error}`);
const fromPlan = step?.fields ? testFirstFiles(step.fields, step.version) : [];
if (fromPlan.length > 0) {
if (tests.length === 0) tests = fromPlan;
@ -229,17 +242,19 @@ export function proveRed(o) {
}
}
if (tests.length === 0) return notRun('no test files given: a step without its test cannot prove red');
// From here on every result names the list it ran (`tests`, operator decision 30).
const ran = (reason, extra = {}) => notRun(reason, { tests, ...extra });
for (const t of tests) {
if (!safeRel(t)) return notRun(`test path must be repo-relative: ${t}`);
if (!existsSync(join(cwd, t))) return notRun(`test file not found in the working tree: ${t}`);
if (!safeRel(t)) return ran(`test path must be repo-relative: ${t}`);
if (!existsSync(join(cwd, t))) return ran(`test file not found in the working tree: ${t}`);
}
for (const l of o.link ?? []) if (!safeRel(l)) return notRun(`--link must be repo-relative: ${l}`);
for (const l of o.link ?? []) if (!safeRel(l)) return ran(`--link must be repo-relative: ${l}`);
const allowed = allowedCommand(o.cmd, { cwd });
if (!allowed.allowed) return notRun(`command not run: ${allowed.reason}`);
if (!allowed.allowed) return ran(`command not run: ${allowed.reason}`);
const screen = screenCommand(o.cmd);
if (!screen.allowed) return notRun(`command blocked: ${screen.rule}`);
if (!screen.allowed) return ran(`command blocked: ${screen.rule}`);
const sha = git(cwd, ['rev-parse', '--verify', '--quiet', `${o.base}^{commit}`]);
if (sha.status !== 0) return notRun(`cannot resolve the start point ${o.base} in ${cwd}`);
if (sha.status !== 0) return ran(`cannot resolve the start point ${o.base} in ${cwd}`);
const baseSha = sha.stdout.trim();
// The result first: it is the working tree as it stands. `redOnly` is the
@ -248,25 +263,25 @@ export function proveRed(o) {
if (!o.redOnly) {
result = run(o.cmd, cwd, timeoutMs);
const resultBroken = unrunnable(result);
if (resultBroken) return notRun(`at the result: ${resultBroken}`, { result });
if (resultBroken) return ran(`at the result: ${resultBroken}`, { result });
}
const tmp = mkdtempSync(join(tmpdir(), 'voyage-red-proof-'));
try {
const tree = join(tmp, 'tree');
const c = git(tmp, ['clone', '--quiet', '--shared', '--no-checkout', cwd, tree]);
if (c.status !== 0) return notRun(`could not clone the start point: ${String(c.stderr).trim()}`, { result });
if (c.status !== 0) return ran(`could not clone the start point: ${String(c.stderr).trim()}`, { result });
const co = git(tree, ['checkout', '--quiet', '--detach', baseSha]);
if (co.status !== 0) return notRun(`could not check out ${baseSha}: ${String(co.stderr).trim()}`, { result });
if (co.status !== 0) return ran(`could not check out ${baseSha}: ${String(co.stderr).trim()}`, { result });
for (const t of tests) copyIn(cwd, tree, t);
const linked = linkDependencies(cwd, tree, o.link ?? []);
const base = run(o.cmd, tree, timeoutMs);
const baseBroken = unrunnable(base);
if (baseBroken) return notRun(`at the start point: ${baseBroken}`, { base, result, linked });
const out = { base_sha: baseSha, linked, base, result };
if (baseBroken) return ran(`at the start point: ${baseBroken}`, { base, result, linked });
const out = { tests, base_sha: baseSha, linked, base, result };
if (base.exit === 0) return { status: 'rejected', reason: 'not-red-at-base', ...testsOnly(step), ...out };
if (o.redOnly) return { status: 'proven', reason: 'red at the start point (--red-only: the result was not run)', red_kind: redKind(base), base_sha: baseSha, linked, base };
if (o.redOnly) return { status: 'proven', reason: 'red at the start point (--red-only: the result was not run)', red_kind: redKind(base), tests, base_sha: baseSha, linked, base };
if (result.exit !== 0) return { status: 'rejected', reason: 'not-green-at-result', ...out };
// Control: the same copy with the whole change laid on. Red here means the
@ -274,9 +289,9 @@ export function proveRed(o) {
applyResult(cwd, tree, baseSha);
const control = run(o.cmd, tree, timeoutMs);
const controlBroken = unrunnable(control);
if (controlBroken) return notRun(`in the control run: ${controlBroken}`, { ...out, control });
if (controlBroken) return ran(`in the control run: ${controlBroken}`, { ...out, control });
if (control.exit !== 0) {
return notRun('the control run (start-point copy + the change) stayed red: the start point failed for an environment reason, not for the missing change', { ...out, control });
return ran('the control run (start-point copy + the change) stayed red: the start point failed for an environment reason, not for the missing change', { ...out, control });
}
return { status: 'proven', reason: 'red at the start point, green at the result, green in the control', red_kind: redKind(base), ...out, control };
} finally {
@ -288,7 +303,7 @@ export function proveRed(o) {
const USAGE =
'usage: red-proof.mjs --base <start_sha> --cmd "<test command>" [--tests <a,b>] [--plan <plan.md> --step <N>] [--red-only] [--cwd <dir>] [--link <dir,...>] [--timeout <ms>] [--json]\n' +
' --tests is required unless --plan/--step name a step whose Test first File line lists its tests; given with them, it must name the same set';
' with --plan/--step the step\'s Test first File line gives the test list and the documented call passes no --tests; a --tests given with them must name the same set; a plan or step that cannot be read is not-run (plan-unreadable). Without --plan, or for a step whose Test first File line lists no test, --tests is required';
const list = (v) => String(v).split(',').map((s) => s.trim()).filter(Boolean);
@ -320,6 +335,9 @@ export function main(argv) {
else {
const counts = (x) => (x?.tap ? ` (tap: ${x.tap.fail} of ${x.tap.tests} failed)` : '');
process.stdout.write(`red proof: ${r.status.toUpperCase()} - ${r.reason}${r.red_kind ? ` (red: ${r.red_kind})` : ''}\n`);
// The exact form --files and --tests take: one comma-joined list.
if (r.tests) process.stdout.write(` tests: ${r.tests.join(',')}\n`);
if (r.plan_tests) process.stdout.write(` plan_tests: ${r.plan_tests.join(',')}\n`);
if (r.tests_only === true) process.stdout.write(' tests_only: true - every file in the step\'s Files: is a test, so it can never be red: up to the plan\n');
if (r.base) process.stdout.write(` start point ${r.base_sha ?? o.base}: exit ${r.base.exit}${counts(r.base)}\n`);
if (r.result) process.stdout.write(` result: exit ${r.result.exit}${counts(r.result)}\n`);