merge: round 2 of the fixes from the release review of 6.3.0
The git runner's --output refusal now holds against quoting, plus the round-2 findings in fieldPaths, --steps-from, regression-check, release-check and skill routing. The review-step loop's limit is stated plainly (operator 2026-10-10: runnable template steps are a later order). Suite 2160 (2158/0/2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
commit
96cb977856
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
10 changed files with 263 additions and 70 deletions
|
|
@ -1734,6 +1734,10 @@ log. It installs nothing.
|
|||
criteria runner's two screens (the allowlist of test runners, then the
|
||||
executor denylist), before the checkpoint:
|
||||
|
||||
`steps.{N}.regression` is the JSON's `progress_status`, copied as it is: the
|
||||
CLI already speaks progress.json's words (`"blocked"` → `"unrunnable"`,
|
||||
`"error"` → `"not-run"`).
|
||||
|
||||
| Exit | `steps.{N}.regression` | Executor |
|
||||
|------|------------------------|----------|
|
||||
| 0 | `"passed"`, or `"absent"` when the step has no `Regression:` (nothing new happens) | continue |
|
||||
|
|
@ -2205,7 +2209,13 @@ never typed by the operator, in this order (Cherny's):
|
|||
**A BLOCK is fixed by new plan steps, in a bounded loop.** The review writes
|
||||
every BLOCKER as a numbered `### Step R<n>` under `## Remediation Steps`
|
||||
(Handover 6: Files, Source finding, Test first, Verify, On failure). The fix is
|
||||
those steps, not improvised. Every stop below is stop class 4 (Phase 5.5):
|
||||
those steps, not improvised. **Today the loop runs only for steps written as
|
||||
runnable plan steps:** `Test first:` with its `File:` and `Verifies:`, and a
|
||||
`Verify:` that is one allowlisted test command. A step in the review
|
||||
template's own form (a prose `Test first:`, `Verify:` "re-run `/trekreview`")
|
||||
does not pass step 3, and the run stops with "re-plan from the review" — the
|
||||
usual outcome until a later order makes the template's steps runnable as
|
||||
written. Every stop below is stop class 4 (Phase 5.5):
|
||||
the run is `stopped`, and the merge stays closed.
|
||||
|
||||
0. **On the wave path there is no loop.** The plan's sha256 binds the
|
||||
|
|
|
|||
|
|
@ -285,22 +285,30 @@ export function onFailureAction(text) {
|
|||
// ---- a step that holds nothing but tests -----------------------------------
|
||||
|
||||
/**
|
||||
* The paths a field names. Each entry (comma- or line-separated, a sub-bullet's
|
||||
* `-` dropped) gives its backticked spans, or else its first word: a plain
|
||||
* path beside a backticked one is still a path (independent review, order 10).
|
||||
* A parenthesised note is not an entry: "`a.test.mjs` (new: covers x, y)"
|
||||
* names one path, not "y)" too (release review of 6.3.0, 2026-10-10).
|
||||
* The paths a field names, in order. A backticked span is a path as written,
|
||||
* parentheses and all (`app/(auth)/x.sh`). In the plain text around the spans
|
||||
* a parenthesised note is not a path ("(new: covers x, y)"), and every word
|
||||
* that looks like a path is one ("tests/a.test.mjs + src/b.mjs" names both);
|
||||
* an entry (comma- or line-separated) with no such word gives its first word,
|
||||
* 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) {
|
||||
const spans = [];
|
||||
const masked = String(text ?? '').replace(/`([^`\n]+)`/g, (_, p) => `\u0000${spans.push(p.trim()) - 1}\u0000`);
|
||||
const out = [];
|
||||
for (const raw of String(text ?? '').replace(/\([^()\n]*\)/g, ' ').split(/[,\n]/)) {
|
||||
for (const raw of masked.replace(/\([^()\n]*\)/g, ' ').split(/[,\n]/)) {
|
||||
const entry = raw.trim().replace(/^[-*]\s+/, '');
|
||||
if (entry === '') continue;
|
||||
const spans = [...entry.matchAll(/`([^`\n]+)`/g)].map((m) => m[1].trim());
|
||||
if (spans.length > 0) out.push(...spans);
|
||||
else out.push(entry.split(/\s+/)[0]);
|
||||
const found = [];
|
||||
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(/[.;:]+$/, ''));
|
||||
}
|
||||
out.push(...(found.length > 0 ? found : [entry.split(/\s+/)[0]]));
|
||||
}
|
||||
return out.filter(Boolean);
|
||||
return out.filter((p) => p && !p.includes('\u0000'));
|
||||
}
|
||||
|
||||
// A document is never the test here: `docs/specs/api.md` sits under a `specs/`
|
||||
|
|
|
|||
|
|
@ -102,7 +102,10 @@ export function buildCorpus(entries) {
|
|||
const df = new Map();
|
||||
for (const tf of docs.values()) for (const term of tf.keys()) df.set(term, (df.get(term) || 0) + 1);
|
||||
const n = docs.size;
|
||||
return { docs, idf: (term) => Math.log(1 + n / (1 + (df.get(term) || 0))), entries };
|
||||
const idf = (term) => Math.log(1 + n / (1 + (df.get(term) || 0)));
|
||||
// The corpus never changes: its vectors are built once, not per prompt or pair.
|
||||
const vecs = new Map([...docs].map(([id, tf]) => [id, vec(tf, idf)]));
|
||||
return { docs, idf, vecs, entries };
|
||||
}
|
||||
|
||||
function vec(tf, idf) {
|
||||
|
|
@ -122,7 +125,7 @@ function cosine(a, b) {
|
|||
export function rank(prompt, corpus) {
|
||||
const pv = vec(termFreq(tokenize(prompt)), corpus.idf);
|
||||
const out = [];
|
||||
for (const [id, tf] of corpus.docs) out.push({ id, score: cosine(pv, vec(tf, corpus.idf)) });
|
||||
for (const [id, v] of corpus.vecs) out.push({ id, score: cosine(pv, v) });
|
||||
return out.sort((a, b) => b.score - a.score || a.id.localeCompare(b.id));
|
||||
}
|
||||
|
||||
|
|
@ -132,7 +135,7 @@ export function collisions(corpus, t = THRESHOLDS) {
|
|||
const out = [];
|
||||
for (let i = 0; i < ids.length; i++) {
|
||||
for (let j = i + 1; j < ids.length; j++) {
|
||||
const sim = cosine(vec(corpus.docs.get(ids[i]), corpus.idf), vec(corpus.docs.get(ids[j]), corpus.idf));
|
||||
const sim = cosine(corpus.vecs.get(ids[i]), corpus.vecs.get(ids[j]));
|
||||
if (sim >= t.COLLISION_ERROR) out.push({ a: ids[i], b: ids[j], sim, level: 'error' });
|
||||
else if (sim >= t.COLLISION_WARN) out.push({ a: ids[i], b: ids[j], sim, level: 'warn' });
|
||||
}
|
||||
|
|
@ -205,8 +208,11 @@ export function loadCases(root) {
|
|||
export function checkRouting(root, t = THRESHOLDS) {
|
||||
const entries = loadEntries(root);
|
||||
const corpus = buildCorpus(entries);
|
||||
const byName = new Map(entries.map((e) => [e.name, e]));
|
||||
const skills = entries.filter((e) => e.kind === 'skill');
|
||||
// A skill is looked up among skills only: an agent or command with the same
|
||||
// name must not take its place. An owner may be any kind; a skill wins a tie.
|
||||
const skillByName = new Map(skills.map((e) => [e.name, e]));
|
||||
const byName = new Map([...entries.filter((e) => e.kind !== 'skill'), ...skills].map((e) => [e.name, e]));
|
||||
const cases = loadCases(root);
|
||||
const errors = [];
|
||||
const warnings = [];
|
||||
|
|
@ -220,7 +226,7 @@ export function checkRouting(root, t = THRESHOLDS) {
|
|||
|
||||
for (const c of cases) {
|
||||
if (!c.from) errors.push({ code: 'ROUTING_CASE_NO_SOURCE', case: c.case, message: `${c.case}: tag the event the case came from (from=...)` });
|
||||
const self = c.skill && byName.get(c.skill);
|
||||
const self = c.skill && skillByName.get(c.skill);
|
||||
if (!self || self.kind !== 'skill') {
|
||||
errors.push({ code: 'ROUTING_UNKNOWN_SKILL', case: c.case, message: `${c.case}: skill=${c.skill} is not a skill in skills/` });
|
||||
continue;
|
||||
|
|
|
|||
|
|
@ -99,7 +99,9 @@ const WAY_IN = 'A check the allowlist has no name for (a pipe, `jq`, `grep`, the
|
|||
function checkRunnable(text, sections, opts) {
|
||||
const out = [];
|
||||
const criteria = parsePlanVerification(text);
|
||||
for (const c of criteria) {
|
||||
// opts.stepsOnly: only the given steps are judged; ## Verification is read
|
||||
// for the commands it lists, never reported on (the --steps-from path).
|
||||
for (const c of opts.stepsOnly ? [] : criteria) {
|
||||
const r = runnableCheck(c, opts);
|
||||
if (!r.runnable) {
|
||||
out.push(issue('PLAN_VERIFICATION_NOT_RUNNABLE',
|
||||
|
|
@ -302,7 +304,7 @@ export function validatePlanContent(rawText, opts = {}) {
|
|||
const appended = sections.filter((s) => Number(s.n) >= stepsFrom);
|
||||
const stepIssues = [
|
||||
...checkSteps(appended).filter((i) => i.code !== 'PLAN_NO_END_TO_END'),
|
||||
...checkRunnable('', appended, { cwd: opts.cwd }),
|
||||
...checkRunnable(text, appended, { cwd: opts.cwd, stepsOnly: true }),
|
||||
];
|
||||
for (const s of appended) {
|
||||
if (!parseStepFields(s.body).has('Verify')) {
|
||||
|
|
@ -310,7 +312,8 @@ export function validatePlanContent(rawText, opts = {}) {
|
|||
'An appended step needs one test command: proof of red and the done line run it.'));
|
||||
}
|
||||
}
|
||||
errors.push(...stepIssues);
|
||||
if (strict) errors.push(...stepIssues);
|
||||
else warnings.push(...stepIssues);
|
||||
}
|
||||
|
||||
const ceremony = extractPlanCeremony(text);
|
||||
|
|
|
|||
|
|
@ -268,6 +268,13 @@ const SHELL_OPERATORS = /[;&|<>]|\$\(|`/;
|
|||
|
||||
// Read-only git subcommands: they report, they do not write a repository.
|
||||
const READONLY_GIT = new Set(['status', 'log', 'diff', 'show', 'ls-files']);
|
||||
// A word /bin/sh hands to git unchanged (refs like HEAD~1 and HEAD^, formats like %h, paths).
|
||||
const GIT_LITERAL_WORD = /^[A-Za-z0-9_@%^~:.,=+/-]+$/;
|
||||
// `--output`, `--output=<file>`, or an abbreviation of it git may accept.
|
||||
function writesOutput(word) {
|
||||
const name = word.split('=')[0];
|
||||
return name.length >= 5 && '--output'.startsWith(name);
|
||||
}
|
||||
|
||||
const deny = (reason) => ({ allowed: false, reason });
|
||||
const permit = () => ({ allowed: true, reason: '' });
|
||||
|
|
@ -317,9 +324,15 @@ const RUNNERS = {
|
|||
'/bin/bash': (t) => (isTestScript(t[1]) ? permit() : deny('/bin/bash, but not a script under tests/')),
|
||||
git: (t) => {
|
||||
if (!READONLY_GIT.has(t[1])) return deny(`git, but not a read-only subcommand first (${[...READONLY_GIT].join('|')})`);
|
||||
// `--output` makes diff/log/show write their output to a file (release
|
||||
// review of 6.3.0, 2026-10-10: a plan's check could overwrite any file).
|
||||
if (t.some((w) => w === '--output' || w.startsWith('--output='))) return deny('git with --output writes a file, so it is not a read-only check');
|
||||
// The command runs through /bin/sh, which unquotes and expands what it is
|
||||
// given, so a check on the words here holds only for words the shell passes
|
||||
// through unchanged: no quote, backslash, `$`, glob or brace. Then
|
||||
// `--output` (and an abbreviation of it) is refused: it makes diff/log/show
|
||||
// write to a file (release review of 6.3.0, 2026-10-10: a plan's check
|
||||
// could overwrite any file, first plainly, then through quoting).
|
||||
const odd = t.find((w) => !GIT_LITERAL_WORD.test(w));
|
||||
if (odd !== undefined) return deny(`git with a word the shell would rewrite (${odd}): only plain words are checked`);
|
||||
if (t.some(writesOutput)) return deny('git with --output writes a file, so it is not a read-only check');
|
||||
return permit();
|
||||
},
|
||||
};
|
||||
|
|
|
|||
|
|
@ -10,8 +10,11 @@
|
|||
// (no command, or a command outside the test-runner allowlist): the
|
||||
// step is not done, and that is a finding for the fix rounds
|
||||
// 2 = could not run: bad usage, a plan that cannot be read, no step N
|
||||
// JSON on stdout: {step, status, command, exit_code, output}
|
||||
// JSON on stdout: {step, status, progress_status, command, exit_code, output}
|
||||
// status: passed | failed | blocked | unrunnable | absent | error
|
||||
// progress_status: the value progress.json takes for steps.N.regression
|
||||
// (blocked counts as unrunnable, error as not-run), so the executor
|
||||
// copies one field and never translates
|
||||
//
|
||||
// The command is the field's first backticked span — the same reading as the
|
||||
// runner's `## Verification` bullets — and it goes through the same two
|
||||
|
|
@ -44,6 +47,8 @@ function exec(command, { cwd, timeoutMs }) {
|
|||
|
||||
export const REGRESSION_FIELD = 'Regression';
|
||||
const EXIT = Object.freeze({ passed: 0, absent: 0, failed: 1, blocked: 1, unrunnable: 1, error: 2 });
|
||||
const PROGRESS = Object.freeze({ passed: 'passed', absent: 'absent', failed: 'failed', blocked: 'unrunnable', unrunnable: 'unrunnable', error: 'not-run' });
|
||||
const withProgress = (r) => ({ ...r, progress_status: PROGRESS[r.status] ?? 'not-run' });
|
||||
|
||||
/**
|
||||
* @param {{ planText: string, step: number, cwd?: string, run?: Function }} o
|
||||
|
|
@ -52,19 +57,19 @@ const EXIT = Object.freeze({ passed: 0, absent: 0, failed: 1, blocked: 1, unrunn
|
|||
export function checkRegression({ planText, step, cwd = process.cwd(), run = (c, o) => runCriteria(c, { ...o, exec }) }) {
|
||||
const base = { step, command: null, exit_code: null, output: '' };
|
||||
const found = sliceSteps(planText).find((s) => Number(s.n) === step);
|
||||
if (!found) return { ...base, status: 'error', output: `no "### Step ${step}:" in the plan`, exit: EXIT.error };
|
||||
if (!found) return withProgress({ ...base, status: 'error', output: `no "### Step ${step}:" in the plan`, exit: EXIT.error });
|
||||
const field = parseStepFields(found.body).get(REGRESSION_FIELD);
|
||||
if (!field) return { ...base, status: 'absent', output: 'no Regression: field — nothing to run', exit: EXIT.absent };
|
||||
if (!field) return withProgress({ ...base, status: 'absent', output: 'no Regression: field — nothing to run', exit: EXIT.absent });
|
||||
const { command, reason } = firstCommand(field.text);
|
||||
const [r] = run([{ label: `R${step}`, text: field.text, command, reason }], { cwd });
|
||||
return {
|
||||
return withProgress({
|
||||
step,
|
||||
status: r.status,
|
||||
command: r.command ?? null,
|
||||
exit_code: r.exitCode ?? null,
|
||||
output: r.output ?? '',
|
||||
exit: EXIT[r.status] ?? EXIT.error,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
export function main(argv) {
|
||||
|
|
@ -92,7 +97,7 @@ export function main(argv) {
|
|||
}
|
||||
let planText;
|
||||
try { planText = readFileSync(plan, 'utf8'); } catch (e) {
|
||||
return say({ step, status: 'error', command: null, exit_code: null, output: `cannot read ${plan}: ${e.message}`, exit: 2 });
|
||||
return say(withProgress({ step, status: 'error', command: null, exit_code: null, output: `cannot read ${plan}: ${e.message}`, exit: 2 }));
|
||||
}
|
||||
return say(checkRegression({ planText, step, cwd }));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -131,7 +131,7 @@ export function changedLines(diff) {
|
|||
}
|
||||
|
||||
function checkTree(repo) {
|
||||
const r = git(repo, ['status', '--porcelain']);
|
||||
const r = git(repo, ['status', '--porcelain', '--', '.']);
|
||||
if (r.status !== 0) return { id: 'tree', status: 'error', detail: `git status failed: ${r.stderr.trim()}` };
|
||||
const changed = r.stdout.split('\n').filter(Boolean);
|
||||
if (changed.length > 0) {
|
||||
|
|
@ -177,13 +177,15 @@ function checkCoverage(repo, reviewText) {
|
|||
const anc = git(repo, ['merge-base', '--is-ancestor', full, 'HEAD']);
|
||||
if (anc.status !== 0) return { id: 'coverage', status: 'fail', detail: `the reviewed commit ${full.slice(0, 12)} is not in the history of HEAD` };
|
||||
|
||||
const names = git(repo, ['diff', '--name-only', full, 'HEAD']);
|
||||
// --relative: the paths, and the files that count, are those under the plugin's
|
||||
// own directory, also when it sits in a subdirectory of a larger repo.
|
||||
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));
|
||||
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}`]);
|
||||
const before = git(repo, ['show', `${full}:./${file}`]);
|
||||
const after = git(repo, ['show', `HEAD:./${file}`]);
|
||||
if (before.status !== 0 || after.status !== 0) { outside.push(`${file} (added or removed after the review)`); continue; }
|
||||
let same;
|
||||
try { same = withoutVersion(before.stdout) === withoutVersion(after.stdout); } catch (e) {
|
||||
|
|
@ -192,7 +194,7 @@ function checkCoverage(repo, reviewText) {
|
|||
if (!same) outside.push(`${file} (lines other than the version)`);
|
||||
}
|
||||
if (changed.includes('README.md')) {
|
||||
const d = git(repo, ['diff', '-U0', full, 'HEAD', '--', 'README.md']);
|
||||
const d = git(repo, ['diff', '--relative', '-U0', full, 'HEAD', '--', 'README.md']);
|
||||
if (d.status !== 0) return { id: 'coverage', status: 'error', detail: `git diff of README.md failed: ${d.stderr.trim()} — not measured` };
|
||||
if (changedLines(d.stdout).some((l) => !BADGE_RE.test(l))) outside.push('README.md (lines other than the version badge)');
|
||||
}
|
||||
|
|
@ -235,15 +237,22 @@ function resolveVersion(repo, explicit) {
|
|||
export function releaseCheck({ repo, review, version: explicitVersion }) {
|
||||
const top = git(repo, ['rev-parse', '--show-toplevel']);
|
||||
if (top.error || top.status !== 0) return { exit: EXIT.UNDECIDED, version: null, repo, checks: [], reason: `${repo} is not a git repository` };
|
||||
const root = top.stdout.trim();
|
||||
// The plugin's own directory, not the git top level: a plugin may sit in a
|
||||
// subdirectory of a larger repo (release review of 6.3.0, round 2).
|
||||
const root = resolve(repo);
|
||||
const version = resolveVersion(root, explicitVersion);
|
||||
if (!version) return { exit: EXIT.UNDECIDED, version: null, repo: root, checks: [], reason: 'no version to release: pass --version or set .claude-plugin/plugin.json version' };
|
||||
|
||||
// The cheap checks first: when one of them cannot be measured the answer is
|
||||
// already exit 2, and the suite (minutes) is not run for nothing.
|
||||
const tree = checkTree(root);
|
||||
const suite = checkSuite(root, tree.status === 'pass');
|
||||
const { check: rev, text } = checkReview(review ? resolve(review) : null);
|
||||
const coverage = checkCoverage(root, text);
|
||||
const door = checkDoor(root, version);
|
||||
const undecided = [tree, rev, coverage, door].filter((c) => c.status === 'error');
|
||||
const suite = undecided.length > 0
|
||||
? { id: 'suite', status: 'skip', detail: `not run: ${undecided.map((c) => c.id).join(', ')} could not be measured, so the answer is exit 2 already` }
|
||||
: checkSuite(root, tree.status === 'pass');
|
||||
const checks = [tree, suite, rev, coverage, door];
|
||||
|
||||
let exit = EXIT.READY;
|
||||
|
|
@ -284,10 +293,11 @@ function parseArgs(argv) {
|
|||
const out = { repo: process.cwd(), review: null, version: null, json: false };
|
||||
for (let i = 0; i < argv.length; i++) {
|
||||
const a = argv[i];
|
||||
if (a === '--repo') out.repo = argv[++i];
|
||||
else if (a === '--review') out.review = argv[++i];
|
||||
else if (a === '--version') out.version = argv[++i];
|
||||
else if (a === '--json') out.json = true;
|
||||
if (a === '--repo' || a === '--review' || a === '--version') {
|
||||
const v = argv[++i];
|
||||
if (v === undefined || v.startsWith('--')) throw new Error(`${a} needs a value`);
|
||||
out[a.slice(2)] = v;
|
||||
} else if (a === '--json') out.json = true;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
|
@ -299,8 +309,17 @@ function isMain() {
|
|||
}
|
||||
|
||||
if (isMain()) {
|
||||
const args = parseArgs(process.argv.slice(2));
|
||||
const result = releaseCheck({ repo: resolve(args.repo), review: args.review, version: args.version });
|
||||
process.stdout.write((args.json ? JSON.stringify(result, null, 2) : render(result)) + '\n');
|
||||
// Anything that throws is "could not run" (2), never "not ready" (1): exit 1
|
||||
// promises a list of failed checks, and none ran.
|
||||
let result;
|
||||
let json = process.argv.includes('--json');
|
||||
try {
|
||||
const args = parseArgs(process.argv.slice(2));
|
||||
json = args.json;
|
||||
result = releaseCheck({ repo: resolve(args.repo), review: args.review, version: args.version });
|
||||
} catch (e) {
|
||||
result = { exit: EXIT.UNDECIDED, version: null, repo: null, checks: [], reason: e.message };
|
||||
}
|
||||
process.stdout.write((json ? JSON.stringify(result, null, 2) : render(result)) + '\n');
|
||||
process.exit(result.exit);
|
||||
}
|
||||
|
|
|
|||
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:2867-2889 | 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:2877-2899 | 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 |
|
||||
|
|
|
|||
|
|
@ -16,6 +16,9 @@ import { validatePlanContent } from '../../lib/validators/plan-validator.mjs';
|
|||
import { stepTestsOnly } from '../../lib/parsers/plan-schema.mjs';
|
||||
import { validateFindings } from '../../lib/review/findings-schema.mjs';
|
||||
import { lintSkill } from '../../lib/skills/skill-lint.mjs';
|
||||
import * as routing from '../../lib/skills/routing.mjs';
|
||||
|
||||
const awaitImportSync = () => routing;
|
||||
|
||||
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
|
||||
const read = (rel) => readFileSync(join(ROOT, rel), 'utf8');
|
||||
|
|
@ -164,3 +167,101 @@ test('review 630 / security: `git diff --output=<path>` is not a read-only check
|
|||
assert.match(r.reason, /--output/);
|
||||
}
|
||||
});
|
||||
|
||||
// ==== round 2 of the release review (2026-10-10), each red on 0d044f6 ===========
|
||||
|
||||
test('review 630 r2 / BLOCKER: a quoted, escaped or expanded --output is refused too', async () => {
|
||||
const { allowedCommand } = await import('../../lib/verification/criteria-runner.mjs');
|
||||
for (const cmd of [
|
||||
'git diff "--output=/tmp/x"', "git log -p '--output'=x", "git diff '--output=q1'", 'git log "--output" q1',
|
||||
'git diff --output\\=x', 'git diff --\\output=q1', 'git diff --out{put,x}=y', 'git diff --outp$EMPTY=x',
|
||||
'git diff --outp?t=x', 'git diff --outpu=x', 'git log --out x',
|
||||
]) {
|
||||
assert.equal(allowedCommand(cmd).allowed, false, `${cmd} must not reach the shell`);
|
||||
}
|
||||
for (const cmd of ['git log --oneline -1', 'git diff --stat HEAD~1', 'git show HEAD^:package.json', 'git log --format=%h -1',
|
||||
'git ls-files tests/', 'git log --output-indicator-new=+ -1']) {
|
||||
assert.equal(allowedCommand(cmd).allowed, true, `the control: ${cmd} is a read-only check`);
|
||||
}
|
||||
});
|
||||
|
||||
test('review 630 r2: a Files: entry without backticks names every path in it', () => {
|
||||
const body = '- **Files:** tests/a.test.mjs + src/b.mjs\n- **Changes:** x\n';
|
||||
assert.deepEqual(stepTestsOnly(body), { testsOnly: false, files: ['tests/a.test.mjs', 'src/b.mjs'] });
|
||||
const and = '- **Files:** tests/a.test.mjs and src/b.mjs\n- **Changes:** x\n';
|
||||
assert.equal(stepTestsOnly(and).testsOnly, false);
|
||||
const plain = '- **Files:** Makefile\n- **Changes:** x\n';
|
||||
assert.deepEqual(stepTestsOnly(plain).files, ['Makefile'], 'the control: a bare file name is still a path');
|
||||
});
|
||||
|
||||
test('review 630 r2: a backticked path with parentheses keeps them', () => {
|
||||
const body = '- **Files:** `app/(auth)/login.check.sh` (new)\n- **Changes:** x\n' +
|
||||
'- **Test first:**\n - File: `app/(auth)/login.check.sh` (new)\n - Verifies: y\n';
|
||||
assert.deepEqual(stepTestsOnly(body), { testsOnly: true, files: ['app/(auth)/login.check.sh'] });
|
||||
});
|
||||
|
||||
test('review 630 r2: --steps-from reads the plan\'s ## Verification for an appended End-to-end step', () => {
|
||||
const plan = buildPlan({ version: '1.7', design: false });
|
||||
const e2e = '### Step 3: Run it\n\n- **Files:** `src/run.mjs`\n- **Changes:** wire it\n' +
|
||||
'- **Test first:**\n - File: `tests/run.test.mjs` (new)\n - Verifies: it runs\n' +
|
||||
'- **Verify:** `node --test tests/run.test.mjs` → expected: exit 0\n' +
|
||||
'- **End-to-end:** `bash tests/e2e.sh` → expected: the CLI prints ok\n' +
|
||||
'- **Manifest:**\n ```yaml\n manifest:\n expected_paths:\n - src/run.mjs\n min_file_count: 1\n' +
|
||||
' commit_message_pattern: "^fix"\n bash_syntax_check: []\n forbidden_paths: []\n must_contain: []\n ```\n';
|
||||
const edited = plan.replace('\n## Test Strategy', `\n${e2e}\n## Test Strategy`);
|
||||
assert.match(edited, /- \[ \] `bash tests\/e2e\.sh`/, 'the fixture lists the command under ## Verification');
|
||||
const r = validatePlanContent(edited, { strict: true, stepsFrom: 3 });
|
||||
assert.deepEqual(r.errors.map((e) => e.code), [], JSON.stringify(r.errors));
|
||||
});
|
||||
|
||||
test('review 630 r2: --soft --steps-from warns instead of failing', () => {
|
||||
const plan = withTemplateStep(buildPlan({ version: '1.7', design: false }), 3);
|
||||
const soft = validatePlanContent(plan, { strict: false, stepsFrom: 3 });
|
||||
assert.equal(soft.valid, true, JSON.stringify(soft.errors));
|
||||
assert.ok(soft.warnings.some((w) => /VERIFY_NOT_RUNNABLE|TEST_FIRST/.test(w.code)), 'the issue is still reported');
|
||||
assert.equal(validatePlanContent(plan, { strict: true, stepsFrom: 3 }).valid, false, 'the control: strict fails');
|
||||
});
|
||||
|
||||
test('review 630 r2: regression-check prints the status progress.json takes', async () => {
|
||||
const { checkRegression } = await import('../../lib/verification/regression-check.mjs');
|
||||
const { PROGRESS_REGRESSION_VALUES } = await import('../../lib/validators/progress-validator.mjs');
|
||||
const plan = '## Implementation Plan\n\n### Step 1: x\n\n- **Regression:** `npm test`\n';
|
||||
const blocked = checkRegression({ planText: plan, step: 1, run: () => [{ status: 'blocked', command: 'npm test' }] });
|
||||
assert.equal(blocked.progress_status, 'unrunnable');
|
||||
const missing = checkRegression({ planText: plan, step: 9 });
|
||||
assert.equal(missing.progress_status, 'not-run');
|
||||
for (const r of [blocked, missing]) assert.ok(PROGRESS_REGRESSION_VALUES.includes(r.progress_status));
|
||||
assert.match(read('commands/trekexecute.md'), /`progress_status`/, 'the executor writes that field');
|
||||
});
|
||||
|
||||
test('review 630 r2: a skill is found by its name even when an agent has the same name', () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'r630-route-'));
|
||||
try {
|
||||
for (const d of ['skills', 'commands', 'agents', 'evals']) cpSync(join(ROOT, d), join(dir, d), { recursive: true });
|
||||
writeFileSync(join(dir, 'agents', 'debugging.md'),
|
||||
'---\nname: debugging\ndescription: Bakes sourdough loaves and schedules oven time for the shared kitchen rota.\nmodel: opus\n---\n\nBread.\n');
|
||||
const { checkRouting } = awaitImportSync();
|
||||
const codes = checkRouting(dir).errors.map((e) => e.code);
|
||||
assert.ok(!codes.includes('ROUTING_UNKNOWN_SKILL'), codes.join(','));
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test('review 630 r2: rank() does not rebuild the document vectors per call', () => {
|
||||
const { buildCorpus, rank } = awaitImportSync();
|
||||
const corpus = buildCorpus([
|
||||
{ id: 'a', name: 'alpha', description: 'parses plans and validates steps' },
|
||||
{ id: 'b', name: 'beta', description: 'runs tests and reports failures' },
|
||||
]);
|
||||
let calls = 0;
|
||||
const idf = corpus.idf;
|
||||
corpus.idf = (t) => { calls++; return idf(t); };
|
||||
rank('validates plans', corpus);
|
||||
rank('reports failures', corpus);
|
||||
assert.ok(calls <= 4, `idf was called ${calls} times for two two-word prompts`);
|
||||
});
|
||||
|
||||
test('review 630 r2 (operator 2026-10-10): Phase 7.2 says plainly that template-form steps stop with a re-plan', () => {
|
||||
const cmd = read('commands/trekexecute.md');
|
||||
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/);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -85,55 +85,57 @@ const DOORS = `### Doors
|
|||
function fixture({
|
||||
suiteExit = 0, doors = DOORS, verdict = 'ALLOW', writeReview = true,
|
||||
reviewSha = 'work', afterRelease = null, dirty = false, version = '1.0.1',
|
||||
testScript = null, files = {}, attributes = null,
|
||||
testScript = null, files = {}, attributes = null, sub = null,
|
||||
} = {}) {
|
||||
const base = mkdtempSync(join(tmpdir(), 'release-check-'));
|
||||
const repo = join(base, 'repo');
|
||||
mkdirSync(join(repo, '.claude-plugin'), { recursive: true });
|
||||
// sub: the plugin lives in a subdirectory of a larger repo (release review of 6.3.0, round 2).
|
||||
const dir = sub ? join(repo, sub) : repo;
|
||||
mkdirSync(join(dir, '.claude-plugin'), { recursive: true });
|
||||
git(repo, 'init', '-q', '-b', 'main');
|
||||
git(repo, 'config', 'user.email', 't@example.invalid');
|
||||
git(repo, 'config', 'user.name', 't');
|
||||
git(repo, 'config', 'commit.gpgsign', 'false');
|
||||
|
||||
const pkg = (v) => JSON.stringify({ name: 'fx', version: v, scripts: { test: testScript ?? `node -e "process.exit(${suiteExit})"` } }, null, 2) + '\n';
|
||||
writeFileSync(join(repo, 'package.json'), pkg('1.0.0'));
|
||||
writeFileSync(join(repo, '.claude-plugin', 'plugin.json'), JSON.stringify({ name: 'fx', version: '1.0.0' }) + '\n');
|
||||
writeFileSync(join(repo, 'README.md'), '# fx\n\n\n\nText.\n');
|
||||
writeFileSync(join(repo, 'CHANGELOG.md'), '# Changelog\n\n## v1.0.0 — 2026-10-01 — first\n\n- first\n');
|
||||
writeFileSync(join(repo, 'a.mjs'), 'export const a = 1;\n');
|
||||
for (const [rel, text] of Object.entries(files)) writeFileSync(join(repo, rel), text);
|
||||
writeFileSync(join(dir, 'package.json'), pkg('1.0.0'));
|
||||
writeFileSync(join(dir, '.claude-plugin', 'plugin.json'), JSON.stringify({ name: 'fx', version: '1.0.0' }) + '\n');
|
||||
writeFileSync(join(dir, 'README.md'), '# fx\n\n\n\nText.\n');
|
||||
writeFileSync(join(dir, 'CHANGELOG.md'), '# Changelog\n\n## v1.0.0 — 2026-10-01 — first\n\n- first\n');
|
||||
writeFileSync(join(dir, 'a.mjs'), 'export const a = 1;\n');
|
||||
for (const [rel, text] of Object.entries(files)) writeFileSync(join(dir, rel), text);
|
||||
if (attributes) writeFileSync(join(repo, '.git', 'info', 'attributes'), attributes);
|
||||
git(repo, 'add', '-A');
|
||||
git(repo, 'commit', '-q', '-m', 'feat: first');
|
||||
writeFileSync(join(repo, 'a.mjs'), 'export const a = 2;\n');
|
||||
git(repo, 'add', 'a.mjs');
|
||||
git(repo, 'commit', '-q', '-m', 'fix: the work');
|
||||
const workSha = git(repo, 'rev-parse', 'HEAD');
|
||||
git(dir, 'add', '-A');
|
||||
git(dir, 'commit', '-q', '-m', 'feat: first');
|
||||
writeFileSync(join(dir, 'a.mjs'), 'export const a = 2;\n');
|
||||
git(dir, 'add', 'a.mjs');
|
||||
git(dir, 'commit', '-q', '-m', 'fix: the work');
|
||||
const workSha = git(dir, 'rev-parse', 'HEAD');
|
||||
|
||||
if (version !== null) {
|
||||
writeFileSync(join(repo, 'package.json'), pkg(version));
|
||||
writeFileSync(join(repo, '.claude-plugin', 'plugin.json'), JSON.stringify({ name: 'fx', version }) + '\n');
|
||||
writeFileSync(join(dir, 'package.json'), pkg(version));
|
||||
writeFileSync(join(dir, '.claude-plugin', 'plugin.json'), JSON.stringify({ name: 'fx', version }) + '\n');
|
||||
} else {
|
||||
writeFileSync(join(repo, '.claude-plugin', 'plugin.json'), JSON.stringify({ name: 'fx' }) + '\n');
|
||||
writeFileSync(join(repo, 'package.json'), JSON.stringify({ name: 'fx', scripts: { test: 'node -e "process.exit(0)"' } }) + '\n');
|
||||
writeFileSync(join(dir, '.claude-plugin', 'plugin.json'), JSON.stringify({ name: 'fx' }) + '\n');
|
||||
writeFileSync(join(dir, 'package.json'), JSON.stringify({ name: 'fx', scripts: { test: 'node -e "process.exit(0)"' } }) + '\n');
|
||||
}
|
||||
writeFileSync(join(repo, 'README.md'), `# fx\n\n\n\nText.\n`);
|
||||
writeFileSync(join(repo, 'CHANGELOG.md'),
|
||||
writeFileSync(join(dir, 'README.md'), `# fx\n\n\n\nText.\n`);
|
||||
writeFileSync(join(dir, 'CHANGELOG.md'),
|
||||
`# Changelog\n\n## v${version ?? '9.9.9'} — 2026-10-08 — fixture\n\n${doors ?? ''}\n### Fixed\n\n- the work\n\n## v1.0.0 — 2026-10-01 — first\n\n- first\n`);
|
||||
git(repo, 'add', '-A');
|
||||
git(repo, 'commit', '-q', '-m', 'chore(release): fixture');
|
||||
git(dir, 'add', '-A');
|
||||
git(dir, 'commit', '-q', '-m', 'chore(release): fixture');
|
||||
|
||||
if (afterRelease) {
|
||||
for (const [rel, text] of Object.entries(afterRelease)) writeFileSync(join(repo, rel), text);
|
||||
git(repo, 'add', '-A');
|
||||
git(repo, 'commit', '-q', '-m', 'after the review');
|
||||
for (const [rel, text] of Object.entries(afterRelease)) writeFileSync(join(dir, rel), text);
|
||||
git(dir, 'add', '-A');
|
||||
git(dir, 'commit', '-q', '-m', 'after the review');
|
||||
}
|
||||
if (dirty) writeFileSync(join(repo, 'a.mjs'), 'export const a = 3;\n');
|
||||
if (dirty) writeFileSync(join(dir, 'a.mjs'), 'export const a = 3;\n');
|
||||
|
||||
const review = join(base, 'review.md');
|
||||
const sha = reviewSha === 'work' ? workSha : reviewSha;
|
||||
if (writeReview) writeFileSync(review, reviewText({ verdict, sha }));
|
||||
return { base, repo, review, cleanup: () => rmSync(base, { recursive: true, force: true }) };
|
||||
return { base, repo: dir, review, cleanup: () => rmSync(base, { recursive: true, force: true }) };
|
||||
}
|
||||
|
||||
function run(fx, extra = []) {
|
||||
|
|
@ -360,3 +362,29 @@ test('release-check — a red suite is red when the check itself runs under node
|
|||
assert.match(r.stdout, /npm test exited/);
|
||||
} finally { fx.cleanup(); }
|
||||
});
|
||||
|
||||
// ---- round 2 of the release review (2026-10-10) --------------------------------
|
||||
|
||||
test('release-check — a plugin in a subdirectory of a larger repo is checked there → 0', () => {
|
||||
const fx = fixture({ sub: 'voyage' });
|
||||
try {
|
||||
const r = run(fx);
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
} finally { fx.cleanup(); }
|
||||
});
|
||||
|
||||
test('release-check — no review: the suite is not run, exit 2 at once', () => {
|
||||
const fx = fixture({ writeReview: false, testScript: 'node -e "require(\'fs\').writeFileSync(\'suite-ran\', \'\')"' });
|
||||
try {
|
||||
const r = run(fx);
|
||||
assert.equal(r.status, 2, r.stdout + r.stderr);
|
||||
assert.equal(existsSync(join(fx.repo, 'suite-ran')), false, 'the suite did not run');
|
||||
assert.match(r.stdout, /suite\s+not run/);
|
||||
} finally { fx.cleanup(); }
|
||||
});
|
||||
|
||||
test('release-check — a flag with no value is exit 2, not 1', () => {
|
||||
const r = spawnSync(process.execPath, [CHECK, '--repo'], { encoding: 'utf8' });
|
||||
assert.equal(r.status, 2, r.stdout + r.stderr);
|
||||
assert.match(r.stdout + r.stderr, /not measured|NOT MEASURED/i);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue