merge: round 3 of the fixes from the release review of 6.3.0
Every allowlisted runner now takes only plain words and options that only read (round 3's BLOCKER: rounds 1-2 closed the write-a-file class for git only), plus the eight MAJOR and two MINOR findings: refused restores in /trekexecute, stepTestsOnly and one Files: reader, --steps-from Verify on 1.8, skill frontmatter, routing, and release-check's sha and artifacts. Suite 2170 (2168/0/2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
commit
41ea8fc317
14 changed files with 284 additions and 40 deletions
|
|
@ -1863,6 +1863,12 @@ directly (Sub-step B).
|
|||
```
|
||||
The new files stay on disk, untracked, and `step-{N}-upward.md` names them —
|
||||
nothing the step made is deleted, and none of it is left registered.
|
||||
When the guard refuses the restore (it is destructive git: in the permission
|
||||
mode that shows no prompts the guard refuses it, exit 2, "no one to ask"), or
|
||||
the operator declines it, nothing was restored: write in `step-{N}-upward.md`
|
||||
and the final report that the step's changes are still in the working tree,
|
||||
with the paths, and that the operator restores them by hand. Never call the
|
||||
step reverted.
|
||||
Record failure. **Do NOT proceed to next step.** Jump to Phase 7.5 (Phase 7
|
||||
runs only when every step passed).
|
||||
|
||||
|
|
@ -2177,6 +2183,14 @@ never typed by the operator, in this order (Cherny's):
|
|||
`refactor: simplify pass (trekexecute)`; `simplify = "applied"`. Exit 1 or
|
||||
2 → restore every path it changed as above, `simplify = "reverted"`, and
|
||||
say why in the report: the cleanup broke the verification.
|
||||
- **A restore did not run** (the guard refused it — `git checkout --` is
|
||||
destructive git, refused in the permission mode that shows no prompts —
|
||||
or the operator declined it): the paths are still changed.
|
||||
`simplify = "revert-refused"`, never `"reverted"`. When the cleanup broke
|
||||
the verification, stop here: the run is `partial`, no review runs over a
|
||||
tree that failed it, and the report lists the paths for the operator to
|
||||
restore by hand. Paths outside the run's files that were not restored are
|
||||
listed too.
|
||||
- **It could not start** (the Skill tool refused, the skill is not there):
|
||||
`simplify = "not-run"`, with the reason. Not a stop: `/simplify` is a
|
||||
cleanup pass, not a gate. Continue.
|
||||
|
|
@ -2855,7 +2869,7 @@ To continue (multi-session waves): re-run the same /trekexecute command (not --r
|
|||
"steps_check_not_measured": 0,
|
||||
"arch_rules": "{pass | violation | not-measured | n/a}",
|
||||
"scope_sweep": "{pass | deviation | not-measured | n/a}",
|
||||
"simplify": "{applied | clean | reverted | not-run | n/a}",
|
||||
"simplify": "{applied | clean | reverted | revert-refused | not-run | n/a}",
|
||||
"review_verdict": "{ALLOW | WARN | BLOCK | none}",
|
||||
"review_gate_exit": "{0 | 1 | 2}",
|
||||
"main_merge": "{merged | nothing-to-merge | closed | declined | n/a}"
|
||||
|
|
|
|||
|
|
@ -375,7 +375,7 @@ contract CLI's exit said in words; `not-measured` / `not-run` are never green.
|
|||
| `plan_approval` | run | `approved \| not-checked` | K2 `approval-log.mjs verify --kind plan` (Phase 1); `not-checked` = a session spec |
|
||||
| `arch_rules` | run | `pass \| violation \| not-measured` | K5 `lib/verification/arch-rules.mjs` exit 0 / 1 / 2 (Phase 7) |
|
||||
| `scope_sweep` | run | `{status: pass \| deviation \| not-measured, outside: [paths]}` | K6 `lib/verification/plan-scope-sweep.mjs` exit 0 / 1 / 2 (Phase 7) |
|
||||
| `simplify` | run | `applied \| clean \| reverted \| not-run` | `/simplify` in Phase 7.2 |
|
||||
| `simplify` | run | `applied \| clean \| reverted \| revert-refused \| not-run` | `/simplify` in Phase 7.2; `revert-refused` = the restore was refused or declined |
|
||||
| `review` | run | `{round ≥ 1, path, gate_exit: 0 \| 1 \| 2, verdict}` | K7 `lib/review/verdict-gate.mjs` on `review.md` (Phase 7.2; Phase 8 locks on it) |
|
||||
|
||||
`plan_verification.failed_criteria[]` carries `exit_code` (integer, or `null`
|
||||
|
|
|
|||
|
|
@ -81,8 +81,9 @@ do. `scripts/release-check.mjs` refuses a release whose section has no
|
|||
It checks, in order: the working tree is clean (otherwise the suite would
|
||||
not measure HEAD), `npm test` exits 0 at HEAD, the review verdict is not
|
||||
BLOCK, the review covers HEAD (its `scope_sha_end` is HEAD or an ancestor,
|
||||
and the commits after it touch only the release bookkeeping above), and the
|
||||
door type is set. 0 = ready, 1 = not ready (each failure listed), 2 = could
|
||||
and the commits after it touch only the release bookkeeping above and
|
||||
Voyage's own artifacts under `.claude/projects/` and `.claude/voyage/`, such
|
||||
as the review itself and the approval log), and the door type is set. 0 = ready, 1 = not ready (each failure listed), 2 = could
|
||||
not run — never green.
|
||||
6. **The operator's yes, logged.** The operator reads the release section and
|
||||
says yes (an answer like "sounds good" is not a yes). The check prints the
|
||||
|
|
|
|||
|
|
@ -293,7 +293,15 @@ export function onFailureAction(text) {
|
|||
* so a bare `Makefile` is still a path. Release review of 6.3.0, rounds 1-2.
|
||||
*/
|
||||
const PATH_WORD = /^(?:[\w@.-]+\/)+[\w@.-]+$|^[\w@-][\w@.-]*\.[A-Za-z0-9]+$/;
|
||||
function fieldPaths(text) {
|
||||
// Abbreviations that PATH_WORD would read as a name with an extension.
|
||||
const NOT_A_PATH = /^(?:e\.g|i\.e|n\.b|cf|etc|vs)$/i;
|
||||
const dotSlash = (p) => p.replace(/^\.\//, '');
|
||||
/**
|
||||
* The one reader of a step's `Files:` field: this file's tests-only check and
|
||||
* the plan-file sweep (lib/verification/plan-scope-sweep.mjs) both use it, so
|
||||
* the two never judge different lists for one step. A leading `./` is dropped.
|
||||
*/
|
||||
export function fieldPaths(text) {
|
||||
const spans = [];
|
||||
const masked = String(text ?? '').replace(/`([^`\n]+)`/g, (_, p) => `\u0000${spans.push(p.trim()) - 1}\u0000`);
|
||||
const out = [];
|
||||
|
|
@ -304,17 +312,20 @@ function fieldPaths(text) {
|
|||
for (const word of entry.split(/\s+/)) {
|
||||
const span = /^\u0000(\d+)\u0000$/.exec(word);
|
||||
if (span) found.push(spans[Number(span[1])]);
|
||||
else if (PATH_WORD.test(word.replace(/[.;:]+$/, ''))) found.push(word.replace(/[.;:]+$/, ''));
|
||||
else {
|
||||
const bare = word.replace(/[.;:]+$/, '');
|
||||
if (PATH_WORD.test(bare) && !NOT_A_PATH.test(bare)) found.push(bare);
|
||||
}
|
||||
}
|
||||
out.push(...(found.length > 0 ? found : [entry.split(/\s+/)[0]]));
|
||||
}
|
||||
return out.filter((p) => p && !p.includes('\u0000'));
|
||||
return out.filter((p) => p && !p.includes('\u0000')).map(dotSlash);
|
||||
}
|
||||
|
||||
// A document is never the test here: `docs/specs/api.md` sits under a `specs/`
|
||||
// folder, which lib/util/test-path.mjs reads as tests for the floor guard.
|
||||
// Nor is a data file under a `spec/` or `specs/` folder (`api/specs/openapi.yaml`
|
||||
// is an API spec); under tests/ it is a fixture, and a fixture is a test file.
|
||||
// A document or a data file outside tests/ is never the test here:
|
||||
// `docs/specs/api.md` and `api/specs/openapi.yaml` sit under a `specs/` folder,
|
||||
// which lib/util/test-path.mjs reads as tests for the floor guard. Under tests/
|
||||
// either one is a fixture, and a fixture is a test file.
|
||||
const DOC_RE = /\.(md|markdown|mdx|txt|rst|adoc)$/i;
|
||||
const DATA_RE = /\.(ya?ml|json|toml|xml|csv)$/i;
|
||||
const UNDER_TESTS_RE = /(^|\/)(tests?|__tests__)\//;
|
||||
|
|
@ -333,13 +344,17 @@ const UNDER_TESTS_RE = /(^|\/)(tests?|__tests__)\//;
|
|||
* @returns {{testsOnly: boolean, files: string[]}}
|
||||
*/
|
||||
export function stepTestsOnly(body) {
|
||||
const f = parseStepFields(body);
|
||||
return stepTestsOnlyFromFields(parseStepFields(body));
|
||||
}
|
||||
|
||||
/** `stepTestsOnly` for a caller that already holds the step's parsed fields. */
|
||||
export function stepTestsOnlyFromFields(f) {
|
||||
const files = f.has('Files') ? fieldPaths(f.get('Files').text) : [];
|
||||
const testFirst = f.has('Test first')
|
||||
? [...f.get('Test first').text.matchAll(/\bFile:\**[ \t]*`([^`\n]+)`/gi)].map((m) => m[1].trim())
|
||||
? [...f.get('Test first').text.matchAll(/\bFile:\**[ \t]*`([^`\n]+)`/gi)].map((m) => dotSlash(m[1].trim()))
|
||||
: [];
|
||||
const isTest = (p) => testFirst.includes(p) ||
|
||||
(isTestPath(p) && !DOC_RE.test(p) && !(DATA_RE.test(p) && !UNDER_TESTS_RE.test(p)));
|
||||
(isTestPath(p) && (UNDER_TESTS_RE.test(p) || !(DOC_RE.test(p) || DATA_RE.test(p))));
|
||||
return { testsOnly: files.length > 0 && files.every(isTest), files };
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -190,7 +190,7 @@ export function loadCases(root) {
|
|||
const text = readFileSync(f, 'utf8');
|
||||
const tagsField = readFrontmatterFields(text).fields.tags;
|
||||
const tags = tagsField && Array.isArray(tagsField.value) ? tagsField.value.map(String) : [];
|
||||
const kindTag = tags.find((t) => t in KIND_TAGS);
|
||||
const kindTag = tags.find((t) => Object.hasOwn(KIND_TAGS, t));
|
||||
if (!kindTag) continue;
|
||||
const val = (k) => { const t = tags.find((x) => x.startsWith(`${k}=`)); return t ? t.slice(k.length + 1) : null; };
|
||||
const body = text.replace(/^---\n[\s\S]*?\n---\n?/, '').trim();
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ export function splitFile(text) {
|
|||
export function plainScalarHazards(value) {
|
||||
const out = [];
|
||||
if (/:\s/.test(value) || /:$/.test(value)) out.push('unquoted ": " (YAML reads a mapping and the line fails)');
|
||||
if (/^#/.test(value)) out.push('"#" at the start is a YAML comment: the value is empty');
|
||||
if (/\s#/.test(value)) out.push('" #" starts a YAML comment: the rest of the value is cut');
|
||||
if (/^[`@%*&!|>,?{]/.test(value)) out.push(`a value may not start with "${value[0]}" unquoted`);
|
||||
if (/^\[/.test(value) && !/\]$/.test(value)) out.push('an unclosed flow list');
|
||||
|
|
@ -40,7 +41,9 @@ export function plainScalarHazards(value) {
|
|||
|
||||
function unquote(v) {
|
||||
if (v.startsWith('"')) {
|
||||
if (v.length < 2 || !v.endsWith('"') || v.endsWith('\\"')) return { error: 'unclosed double quote' };
|
||||
// The last quote closes the value unless an odd run of backslashes escapes it.
|
||||
const escaped = (v.slice(1, -1).match(/\\*$/)[0].length % 2) === 1;
|
||||
if (v.length < 2 || !v.endsWith('"') || escaped) return { error: 'unclosed double quote' };
|
||||
try { return { value: JSON.parse(v) }; } catch { return { error: 'bad escape in a double-quoted value' }; }
|
||||
}
|
||||
if (v.startsWith("'")) {
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ import {
|
|||
sliceSteps, validatePlanHeadings, extractPlanVersion, extractPlanCeremony,
|
||||
parseStepFields, findSection, splitSections, findModuleCycle, parseMermaid, fencedBlocks,
|
||||
versionAtLeast, PLAN_VERSION_CURRENT, PLAN_VERSIONS_SUPPORTED, CEREMONY_VALUES,
|
||||
ON_FAILURE_ACTIONS, onFailureAction, stepTestsOnly,
|
||||
ON_FAILURE_ACTIONS, onFailureAction, stepTestsOnlyFromFields,
|
||||
} from '../parsers/plan-schema.mjs';
|
||||
import { validateAllManifests } from '../parsers/manifest-yaml.mjs';
|
||||
import { parseDocument } from '../util/frontmatter.mjs';
|
||||
|
|
@ -165,7 +165,7 @@ function checkRecovery(sections, opts) {
|
|||
`Step ${s.n}: \`On failure:\` does not open with a recovery word the executor reads`,
|
||||
`Open with one of ${ON_FAILURE_ACTIONS.join(' | ')}, then the specifics (\`retry once, then escalate\`).`));
|
||||
}
|
||||
if (stepTestsOnly(s.body).testsOnly) {
|
||||
if (stepTestsOnlyFromFields(f).testsOnly) {
|
||||
// The run-the-thing script alone is such a step too: `End-to-end:` rides on
|
||||
// the last step that builds behaviour (release review of 6.3.0, 2026-10-10).
|
||||
const e2e = f.has('End-to-end')
|
||||
|
|
@ -298,11 +298,13 @@ export function validatePlanContent(rawText, opts = {}) {
|
|||
// Phase 7.2) must be runnable steps whatever the plan's version: below 1.8
|
||||
// the step rules never ran, and a review step whose Verify is "re-run
|
||||
// /trekreview" validated and was executed (release review of 6.3.0,
|
||||
// 2026-10-10). From 1.8 the rules below already cover every step.
|
||||
// 2026-10-10). From 1.8 the rules below cover every step except one: none of
|
||||
// them asks for a `Verify:`, so an appended step needs one on any version
|
||||
// (round 3).
|
||||
const stepsFrom = Number.isInteger(opts.stepsFrom) && opts.stepsFrom > 0 ? opts.stepsFrom : null;
|
||||
if (stepsFrom !== null && !versionAtLeast(planVersion, '1.8')) {
|
||||
if (stepsFrom !== null) {
|
||||
const appended = sections.filter((s) => Number(s.n) >= stepsFrom);
|
||||
const stepIssues = [
|
||||
const stepIssues = versionAtLeast(planVersion, '1.8') ? [] : [
|
||||
...checkSteps(appended).filter((i) => i.code !== 'PLAN_NO_END_TO_END'),
|
||||
...checkRunnable(text, appended, { cwd: opts.cwd, stepsOnly: true }),
|
||||
];
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ export const PROGRESS_REGRESSION_VALUES = ['passed', 'failed', 'unrunnable', 'no
|
|||
export const PROGRESS_PLAN_APPROVAL_VALUES = ['approved', 'not-checked']; // K2; not-checked = a session spec
|
||||
export const PROGRESS_ARCH_RULES_VALUES = ['pass', 'violation', 'not-measured']; // K5 exit 0 / 1 / 2
|
||||
export const PROGRESS_SCOPE_SWEEP_VALUES = ['pass', 'deviation', 'not-measured']; // K6 exit 0 / 1 / 2
|
||||
export const PROGRESS_SIMPLIFY_VALUES = ['applied', 'clean', 'reverted', 'not-run'];
|
||||
export const PROGRESS_SIMPLIFY_VALUES = ['applied', 'clean', 'reverted', 'revert-refused', 'not-run'];
|
||||
export const PROGRESS_REVIEW_GATE_EXITS = [0, 1, 2]; // K7: ALLOW/WARN, BLOCK, could not decide
|
||||
|
||||
function checkStepFields(k, s, errors) {
|
||||
|
|
|
|||
|
|
@ -257,7 +257,13 @@ export function screenCommand(command, opts = {}) {
|
|||
// - It is not a sandbox. `npm test`, `npm run <script>` and `make test` run
|
||||
// whatever the repo's own package.json/Makefile says they run, including a
|
||||
// script that pushes. That is the repo's responsibility, not this file's.
|
||||
// - It has no opinion about arguments beyond the shape rules below.
|
||||
// - It is not an option parser. Each runner takes only plain words (no
|
||||
// quote, backslash, `$` or brace; a glob only inside a path) and a short
|
||||
// list of options that only read (RUNNER_ARGS); an option that writes a
|
||||
// file or loads code (`--test-reporter-destination`, `--outputFile`,
|
||||
// `--junitxml`, `--basetemp`, `--import`, `--config`, `-o`, `-exec`) is on
|
||||
// no list (release review of 6.3.0, round 3: rounds 1-2 closed this for
|
||||
// git only).
|
||||
// - It rejects plenty of honest commands: an env prefix (`CI=1 npm test`), a
|
||||
// project's own binary (`./build/app --check`), anything piped. A check
|
||||
// that needs one of those is declared through a runner instead —
|
||||
|
|
@ -279,6 +285,60 @@ function writesOutput(word) {
|
|||
const deny = (reason) => ({ allowed: false, reason });
|
||||
const permit = () => ({ allowed: true, reason: '' });
|
||||
|
||||
// A word /bin/sh hands on unchanged, or a path with a glob in it: a glob can
|
||||
// only expand to names that start with the word's first character, so it
|
||||
// cannot make an option out of a path.
|
||||
const PLAIN_WORD = /^(?:[A-Za-z0-9_@%^~:.,=+/-]+|[A-Za-z0-9_./][A-Za-z0-9_@%^~:.,=+/*?-]*)$/;
|
||||
|
||||
// After each runner's fixed words: the options it may take (`name` or
|
||||
// `name=value`; `value` lists the ones whose value may be the next word, and
|
||||
// `only` the ones whose value must be one of a few words: a built-in node test
|
||||
// reporter only formats, a reporter path would load code),
|
||||
// whether `--` may hand the rest to the repo's own script (`dashDash`: then
|
||||
// only paths follow), and whether a plain word may carry `=` (make reads
|
||||
// `VAR=value` as an override). The lists hold options that only read.
|
||||
const RUNNER_ARGS = {
|
||||
npm: { fixed: (t) => (t[1] === 'run' ? 3 : 2), opts: [], dashDash: true },
|
||||
node: {
|
||||
fixed: 2,
|
||||
opts: ['--test-name-pattern', '--test-skip-pattern', '--test-only', '--test-concurrency', '--test-timeout', '--test-reporter'],
|
||||
value: ['--test-reporter'],
|
||||
only: { '--test-reporter': ['spec', 'tap', 'dot', 'junit', 'lcov'] },
|
||||
},
|
||||
vitest: { fixed: 1, opts: ['--run', '--passWithNoTests', '--silent', '--bail'] },
|
||||
jest: { fixed: 1, opts: ['--ci', '--runInBand', '-i', '--silent', '--passWithNoTests', '--bail', '--verbose'] },
|
||||
pytest: { fixed: 1, opts: ['-q', '-qq', '-v', '-vv', '-x', '-s', '--quiet', '--verbose', '--exitfirst', '--tb', '--maxfail'] },
|
||||
cargo: { fixed: 2, opts: ['--release', '--workspace', '--all', '--lib', '--doc', '-q', '--quiet', '--no-fail-fast'] },
|
||||
go: { fixed: 2, opts: ['-v', '-short', '-race', '-failfast', '-cover', '-count', '-run'], value: ['-count', '-run'] },
|
||||
make: { fixed: 2, opts: [], noAssign: true },
|
||||
};
|
||||
RUNNER_ARGS.python = RUNNER_ARGS.python3 = RUNNER_ARGS.uv = { ...RUNNER_ARGS.pytest, fixed: 3 };
|
||||
|
||||
function screenArgs(t, spec) {
|
||||
const fixed = typeof spec.fixed === 'function' ? spec.fixed(t) : spec.fixed;
|
||||
for (let i = fixed, paths = false; i < t.length; i++) {
|
||||
const w = t[i];
|
||||
if (paths) {
|
||||
if (w.startsWith('-')) return deny(`an option after \`--\` (${w}): only paths may follow it`);
|
||||
continue;
|
||||
}
|
||||
if (w === '--' && spec.dashDash) { paths = true; continue; }
|
||||
if (w.startsWith('-')) {
|
||||
const name = w.split('=')[0];
|
||||
if (!spec.opts.includes(name)) return deny(`\`${t[0]}\` with an option that is not on its read-only list (${name})`);
|
||||
const inline = w.includes('=');
|
||||
const value = inline ? w.slice(name.length + 1) : spec.value?.includes(name) ? t[i + 1] : undefined;
|
||||
if (!inline && spec.value?.includes(name)) i++;
|
||||
if (spec.only?.[name] && !spec.only[name].includes(value)) {
|
||||
return deny(`\`${t[0]}\` with ${name} ${value ?? '(no value)'}: only ${spec.only[name].join('|')}`);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (spec.noAssign && w.includes('=')) return deny(`\`${t[0]}\` with a variable override (${w})`);
|
||||
}
|
||||
return permit();
|
||||
}
|
||||
|
||||
// `bash <path>` is the way a repo declares a check the allowlist has no name
|
||||
// for. The path must live under tests/, be relative, and not climb out of it.
|
||||
function isTestScript(path) {
|
||||
|
|
@ -371,7 +431,11 @@ function screenShape(command, opts, writeTime) {
|
|||
|
||||
const tokens = text.trim().split(/\s+/);
|
||||
if (!Object.hasOwn(RUNNERS, tokens[0])) return deny(`\`${tokens[0]}\` is not a known test runner`);
|
||||
return RUNNERS[tokens[0]](tokens, cwd, writeTime);
|
||||
const form = RUNNERS[tokens[0]](tokens, cwd, writeTime);
|
||||
if (!form.allowed || tokens[0] === 'git') return form;
|
||||
const odd = tokens.find((w) => !PLAIN_WORD.test(w));
|
||||
if (odd !== undefined) return deny(`a word the shell would rewrite (${odd}): only plain words are checked`);
|
||||
return Object.hasOwn(RUNNER_ARGS, tokens[0]) ? screenArgs(tokens, RUNNER_ARGS[tokens[0]]) : permit();
|
||||
}
|
||||
|
||||
// --- running ----------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ import { readFileSync, existsSync, realpathSync } from 'node:fs';
|
|||
import { spawnSync } from 'node:child_process';
|
||||
import { resolve, relative, dirname, isAbsolute, sep } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { sliceSteps, parseStepFields } from '../parsers/plan-schema.mjs';
|
||||
import { sliceSteps, parseStepFields, fieldPaths } from '../parsers/plan-schema.mjs';
|
||||
import { parseManifest } from '../parsers/manifest-yaml.mjs';
|
||||
|
||||
// Files Voyage's own process writes while planning and executing.
|
||||
|
|
@ -44,9 +44,8 @@ export const PROCESS_FILES = Object.freeze([
|
|||
]);
|
||||
|
||||
const BACKTICK_SPAN = /`([^`\n]+)`/g;
|
||||
// Backticked: spaces allowed (`docs/my file.md`); bare: no whitespace.
|
||||
// A backticked path in `Test first:` (spaces allowed: `docs/my file.md`).
|
||||
const looksLikeTickedPath = s => /[./]/.test(s) && !/^https?:/.test(s) && !s.startsWith('-') && s.trim() === s;
|
||||
const looksLikePath = s => !/\s/.test(s) && looksLikeTickedPath(s);
|
||||
|
||||
function pathsIn(text) {
|
||||
const out = [];
|
||||
|
|
@ -62,12 +61,9 @@ export function plannedPaths(planText) {
|
|||
for (const s of sliceSteps(planText)) {
|
||||
const f = parseStepFields(s.body);
|
||||
const files = f.get('Files');
|
||||
if (files) {
|
||||
const ticked = pathsIn(files.text);
|
||||
// Older plans write bare paths: `- Files: a.ts, b.ts`.
|
||||
const bare = ticked.length ? [] : files.text.split(/[,\s]+/).filter(looksLikePath);
|
||||
for (const p of [...ticked, ...bare]) out.add(norm(p));
|
||||
}
|
||||
// One reader with the tests-only check (plan-schema.mjs fieldPaths):
|
||||
// backticked spans as written, else every path-like word.
|
||||
if (files) for (const p of fieldPaths(files.text)) out.add(norm(p));
|
||||
const tf = f.get('Test first');
|
||||
if (tf) {
|
||||
for (const line of tf.text.split('\n')) {
|
||||
|
|
|
|||
|
|
@ -39,7 +39,6 @@ import { dirname, join, relative, resolve } from 'node:path';
|
|||
import { fileURLToPath } from 'node:url';
|
||||
import { decideVerdict } from '../lib/review/verdict-gate.mjs';
|
||||
import { commandEnv } from '../lib/util/command-env.mjs';
|
||||
import { parseDocument } from '../lib/util/frontmatter.mjs';
|
||||
|
||||
const PLUGIN_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..');
|
||||
|
||||
|
|
@ -54,6 +53,11 @@ export const CHECK_IDS = Object.freeze(['tree', 'suite', 'review', 'coverage', '
|
|||
export const RELEASE_BOOKKEEPING = Object.freeze([
|
||||
'CHANGELOG.md', 'package.json', 'package-lock.json', '.claude-plugin/plugin.json', 'README.md',
|
||||
]);
|
||||
// Voyage's own pipeline artifacts, committed by a repo that tracks them
|
||||
// (`artifact_store: git`, `.claude/voyage/approvals.jsonl`): the review itself
|
||||
// and the approval log land after the reviewed commit by design (release
|
||||
// review of 6.3.0, round 3). The plan-file sweep treats them the same way.
|
||||
const PROCESS_PREFIXES = Object.freeze(['.claude/projects/', '.claude/voyage/']);
|
||||
const BADGE_RE = /badge\/version-/;
|
||||
const VERSION_FILES = Object.freeze(['package.json', 'package-lock.json', '.claude-plugin/plugin.json']);
|
||||
|
||||
|
|
@ -165,11 +169,20 @@ function checkReview(reviewPath) {
|
|||
return { check: { id: 'review', status: 'pass', detail: `review verdict ${v.verdict}` }, text };
|
||||
}
|
||||
|
||||
/**
|
||||
* The review's `scope_sha_end` as written. Read from the frontmatter text, not
|
||||
* through the YAML reader, which turns an all-digit value into a number and
|
||||
* drops a leading zero (`0123456` → 123456; release review of 6.3.0, round 3).
|
||||
*/
|
||||
export function reviewedSha(reviewText) {
|
||||
const fm = /^---\r?\n([\s\S]*?)\r?\n---\r?\n/.exec(String(reviewText ?? ''));
|
||||
const m = fm && /^scope_sha_end:[ \t]*(["']?)([^"'\s#]*)\1[ \t]*(?:#.*)?$/m.exec(fm[1]);
|
||||
return m ? m[2] : '';
|
||||
}
|
||||
|
||||
function checkCoverage(repo, reviewText) {
|
||||
if (reviewText === null) return { id: 'coverage', status: 'skip', detail: 'not checked: no readable review' };
|
||||
let sha;
|
||||
try { sha = parseDocument(reviewText).parsed?.frontmatter?.scope_sha_end; } catch { sha = undefined; }
|
||||
sha = sha === undefined || sha === null ? '' : String(sha).trim();
|
||||
const sha = reviewedSha(reviewText);
|
||||
if (!sha) return { id: 'coverage', status: 'fail', detail: 'review.md has no scope_sha_end — cannot tell what it reviewed' };
|
||||
const resolved = git(repo, ['rev-parse', '--verify', '--quiet', `${sha}^{commit}`]);
|
||||
if (resolved.status !== 0) return { id: 'coverage', status: 'fail', detail: `the reviewed commit ${sha} is not in the history of HEAD (unknown commit)` };
|
||||
|
|
@ -182,7 +195,7 @@ function checkCoverage(repo, reviewText) {
|
|||
const names = git(repo, ['diff', '--relative', '--name-only', full, 'HEAD']);
|
||||
if (names.status !== 0) return { id: 'coverage', status: 'error', detail: `git diff failed: ${names.stderr.trim()}` };
|
||||
const changed = names.stdout.split('\n').filter(Boolean);
|
||||
const outside = changed.filter((f) => !RELEASE_BOOKKEEPING.includes(f));
|
||||
const outside = changed.filter((f) => !RELEASE_BOOKKEEPING.includes(f) && !PROCESS_PREFIXES.some((p) => f.startsWith(p)));
|
||||
for (const file of changed.filter((f) => VERSION_FILES.includes(f))) {
|
||||
const before = git(repo, ['show', `${full}:./${file}`]);
|
||||
const after = git(repo, ['show', `HEAD:./${file}`]);
|
||||
|
|
|
|||
2
tests/fixtures/jsonl-schemas.md
vendored
2
tests/fixtures/jsonl-schemas.md
vendored
|
|
@ -22,7 +22,7 @@
|
|||
| trekbrief-stats | ts, task, slug, mode, interview_turns, review_iterations, brief_quality, research_topics, auto_research, auto_result, intent_approved, project_dir, order_id, order_claimed_at, order_reason | commands/trekbrief.md (orchestrator-emit Phase 7; order_* added by lib/stats/stats-append.mjs) | trekbrief.md:1130-1145 | profile, phase_models, profile_source | none |
|
||||
| trekresearch-stats | ts, question, mode, scope, engine, slug, project_dir, brief_path, dimensions, dimensions_baseline, dimensions_baseline_preserved, effort, conv_turns, empty_turns, unique_sources, agents_local, agents_external, gemini_used, confidence, contradictions, open_questions | lib/stats/stats-append.mjs (trekresearch.md Stats tracking) | trekresearch.md:782-808 | profile, phase_models, parallel_agents, external_research_enabled, profile_source | none |
|
||||
| trekplan-stats | ts, task, mode, slug, brief_path, project_dir, codebase_size, codebase_files, agents_deployed, deep_dives, research_briefs_used, research_scout_used, critic_verdict, guardian_verdict, outcome | lib/stats/stats-append.mjs (trekplan.md Phase 12) | trekplan.md:1115-1135 | profile, phase_models, parallel_agents, profile_source | none |
|
||||
| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:2877-2899 | profile, phase_models, profile_source | none |
|
||||
| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:2891-2913 | profile, phase_models, profile_source | none |
|
||||
| trekexecute-stats (autonomy events) | ts, event, known_event, payload | lib/stats/event-emit.mjs `emit()` | event-emit.mjs:78-99 | payload.profile, payload.phase_models, payload.profile_source | none |
|
||||
| trekexecute-stats (PostToolUse Bash) | ts, session_id, command_excerpt, duration_ms, success | hooks/scripts/post-bash-stats.mjs (Bash PostToolUse) | post-bash-stats.mjs:42-54 | none (hook is plugin-level, not profile-aware) | command_excerpt (CWE-212) |
|
||||
| trekreview-stats | ts, slug, verdict, counts (BLOCKER/MAJOR/MINOR/SUGGESTION), reviewed_files_count, mode, duration_ms | lib/stats/stats-append.mjs (trekreview.md Phase 8) | trekreview.md:628-632 | profile, phase_models, profile_source | none |
|
||||
|
|
|
|||
|
|
@ -265,3 +265,116 @@ test('review 630 r2 (operator 2026-10-10): Phase 7.2 says plainly that template-
|
|||
assert.match(cmd, /Today the loop runs only for steps written as\s+runnable plan steps/);
|
||||
assert.match(cmd, /re-plan from the review" — the\s+usual outcome until a later order/);
|
||||
});
|
||||
|
||||
// ==== round 3 of the release review (2026-10-10), each red on 96cb977 ===========
|
||||
|
||||
test('review 630 r3 / BLOCKER: no runner takes an option that writes a file or loads code', async () => {
|
||||
const { allowedCommand } = await import('../../lib/verification/criteria-runner.mjs');
|
||||
for (const cmd of [
|
||||
'node --test --test-reporter=tap --test-reporter-destination=/tmp/x',
|
||||
'node --test --import=data:text/javascript,1',
|
||||
'node --test "--import=data:text/javascript,import(\'node:fs\')"',
|
||||
'node --test --require ./evil.cjs',
|
||||
'node --test --test-reporter=./evil-reporter.mjs',
|
||||
'node --test --test-reporter ./evil-reporter.mjs',
|
||||
'jest --json --outputFile=/tmp/x',
|
||||
'jest --config=evil.js',
|
||||
'vitest --outputFile=/tmp/x',
|
||||
'vitest run --reporter=./evil.mjs',
|
||||
'pytest --junitxml=/tmp/x',
|
||||
'pytest --basetemp=/tmp/anydir',
|
||||
'pytest -p evil_plugin',
|
||||
'python -m pytest --junitxml=/tmp/x',
|
||||
'uv run pytest --basetemp=/tmp/x',
|
||||
'go test -o /tmp/x',
|
||||
'go test -exec /tmp/evil ./...',
|
||||
'cargo test --target-dir=/tmp/x',
|
||||
'npm test -- --test-reporter-destination=/tmp/x',
|
||||
'npm test --script-shell=/tmp/evil',
|
||||
'npm run verify -- --outputFile=/tmp/x',
|
||||
'make test SHELL=/tmp/evil',
|
||||
'make test -f /tmp/evil.mk',
|
||||
'bash tests/x.sh "$(id)"',
|
||||
]) {
|
||||
assert.equal(allowedCommand(cmd, { cwd: ROOT }).allowed, false, `${cmd} must not reach the shell`);
|
||||
}
|
||||
for (const cmd of [
|
||||
'npm test', 'npm test -- tests/lib/criteria-runner.test.mjs', 'node --test tests/lib/criteria-runner.test.mjs',
|
||||
'node --test tests/lib/*.test.mjs', 'node --test --test-reporter=tap tests/lib/review-630.test.mjs', 'node --test --test-reporter dot', 'node --test --test-name-pattern=review tests/lib/review-630.test.mjs',
|
||||
'vitest run', 'jest --ci', 'pytest -q', 'pytest -x -q tests/', 'python3 -m pytest tests/', 'uv run pytest',
|
||||
'cargo test', 'cargo test --release', 'go test ./...', 'go test -run TestX -count=1 ./pkg', 'make test',
|
||||
'bash tests/fixtures/criteria-exit-0.sh', 'bash tests/fixtures/criteria-exit-0.sh --fast',
|
||||
]) {
|
||||
const r = allowedCommand(cmd, { cwd: ROOT });
|
||||
assert.equal(r.allowed, true, `the control: ${cmd} is a test run (${r.reason})`);
|
||||
}
|
||||
});
|
||||
|
||||
test('review 630 r3: a restore the guard refuses is never recorded as reverted', async () => {
|
||||
const { PROGRESS_SIMPLIFY_VALUES } = await import('../../lib/validators/progress-validator.mjs');
|
||||
assert.ok(PROGRESS_SIMPLIFY_VALUES.includes('revert-refused'), 'progress.json can say the restore was refused');
|
||||
assert.match(read('docs/HANDOVER-CONTRACTS.md'), /`applied \\| clean \\| reverted \\| revert-refused \\| not-run`/);
|
||||
const cmd = read('commands/trekexecute.md');
|
||||
const onFailure = cmd.slice(cmd.indexOf('**`On failure: revert`** and **`On failure: retry`**'), cmd.indexOf('**`On failure: skip`**'));
|
||||
assert.match(onFailure, /guard refuses the restore/, 'the step revert says what happens when the guard refuses it');
|
||||
const simplify = cmd.slice(cmd.indexOf('2. **`/simplify`**'), cmd.indexOf('3. **`/trekreview`**'));
|
||||
assert.match(simplify, /simplify = "revert-refused"/);
|
||||
assert.match(simplify, /never `"reverted"`/);
|
||||
});
|
||||
|
||||
test('review 630 r3: a fixture document under tests/ is a test file, and "e.g." is not a path', () => {
|
||||
const step = (files) => `- **Files:** ${files}\n- **Changes:** x\n`;
|
||||
assert.equal(stepTestsOnly(step('`tests/a.test.mjs`, `tests/fixtures/sample.md`')).testsOnly, true);
|
||||
assert.deepEqual(stepTestsOnly(step('tests/a.test.mjs (new), e.g. the fixture tests/fixtures/x.json')),
|
||||
{ testsOnly: true, files: ['tests/a.test.mjs', 'tests/fixtures/x.json'] });
|
||||
assert.equal(stepTestsOnly(step('`docs/specs/api.md`')).testsOnly, false, 'a document outside tests/ is still not a test');
|
||||
assert.deepEqual(stepTestsOnly(step('`./tests/a.test.mjs`')).files, ['tests/a.test.mjs'], './ is stripped');
|
||||
});
|
||||
|
||||
test('review 630 r3: the plan-file sweep reads Files: with the same reader as the tests-only check', async () => {
|
||||
const { plannedPaths } = await import('../../lib/verification/plan-scope-sweep.mjs');
|
||||
const plan = '## Implementation Plan\n\n### Step 1: x\n\n- **Files:** `./src/a.mjs`, tests/b.test.mjs + src/c.mjs\n- **Changes:** x\n';
|
||||
const body = plan.slice(plan.indexOf('### Step 1'));
|
||||
assert.deepEqual(plannedPaths(plan), [...stepTestsOnly(body).files].sort());
|
||||
});
|
||||
|
||||
test('review 630 r3: --steps-from refuses an appended step with no Verify: on a 1.8 plan too', () => {
|
||||
const plan = buildPlan({});
|
||||
const step = '### Step 3: Fix it\n\n- **Files:** `src/fix.mjs`, `tests/fix.test.mjs`\n- **Changes:** fix it\n' +
|
||||
'- **Test first:**\n - File: `tests/fix.test.mjs` (new)\n - Verifies: it is fixed\n- **On failure:** escalate\n';
|
||||
const edited = plan.replace('\n## Test Strategy', `\n${step}\n## Test Strategy`);
|
||||
assert.ok(edited.includes('### Step 3: Fix it'), 'the fixture appends the step');
|
||||
const r = validatePlanContent(edited, { strict: true, stepsFrom: 3 });
|
||||
assert.ok(r.errors.some((e) => e.code === 'PLAN_STEP_VERIFY_NOT_RUNNABLE' && /Step 3/.test(e.message)),
|
||||
JSON.stringify(r.errors.map((e) => e.code)));
|
||||
});
|
||||
|
||||
test('review 630 r3: the plan validator hands its parsed fields to the tests-only check', async () => {
|
||||
const schema = await import('../../lib/parsers/plan-schema.mjs');
|
||||
assert.equal(typeof schema.stepTestsOnlyFromFields, 'function');
|
||||
const body = '- **Files:** `tests/a.test.mjs`\n- **Changes:** x\n';
|
||||
assert.deepEqual(schema.stepTestsOnlyFromFields(schema.parseStepFields(body)), stepTestsOnly(body));
|
||||
assert.match(read('lib/validators/plan-validator.mjs'), /stepTestsOnlyFromFields\(f\)/);
|
||||
});
|
||||
|
||||
test('review 630 r3: skill frontmatter - a value starting with # is a hazard, an escaped backslash closes a quote', async () => {
|
||||
const { plainScalarHazards, readFrontmatterFields } = await import('../../lib/skills/skill-frontmatter.mjs');
|
||||
assert.ok(plainScalarHazards('#1 helper. Use when testing.').length > 0, 'YAML reads "#..." as a comment: the value is null');
|
||||
const r = readFrontmatterFields('---\nname: x\ndescription: "Path C:\\\\"\n---\nbody\n');
|
||||
assert.equal(r.fields.description?.value, 'Path C:\\', JSON.stringify(r));
|
||||
const open = readFrontmatterFields('---\nname: x\ndescription: "a \\"\n---\nbody\n');
|
||||
assert.ok(JSON.stringify(open).includes('unclosed double quote'), 'a quote escaped by one backslash is still open');
|
||||
});
|
||||
|
||||
test('review 630 r3: routing reads a kind tag only from its own table, never Object.prototype', () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'r630-proto-'));
|
||||
try {
|
||||
for (const d of ['skills', 'commands', 'agents', 'evals']) cpSync(join(ROOT, d), join(dir, d), { recursive: true });
|
||||
const c = join(dir, 'evals', 'zz-proto');
|
||||
cpSync(join(ROOT, 'evals', 'skill-test-first-quiet-1'), c, { recursive: true });
|
||||
writeFileSync(join(c, 'prompt.md'), '---\ntags: [constructor, owner=nobody]\n---\n\nWhat time is it?\n');
|
||||
const { checkRouting } = awaitImportSync();
|
||||
const r = checkRouting(dir);
|
||||
assert.ok(!r.errors.some((e) => String(e.message ?? '').includes('zz-proto') || e.case === 'zz-proto'), JSON.stringify(r.errors));
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
|
|
|
|||
|
|
@ -126,7 +126,10 @@ function fixture({
|
|||
git(dir, 'commit', '-q', '-m', 'chore(release): fixture');
|
||||
|
||||
if (afterRelease) {
|
||||
for (const [rel, text] of Object.entries(afterRelease)) writeFileSync(join(dir, rel), text);
|
||||
for (const [rel, text] of Object.entries(afterRelease)) {
|
||||
mkdirSync(dirname(join(dir, rel)), { recursive: true });
|
||||
writeFileSync(join(dir, rel), text);
|
||||
}
|
||||
git(dir, 'add', '-A');
|
||||
git(dir, 'commit', '-q', '-m', 'after the review');
|
||||
}
|
||||
|
|
@ -388,3 +391,23 @@ test('release-check — a flag with no value is exit 2, not 1', () => {
|
|||
assert.equal(r.status, 2, r.stdout + r.stderr);
|
||||
assert.match(r.stdout + r.stderr, /not measured|NOT MEASURED/i);
|
||||
});
|
||||
|
||||
// ==== round 3 of the release review of 6.3.0 (2026-10-10) ====================
|
||||
|
||||
test('release-check r3 — the review and the approval log committed after the review do not count as unreviewed → 0', () => {
|
||||
const fx = fixture({ afterRelease: {
|
||||
'.claude/projects/2026-10-08-s/review.md': 'the review, committed (artifact_store: git)\n',
|
||||
'.claude/voyage/approvals.jsonl': '{"kind":"release"}\n',
|
||||
} });
|
||||
try {
|
||||
const r = run(fx);
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
} finally { fx.cleanup(); }
|
||||
});
|
||||
|
||||
test('release-check r3 — scope_sha_end is read as written: an all-digit sha keeps its leading zero', () => {
|
||||
assert.equal(typeof mod.reviewedSha, 'function');
|
||||
assert.equal(mod.reviewedSha('---\ntype: trekreview\nscope_sha_end: 0123456\n---\nbody\n'), '0123456');
|
||||
assert.equal(mod.reviewedSha('---\nscope_sha_end: "00ab12"\n---\n'), '00ab12');
|
||||
assert.equal(mod.reviewedSha('---\ntype: trekreview\n---\nscope_sha_end: 1\n'), '', 'only the frontmatter counts');
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue