fix(execute): run the plan's Verification on the single-session path (D-03)
A trekplan's `## Verification` section is where the brief's success criteria land. Phase 7 opened with "**Skip for trekplans.**", and only the multi-session wave path (Phase 2.6 Step 3) ran master verification. A plan executed in ONE session therefore reported `completed` without ever running the criteria it was measured against — the executor's own belief was the only evidence. The check now exists as code, not as an instruction: - `lib/verification/criteria-runner.mjs` parses the criteria an artifact DECLARES (a plan's `## Verification`, a brief's `## Success Criteria`), runs each command, and returns a verdict built from exit codes. Fail-closed throughout: a placeholder, a prose-only criterion, or an unavailable screen is `unrunnable`/`blocked`, never `passed`. A plan with no `## Verification` section exits 1 — a plan that promises no end-to-end check cannot be reported as verified. - Every command is screened through the plugin's own PreToolUse denylist (`hooks/scripts/pre-bash-executor.mjs`) before it reaches a shell. Chose invoking that hook over its documented stdin protocol rather than copying its rules, because a command spawned from node never passes through the Bash tool and so the hook cannot fire by itself — this keeps exactly one denylist. - Phase 7 is now "Exit / verification check": session specs run the exit condition, trekplans run the criteria runner. Phase 4's entry-condition skip for trekplans stands — a plan carries no entry condition; the exit side is not symmetrical. - A failing criterion FELLS the run: `plan_verification.status != "passed"` forbids `result: completed`. That is clause 2 of the stop-signal contract, now enforced on the single-session path too. Red first: `tests/lib/criteria-runner.test.mjs` (26 tests) against two committed fixture plans, one of which declares a criterion that fails on purpose. The doc pin in `tests/lib/doc-consistency.test.mjs` guards the wiring — a capability no phase calls is the same defect wearing a lib/ file; verified red against the pre-fix Phase 7 (skip present, runner absent, no fell-the-run clause). Suite: 1108 (1106/0/2), up 27 from 1081. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
8d1669ef51
commit
e55ca9fc89
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
6 changed files with 756 additions and 6 deletions
280
tests/lib/criteria-runner.test.mjs
Normal file
280
tests/lib/criteria-runner.test.mjs
Normal file
|
|
@ -0,0 +1,280 @@
|
|||
// tests/lib/criteria-runner.test.mjs
|
||||
// The criteria runner is what makes a declared check a RUN check: it parses the
|
||||
// falsifiable criteria a plan (`## Verification`) or a brief (`## Success
|
||||
// Criteria`) declares, screens each command through the executor's own
|
||||
// PreToolUse denylist, runs it, and returns a verdict built from exit codes.
|
||||
//
|
||||
// Fail-closed is the whole point: a criterion that cannot run (placeholder, no
|
||||
// command, screen unavailable) must never read as "passed".
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { mkdtempSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
|
||||
import {
|
||||
parsePlanVerification,
|
||||
parseSuccessCriteria,
|
||||
screenCommand,
|
||||
runCriteria,
|
||||
summarize,
|
||||
runPlanVerification,
|
||||
render,
|
||||
} from '../../lib/verification/criteria-runner.mjs';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const ROOT = join(HERE, '..', '..');
|
||||
const CLI = join(ROOT, 'lib', 'verification', 'criteria-runner.mjs');
|
||||
const FIX = join(ROOT, 'tests', 'fixtures');
|
||||
const HOOK = join(ROOT, 'hooks', 'scripts', 'pre-bash-executor.mjs');
|
||||
|
||||
// An exec double: maps a command string to {status, stdout, stderr}.
|
||||
function execDouble(table) {
|
||||
const calls = [];
|
||||
const exec = (command) => {
|
||||
calls.push(command);
|
||||
return table[command] ?? { status: 127, stdout: '', stderr: 'not in table' };
|
||||
};
|
||||
exec.calls = calls;
|
||||
return exec;
|
||||
}
|
||||
|
||||
// A screen double that allows everything, so exec behaviour can be tested alone.
|
||||
const allowAll = () => ({ allowed: true, rule: '' });
|
||||
|
||||
// --- parsing ---------------------------------------------------------------
|
||||
|
||||
test('parsePlanVerification: checkbox bullets become V1..Vn with their command', () => {
|
||||
const md = [
|
||||
'# Plan',
|
||||
'',
|
||||
'## Verification',
|
||||
'',
|
||||
'- [ ] `npm test` -> expected: exit 0',
|
||||
'- [x] `node --test tests/lib/x.test.mjs` -> expected: 3 passing',
|
||||
'',
|
||||
'## Estimated Scope',
|
||||
'',
|
||||
'- [ ] `not-a-criterion` (outside the section)',
|
||||
].join('\n');
|
||||
|
||||
const criteria = parsePlanVerification(md);
|
||||
assert.equal(criteria.length, 2);
|
||||
assert.deepEqual(criteria.map((c) => c.label), ['V1', 'V2']);
|
||||
assert.equal(criteria[0].command, 'npm test');
|
||||
assert.equal(criteria[1].command, 'node --test tests/lib/x.test.mjs');
|
||||
assert.match(criteria[0].text, /expected: exit 0/);
|
||||
});
|
||||
|
||||
test('parsePlanVerification: a template placeholder is unrunnable, not a command', () => {
|
||||
const md = '## Verification\n\n- [ ] `{exact command}` -> expected: `{exact output}`\n';
|
||||
const criteria = parsePlanVerification(md);
|
||||
assert.equal(criteria.length, 1);
|
||||
assert.equal(criteria[0].command, null);
|
||||
assert.equal(criteria[0].reason, 'placeholder');
|
||||
});
|
||||
|
||||
test('parsePlanVerification: a missing section yields no criteria', () => {
|
||||
assert.deepEqual(parsePlanVerification('# Plan\n\n## Steps\n\n- do a thing\n'), []);
|
||||
});
|
||||
|
||||
test('parseSuccessCriteria: bullets become SC1..SCn and take the FIRST backticked span', () => {
|
||||
const md = [
|
||||
'## Success Criteria',
|
||||
'',
|
||||
'- All existing tests pass: `npm test` exits 0',
|
||||
'- Endpoint returns 200: `curl -s localhost:3000/health` -> `"ok"`',
|
||||
'- No new runtime dependencies are introduced',
|
||||
'',
|
||||
'## Research Plan',
|
||||
].join('\n');
|
||||
|
||||
const criteria = parseSuccessCriteria(md);
|
||||
assert.deepEqual(criteria.map((c) => c.label), ['SC1', 'SC2', 'SC3']);
|
||||
assert.equal(criteria[0].command, 'npm test');
|
||||
assert.equal(criteria[1].command, 'curl -s localhost:3000/health');
|
||||
assert.equal(criteria[2].command, null);
|
||||
assert.equal(criteria[2].reason, 'no-command');
|
||||
});
|
||||
|
||||
// --- screening -------------------------------------------------------------
|
||||
|
||||
test('screenCommand: the real executor denylist blocks a catastrophic command', () => {
|
||||
const verdict = screenCommand('rm -rf ~', { hookPath: HOOK });
|
||||
assert.equal(verdict.allowed, false);
|
||||
assert.match(verdict.rule, /rm -rf|destruction/i);
|
||||
});
|
||||
|
||||
test('screenCommand: an ordinary command passes the real denylist', () => {
|
||||
assert.equal(screenCommand('npm test', { hookPath: HOOK }).allowed, true);
|
||||
});
|
||||
|
||||
test('screenCommand: an unavailable screen denies (fail-closed, never silently allows)', () => {
|
||||
const verdict = screenCommand('npm test', { hookPath: join(ROOT, 'hooks', 'scripts', 'no-such-hook.mjs') });
|
||||
assert.equal(verdict.allowed, false);
|
||||
assert.match(verdict.rule, /screen unavailable/i);
|
||||
});
|
||||
|
||||
// --- running ---------------------------------------------------------------
|
||||
|
||||
test('runCriteria: exit 0 passes, a non-zero exit fails, output is captured', () => {
|
||||
const criteria = parsePlanVerification(
|
||||
'## Verification\n\n- [ ] `good`\n- [ ] `bad`\n'
|
||||
);
|
||||
const exec = execDouble({
|
||||
good: { status: 0, stdout: 'all green\n', stderr: '' },
|
||||
bad: { status: 1, stdout: '', stderr: '1 failing\n' },
|
||||
});
|
||||
const results = runCriteria(criteria, { exec, screen: allowAll });
|
||||
|
||||
assert.deepEqual(results.map((r) => r.status), ['passed', 'failed']);
|
||||
assert.equal(results[0].exitCode, 0);
|
||||
assert.equal(results[1].exitCode, 1);
|
||||
assert.match(results[1].output, /1 failing/);
|
||||
assert.deepEqual(exec.calls, ['good', 'bad']);
|
||||
});
|
||||
|
||||
test('runCriteria: a blocked command is marked blocked and is NEVER executed', () => {
|
||||
const criteria = parsePlanVerification('## Verification\n\n- [ ] `rm -rf ~`\n');
|
||||
const exec = execDouble({});
|
||||
const results = runCriteria(criteria, {
|
||||
exec,
|
||||
screen: () => ({ allowed: false, rule: 'Filesystem root/home destruction' }),
|
||||
});
|
||||
|
||||
assert.equal(results[0].status, 'blocked');
|
||||
assert.equal(results[0].exitCode, null);
|
||||
assert.match(results[0].output, /Filesystem root\/home destruction/);
|
||||
assert.deepEqual(exec.calls, [], 'a blocked command must not reach the shell');
|
||||
});
|
||||
|
||||
test('runCriteria: a criterion with no command is unrunnable, not passed', () => {
|
||||
const criteria = parseSuccessCriteria('## Success Criteria\n\n- No new dependencies\n');
|
||||
const results = runCriteria(criteria, { exec: execDouble({}), screen: allowAll });
|
||||
assert.equal(results[0].status, 'unrunnable');
|
||||
assert.equal(results[0].exitCode, null);
|
||||
});
|
||||
|
||||
test('runCriteria: output is capped so a verbose command cannot flood a prompt', () => {
|
||||
const criteria = parsePlanVerification('## Verification\n\n- [ ] `loud`\n');
|
||||
const exec = execDouble({ loud: { status: 0, stdout: 'x'.repeat(10000), stderr: '' } });
|
||||
const results = runCriteria(criteria, { exec, screen: allowAll, maxOutput: 200 });
|
||||
assert.ok(results[0].output.length < 400, `capped, got ${results[0].output.length}`);
|
||||
assert.match(results[0].output, /truncated/);
|
||||
});
|
||||
|
||||
// --- the verdict -----------------------------------------------------------
|
||||
|
||||
test('summarize: one failing criterion makes the run NOT ok', () => {
|
||||
const results = [
|
||||
{ status: 'passed' }, { status: 'failed' }, { status: 'passed' },
|
||||
];
|
||||
const s = summarize(results, { requireCommand: true });
|
||||
assert.equal(s.ok, false);
|
||||
assert.equal(s.failed, 1);
|
||||
assert.equal(s.passed, 2);
|
||||
assert.equal(s.total, 3);
|
||||
});
|
||||
|
||||
test('summarize: in plan mode an unrunnable criterion makes the run NOT ok', () => {
|
||||
const s = summarize([{ status: 'passed' }, { status: 'unrunnable' }], { requireCommand: true });
|
||||
assert.equal(s.ok, false);
|
||||
assert.equal(s.unrunnable, 1);
|
||||
});
|
||||
|
||||
test('summarize: in brief mode an unrunnable criterion is reported, not failed', () => {
|
||||
const s = summarize([{ status: 'passed' }, { status: 'unrunnable' }], { requireCommand: false });
|
||||
assert.equal(s.ok, true);
|
||||
assert.equal(s.unrunnable, 1);
|
||||
});
|
||||
|
||||
test('summarize: a blocked criterion is never ok, in either mode', () => {
|
||||
for (const requireCommand of [true, false]) {
|
||||
assert.equal(summarize([{ status: 'blocked' }], { requireCommand }).ok, false);
|
||||
}
|
||||
});
|
||||
|
||||
test('summarize: zero criteria is NOT ok in plan mode (a plan that promises nothing)', () => {
|
||||
assert.equal(summarize([], { requireCommand: true }).ok, false);
|
||||
});
|
||||
|
||||
// --- the single-session path, end to end -----------------------------------
|
||||
|
||||
test('runPlanVerification: a plan whose success criterion FAILS fells the run', () => {
|
||||
const report = runPlanVerification(join(FIX, 'plan-verification-fails.md'));
|
||||
assert.equal(report.kind, 'plan');
|
||||
assert.equal(report.summary.ok, false);
|
||||
assert.equal(report.summary.failed, 1);
|
||||
const failed = report.results.find((r) => r.status === 'failed');
|
||||
assert.ok(failed, 'the failing criterion is reported by label');
|
||||
assert.match(failed.label, /^V\d+$/);
|
||||
});
|
||||
|
||||
test('runPlanVerification: a plan whose criteria all pass is ok', () => {
|
||||
const report = runPlanVerification(join(FIX, 'plan-verification-passes.md'));
|
||||
assert.equal(report.summary.ok, true);
|
||||
assert.equal(report.summary.failed, 0);
|
||||
assert.equal(report.summary.total, 2);
|
||||
});
|
||||
|
||||
test('runPlanVerification: a plan with no ## Verification section is NOT ok', () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'criteria-runner-'));
|
||||
const p = join(dir, 'plan.md');
|
||||
writeFileSync(p, '# Plan\n\n## Steps\n\n- do a thing\n');
|
||||
const report = runPlanVerification(p);
|
||||
assert.equal(report.summary.ok, false);
|
||||
assert.equal(report.error.code, 'NO_VERIFICATION_SECTION');
|
||||
});
|
||||
|
||||
test('render: the report names every non-passing criterion and its exit code', () => {
|
||||
const report = runPlanVerification(join(FIX, 'plan-verification-fails.md'));
|
||||
const text = render(report);
|
||||
assert.match(text, /FAILED/);
|
||||
assert.match(text, /exit 1/);
|
||||
});
|
||||
|
||||
// --- the CLI ---------------------------------------------------------------
|
||||
|
||||
function cli(args) {
|
||||
return spawnSync(process.execPath, [CLI, ...args], { encoding: 'utf8', cwd: ROOT });
|
||||
}
|
||||
|
||||
test('CLI: --plan exits 1 when a criterion fails', () => {
|
||||
const r = cli(['--plan', join(FIX, 'plan-verification-fails.md')]);
|
||||
assert.equal(r.status, 1, r.stderr);
|
||||
assert.match(r.stdout, /FAILED/);
|
||||
});
|
||||
|
||||
test('CLI: --plan exits 0 when every criterion passes', () => {
|
||||
const r = cli(['--plan', join(FIX, 'plan-verification-passes.md')]);
|
||||
assert.equal(r.status, 0, r.stderr + r.stdout);
|
||||
});
|
||||
|
||||
test('CLI: --json emits a parseable report with the summary', () => {
|
||||
const r = cli(['--plan', join(FIX, 'plan-verification-fails.md'), '--json']);
|
||||
assert.equal(r.status, 1);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.summary.ok, false);
|
||||
assert.equal(out.results.length, out.summary.total);
|
||||
});
|
||||
|
||||
test('CLI: a missing file exits 2 — a read error is not a failed criterion', () => {
|
||||
const r = cli(['--plan', join(FIX, 'no-such-plan.md')]);
|
||||
assert.equal(r.status, 2);
|
||||
assert.match(r.stderr, /criteria-runner/);
|
||||
});
|
||||
|
||||
test('CLI: an unknown argument exits 2 with usage', () => {
|
||||
const r = cli(['--nope']);
|
||||
assert.equal(r.status, 2);
|
||||
assert.match(r.stderr, /usage/);
|
||||
});
|
||||
|
||||
test('CLI: no mode flag exits 2 — it never guesses which artifact it was given', () => {
|
||||
const r = cli([]);
|
||||
assert.equal(r.status, 2);
|
||||
assert.match(r.stderr, /usage/);
|
||||
});
|
||||
|
|
@ -1829,3 +1829,26 @@ test('D-07: trekresearch launch rules inject the resolved model instead of a bla
|
|||
);
|
||||
assert.match(rules, /phase_signal_result\.model/, 'the launch rules must name the resolved model the spawn sites inject');
|
||||
});
|
||||
|
||||
// End-state defect D-03: a trekplan's `## Verification` is where the brief's success
|
||||
// criteria land. Phase 7 used to say "Skip for trekplans", so on the single-session path
|
||||
// the criteria were never run and `completed` meant "the executor believed it". The
|
||||
// behaviour lives in lib/verification/criteria-runner.mjs; this pin guards the WIRING —
|
||||
// a capability no phase calls is the same defect wearing a lib/ file. Fix the SOURCE.
|
||||
test('D-03: trekexecute Phase 7 runs the plan Verification on the single-session path', () => {
|
||||
const t = read('commands/trekexecute.md');
|
||||
const phase7 = (t.split('\n## Phase 7 — ')[1] || '').split('\n## ')[0];
|
||||
assert.ok(phase7.length > 0, 'trekexecute.md must still carry a Phase 7 section');
|
||||
assert.ok(
|
||||
!/^\*\*Skip for trekplans\.\*\*/m.test(phase7),
|
||||
'Phase 7 may no longer skip trekplans — that skip IS defect D-03',
|
||||
);
|
||||
assert.match(
|
||||
phase7, /criteria-runner\.mjs" --plan/,
|
||||
'Phase 7 must invoke the criteria runner over the plan file, not describe the check in prose',
|
||||
);
|
||||
assert.match(
|
||||
phase7, /MUST NOT emit `result: completed`/,
|
||||
'a failing criterion must fell the run, not be noted in the final report',
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue