voyage/lib/validators/brief-validator.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

355 lines
17 KiB
JavaScript

// lib/validators/brief-validator.mjs
// Validate trekbrief frontmatter + body invariants.
//
// 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 on trekreview) can be
// added without a brief_version bump.
import { readFileSync, existsSync } from 'node:fs';
import { parseDocument } from '../util/frontmatter.mjs';
import { issue, ok, fail } from '../util/result.mjs';
import { BASE_ALLOWED_MODELS } from './profile-validator.mjs';
import { parseSuccessCriteria, runnableCheck } from '../verification/criteria-runner.mjs';
import { isMain } from '../util/is-main.mjs';
export const BRIEF_REQUIRED_FRONTMATTER = ['type', 'brief_version', 'task', 'slug', 'research_topics', 'research_status'];
export const REVIEW_AS_BRIEF_REQUIRED_FRONTMATTER = ['type', 'task', 'slug', 'project_dir', 'findings'];
export const BRIEF_TYPE_VALUES = Object.freeze(['trekbrief', 'trekreview']);
export const BRIEF_RESEARCH_STATUS_VALUES = ['pending', 'in_progress', 'complete', 'skipped'];
export const BRIEF_BODY_SECTIONS = ['Intent', 'Goal', 'Success Criteria'];
export const PHASE_SIGNAL_PHASES = Object.freeze(['research', 'plan', 'execute', 'review']);
export const EFFORT_LEVELS = Object.freeze(['low', 'standard', 'high']);
// v5.5 — framing: how this brief relates to prior operator intent (the first layer
// of the framing-alignment defense). Required at brief_version ≥ 2.2.
export const BRIEF_FRAMING_VALUES = Object.freeze(['preserve', 'refine', 'replace', 'new-direction']);
// v5.5 — obligatory TL;DR section (≤ 5 content lines) at the top of brief.md,
// gated at brief_version ≥ 2.2. Soft cap is a warning, not a blocker.
export const BRIEF_TLDR_MAX_LINES = 5;
// K1 — the brief's size class (ceremony by size). Optional, additive, no
// brief_version bump; absent (or invalid) reads as the heaviest class.
export const BRIEF_CEREMONY_VALUES = Object.freeze(['small', 'bounded', 'architectural']);
export const BRIEF_CEREMONY_DEFAULT = 'architectural';
// Where the brief and the plan live: committed (`git`) or kept out of git
// (`local`). The approval log is committed either way. Optional, additive;
// absent reads as `local`, so nothing is published without the operator's yes.
export const BRIEF_ARTIFACT_STORE_VALUES = Object.freeze(['git', 'local']);
export const BRIEF_ARTIFACT_STORE_DEFAULT = 'local';
/** The brief's size class; absent or invalid → architectural (never lighter in doubt). */
export function briefCeremony(fm) {
return BRIEF_CEREMONY_VALUES.includes(fm?.ceremony) ? fm.ceremony : BRIEF_CEREMONY_DEFAULT;
}
/** Where the brief and plan are stored; absent or invalid → local. */
export function briefArtifactStore(fm) {
return BRIEF_ARTIFACT_STORE_VALUES.includes(fm?.artifact_store) ? fm.artifact_store : BRIEF_ARTIFACT_STORE_DEFAULT;
}
// Extract the raw text of a `## {heading}` section body (between its heading line
// and the next `## ` heading, or end of document). Returns null if absent.
function extractSection(body, heading) {
const re = new RegExp(`^##\\s+${heading}\\b.*$`, 'm');
const m = re.exec(body);
if (!m) return null;
const after = body.slice(m.index + m[0].length);
const next = after.search(/^##\s/m);
return next === -1 ? after : after.slice(0, next);
}
function getRequiredFields(type) {
return type === 'trekreview' ? REVIEW_AS_BRIEF_REQUIRED_FRONTMATTER : BRIEF_REQUIRED_FRONTMATTER;
}
export function validateBriefContent(text, opts = {}) {
const strict = opts.strict !== false;
const doc = parseDocument(text);
if (!doc.valid) return doc;
const fm = doc.parsed.frontmatter || {};
const body = doc.parsed.body || '';
const errors = [];
const warnings = [];
for (const k of getRequiredFields(fm.type)) {
if (!(k in fm)) {
errors.push(issue('BRIEF_MISSING_FIELD', `Required frontmatter field missing: ${k}`));
}
}
// v5.1 — phase_signals (additive optional field) + version-conditional sequencing gate.
// Composition rule documented in each downstream command's "Composition rule (v5.1)" section.
const hasSignals = 'phase_signals' in fm;
const hasPartial = 'phase_signals_partial' in fm;
if (hasSignals && hasPartial) {
errors.push(issue(
'BRIEF_SIGNALS_MUTUALLY_EXCLUSIVE',
'phase_signals and phase_signals_partial are mutually exclusive — set exactly one',
'Either commit per-phase signals OR record phase_signals_partial: true (force-stop).',
));
}
if (hasSignals) {
if (!Array.isArray(fm.phase_signals)) {
errors.push(issue(
'BRIEF_INVALID_PHASE_SIGNALS',
'phase_signals must be a list of {phase, effort?, model?} entries',
));
} else {
for (const entry of fm.phase_signals) {
if (!entry || typeof entry !== 'object' || !('phase' in entry)) {
errors.push(issue('BRIEF_INVALID_PHASE_SIGNALS', `phase_signals entry must include a "phase" key`));
continue;
}
if (!PHASE_SIGNAL_PHASES.includes(entry.phase)) {
errors.push(issue(
'BRIEF_INVALID_PHASE_SIGNAL_PHASE',
`phase_signals.phase "${entry.phase}" not in [${PHASE_SIGNAL_PHASES.join(', ')}]`,
));
}
if ('effort' in entry && !EFFORT_LEVELS.includes(entry.effort)) {
errors.push(issue(
'BRIEF_INVALID_EFFORT',
`phase_signals.effort "${entry.effort}" not in [${EFFORT_LEVELS.join(', ')}]`,
));
}
if ('model' in entry && !BASE_ALLOWED_MODELS.includes(entry.model)) {
errors.push(issue(
'BRIEF_INVALID_MODEL',
`phase_signals.model "${entry.model}" not in [${BASE_ALLOWED_MODELS.join(', ')}]`,
));
}
}
}
}
// Sequencing gate: brief_version ≥ 2.1 requires phase_signals OR phase_signals_partial.
// Coerce to String so the gate fires regardless of whether YAML parsed the value as
// a string ("2.1") or a number (2.1). v5.1.0 shipped with an unquoted-2.1 template
// that silently bypassed this gate — fix locked in by quoting the template AND
// accepting both shapes here as defense-in-depth (v5.1.1, finding 3c834097/df1435a2).
// v5.5 — framing enum check fires on ANY version when the field is present but
// malformed. The missing-framing BLOCKER below is version-gated (≥ 2.2).
if ('framing' in fm && !BRIEF_FRAMING_VALUES.includes(fm.framing)) {
errors.push(issue(
'BRIEF_INVALID_FRAMING',
`framing "${fm.framing}" not in [${BRIEF_FRAMING_VALUES.join(', ')}]`,
'framing declares how this brief relates to prior operator intent.',
));
}
if ('ceremony' in fm && !BRIEF_CEREMONY_VALUES.includes(fm.ceremony)) {
errors.push(issue(
'BRIEF_INVALID_CEREMONY',
`ceremony "${fm.ceremony}" not in [${BRIEF_CEREMONY_VALUES.join(', ')}]`,
'ceremony is the brief\'s size class; leave it out to mean architectural (the heaviest).',
));
}
if ('artifact_store' in fm && !BRIEF_ARTIFACT_STORE_VALUES.includes(fm.artifact_store)) {
errors.push(issue(
'BRIEF_INVALID_ARTIFACT_STORE',
`artifact_store "${fm.artifact_store}" not in [${BRIEF_ARTIFACT_STORE_VALUES.join(', ')}]`,
'git = the brief and plan are committed; local = kept out of git. Leave it out to mean local.',
));
}
if (typeof fm.brief_version === 'string' || typeof fm.brief_version === 'number') {
const vm = String(fm.brief_version).match(/^(\d+)\.(\d+)$/);
if (vm) {
const major = Number(vm[1]);
const minor = Number(vm[2]);
const atLeast21 = major > 2 || (major === 2 && minor >= 1);
const atLeast22 = major > 2 || (major === 2 && minor >= 2);
if (atLeast21 && !hasSignals && !hasPartial && fm.type !== 'trekreview') {
errors.push(issue(
'BRIEF_V51_MISSING_SIGNALS',
'brief_version ≥ 2.1 requires phase_signals (or phase_signals_partial: true)',
'Re-run /trekbrief — Phase 3.5 collects per-phase effort + model signals.',
));
}
// v5.5 framing enforcement — gated at ≥ 2.2 (trekreview briefs are exempt).
if (atLeast22 && fm.type !== 'trekreview') {
if (!('framing' in fm)) {
errors.push(issue(
'BRIEF_MISSING_FRAMING',
'brief_version ≥ 2.2 requires a framing: field',
`Set framing to one of [${BRIEF_FRAMING_VALUES.join(', ')}] — /trekbrief Phase 2.5 collects it before any brief prose is written.`,
));
}
const tldr = extractSection(body, 'TL;DR');
if (tldr === null) {
const tldrIssue = issue('BRIEF_MISSING_SECTION', 'Required body section missing: ## TL;DR');
if (strict) errors.push(tldrIssue); else warnings.push(tldrIssue);
} else {
const lines = tldr.split('\n').map(l => l.trim()).filter(Boolean);
if (lines.length > BRIEF_TLDR_MAX_LINES) {
warnings.push(issue(
'BRIEF_TLDR_TOO_LONG',
`## TL;DR has ${lines.length} content lines (max ${BRIEF_TLDR_MAX_LINES}) — keep it to a one-glance summary`,
));
}
}
}
}
}
// S18 — opt-in version floor. When the caller passes minBriefVersion (commands
// forward --min-brief-version), WARN (never block) if the brief declares a
// version below it: framing enforcement only fires at ≥ 2.2, so an older brief
// sidesteps the framing-alignment defense silently. Absent opt → no check.
// trekreview briefs have no framing and are exempt.
if (opts.minBriefVersion && fm.type !== 'trekreview') {
const mm = String(opts.minBriefVersion).match(/^(\d+)\.(\d+)$/);
if (mm) {
const minMajor = Number(mm[1]);
const minMinor = Number(mm[2]);
const bm = fm.brief_version === undefined
? null
: String(fm.brief_version).match(/^(\d+)\.(\d+)$/);
const below = bm
? (Number(bm[1]) < minMajor || (Number(bm[1]) === minMajor && Number(bm[2]) < minMinor))
: true; // absent or unparseable version is below any floor
if (below) {
warnings.push(issue(
'BRIEF_VERSION_BELOW_MINIMUM',
`brief_version ${fm.brief_version ?? '(absent)'} is below the requested minimum ${opts.minBriefVersion} — framing enforcement (≥ 2.2) is bypassed`,
'Re-run /trekbrief to produce a brief_version 2.2 brief, or drop --min-brief-version to accept the older brief as-is.',
));
}
}
}
if (fm.type !== undefined && !BRIEF_TYPE_VALUES.includes(fm.type)) {
errors.push(issue(
'BRIEF_WRONG_TYPE',
`frontmatter.type must be one of [${BRIEF_TYPE_VALUES.join(', ')}], got "${fm.type}"`,
));
}
if (fm.type === 'trekreview' && fm.findings !== undefined && !Array.isArray(fm.findings)) {
errors.push(issue(
'BRIEF_BAD_FINDINGS_TYPE',
'Field "findings" must be an array of finding-IDs for type:trekreview',
'Use block-style YAML: `findings:\\n - <id1>\\n - <id2>`',
));
}
if (fm.research_status !== undefined && !BRIEF_RESEARCH_STATUS_VALUES.includes(fm.research_status)) {
errors.push(issue(
'BRIEF_BAD_STATUS',
`research_status "${fm.research_status}" not in [${BRIEF_RESEARCH_STATUS_VALUES.join(', ')}]`,
));
}
if (typeof fm.research_topics === 'number' && fm.research_topics > 0 && fm.research_status === 'skipped') {
if (fm.brief_quality !== 'partial') {
errors.push(issue(
'BRIEF_STATE_INCOHERENT',
`research_topics=${fm.research_topics} but research_status=skipped`,
'Either set research_status to a real progress value, or mark brief_quality: partial.',
));
} else {
warnings.push(issue(
'BRIEF_PARTIAL_SKIPPED',
`Brief has unresolved research topics (${fm.research_topics}) but is partial`,
));
}
}
for (const section of BRIEF_BODY_SECTIONS) {
const re = new RegExp(`^##\\s+${section}\\b`, 'm');
if (!re.test(body)) {
const issueObj = issue('BRIEF_MISSING_SECTION', `Required body section missing: ## ${section}`);
if (strict) errors.push(issueObj);
else warnings.push(issueObj);
}
}
// A criterion that opens with a command the review's criteria runner can
// never run is reported NOT RUN at /trekreview, long after anyone could fix
// it: 11 of 13 in the voyage-lekerepo trial (2026-10-06), and nothing in
// /trekbrief warned. A warning, not an error: a criterion may be one a human
// judges by reading, and that stays allowed. Prose-only criteria (no
// command) are not flagged.
if (fm.type !== 'trekreview') {
// bash 3.2 required but measured under whatever `bash` is first on PATH:
// on a Mac with a newer bash installed that is 5.x, so the constraint is
// never measured (voyage-lekerepo trial 2, 2026-10-08).
const constraints = (extractSection(body, 'Constraints') ?? '') + (extractSection(body, 'Non-Functional Requirements') ?? '');
// Line by line, and a line that negates it ("not required") does not count.
const needsBash32 = constraints.split('\n').some((l) =>
/\bbash\s*3\.2\b/i.test(l) && !/\b(?:not|no longer|never|no need)\b/i.test(l));
for (const c of parseSuccessCriteria(body)) {
if (c.subpoints && !c.command) {
warnings.push(issue(
'BRIEF_SC_SUBPOINTS',
`Success criterion ${c.label} has ${c.subpoints} sub-point(s) and no command of its own: the review measures none of them`,
'Give the criterion one command that checks all its sub-points (`bash tests/<script>.sh` exits 0), ' +
'or write each sub-point as its own criterion with its own command.',
));
}
if (!c.command) continue;
const bare = c.command.replace(/^(?:[A-Za-z_][A-Za-z0-9_]*=\S*\s+)+/, '');
if (needsBash32 && /^bash\s/.test(bare)) {
const script = bare.split(/\s+/)[1] ?? '';
warnings.push(issue(
'BRIEF_SC_BASH_NOT_PINNED',
`Success criterion ${c.label} runs \`${c.command}\`, but the brief requires bash 3.2: \`bash\` is whichever bash comes first on PATH, which may be newer`,
script.startsWith('tests/')
? `Write it as \`/bin/bash ${bare.replace(/^bash\s+/, '')}\` (/bin/bash tests/<script>.sh) so the check runs under the system bash.`
: 'Put the check in a script under tests/ and run it as `/bin/bash tests/<script>.sh`, so it runs under the system bash.',
));
}
const r = runnableCheck(c, { cwd: opts.cwd });
if (r.runnable) continue;
warnings.push(issue(
'BRIEF_SC_NOT_RUNNABLE',
`Success criterion ${c.label} opens with \`${c.command}\`, which the review's criteria runner will never run (${r.reason})`,
'Wrap the check in a script under tests/ and write the criterion as `bash tests/<script>.sh` exits 0, ' +
'or name a test runner (`npm test`, `node --test <file>`); otherwise it is reported NOT RUN at review.',
));
}
}
if (typeof fm.brief_version === 'string' || typeof fm.brief_version === 'number') {
const m = String(fm.brief_version).match(/^(\d+)\.(\d+)$/);
if (!m) {
warnings.push(issue('BRIEF_VERSION_FORMAT', `brief_version "${fm.brief_version}" not in N.M form`));
}
}
return { valid: errors.length === 0, errors, warnings, parsed: { frontmatter: fm, body } };
}
export function validateBrief(filePath, opts = {}) {
if (!existsSync(filePath)) return fail(issue('BRIEF_NOT_FOUND', `File not found: ${filePath}`));
let text;
try { text = readFileSync(filePath, 'utf-8'); }
catch (e) { return fail(issue('BRIEF_READ_ERROR', `Cannot read ${filePath}: ${e.message}`)); }
const r = validateBriefContent(text, opts);
return { ...r, parsed: { ...r.parsed, filePath } };
}
if (isMain(import.meta.url)) {
const args = process.argv.slice(2);
const strict = !args.includes('--soft');
const minIdx = args.indexOf('--min-version');
const minBriefVersion = minIdx >= 0 ? args[minIdx + 1] : undefined;
// filePath is the first positional, skipping the --min-version value token.
// Guard: when --min-version is absent (minIdx === -1) the skip index must be -1,
// not 0 — otherwise the no-flag invocation `brief-validator.mjs <brief.md>` drops
// the file (which sits at index 0) and bails to Usage.
const skipIdx = minIdx >= 0 ? minIdx + 1 : -1;
const filePath = args.find((a, i) => !a.startsWith('--') && i !== skipIdx);
if (!filePath) {
process.stderr.write('Usage: brief-validator.mjs [--soft] [--min-version <x.y>] <brief.md>\n');
process.exit(2);
}
const r = validateBrief(filePath, { strict, minBriefVersion });
if (args.includes('--json')) {
process.stdout.write(JSON.stringify({ valid: r.valid, errors: r.errors, warnings: r.warnings }, null, 2) + '\n');
} else {
process.stdout.write(`brief-validator: ${r.valid ? 'PASS' : 'FAIL'} ${filePath}\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);
}