voyage/lib/validators/plan-validator.mjs
Kjell Tore Guttormsen c56f852ab6
fix(validate): a check that can never run is caught when the brief or plan is written
voyage-lekerepo end-to-end trial (2026-10-06), report points 2, 3, 4, 9:
11 of 13 brief criteria were NOT RUN at review with no warning in /trekbrief;
a plan `git grep` criterion passed validator, gate and three critic rounds,
then felled the run in Phase 7; the End-to-end field and Phase 7's text
search never met; a prose Verify left the test command undefined.

- criteria-runner: runnableCheck() screens a declared check without running
  it (same allowlist; an npm run script is not looked up at write time);
  /bin/bash tests/<script> allowed; the plan report carries end_to_end,
  verified when a passed criterion runs a step's End-to-end command (the
  older text form still counts).
- plan-validator 1.8: PLAN_VERIFICATION_NOT_RUNNABLE,
  PLAN_STEP_VERIFY_NOT_RUNNABLE, PLAN_END_TO_END_NOT_RUNNABLE,
  PLAN_END_TO_END_NOT_VERIFIED (the gate inherits them).
- brief-validator: BRIEF_SC_NOT_RUNNABLE warning; /trekbrief offers a
  tests/ script; brief-reviewer no longer proposes unrunnable commands.
- trekexecute: Phase 7 copies end_to_end from the runner; Verify's test
  command is its first backticked span.

Measured on the trial's own brief and plan: 11 warnings (the 11 NOT RUN),
and the plan stops on V5 git grep and the unlisted End-to-end command.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-08 06:42:33 +02:00

320 lines
14 KiB
JavaScript

// lib/validators/plan-validator.mjs
// Wraps plan-schema (heading shape) + manifest-yaml (per-step Manifest blocks).
// This is the JS equivalent of Phase 5.5 grep checks in planning-orchestrator.
//
// Schema is forward-compatible: unknown top-level frontmatter keys are
// tolerated silently. Strict-key checks are intentionally avoided so new
// optional fields (jf. the source_findings precedent) can be added without
// a plan_version bump.
//
// plan_version 1.8 rules (apply only to plans declaring >= 1.8; 1.7 plans
// validate exactly as before):
// - every step has `Test first:` naming a File and what it Verifies;
// - the plan has a "run the thing" step: an `End-to-end:` field with a
// backticked command (without it a result is "not verified end to end");
// - the optional step fields `Interface in:`, `Interface out:`,
// `Review focus:` are never empty when present;
// - `ceremony:` is small | bounded | architectural (absent = architectural);
// an architectural plan carries `## Design` (see checkDesign);
// - every check the run will execute can run (checkRunnable): each
// `## Verification` criterion, each step's `Verify:` (its first backticked
// span is the step's test command) and each `End-to-end:` command pass the
// criteria runner's allowlist, and each `End-to-end:` command also stands
// under `## Verification`, where Phase 7 runs it.
// Strict mode reports these as errors; --soft downgrades them to warnings.
import { readFileSync, existsSync, realpathSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import {
sliceSteps, validatePlanHeadings, extractPlanVersion, extractPlanCeremony,
parseStepFields, findSection, splitSections, findModuleCycle, parseMermaid, fencedBlocks,
versionAtLeast, PLAN_VERSION_CURRENT, PLAN_VERSIONS_SUPPORTED, CEREMONY_VALUES,
} from '../parsers/plan-schema.mjs';
import { validateAllManifests } from '../parsers/manifest-yaml.mjs';
import { parseDocument } from '../util/frontmatter.mjs';
import { issue, fail } from '../util/result.mjs';
import { parsePlanVerification, firstCommand, runnableCheck } from '../verification/criteria-runner.mjs';
export const OPTIONAL_STEP_FIELDS = Object.freeze(['Interface in', 'Interface out', 'Review focus']);
export const DESIGN_SUBSECTIONS = Object.freeze([
'Architecture', 'Data Flow', 'Interfaces', 'Data Models', 'Error Handling',
'Module Map', 'Architecture Rules', 'Requirements Check',
]);
const BACKTICKED = /`[^`\n]+`/;
function checkSteps(sections) {
const out = [];
let endToEnd = 0;
for (const s of sections) {
const f = parseStepFields(s.body);
const tf = f.get('Test first');
const names = tf && /\bFile:\**[ \t]*`[^`\n]+`/i.test(tf.text) && /\bVerifies:\**[ \t]*\S/i.test(tf.text);
if (!names) {
out.push(issue(
'PLAN_STEP_NO_TEST_FIRST',
`Step ${s.n}: ${tf ? '`Test first:` does not name a File (backticked path) and what it Verifies' : 'no `Test first:` field'}`,
'plan_version 1.8 requires `Test first:` on every step, with `File:` and `Verifies:` sub-bullets.',
));
}
for (const name of OPTIONAL_STEP_FIELDS) {
if (f.has(name) && f.get(name).text === '') {
out.push(issue('PLAN_STEP_FIELD_EMPTY', `Step ${s.n}: \`${name}:\` is present but empty`,
'Fill the field or remove it; it is optional.'));
}
}
if (f.has('End-to-end') && BACKTICKED.test(f.get('End-to-end').text)) endToEnd += 1;
}
if (sections.length > 0 && endToEnd === 0) {
out.push(issue(
'PLAN_NO_END_TO_END',
'No "run the thing" step: no step carries `End-to-end:` with a backticked command',
'Add a step that runs the product the way a user does (`End-to-end: `<command>` → expected: …`). ' +
'Without it the result is "not verified end to end", never green.',
));
}
return out;
}
const WAY_IN = 'A check the allowlist has no name for (a pipe, `jq`, `grep`, the product\'s own binary) is ' +
'wrapped in a script under tests/ and declared as `bash tests/<script>.sh`.';
/**
* Every check the run will execute can run. Without this a plan passed the
* validator, the plan gate and three critic rounds with a `git grep` criterion
* the runner refuses, and Phase 7 felled a run whose steps had all passed
* (voyage-lekerepo trial, 2026-10-06).
*/
function checkRunnable(text, sections, opts) {
const out = [];
const criteria = parsePlanVerification(text);
for (const c of criteria) {
const r = runnableCheck(c, opts);
if (!r.runnable) {
out.push(issue('PLAN_VERIFICATION_NOT_RUNNABLE',
`## Verification ${c.label} can never run (${r.reason}): ${c.text}`,
`Phase 7 runs every criterion and fells the run on one it cannot run. ${WAY_IN}`));
}
}
const verified = new Set(criteria.filter(c => c.command).map(c => c.command.trim().replace(/\s+/g, ' ')));
for (const s of sections) {
const f = parseStepFields(s.body);
if (f.has('Verify')) {
const v = firstCommand(f.get('Verify').text);
const r = runnableCheck(v, opts);
if (!r.runnable) {
out.push(issue('PLAN_STEP_VERIFY_NOT_RUNNABLE',
`Step ${s.n}: \`Verify:\` does not open with one runnable test command (${r.reason})`,
'The first backticked span of `Verify:` is the step\'s test command: proof of red and the done line run it, ' +
`and nothing after it is run. ${WAY_IN}`));
}
}
if (f.has('End-to-end') && BACKTICKED.test(f.get('End-to-end').text)) {
const e = firstCommand(f.get('End-to-end').text);
const r = runnableCheck(e, opts);
if (!r.runnable) {
out.push(issue('PLAN_END_TO_END_NOT_RUNNABLE',
`Step ${s.n}: \`End-to-end:\` command can never run (${r.reason})`,
`Run the product from a script: \`bash tests/e2e.sh\`. ${WAY_IN}`));
} else if (!verified.has(e.command.trim().replace(/\s+/g, ' '))) {
out.push(issue('PLAN_END_TO_END_NOT_VERIFIED',
`Step ${s.n}: \`End-to-end:\` command \`${e.command}\` does not stand under ## Verification`,
'Phase 7 runs `## Verification`; list the same command there, or the result is "not verified end to end".'));
}
}
}
return out;
}
/** Table body rows (skips the header and the |---| separator). */
function tableRows(text) {
const rows = text.split('\n').filter(l => l.trim().startsWith('|'));
return rows.slice(2).map(r => r.trim().replace(/^\||\|$/g, '').split('|').map(c => c.trim()));
}
function checkDesign(text, stepNumbers) {
const out = [];
const design = findSection(text, 'Design', 2);
if (!design) {
return [issue('PLAN_DESIGN_MISSING', 'Architectural plan without a `## Design` section',
'Add the design part (templates/design-template.md), or plan under `ceremony: small|bounded` if the brief says so.')];
}
const subs = splitSections(design.text, 3);
const titles = new Set(subs.filter(s => s.title).map(s => s.title.toLowerCase()));
const missing = DESIGN_SUBSECTIONS.filter(t => !titles.has(t.toLowerCase()));
if (missing.length > 0) {
out.push(issue('PLAN_DESIGN_SECTION_MISSING', `## Design lacks: ${missing.join(', ')}`,
`Required subsections: ${DESIGN_SUBSECTIONS.join(', ')}.`));
}
if (!findSection(text, 'Test Strategy', 2)) {
out.push(issue('PLAN_DESIGN_SECTION_MISSING', 'Architectural plan lacks `## Test Strategy`',
'The design part\'s test strategy lives in the plan\'s `## Test Strategy` section.'));
}
const sub = t => subs.find(s => s.title && s.title.toLowerCase() === t.toLowerCase());
const map = sub('Module Map');
if (map) {
const blocks = fencedBlocks(map.text, 'mermaid');
if (blocks.length === 0) {
out.push(issue('PLAN_MODULE_MAP_EMPTY', '### Module Map has no ```mermaid block',
'Draw the modules and their dependency direction as a mermaid graph.'));
}
// All blocks form one map: a cycle may run across two of them.
const parsed = blocks.map(b => parseMermaid(b));
const unparsed = parsed.flatMap(p => p.unparsed);
if (unparsed.length > 0) {
out.push(issue('PLAN_MODULE_MAP_UNPARSED', `Module map lines that look like links but give no edge: ${unparsed.join(' | ')}`,
'Write links as `a --> b` (ids of letters, digits, _ . - /); the validator cannot check what it cannot read.'));
}
const cycle = findModuleCycle(parsed.flatMap(p => p.edges));
if (cycle) {
out.push(issue('PLAN_MODULE_CYCLE', `Module map has a dependency cycle: ${cycle.join(' -> ')}`,
'Dependencies point one way. Break the cycle (extract the shared part, or invert one edge).'));
}
}
const rules = sub('Architecture Rules');
if (rules) {
const rows = tableRows(rules.text);
if (rows.length === 0) {
out.push(issue('PLAN_ARCH_RULE_NO_ENFORCER', '### Architecture Rules lists no rule',
'List at least one rule as a table row: | Rule | Enforced by | Proof |.'));
}
for (const row of rows) {
const line = row.join(' | ');
const checked = /checked by\s+`[^`]+`/i.test(line);
const advisory = /\badvisory\b/i.test(line) && /\.claude\/rules\/[^\s`|]+\.md/.test(line);
if (!checked && !advisory) {
out.push(issue('PLAN_ARCH_RULE_NO_ENFORCER', `Rule without an enforcer: ${row[0] || line}`,
'Each rule says "checked by `<command>`" or "advisory — `.claude/rules/<file>.md`".'));
continue;
}
if (checked) {
const proof = (row[2] ?? '').match(/Step (\d+)/);
if (!proof || !stepNumbers.includes(Number(proof[1]))) {
out.push(issue('PLAN_ARCH_RULE_NO_PROOF',
`Checked rule without an existing proving step: ${row[0] || line}`,
'Name the step (Proof column, "Step N") that shows the check pass, fail on a planted violation, and pass again.'));
}
}
}
}
return out;
}
export function validatePlanContent(rawText, opts = {}) {
const strict = opts.strict !== false;
// CRLF plans (an editor on Windows) used to fail every manifest with
// FM_SYNTAX: the YAML line parser does not expect a trailing \r.
const text = typeof rawText === 'string' ? rawText.replace(/\r\n?/g, '\n') : rawText;
const headRes = validatePlanHeadings(text, { strict });
const errors = [...headRes.errors];
const warnings = [...headRes.warnings];
const steps = headRes.parsed?.steps || [];
const sections = sliceSteps(text);
const manRes = validateAllManifests(sections);
errors.push(...manRes.errors);
warnings.push(...manRes.warnings);
if (steps.length > 0 && manRes.parsed.length !== steps.length) {
errors.push(issue(
'PLAN_MANIFEST_COUNT_MISMATCH',
`Step count (${steps.length}) does not equal manifest count (${manRes.parsed.length})`,
));
}
const planVersion = extractPlanVersion(text);
if (planVersion === null) {
warnings.push(issue('PLAN_NO_VERSION', `No plan_version detected; current target is ${PLAN_VERSION_CURRENT}`));
} else if (!PLAN_VERSIONS_SUPPORTED.includes(planVersion)) {
warnings.push(issue('PLAN_VERSION_MISMATCH', `plan_version=${planVersion}, current target is ${PLAN_VERSION_CURRENT}`));
}
const ceremony = extractPlanCeremony(text);
if (versionAtLeast(planVersion, '1.8')) {
const v18 = [...checkSteps(sections), ...checkRunnable(text, sections, { cwd: opts.cwd })];
if (!ceremony.valid) {
v18.push(issue('PLAN_CEREMONY_INVALID', `ceremony=${ceremony.raw}`,
`Allowed: ${CEREMONY_VALUES.join(' | ')} (absent = architectural).`));
} else if (ceremony.value === 'architectural') {
v18.push(...checkDesign(text, steps.map(s => s.n)));
}
if (strict) errors.push(...v18);
else warnings.push(...v18);
}
// v4.1 SC #20 — MANIFEST_PROFILE_DRIFT detection. Strict-mode only.
// If plan frontmatter declares `profile: <name>` and any step manifest
// declares `profile_used: <other>`, emit a warning (NOT an error) so
// operators see drift but parsing remains forward-compat.
if (strict) {
const planFm = parseDocument(text).parsed?.frontmatter;
const planProfile =
planFm && typeof planFm.profile === 'string' ? planFm.profile : null;
if (planProfile) {
for (const m of manRes.parsed) {
const stepProfile = m.manifest && m.manifest.profile_used;
if (typeof stepProfile === 'string' && stepProfile !== planProfile) {
warnings.push(issue(
'MANIFEST_PROFILE_DRIFT',
`step ${m.n}: profile_used = ${stepProfile}, plan profile = ${planProfile}`,
'A step manifest declares a different profile than the plan frontmatter; ' +
'verify whether this is intentional (manual override) or accidental drift.',
));
}
}
}
}
return {
valid: errors.length === 0,
errors,
warnings,
parsed: { steps, manifests: manRes.parsed, planVersion, ceremony: ceremony.value },
};
}
export function validatePlan(filePath, opts = {}) {
if (!existsSync(filePath)) return fail(issue('PLAN_NOT_FOUND', `File not found: ${filePath}`));
let text;
try { text = readFileSync(filePath, 'utf-8'); }
catch (e) { return fail(issue('PLAN_READ_ERROR', `Cannot read ${filePath}: ${e.message}`)); }
const r = validatePlanContent(text, opts);
return { ...r, parsed: { ...r.parsed, filePath } };
}
// realpath on both sides: `file://${argv[1]}` silently no-ops under a symlink
// or a path with a space (the CLI then exits 0 having checked nothing).
function isMain() {
try {
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
} catch {
return false;
}
}
if (isMain()) {
const args = process.argv.slice(2);
const strict = !args.includes('--soft');
const filePath = args.find(a => !a.startsWith('--'));
if (!filePath) {
process.stderr.write('Usage: plan-validator.mjs [--strict|--soft] <plan.md>\n');
process.exit(2);
}
const r = validatePlan(filePath, { strict });
if (args.includes('--json')) {
process.stdout.write(JSON.stringify({
valid: r.valid,
errors: r.errors,
warnings: r.warnings,
steps: r.parsed?.steps?.length ?? 0,
planVersion: r.parsed?.planVersion ?? null,
ceremony: r.parsed?.ceremony ?? null,
}, null, 2) + '\n');
} else {
process.stdout.write(`plan-validator: ${r.valid ? 'READY' : 'FAIL'} ${filePath} (${r.parsed?.steps?.length ?? 0} steps)\n`);
for (const e of r.errors) process.stderr.write(` ERROR [${e.code}] ${e.message}\n`);
for (const w of r.warnings) process.stderr.write(` WARN [${w.code}] ${w.message}\n`);
}
process.exit(r.valid ? 0 : 1);
}