fix: the fix round before 6.3.0 (order 10)
1. A step of only tests can never be proven red. One reading, stepTestsOnly (lib/parsers/plan-schema.mjs, test paths from the new lib/util/test-path.mjs): the plan validator warns PLAN_STEP_TESTS_ONLY, red-proof --plan/--step adds tests_only to a not-red-at-base, and /trekexecute and the step helper send such a step straight up to the plan instead of into fix rounds that cannot succeed. /trekplan, the plan template and plan-critic (dimension 8, still 12) say the test belongs in the step that builds the behaviour. 2. Timing tests under load: the hook helper's kill limit is 30 s and per call overridable; SC #14 passes on the fastest of up to three runs against the same 800 ms limit (tests/helpers/timing.mjs). 3. The example plan's On failure lines open with a K11 word. 4. /trekreview saves the reviewer's whole answer for the findings check and names FINDING_PROSE_LINE_OUT_OF_RANGE (error: re-ask; warning: report). 5. The criteria runner's commands run without NODE_TEST_CONTEXT (commandEnv, moved to lib/util/command-env.mjs). 6. The resolver returns effort only when the brief's phase signal sets one; the four commands said it always did. Also: the K2 hook tests used fixed session ids in a per-user temp dir, so two suites running at once took each other's approval records; the ids are now unique per run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
19ff772773
commit
33b58f460a
25 changed files with 680 additions and 74 deletions
|
|
@ -12,6 +12,7 @@
|
|||
|
||||
import { ok, fail, issue } from '../util/result.mjs';
|
||||
import { parseDocument } from '../util/frontmatter.mjs';
|
||||
import { isTestPath } from '../util/test-path.mjs';
|
||||
|
||||
export const STEP_HEADING_REGEX = /^### Step (\d+):\s+(.+?)\s*$/m;
|
||||
export const STEP_HEADING_GLOBAL = /^### Step (\d+):\s+(.+?)\s*$/gm;
|
||||
|
|
@ -281,6 +282,52 @@ export function onFailureAction(text) {
|
|||
return m && ON_FAILURE_ACTIONS.includes(m[1]) ? m[1] : null;
|
||||
}
|
||||
|
||||
// ---- 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).
|
||||
*/
|
||||
function fieldPaths(text) {
|
||||
const out = [];
|
||||
for (const raw of String(text ?? '').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]);
|
||||
}
|
||||
return out.filter(Boolean);
|
||||
}
|
||||
|
||||
// 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.
|
||||
const DOC_RE = /\.(md|markdown|mdx|txt|rst|adoc)$/i;
|
||||
|
||||
/**
|
||||
* Whether every file in a step's `Files:` is a test: the step's own `Test
|
||||
* first:` file, or a path that is clearly a test (lib/util/test-path.mjs). Such
|
||||
* a step can never be proven red - the behaviour its tests cover already
|
||||
* exists at its start point (an earlier step built it, or it was there before
|
||||
* the plan), so they already pass there (voyage-lekerepo
|
||||
* trial 3, 2026-10-09: `not-red-at-base`, stop class 4). The plan validator
|
||||
* warns on it (PLAN_STEP_TESTS_ONLY) and red-proof.mjs reports it
|
||||
* (`tests_only`), so the executor sends the step up to the plan. A step
|
||||
* without `Files:` is not such a step.
|
||||
* @param {string} body - the step's section (sliceSteps)
|
||||
* @returns {{testsOnly: boolean, files: string[]}}
|
||||
*/
|
||||
export function stepTestsOnly(body) {
|
||||
const f = parseStepFields(body);
|
||||
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())
|
||||
: [];
|
||||
const isTest = (p) => testFirst.includes(p) || (isTestPath(p) && !DOC_RE.test(p));
|
||||
return { testsOnly: files.length > 0 && files.every(isTest), files };
|
||||
}
|
||||
|
||||
// ---- sections (fence-aware) ------------------------------------------------
|
||||
|
||||
/**
|
||||
|
|
|
|||
12
lib/util/command-env.mjs
Normal file
12
lib/util/command-env.mjs
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
// lib/util/command-env.mjs
|
||||
// The environment a check command runs in. The command's exit code must be its
|
||||
// own: under `node --test` a child inherits NODE_TEST_CONTEXT, and a nested
|
||||
// `node --test` then reports to that parent and exits 0 whatever its tests did
|
||||
// (measured 2026-10-06: a red start point read as green). So the variable
|
||||
// never reaches the command. Used by red-proof.mjs, regression-check.mjs and
|
||||
// the criteria runner's default exec.
|
||||
|
||||
export function commandEnv(env = process.env) {
|
||||
const { NODE_TEST_CONTEXT, ...rest } = env;
|
||||
return rest;
|
||||
}
|
||||
15
lib/util/test-path.mjs
Normal file
15
lib/util/test-path.mjs
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
// lib/util/test-path.mjs
|
||||
// What counts as a test file, by its path. One reading for every caller: the
|
||||
// floor guard (test-integrity.mjs) reads a diff with it, and the plan reader
|
||||
// (plan-schema.mjs `stepTestsOnly`) asks whether a step holds nothing but tests.
|
||||
|
||||
const TEST_PATH_RE = [
|
||||
/(^|\/)(tests?|__tests__|spec|specs)\//,
|
||||
/\.(test|spec)\.[a-z0-9]+$/i,
|
||||
/_(test|spec)\.[a-z0-9]+$/i,
|
||||
/(^|\/)test_[^/]*\.py$/,
|
||||
];
|
||||
|
||||
export function isTestPath(p) {
|
||||
return TEST_PATH_RE.some((re) => re.test(p));
|
||||
}
|
||||
|
|
@ -30,6 +30,9 @@
|
|||
// can never run is an error (a warning under --soft).
|
||||
// - K11: `On failure:` opens with revert | retry | skip | escalate, the words
|
||||
// /trekexecute reads; any other opening is a warning.
|
||||
// - A step whose `Files:` are all tests (plan-schema `stepTestsOnly`) is the
|
||||
// warning PLAN_STEP_TESTS_ONLY: its tests pass at its start point, so it can
|
||||
// never be proven red (voyage-lekerepo trial 3, 2026-10-09).
|
||||
|
||||
import { readFileSync, existsSync, realpathSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
|
@ -37,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,
|
||||
ON_FAILURE_ACTIONS, onFailureAction, stepTestsOnly,
|
||||
} from '../parsers/plan-schema.mjs';
|
||||
import { validateAllManifests } from '../parsers/manifest-yaml.mjs';
|
||||
import { parseDocument } from '../util/frontmatter.mjs';
|
||||
|
|
@ -160,6 +163,15 @@ 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) {
|
||||
warnings.push(issue('PLAN_STEP_TESTS_ONLY',
|
||||
`Step ${s.n}: every file in \`Files:\` is a test; a step of only tests can never be proven red`,
|
||||
'The test for a behaviour belongs in the step that builds it: at this step\'s start point the behaviour ' +
|
||||
'already exists (an earlier step built it, or it was there before the plan), so its test passes there and ' +
|
||||
'red proof rejects it (`not-red-at-base`). Move the test into the step that builds the behaviour; a test ' +
|
||||
'meant to pass from the start (a golden or characterization test) is the `Regression:` of the step that ' +
|
||||
'changes the code, not a step of its own.'));
|
||||
}
|
||||
}
|
||||
return { errors, warnings };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ import { resolve, dirname, join } from 'node:path';
|
|||
import { fileURLToPath } from 'node:url';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { sliceSteps, parseStepFields } from '../parsers/plan-schema.mjs';
|
||||
import { commandEnv } from '../util/command-env.mjs';
|
||||
|
||||
const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..');
|
||||
|
||||
|
|
@ -359,8 +360,11 @@ function screenShape(command, opts, writeTime) {
|
|||
|
||||
// --- running ----------------------------------------------------------------
|
||||
|
||||
// Without NODE_TEST_CONTEXT (lib/util/command-env.mjs): started under
|
||||
// `node --test`, a nested `node --test` criterion reported to the parent and
|
||||
// exited 0 on a red test - a false green (order 10, 2026-10-09).
|
||||
function defaultExec(command, { cwd, timeoutMs }) {
|
||||
const r = spawnSync('/bin/sh', ['-c', command], { cwd, encoding: 'utf8', timeout: timeoutMs });
|
||||
const r = spawnSync('/bin/sh', ['-c', command], { cwd, encoding: 'utf8', timeout: timeoutMs, env: commandEnv() });
|
||||
return { status: r.status, stdout: r.stdout ?? '', stderr: r.stderr ?? '' };
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -14,6 +14,10 @@
|
|||
// builds (and locks the test). The executor's own check afterwards is the full
|
||||
// run, and only the full run is authoritative.
|
||||
//
|
||||
// `--plan <plan.md> --step <N>` adds `tests_only` to a `not-red-at-base`: true
|
||||
// when every file in the step's `Files:` is a test, so the step can never be
|
||||
// red and goes up to the plan (see `testsOnly` below).
|
||||
//
|
||||
// Exit 0 = proven, 1 = rejected (`not-red-at-base` or `not-green-at-result`),
|
||||
// 2 = could not run (`not-run`: no test files, an unknown start point, a command
|
||||
// outside the test-runner allowlist, a command that cannot start, a control run
|
||||
|
|
@ -49,12 +53,14 @@
|
|||
// Borrowed: GitHub Spec Kit (MIT, github/spec-kit@2e941fe) - its test-first
|
||||
// rule is an instruction there; here a command proves it.
|
||||
|
||||
import { cpSync, existsSync, lstatSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync } from 'node:fs';
|
||||
import { cpSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync } from 'node:fs';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { dirname, isAbsolute, join, resolve } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { allowedCommand, screenCommand } from './criteria-runner.mjs';
|
||||
import { commandEnv } from '../util/command-env.mjs';
|
||||
import { sliceSteps, stepTestsOnly } from '../parsers/plan-schema.mjs';
|
||||
|
||||
export const DEFAULT_TIMEOUT_MS = 900_000;
|
||||
const TAIL = 2000;
|
||||
|
|
@ -65,14 +71,9 @@ function tap(output) {
|
|||
return tests && fail ? { tests: Number(tests[1]), fail: Number(fail[1]) } : null;
|
||||
}
|
||||
|
||||
// The command's exit code must be its own. Under `node --test` a child inherits
|
||||
// NODE_TEST_CONTEXT, and a nested `node --test` then reports to that parent and
|
||||
// exits 0 whatever its tests did (measured 2026-10-06: a red start point read as
|
||||
// green). So the variable never reaches the command.
|
||||
export function commandEnv(env = process.env) {
|
||||
const { NODE_TEST_CONTEXT, ...rest } = env;
|
||||
return rest;
|
||||
}
|
||||
// The command's exit code must be its own: NODE_TEST_CONTEXT never reaches it
|
||||
// (lib/util/command-env.mjs).
|
||||
export { commandEnv };
|
||||
|
||||
export function repoRoot(dir) {
|
||||
const top = spawnSync('git', ['rev-parse', '--show-toplevel'], { cwd: dir, encoding: 'utf8' });
|
||||
|
|
@ -100,6 +101,23 @@ const redKind = (base) => (base.loadError ? 'load-error' : 'test-failure');
|
|||
|
||||
const notRun = (reason, extra = {}) => ({ status: 'not-run', reason, ...extra });
|
||||
|
||||
// With `--plan` and `--step`, a `not-red-at-base` also says whether the step
|
||||
// holds nothing but tests (plan-schema `stepTestsOnly`, the plan validator's
|
||||
// PLAN_STEP_TESTS_ONLY). Such a step can never be red: the behaviour its tests
|
||||
// cover already exists at its start point. Rewriting the test cannot fix that, so
|
||||
// the executor sends the step up to the plan instead of into fix rounds
|
||||
// (voyage-lekerepo trial 3, 2026-10-09). `null` = the plan or the step could
|
||||
// not be read; without `--plan` the field is absent and nothing new happens.
|
||||
function testsOnly(o) {
|
||||
if (!o.plan) return {};
|
||||
let text;
|
||||
try { text = readFileSync(o.plan, 'utf8'); }
|
||||
catch (e) { return { tests_only: null, tests_only_note: `cannot read the plan: ${e.message}` }; }
|
||||
const step = sliceSteps(text).find((s) => Number(s.n) === Number(o.step));
|
||||
if (!step) return { tests_only: null, tests_only_note: `no "### Step ${o.step}:" in the plan` };
|
||||
return { tests_only: stepTestsOnly(step.body).testsOnly };
|
||||
}
|
||||
|
||||
// A shell that could not find or execute the command (126/127), a signal (the
|
||||
// timeout), or a spawn error is not a test verdict at all.
|
||||
function unrunnable(r) {
|
||||
|
|
@ -153,7 +171,7 @@ function applyResult(root, tree, base) {
|
|||
}
|
||||
|
||||
/**
|
||||
* @param {{cwd: string, base: string, cmd: string, tests: string[], link?: string[], redOnly?: boolean, timeoutMs?: number}} o
|
||||
* @param {{cwd: string, base: string, cmd: string, tests: string[], link?: string[], redOnly?: boolean, timeoutMs?: number, plan?: string, step?: number}} o
|
||||
* @returns {{status: 'proven'|'rejected'|'not-run', reason: string, base?: object, result?: object, control?: object}}
|
||||
*/
|
||||
export function proveRed(o) {
|
||||
|
|
@ -200,7 +218,7 @@ export function proveRed(o) {
|
|||
const baseBroken = unrunnable(base);
|
||||
if (baseBroken) return notRun(`at the start point: ${baseBroken}`, { base, result, linked });
|
||||
const out = { base_sha: baseSha, linked, base, result };
|
||||
if (base.exit === 0) return { status: 'rejected', reason: 'not-red-at-base', ...out };
|
||||
if (base.exit === 0) return { status: 'rejected', reason: 'not-red-at-base', ...testsOnly(o), ...out };
|
||||
if (o.redOnly) return { status: 'proven', reason: 'red at the start point (--red-only: the result was not run)', red_kind: redKind(base), base_sha: baseSha, linked, base };
|
||||
if (result.exit !== 0) return { status: 'rejected', reason: 'not-green-at-result', ...out };
|
||||
|
||||
|
|
@ -220,7 +238,7 @@ export function proveRed(o) {
|
|||
// --- CLI ------------------------------------------------------------------------
|
||||
|
||||
const USAGE =
|
||||
'usage: red-proof.mjs --base <start_sha> --cmd "<test command>" --tests <a,b> [--red-only] [--cwd <dir>] [--link <dir,...>] [--timeout <ms>] [--json]';
|
||||
'usage: red-proof.mjs --base <start_sha> --cmd "<test command>" --tests <a,b> [--plan <plan.md> --step <N>] [--red-only] [--cwd <dir>] [--link <dir,...>] [--timeout <ms>] [--json]';
|
||||
|
||||
const list = (v) => String(v).split(',').map((s) => s.trim()).filter(Boolean);
|
||||
|
||||
|
|
@ -234,12 +252,16 @@ export function main(argv) {
|
|||
else if (a === '--tests' && argv[i + 1]) o.tests = list(argv[++i]);
|
||||
else if (a === '--cwd' && argv[i + 1]) o.cwd = resolve(argv[++i]);
|
||||
else if (a === '--link' && argv[i + 1]) o.link = list(argv[++i]);
|
||||
else if (a === '--plan' && argv[i + 1]) o.plan = resolve(argv[++i]);
|
||||
else if (a === '--step' && argv[i + 1]) o.step = Number(argv[++i]);
|
||||
else if (a === '--red-only') o.redOnly = true;
|
||||
else if (a === '--timeout' && argv[i + 1]) o.timeoutMs = Number(argv[++i]);
|
||||
else if (a === '--json') json = true;
|
||||
else { process.stderr.write(`red-proof: unknown argument ${a} (--tests takes ONE comma-separated list)\n${USAGE}\n`); return 2; }
|
||||
}
|
||||
if (!o.base || !o.cmd) { process.stderr.write(`red-proof: --base and --cmd are required\n${USAGE}\n`); return 2; }
|
||||
if (o.plan && !(Number.isInteger(o.step) && o.step > 0)) { process.stderr.write(`red-proof: --plan needs --step <N>\n${USAGE}\n`); return 2; }
|
||||
if (o.step !== undefined && !o.plan) { process.stderr.write(`red-proof: --step needs --plan <plan.md>\n${USAGE}\n`); return 2; }
|
||||
if (o.timeoutMs !== undefined && !Number.isFinite(o.timeoutMs)) { process.stderr.write(`red-proof: --timeout must be a number\n${USAGE}\n`); return 2; }
|
||||
let r;
|
||||
try { r = proveRed(o); }
|
||||
|
|
@ -248,6 +270,7 @@ export function main(argv) {
|
|||
else {
|
||||
const counts = (x) => (x?.tap ? ` (tap: ${x.tap.fail} of ${x.tap.tests} failed)` : '');
|
||||
process.stdout.write(`red proof: ${r.status.toUpperCase()} - ${r.reason}${r.red_kind ? ` (red: ${r.red_kind})` : ''}\n`);
|
||||
if (r.tests_only === true) process.stdout.write(' tests_only: true - every file in the step\'s Files: is a test, so it can never be red: up to the plan\n');
|
||||
if (r.base) process.stdout.write(` start point ${r.base_sha ?? o.base}: exit ${r.base.exit}${counts(r.base)}\n`);
|
||||
if (r.result) process.stdout.write(` result: exit ${r.result.exit}${counts(r.result)}\n`);
|
||||
if (r.control) process.stdout.write(` control: exit ${r.control.exit}${counts(r.control)}\n`);
|
||||
|
|
|
|||
|
|
@ -27,23 +27,16 @@ import { readFileSync, realpathSync } from 'node:fs';
|
|||
import { spawnSync } from 'node:child_process';
|
||||
import { resolve } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { isTestPath } from '../util/test-path.mjs';
|
||||
|
||||
export { isTestPath };
|
||||
|
||||
// --- what counts as a test / a check config ----------------------------------
|
||||
|
||||
const TEST_PATH_RE = [
|
||||
/(^|\/)(tests?|__tests__|spec|specs)\//,
|
||||
/\.(test|spec)\.[a-z0-9]+$/i,
|
||||
/_(test|spec)\.[a-z0-9]+$/i,
|
||||
/(^|\/)test_[^/]*\.py$/,
|
||||
];
|
||||
// A test path is lib/util/test-path.mjs (shared with the plan reader).
|
||||
|
||||
const CHECK_CONFIG_RE =
|
||||
/(^|\/)(\.github|\.forgejo|\.gitea|\.woodpecker)\/|(^|\/)(\.gitlab-ci\.yml|Makefile|package\.json|pyproject\.toml|setup\.cfg|tox\.ini|\.pre-commit-config\.yaml|tsconfig[^/]*\.json|\.eslintrc[^/]*|eslint\.config\.[a-z]+|(jest|vitest|playwright)\.config\.[a-z]+|\.c8rc[^/]*|\.nycrc[^/]*|codecov\.ya?ml)$/;
|
||||
|
||||
export function isTestPath(p) {
|
||||
return TEST_PATH_RE.some((re) => re.test(p));
|
||||
}
|
||||
|
||||
const isCheckPath = (p) => isTestPath(p) || CHECK_CONFIG_RE.test(p);
|
||||
const PROSE_RE = /\.(md|markdown|mdx|txt|rst|adoc)$/i;
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue