fix: close the six defects an independent review found in the trial fixes
Independent read-only review of c56f852..2d29966 (2026-10-08), each defect reproduced by the reviewer; each new test fails on that code. - test-lock green ran ANY allowlisted command: `green --cmd "git status"` marked a red step green and the stop guard let it stop. The lock now stores the step's test command (`lock --cmd`); `green` runs only that one (a different --cmd, or a lock without one, exits 2). trekexecute D1 calls `green` with no command, which also removes the quoting hazard. - plan-gate --accept-open worked at round 1 and passed the gate; it now refuses while rounds are left. - findings-schema --repo-root swallowed a following flag, or took a root that does not exist, and silently skipped the line check: exit 2. - brief-review-stamp hashed only the body, so a changed `framing` or `research_topics` stayed "current"; it now covers the reviewed frontmatter too (only the post-review stamps are left out). - red-proof red_kind read an assertion message naming SyntaxError as a load error; the signatures are now specific. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
2d29966f51
commit
5f9faeed4a
12 changed files with 192 additions and 35 deletions
|
|
@ -190,6 +190,10 @@ export function validateReviewerOutput(rawText, opts = {}) {
|
|||
|
||||
// ---- CLI shim ----------------------------------------------------------------
|
||||
|
||||
function isDir(p) {
|
||||
try { return statSync(p).isDirectory(); } catch { return false; }
|
||||
}
|
||||
|
||||
function isMain() {
|
||||
try {
|
||||
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
|
||||
|
|
@ -205,7 +209,10 @@ if (isMain()) {
|
|||
// Without the flag nothing is skipped (rootIdx + 1 would be 0: the file).
|
||||
const skipIdx = rootIdx >= 0 ? rootIdx + 1 : -1;
|
||||
const filePath = args.find((a, i) => !a.startsWith('--') && i !== skipIdx);
|
||||
if (!filePath || (rootIdx >= 0 && !repoRoot)) {
|
||||
// A missing value, a flag in its place, or a root that is not a directory
|
||||
// would switch the line check off without a word: a usage error instead.
|
||||
const badRoot = rootIdx >= 0 && (!repoRoot || repoRoot.startsWith('--') || !isDir(repoRoot));
|
||||
if (!filePath || badRoot) {
|
||||
process.stderr.write('Usage: findings-schema.mjs [--json] [--repo-root <dir>] <reviewer-output.txt|.md>\n');
|
||||
process.exit(2);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,10 +10,10 @@
|
|||
// test-lock.json), so it can never be committed, and each worktree has its own.
|
||||
//
|
||||
// Usage:
|
||||
// node lib/util/test-lock.mjs lock --files <a,b> --step <N> [--cwd <dir>]
|
||||
// node lib/util/test-lock.mjs lock --files <a,b> --step <N> [--cmd "<test command>"] [--cwd <dir>]
|
||||
// node lib/util/test-lock.mjs unlock [--cwd <dir>]
|
||||
// node lib/util/test-lock.mjs status [--cwd <dir>] (JSON on stdout)
|
||||
// node lib/util/test-lock.mjs green --cmd "<test command>" [--cwd <dir>]
|
||||
// node lib/util/test-lock.mjs green [--cmd "<test command>"] [--cwd <dir>]
|
||||
// node lib/util/test-lock.mjs pending [--cwd <dir>]
|
||||
// Exit: 0 = done, 1 = refused (another step holds the lock; for green: no
|
||||
// lock, or the test command did not exit 0), 2 = could not run (not a
|
||||
|
|
@ -24,7 +24,10 @@
|
|||
// three times while the executor waited for its controller, with the step's
|
||||
// test already green (voyage-lekerepo trial, 2026-10-06). `green` RUNS the
|
||||
// step's test command (the same allowlist and denylist as done-line.mjs) and
|
||||
// records `green_at` only on exit 0; the stop guard then lets the session
|
||||
// records `green_at` only on exit 0. The command is the one stored at `lock
|
||||
// --cmd` time, never one chosen at mark time: an independent review showed
|
||||
// `green --cmd "git status"` marking a red step green. A lock without a
|
||||
// stored command is never marked (exit 2); a --cmd that differs is refused; the stop guard then lets the session
|
||||
// stop, while the write guard keeps refusing the locked files. `pending`
|
||||
// clears the mark when a fix round starts; a new `lock` starts without one.
|
||||
// Paths in --files are relative to the --cwd (default: the current directory).
|
||||
|
|
@ -157,7 +160,7 @@ function main(argv) {
|
|||
const l = cur.lock || {};
|
||||
process.stdout.write(JSON.stringify({
|
||||
locked: cur.state === 'locked', step: l.step ?? null, files: l.files ?? [], session: l.session ?? null,
|
||||
locked_at: l.locked_at ?? null, green_at: l.green_at ?? null, lock_path: cur.path,
|
||||
locked_at: l.locked_at ?? null, cmd: l.cmd ?? null, green_at: l.green_at ?? null, lock_path: cur.path,
|
||||
}, null, 2) + '\n');
|
||||
process.exit(0);
|
||||
}
|
||||
|
|
@ -169,12 +172,14 @@ function main(argv) {
|
|||
if (cmd === 'pending') {
|
||||
delete lock.green_at;
|
||||
} else {
|
||||
if (!args.cmd) fail(2, 'green needs --cmd "<the step\'s test command>"');
|
||||
const allowed = allowedCommand(args.cmd, { cwd: cur.repo.root });
|
||||
const testCmd = typeof lock.cmd === 'string' && lock.cmd.trim() !== '' ? lock.cmd : null;
|
||||
if (!testCmd) fail(2, 'the lock names no test command (lock with --cmd "<Verify>"); never marked green');
|
||||
if (args.cmd !== undefined && args.cmd !== testCmd) fail(2, `--cmd is not the step's test command stored in the lock (${testCmd})`);
|
||||
const allowed = allowedCommand(testCmd, { cwd: cur.repo.root });
|
||||
if (!allowed.allowed) fail(2, `command not run: ${allowed.reason}`);
|
||||
const screen = screenCommand(args.cmd);
|
||||
const screen = screenCommand(testCmd);
|
||||
if (!screen.allowed) fail(2, `command blocked: ${screen.rule}`);
|
||||
const r = spawnSync('/bin/sh', ['-c', args.cmd], {
|
||||
const r = spawnSync('/bin/sh', ['-c', testCmd], {
|
||||
cwd: cur.repo.root, env: commandEnv(), encoding: 'utf8', timeout: DEFAULT_TIMEOUT_MS, maxBuffer: 64 * 1024 * 1024,
|
||||
});
|
||||
if (r.error || r.status === null || r.status === 126 || r.status === 127) {
|
||||
|
|
@ -211,6 +216,10 @@ function main(argv) {
|
|||
session: process.env.CLAUDE_CODE_SESSION_ID || null,
|
||||
locked_at: new Date().toISOString(),
|
||||
};
|
||||
if (args.cmd !== undefined) {
|
||||
if (String(args.cmd).trim() === '') fail(2, '--cmd is empty');
|
||||
lock.cmd = String(args.cmd);
|
||||
}
|
||||
mkdirSync(dirname(cur.path), { recursive: true });
|
||||
writeFileSync(cur.path, JSON.stringify(lock, null, 2) + '\n');
|
||||
writeSessionPointer(lock.session, cur.path);
|
||||
|
|
|
|||
|
|
@ -10,10 +10,13 @@
|
|||
// brief_reviewed_at: "<ISO-8601>"
|
||||
//
|
||||
// /trekplan Phase 4b runs `--check`: `current` skips the second review,
|
||||
// `stale` (the body changed after the review) or `absent` (no stamp: a gate
|
||||
// `stale` (the brief changed after the review) or `absent` (no stamp: a gate
|
||||
// that did not pass, or a brief from another producer) runs it as before.
|
||||
// Frontmatter is outside the hash, so the intent approval stamp written after
|
||||
// the review keeps it current; CRLF and trailing whitespace do not count.
|
||||
// The hash covers the body AND the frontmatter the reviewer judges
|
||||
// (`framing`, `research_topics`, ...): an independent review showed a
|
||||
// body-only hash staying current after `framing` changed. Only the stamps
|
||||
// written after the review are left out (this one and the intent approval
|
||||
// marker); CRLF and trailing whitespace do not count.
|
||||
//
|
||||
// WHAT THE STAMP DOES NOT PROVE: the same trust class as the intent marker
|
||||
// (lib/validators/intent-approval.mjs) — any session that can edit the brief
|
||||
|
|
@ -40,10 +43,15 @@ function split(text) {
|
|||
return { m, front: m[2], body: String(text).slice(m.index + m[0].length) };
|
||||
}
|
||||
|
||||
/** sha256 over the body: CRLF -> LF, trailing whitespace per line and at the end dropped. */
|
||||
export function computeBodyHash(body) {
|
||||
const norm = String(body ?? '').replace(/\r\n?/g, '\n').split('\n').map((l) => l.replace(/\s+$/, '')).join('\n').replace(/\n+$/, '');
|
||||
return `sha256:${createHash('sha256').update(norm, 'utf8').digest('hex')}`;
|
||||
// Frontmatter written AFTER the review; everything else is what was reviewed.
|
||||
const POST_REVIEW_FIELDS = Object.freeze([REVIEWED_HASH_FIELD, REVIEWED_AT_FIELD, 'intent_approved_hash', 'intent_approved_at']);
|
||||
|
||||
const normText = (t) => String(t ?? '').replace(/\r\n?/g, '\n').split('\n').map((l) => l.replace(/\s+$/, '')).join('\n').replace(/\n+$/, '');
|
||||
|
||||
/** sha256 over the reviewed frontmatter + the body: CRLF -> LF, trailing whitespace dropped. */
|
||||
export function computeReviewHash(front, body) {
|
||||
const kept = normText(front).split('\n').filter((l) => !POST_REVIEW_FIELDS.some((f) => l.startsWith(`${f}:`))).join('\n');
|
||||
return `sha256:${createHash('sha256').update(`${kept}\n---\n${normText(body)}`, 'utf8').digest('hex')}`;
|
||||
}
|
||||
|
||||
function fieldValue(front, name) {
|
||||
|
|
@ -56,7 +64,7 @@ function fieldValue(front, name) {
|
|||
export function stampBriefReview(text, now = new Date()) {
|
||||
const parts = split(text);
|
||||
if (!parts) return { stamped: false, text, reason: 'no frontmatter block' };
|
||||
const hash = computeBodyHash(parts.body);
|
||||
const hash = computeReviewHash(parts.front, parts.body);
|
||||
const kept = parts.front.split(/\r?\n/).filter((l) =>
|
||||
!l.startsWith(`${REVIEWED_HASH_FIELD}:`) && !l.startsWith(`${REVIEWED_AT_FIELD}:`));
|
||||
kept.push(`${REVIEWED_HASH_FIELD}: "${hash}"`, `${REVIEWED_AT_FIELD}: "${now.toISOString()}"`);
|
||||
|
|
@ -70,7 +78,7 @@ export function checkBriefReview(text) {
|
|||
const parts = split(text);
|
||||
if (!parts) return { state: 'absent', stamped: null, actual: null };
|
||||
const stamped = fieldValue(parts.front, REVIEWED_HASH_FIELD);
|
||||
const actual = computeBodyHash(parts.body);
|
||||
const actual = computeReviewHash(parts.front, parts.body);
|
||||
if (!stamped) return { state: 'absent', stamped: null, actual };
|
||||
return { state: stamped === actual ? 'current' : 'stale', stamped, actual };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -25,8 +25,8 @@
|
|||
// After the operator's explicit yes to "build as is": records the open
|
||||
// MAJORs of the current round as accepted (`accepted_open` in the verdict
|
||||
// block, bound to the round and the plan hash). 0 = written; 1 = refused
|
||||
// (a BLOCKER is open, no MAJOR is open, or the verdict is stale); 2 = could
|
||||
// not run.
|
||||
// (rounds left, a BLOCKER is open, no MAJOR is open, or the verdict is
|
||||
// stale); 2 = could not run.
|
||||
//
|
||||
// Open MAJORs (operator decision 2026-10-08, voyage-lekerepo trial point 6):
|
||||
// they used to pass as warnings, and after the last round there was no way to
|
||||
|
|
@ -271,6 +271,10 @@ export function acceptOpen(rawText, opts = {}) {
|
|||
if (v.findings.some(f => sevOf(f) !== 'major' && sevOf(f) !== 'minor')) return { code: 1, message: 'a BLOCKER is open; a blocker is never accepted' };
|
||||
const majors = v.findings.filter(f => sevOf(f) === 'major');
|
||||
if (majors.length === 0) return { code: 1, message: 'no MAJOR is open; nothing to accept' };
|
||||
// Only the last round: with rounds left the answer is to revise, not accept
|
||||
// (found by the independent review: acceptance at round 1 passed the gate).
|
||||
const left = v.max_rounds + (v.extra_rounds ?? 0) - v.round;
|
||||
if (left > 0) return { code: 1, message: `${left} critic round(s) left: revise the plan and run the next round; acceptance is only for after the last round` };
|
||||
const accepted = {
|
||||
...v,
|
||||
accepted_open: {
|
||||
|
|
|
|||
|
|
@ -84,7 +84,10 @@ export function repoRoot(dir) {
|
|||
// missing — but the result says which kind, in a field (`red_kind`), instead
|
||||
// of leaving it to a reader of `tap` (null without a TAP reporter: the
|
||||
// voyage-lekerepo trial's step 1, 2026-10-06).
|
||||
const LOAD_ERROR = /SyntaxError|ERR_MODULE_NOT_FOUND|does not provide an export named|Cannot find module|ERR_REQUIRE_ESM|ModuleNotFoundError|ImportError/;
|
||||
// Specific signatures only: a bare `SyntaxError` anywhere also matched an
|
||||
// assertion message («Missing expected exception (SyntaxError)») of a named
|
||||
// failing test (independent review, 2026-10-08).
|
||||
const LOAD_ERROR = /^\s*SyntaxError: |ERR_MODULE_NOT_FOUND|does not provide an export named|Cannot find module|ERR_REQUIRE_ESM|ModuleNotFoundError|ImportError: /m;
|
||||
|
||||
function run(cmd, cwd, timeoutMs) {
|
||||
const r = spawnSync('/bin/sh', ['-c', cmd], { cwd, env: commandEnv(), encoding: 'utf8', timeout: timeoutMs, maxBuffer: 64 * 1024 * 1024 });
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue