fix: round 9 of the release review of 6.3.0, for 6.3.1

Round 9 (0f400d3..a10018f, WARN: 0 BLOCKER, 12 MAJOR): seven findings and
three brief notes, by operator choice. Each finding began with a test that
failed on a10018f on its assertion; suite 2230 (2228/0/2).

- A check refused for its comment (a quote, $, backtick or backslash) keeps
  its command and its reason, and every reader refuses it out loud: a nested
  ## Verification bullet stays a criterion (PLAN_VERIFICATION_NOT_RUNNABLE)
  and a brief SC warns (BRIEF_SC_NOT_RUNNABLE). It was folded into a
  sub-point or read as prose, a regression from a10018f. Readers that took a
  command as runnable (the end-to-end lists) skip a refused one.
- The verdict gate reads the last fenced block's info string and body from
  one reading, and exits 2 unless the info string is exactly json: a last
  block opened as ```json {x} held a BLOCKER the gate passed with exit 0
  (also in 6.3.0). A last block never closed is exit 2 too.
- One fence reader (findings-schema fencedBlocks) for the gate and every
  trailing-json reader: a fence closes only on a run at least as long as its
  opener, as in Markdown, so a four-backtick example of a json block is one
  block; CRLF and an indented fence read alike in the gate and the readers.
- Files: a description runs on past a comma and onto the next line until a
  new entry starts (a list item on a new line, or a backticked path), and a
  plain path followed by a colon starts its description. Measured over all
  939 Files: fields in the .md files under the marketplace and the test repo
  (560 of them in the 48 plan.md files outside worktrees): 0 read
  differently, tests-only changed in 0.
- The test lock compares --cmd by its words, so other blanks between the
  same words are the step's command.
- release-check decides "two files" on real paths: one review.md reached
  through a symlinked --repo is one file.
- Brief notes: red proof's stopped control run has its own test (green at
  once, red with the branch deleted); the two tests that assumed jest is not
  installed run with a PATH that finds only git (red on a10018f with a jest
  on PATH, green now); the Files: reader's doc states the measured counts
  (520 fields in 48 plan files at rounds 7-8, not "182 plans"; 560 now).

Not in this round (operator 2026-10-11): a144e9ce, 1dc2e785, dc3a467e,
2fa88853, 26873fa5.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-11 12:49:01 +02:00
commit a1e0e50451
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
11 changed files with 416 additions and 104 deletions

View file

@ -0,0 +1,36 @@
// tests/helpers/bin-path.mjs
// A PATH on which only the named programs are found: one directory of small
// wrappers, each starting the program from where the current PATH finds it.
// A test that needs a test runner to be absent ("could not start") then holds
// on every machine, also one where that runner is installed (release review
// of 6.3.0, round 9: two tests assumed `jest` is not on PATH).
import { accessSync, constants, mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { delimiter, join } from 'node:path';
function onPath(name) {
for (const dir of String(process.env.PATH ?? '').split(delimiter).filter(Boolean)) {
const p = join(dir, name);
try {
accessSync(p, constants.X_OK);
if (statSync(p).isFile()) return p;
} catch { /* not here */ }
}
return null;
}
/**
* @param {string[]} names - the programs the PATH finds (git, for the repo work)
* @returns {{path: string, cleanup: () => void}}
*/
export function pathWith(names) {
const dir = mkdtempSync(join(tmpdir(), 'voyage-bin-'));
for (const name of names) {
const found = onPath(name);
if (!found) throw new Error(`${name} is not on PATH`);
if (found.includes("'")) throw new Error(`${found}: a quote in the path`);
writeFileSync(join(dir, name), `#!/bin/sh\nexec '${found}' "$@"\n`, { mode: 0o755 });
}
return { path: dir, cleanup: () => rmSync(dir, { recursive: true, force: true }) };
}

View file

@ -17,6 +17,7 @@ import { writeFileSync, mkdirSync, readFileSync } from 'node:fs';
import { join } from 'node:path';
import { runHook } from '../helpers/hook-helper.mjs';
import { ROOT, makeRepo, parseJsonOut } from './guard-helpers.mjs';
import { pathWith } from '../helpers/bin-path.mjs';
const CLI = join(ROOT, 'lib', 'util', 'test-lock.mjs');
const STOP = join(ROOT, 'hooks', 'scripts', 'stop-test-lock.mjs');
@ -24,8 +25,8 @@ const PRE_WRITE = join(ROOT, 'hooks', 'scripts', 'pre-write-executor.mjs');
const SESS = `sess-green-${process.pid}-${Date.now()}`;
const CMD = 'node --test tests/step.test.mjs';
function cli(repo, args) {
const r = spawnSync(process.execPath, [CLI, ...args], { cwd: repo, encoding: 'utf8', env: { ...process.env, CLAUDE_CODE_SESSION_ID: SESS } });
function cli(repo, args, env = {}) {
const r = spawnSync(process.execPath, [CLI, ...args], { cwd: repo, encoding: 'utf8', env: { ...process.env, CLAUDE_CODE_SESSION_ID: SESS, ...env } });
return { code: r.status, stdout: r.stdout, stderr: r.stderr };
}
@ -123,9 +124,28 @@ test('green (release review of 6.3.0, round 8): a test program that is not insta
const repo = makeRepo('main');
mkdirSync(join(repo, 'tests'), { recursive: true });
writeFileSync(join(repo, 'tests', 'step.test.mjs'), 'x\n');
// `jest` is on the allowlist and not installed where this suite runs.
// `jest` is on the allowlist; the PATH finds only git, so it cannot start on
// any machine, one with jest installed too (round 9).
assert.equal(cli(repo, ['lock', '--files', 'tests/step.test.mjs', '--step', '3', '--cmd', 'jest tests/step.test.mjs']).code, 0);
const g = cli(repo, ['green', '--cmd', 'jest tests/step.test.mjs']);
assert.equal(g.code, 2, g.stderr);
assert.match(g.stderr, /could not start/, g.stderr);
const bin = pathWith(['git']);
try {
const g = cli(repo, ['green', '--cmd', 'jest tests/step.test.mjs'], { PATH: bin.path });
assert.equal(g.code, 2, g.stderr);
assert.match(g.stderr, /could not start/, g.stderr);
} finally { bin.cleanup(); }
});
test('green (release review of 6.3.0, round 9 / e36a533e): the same words with other blanks between them are the step\'s command', () => {
// runCommand splits a command on any blank, so a double space or a tab from
// the plan span runs the very same words; the --cmd check compared strings.
for (const greenCmd of ['node --test tests/step.test.mjs', 'node\t--test tests/step.test.mjs', ` ${CMD}\t`]) {
const repo = lockedRepo('assert.equal(1, 1);');
const g = cli(repo, ['green', '--cmd', greenCmd]);
assert.equal(g.code, 0, `green ${JSON.stringify(greenCmd)}: ${g.stderr}`);
}
// The control: other words are still refused.
const repo = lockedRepo('assert.equal(1, 1);');
const g = cli(repo, ['green', '--cmd', 'node --test tests/other.test.mjs']);
assert.equal(g.code, 2, g.stderr);
assert.match(g.stderr, /not the step's test command/, g.stderr);
});

View file

@ -14,6 +14,7 @@ import { tmpdir } from 'node:os';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { buildPlan } from '../helpers/plan-v18-builder.mjs';
import { pathWith } from '../helpers/bin-path.mjs';
import { validatePlanContent } from '../../lib/validators/plan-validator.mjs';
import { stepTestsOnly } from '../../lib/parsers/plan-schema.mjs';
import { validateFindings } from '../../lib/review/findings-schema.mjs';
@ -967,22 +968,26 @@ test('review 630 r8 / done-line.mjs:61 + test-lock.mjs:183: a stopped command is
}
});
test('review 630 r8 / red-proof.mjs:101: a run reports one reading of a stop, not a second spawnError beside it', async () => {
const { proveRed } = await import('../../lib/verification/red-proof.mjs');
test('review 630 r8 / red-proof.mjs:101: a run reports one reading of a stop, not a second spawnError beside it', () => {
const repo = gitRepo('r630-r8-spawn-');
// `jest` is on the allowlist; the PATH finds only git, so it cannot start on
// any machine, one with jest installed too (round 9, brief note 4).
const bin = pathWith(['git']);
try {
writeFileSync(join(repo, 'README.md'), 'x\n');
const base = commitAll(repo, 'base');
mkdirSync(join(repo, 'tests'));
writeFileSync(join(repo, 'tests', 'a.test.mjs'), 'x\n');
// `jest` is on the allowlist and not installed where this suite runs.
const r = proveRed({ cwd: repo, base, cmd: 'jest tests/a.test.mjs', tests: ['tests/a.test.mjs'], redOnly: true });
const out = spawnSync(process.execPath, [join(ROOT, 'lib', 'verification', 'red-proof.mjs'), '--cwd', repo, '--base', base,
'--cmd', 'jest tests/a.test.mjs', '--tests', 'tests/a.test.mjs', '--red-only', '--json'],
{ encoding: 'utf8', env: { ...process.env, PATH: bin.path }, timeout: 60_000 });
const r = JSON.parse(out.stdout);
assert.equal(r.status, 'not-run', JSON.stringify(r));
assert.match(r.reason, /could not start/, r.reason);
const runs = [r.base, r.result].filter(Boolean);
assert.ok(runs.length > 0, JSON.stringify(r));
for (const run of runs) assert.ok(!('spawnError' in run), `no spawnError beside stopped: ${JSON.stringify(run)}`);
} finally { rmSync(repo, { recursive: true, force: true }); }
} finally { bin.cleanup(); rmSync(repo, { recursive: true, force: true }); }
});
test('review 630 r8 / findings-schema.mjs:46: the block reader and the verdict gate agree on what a fence is', async () => {
@ -1024,3 +1029,167 @@ test('review 630 r8 / review-validator.mjs:36 + synthesis-digest-schema.mjs:59:
const digest = '```json\n{"note": "a ```bash fence"}\n```\n';
assert.equal(extractDigestBlock(digest), '{"note": "a ```bash fence"}\n');
});
// ==== round 9 of the release review of 6.3.0 (0f400d3..a10018f, 2026-10-11), fixed in 6.3.1 ====
test('review 630 r9 / f8087ca4 criteria-runner.mjs:172: a check refused for its comment stays a command, refused out loud', async () => {
const { parsePlanVerification, runCriteria } = await import('../../lib/verification/criteria-runner.mjs');
const { validateBriefContent } = await import('../../lib/validators/brief-validator.mjs');
const nestedPlan = (comment) => buildPlan().replace('- [ ] `npm test` → expected: exit 0', () =>
`- [ ] \`npm test\` → expected: exit 0\n - [ ] \`node --test tests/e2e.test.mjs # ${comment}\` → expected: exit 0`);
// (1) A nested ## Verification bullet under a parent with its own command is
// a criterion of its own, never a sub-point, and it is NOT RUN with the reason.
const plan = nestedPlan('expect "3 pass"');
const nested = parsePlanVerification(plan).find((c) => c.text.includes('tests/e2e.test.mjs'));
assert.ok(nested, `the nested check is a criterion: ${JSON.stringify(parsePlanVerification(plan))}`);
const ran = [];
const [r] = runCriteria([nested], {
exec: (c) => { ran.push(c); return { status: 0, stdout: '', stderr: '' }; },
screen: () => ({ allowed: true, rule: '' }),
});
assert.equal(r.status, 'unrunnable', JSON.stringify(r));
assert.match(r.output, /trailing comment/, r.output);
assert.deepEqual(ran, [], 'a refused check never runs, its comment dropped or not');
const issues = (p) => { const v = validatePlanContent(p, { strict: false }); return [...v.errors, ...v.warnings]; };
assert.ok(issues(plan).some((i) => i.code === 'PLAN_VERIFICATION_NOT_RUNNABLE' && /tests\/e2e\.test\.mjs/.test(i.message)),
JSON.stringify(issues(plan).map((i) => i.code)));
// The control: a plain comment is still dropped, and the nested check still runs.
assert.ok(!issues(nestedPlan('all green')).some((i) => i.code === 'PLAN_VERIFICATION_NOT_RUNNABLE'));
// (2) A brief SC with such a comment is BRIEF_SC_NOT_RUNNABLE, not prose-only.
const brief = (sc) => [
'---', 'type: trekbrief', 'brief_version: "2.2"', 'task: t', 'slug: s',
'research_topics: 0', 'research_status: skipped', 'framing: new-direction', 'phase_signals_partial: true', '---', '',
'## TL;DR', '', 'A toy.', '', '## Intent', '', 'Why.', '', '## Goal', '', 'What.', '',
'## Success Criteria', '', sc, '',
].join('\n');
const notRunnable = (sc) => validateBriefContent(brief(sc)).warnings.filter((w) => w.code === 'BRIEF_SC_NOT_RUNNABLE');
assert.equal(notRunnable('- `npm test # prints "ok"` exits 0.').length, 1);
assert.equal(notRunnable('- `npm test # prints ok` exits 0.').length, 0, 'the control');
});
// One review.md: a clean WARN block, then `tail` (what the last block is).
const B3 = '`'.repeat(3);
const B4 = '`'.repeat(4);
function r9Review(tail, { eol = '\n' } = {}) {
const fm = ['---', 'type: trekreview', 'review_version: "1.1"', 'created: 2026-10-11', 'task: "t"', 'slug: s',
'project_dir: p/', 'brief_path: p/brief.md', `scope_sha_start: ${'b'.repeat(40)}`, `scope_sha_end: ${'c'.repeat(40)}`,
'reviewed_files_count: 1', 'verdict: WARN', 'findings: []', '---', ''].join('\n');
const body = '# R\n\n## Executive Summary\n\nx\n\n## Coverage\n\nx\n\n## Declined to Judge\n\nNone.\n\nTrimmed: 0\n\n## Remediation Summary\n\nx\n\n';
return `${fm}${body}${tail}`.replace(/\n/g, eol);
}
const R9_WARN = JSON.stringify({ verdict: 'WARN', counts: { BLOCKER: 0, MAJOR: 0, MINOR: 0, SUGGESTION: 0 }, allow_blocked_by: [], findings: [] }, null, 2);
const R9_BLOCK = JSON.stringify({ verdict: 'BLOCK', counts: { BLOCKER: 1, MAJOR: 0, MINOR: 0, SUGGESTION: 0 }, allow_blocked_by: [],
findings: [{ id: 'a'.repeat(40), severity: 'BLOCKER', rule_key: 'MISSING_TEST', file: 'a.mjs', line: 1 }] }, null, 2);
const r9Clean = `${B3}json\n${R9_WARN}\n${B3}\n`;
test('review 630 r9 / 5fb0d492 verdict-gate.mjs:123: the last fence\'s language and body come from one reading', async () => {
const { decideVerdict } = await import('../../lib/review/verdict-gate.mjs');
// The controls: a clean review passes, LF and CRLF, indented or not.
for (const [name, text] of [['LF', r9Review(r9Clean)], ['CRLF', r9Review(r9Clean, { eol: '\r\n' })],
['indented', r9Review(` ${B3}json\n ${R9_WARN.replace(/\n/g, '\n ')}\n ${B3}\n`)]]) {
assert.equal(decideVerdict(text).exit, 0, `${name}: ${decideVerdict(text).reason}`);
}
// A last block whose info string is not exactly `json` held a BLOCKER the
// block reader skipped: the gate read the earlier clean block and exited 0.
for (const opener of [`${B3}json {x}`, `${B3} json`, `${B3}json\t{"x": 1}`]) {
for (const eol of ['\n', '\r\n']) {
for (const indent of ['', ' ']) {
const tail = `${r9Clean}\n${indent}${opener}\n${R9_BLOCK}\n${indent}${B3}\n`;
const v = decideVerdict(r9Review(tail, { eol }));
assert.equal(v.exit, 2, `${JSON.stringify(indent + opener)} (${JSON.stringify(eol)}): ${v.reason}`);
assert.match(v.reason, /last fenced block/, v.reason);
}
}
}
// An indented last block is read as the last block: its BLOCKER is seen.
const indented = `${r9Clean}\n ${B3}json\n ${R9_BLOCK.replace(/\n/g, '\n ')}\n ${B3}\n`;
assert.equal(decideVerdict(r9Review(indented)).exit, 2, decideVerdict(r9Review(indented)).reason);
// A last block that is never closed runs to the end of the file, as in Markdown.
const unclosed = decideVerdict(r9Review(`${r9Clean}\n${B3}json\n${R9_BLOCK}\n`));
assert.equal(unclosed.exit, 2, unclosed.reason);
assert.match(unclosed.reason, /never closed/, unclosed.reason);
});
test('review 630 r9 / 2eb4b921 verdict-gate.mjs:123: a fence is closed only by a run at least as long as its opener', async () => {
const { decideVerdict } = await import('../../lib/review/verdict-gate.mjs');
const { extractFindingsBlock } = await import('../../lib/review/findings-schema.mjs');
const example = `${B4}markdown\n${B3}json\n{"example": true}\n${B3}\n${B4}\n`;
// A four-backtick example of a json block, then the review's own block: valid.
const before = r9Review(`${example}\n${r9Clean}`);
assert.equal(decideVerdict(before).exit, 0, decideVerdict(before).reason);
assert.equal(extractFindingsBlock(before), `${R9_WARN}\n`);
// The same example after it: the last block is the markdown block, and the
// block reader does not take the example inside it.
const after = r9Review(`${r9Clean}\n${example}`);
const v = decideVerdict(after);
assert.equal(v.exit, 2, v.reason);
assert.match(v.reason, /"markdown"/, v.reason);
assert.equal(extractFindingsBlock(after), `${R9_WARN}\n`, 'the review\'s own block, not the example');
// A four-backtick json block is one block; three backticks inside do not end it.
assert.equal(extractFindingsBlock(`${B4}json\n{"a": 1}\n${B3}\n${B4}\n`), `{"a": 1}\n${B3}\n`);
});
test('review 630 r9 / c0465ecd plan-schema.mjs:350: a description runs on past a comma and onto the next line', () => {
const step = (files) => `- **Files:** ${files}\n- **Changes:** x\n`;
for (const files of ['`tests/a.test.mjs` — mocks fs, child_process.spawnSync and path.join',
'`tests/a.test.mjs` — covers parse, Node.js 22 only',
'`tests/a.test.mjs` — mocks fs and\n child_process.spawnSync, path.join']) {
assert.deepEqual(stepTestsOnly(step(files)), { testsOnly: true, files: ['tests/a.test.mjs'] }, files);
}
// A new entry starts with a backticked path, or on a new list line.
for (const [files, paths] of [
['`tests/a.test.mjs` — new test, `src/a.mjs` — the fix', ['tests/a.test.mjs', 'src/a.mjs']],
['\n - `tests/a.test.mjs` — new test, mocks fs\n - src/a.mjs — the fix', ['tests/a.test.mjs', 'src/a.mjs']],
['\n - `tests/a.test.mjs` — new test\n - package.json', ['tests/a.test.mjs', 'package.json']],
]) {
assert.deepEqual(stepTestsOnly(step(files)), { testsOnly: false, files: paths }, files);
}
});
test('review 630 r9 / 7b3750e9 plan-schema.mjs:372: a plain path that ends with a colon starts its description', () => {
const step = (files) => `- **Files:** ${files}\n- **Changes:** x\n`;
for (const [files, paths] of [
['`tests/a.test.mjs`, src/cli.mjs: wires parse and README.md', ['tests/a.test.mjs', 'src/cli.mjs']],
['`tests/a.test.mjs`, Makefile: a target and package.json', ['tests/a.test.mjs', 'Makefile']],
]) {
assert.deepEqual(stepTestsOnly(step(files)).files, paths, files);
}
// The control: a word that is no path, with a colon, is a label, not a description.
assert.deepEqual(stepTestsOnly(step('`tests/a.test.mjs`, note: Makefile and package.json')).files, ['tests/a.test.mjs', 'package.json']);
});
test('review 630 r9 / brief note 3: red proof reports a control run it stopped as not-run, in the control run', async () => {
const { proveRed } = await import('../../lib/verification/red-proof.mjs');
const repo = gitRepo('r630-r9-control-');
try {
writeFileSync(join(repo, 'README.md'), 'x\n');
const base = commitAll(repo, 'base');
mkdirSync(join(repo, 'tests'));
mkdirSync(join(repo, 'src'));
// Red at the start point (no src/impl.mjs), green in the working tree, and
// it hangs only in the control: the start-point copy with the change laid on.
writeFileSync(join(repo, 'tests', 'a.test.mjs'), [
"import { test } from 'node:test';",
"import { existsSync } from 'node:fs';",
"test('impl', async () => {",
" if (!existsSync('src/impl.mjs')) throw new Error('no impl');",
" if (process.cwd().includes('voyage-red-proof-')) await new Promise((r) => setTimeout(r, 6000));",
'});', ''].join('\n'));
writeFileSync(join(repo, 'src', 'impl.mjs'), 'export const x = 1;\n');
const r = proveRed({ cwd: repo, base, cmd: 'node --test tests/a.test.mjs', tests: ['tests/a.test.mjs'], timeoutMs: 2000 });
assert.equal(r.status, 'not-run', JSON.stringify(r));
assert.match(r.reason, /^in the control run: the command was killed .*after the 2 s timeout/, r.reason);
assert.equal(r.base.exit, 1, 'red at the start point');
assert.equal(r.result.exit, 0, 'green at the result');
assert.ok(r.control, 'the control run is reported');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('review 630 r9 / brief note 8: the Files: reader states one count of plan files, the one measured', () => {
const doc = /\/\*\*\n((?: \*[^\n]*\n)+) \*\/\nconst SEG = /.exec(read('lib/parsers/plan-schema.mjs'));
assert.ok(doc, 'fieldPaths has its doc comment');
const text = doc[1].replace(/\n \* ?/g, ' ');
assert.match(text, /520 Files: fields in 48 plan files at rounds 7-8/);
assert.match(text, /560 fields in 48 plan files at round 9/);
assert.doesNotMatch(text.replace(/round 7 wrote "182 plans" here/, ''), /182/, 'the round-7 count is named only as the one replaced');
});

View file

@ -13,7 +13,7 @@
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { spawnSync } from 'node:child_process';
import { mkdtempSync, mkdirSync, readFileSync, writeFileSync, rmSync, existsSync } from 'node:fs';
import { mkdtempSync, mkdirSync, readFileSync, writeFileSync, rmSync, existsSync, symlinkSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
@ -512,3 +512,20 @@ test('release-check r8 — a relative --review that names two different files is
assert.match(r.stdout, /two files/, r.stdout);
} finally { fx.cleanup(); }
});
// ==== round 9 of the release review of 6.3.0 (2026-10-11), fixed in 6.3.1 ====
test('release-check r9 / bdd46799 — one review.md reached through a symlinked --repo is one file, not two → 0', () => {
const fx = fixture();
const link = join(fx.base, 'repo link');
try {
writeFileSync(join(fx.repo, 'review.md'), readFileSync(fx.review, 'utf8'));
writeFileSync(join(fx.repo, '.git', 'info', 'exclude'), 'review.md\n');
symlinkSync(fx.repo, link);
// --repo through the symlink, run from the repo itself: process.cwd() is the
// physical path, so the two path strings differ while the file is one.
const r = spawnSync(process.execPath, [CHECK, '--repo', link, '--review', 'review.md'], { cwd: fx.repo, encoding: 'utf8' });
assert.doesNotMatch(r.stdout, /two files/, r.stdout);
assert.equal(r.status, 0, r.stdout + r.stderr);
} finally { rmSync(link, { force: true }); fx.cleanup(); }
});