refactor: simplify pass (trekexecute)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-26 20:39:40 +01:00
commit 52c382c198
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
8 changed files with 45 additions and 52 deletions

View file

@ -270,13 +270,12 @@ const commandView = commandPositionView(deobfuscated.replace(/\x1B\[[0-9;]*m/g,
// only when exactly one line, trimmed, equals the delimiter and every line // only when exactly one line, trimmed, equals the delimiter and every line
// after it is empty. // after it is empty.
const ARGS_DELIM = 'VOYAGE_ARGS_EOF_7F3C9E'; const ARGS_DELIM = 'VOYAGE_ARGS_EOF_7F3C9E';
const argsOpener = /<<-?\s*(['"\\]?)VOYAGE_ARGS_EOF_7F3C9E(['"]?)/.exec(command); const argsOpener = new RegExp(`<<-?\\s*(['"\\\\]?)${ARGS_DELIM}(['"]?)`).exec(command);
if (argsOpener) { if (argsOpener) {
const [, open, close] = argsOpener; const [, open, close] = argsOpener;
const quoted = (open === '\\' && close === '') || (open !== '' && open !== '\\' && close === open); const quoted = open === '\\' ? close === '' : open !== '' && close === open;
const lines = command.split('\n'); const lines = command.split('\n');
const closing = []; const closing = lines.flatMap((l, i) => (l.trim() === ARGS_DELIM ? [i] : []));
lines.forEach((line, i) => { if (line.trim() === ARGS_DELIM) closing.push(i); });
const closesOnceAtEnd = closing.length === 1 && const closesOnceAtEnd = closing.length === 1 &&
lines.slice(closing[0] + 1).every((line) => line.trim() === ''); lines.slice(closing[0] + 1).every((line) => line.trim() === '');
if (!quoted || !closesOnceAtEnd) { if (!quoted || !closesOnceAtEnd) {

View file

@ -86,12 +86,20 @@ function gitFailed(what, r) {
const short = (c) => c.slice(0, 12); const short = (c) => c.slice(0, 12);
/** Where the managed block's start marker (`s`) and end marker (`e`) are; null when either is missing. */
function blockBounds(text) {
const s = text.indexOf(MARK_START);
if (s === -1) return null;
const e = text.indexOf(MARK_END, s + MARK_START.length);
if (e === -1) return null;
return { s, e };
}
/** The lessons text with its managed block (start marker to end marker, plus its newline) taken out. */ /** The lessons text with its managed block (start marker to end marker, plus its newline) taken out. */
function outsideBlock(text) { function outsideBlock(text) {
const s = text.indexOf(MARK_START); const b = blockBounds(text);
if (s === -1) return text; if (!b) return text;
const e = text.indexOf(MARK_END, s + MARK_START.length); const { s, e } = b;
if (e === -1) return text;
let end = e + MARK_END.length; let end = e + MARK_END.length;
if (text.startsWith('\r\n', end)) end += 2; if (text.startsWith('\r\n', end)) end += 2;
else if (text[end] === '\n') end += 1; else if (text[end] === '\n') end += 1;
@ -100,11 +108,9 @@ function outsideBlock(text) {
/** The non-blank lines inside the managed block (the same block `outsideBlock` takes out); none without one. */ /** The non-blank lines inside the managed block (the same block `outsideBlock` takes out); none without one. */
function blockLines(text) { function blockLines(text) {
const s = text.indexOf(MARK_START); const b = blockBounds(text);
if (s === -1) return []; if (!b) return [];
const e = text.indexOf(MARK_END, s + MARK_START.length); return text.slice(b.s + MARK_START.length, b.e).split(/\r?\n/).filter((l) => l.trim() !== '');
if (e === -1) return [];
return text.slice(s + MARK_START.length, e).split(/\r?\n/).filter((l) => l.trim() !== '');
} }
/** /**

View file

@ -98,8 +98,10 @@ function headIsUnborn(root) {
if (sym.error || sym.status !== 0) return false; if (sym.error || sym.status !== 0) return false;
const ref = String(sym.stdout ?? '').trim(); const ref = String(sym.stdout ?? '').trim();
if (!/^refs\/heads\/./.test(ref)) return false; if (!/^refs\/heads\/./.test(ref)) return false;
if (lexists(resolve(root, git(root, ['rev-parse', '--git-path', ref]).trim()))) return false; // One call, one output line per --git-path, in argument order.
const packed = resolve(root, git(root, ['rev-parse', '--git-path', 'packed-refs']).trim()); const [loosePath = '', packedPath = ''] = git(root, ['rev-parse', '--git-path', ref, '--git-path', 'packed-refs']).split('\n');
if (lexists(resolve(root, loosePath.trim()))) return false;
const packed = resolve(root, packedPath.trim());
if (lexists(packed)) { if (lexists(packed)) {
const lines = readFileSync(packed, 'utf8').split('\n'); const lines = readFileSync(packed, 'utf8').split('\n');
if (lines.some((l) => l.endsWith(` ${ref}`))) return false; if (lines.some((l) => l.endsWith(` ${ref}`))) return false;

View file

@ -36,7 +36,7 @@ import { FULL_SHA_RE } from '../review/reviewed-commit.mjs';
*/ */
export function doneLineRe(step, commit) { export function doneLineRe(step, commit) {
const sha = commit === undefined || commit === null || commit === '' const sha = commit === undefined || commit === null || commit === ''
? '(?:[0-9a-f]{40}|[0-9a-f]{64})' ? FULL_SHA_RE.source.slice(1, -1) // the same 40-or-64 alternation, without its ^ and $
: FULL_SHA_RE.test(String(commit)) ? String(commit).toLowerCase() : '(?!)'; : FULL_SHA_RE.test(String(commit)) ? String(commit).toLowerCase() : '(?!)';
return new RegExp(`^DONE step ${step}: \`.+\` exit 0 at ${sha} \\d{4}-\\d{2}-\\d{2}T\\S+$`, 'm'); return new RegExp(`^DONE step ${step}: \`.+\` exit 0 at ${sha} \\d{4}-\\d{2}-\\d{2}T\\S+$`, 'm');
} }

View file

@ -40,7 +40,7 @@ import { spawnSync } from 'node:child_process';
import { dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { dirname, isAbsolute, join, relative, resolve } from 'node:path';
import { fileURLToPath } from 'node:url'; import { fileURLToPath } from 'node:url';
import { decideVerdict } from '../lib/review/verdict-gate.mjs'; import { decideVerdict } from '../lib/review/verdict-gate.mjs';
import { FULL_SHA_RE, reviewedSha } from '../lib/review/reviewed-commit.mjs'; import { reviewedSha } from '../lib/review/reviewed-commit.mjs';
import { commandEnv } from '../lib/util/command-env.mjs'; import { commandEnv } from '../lib/util/command-env.mjs';
import { DEFAULT_TIMEOUT_MS, stoppedBy } from '../lib/verification/criteria-runner.mjs'; import { DEFAULT_TIMEOUT_MS, stoppedBy } from '../lib/verification/criteria-runner.mjs';
import { isMain } from '../lib/util/is-main.mjs'; import { isMain } from '../lib/util/is-main.mjs';
@ -224,7 +224,7 @@ function checkCoverage(repo, reviewText) {
const sha = reviewedSha(reviewText); const sha = reviewedSha(reviewText);
// A full id only (M11): a short id, or a 64-hex name in a SHA-1 repository, // A full id only (M11): a short id, or a 64-hex name in a SHA-1 repository,
// can be a branch name that resolves to wherever the branch points now. // can be a branch name that resolves to wherever the branch points now.
if (!sha || !FULL_SHA_RE.test(sha)) return { id: 'coverage', status: 'fail', detail: 'review.md has no scope_sha_end that is a commit sha (a full 40- or 64-hex commit id) — cannot tell what it reviewed' }; if (!sha) return { id: 'coverage', status: 'fail', detail: 'review.md has no scope_sha_end that is a commit sha (a full 40- or 64-hex commit id) — cannot tell what it reviewed' };
const resolved = git(repo, ['rev-parse', '--verify', '--quiet', `${sha}^{commit}`]); 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)` }; if (resolved.status !== 0) return { id: 'coverage', status: 'fail', detail: `the reviewed commit ${sha} is not in the history of HEAD (unknown commit)` };
const full = resolved.stdout.trim(); const full = resolved.stdout.trim();

View file

@ -68,10 +68,13 @@ const DIFF = [
'', '',
].join('\n'); ].join('\n');
/** The CLI run with `--cwd <cwd>` (the repo by default) on `diff` written into the repo's git dir. */ /**
function run(dir, diff = DIFF, cwd = dir) { * The CLI run with `--cwd <cwd>` (the repo by default) on `diff` written into the repo's git dir.
* With `env`, the child gets that environment; without it, it inherits this process's.
*/
function run(dir, diff = DIFF, cwd = dir, env) {
writeFileSync(join(dir, '.git', 'upward.diff'), diff); writeFileSync(join(dir, '.git', 'upward.diff'), diff);
return spawnSync(process.execPath, [CLI, '--diff', join(dir, '.git', 'upward.diff'), '--step', '1', '--attempt', '1', '--cwd', cwd], { encoding: 'utf8' }); return spawnSync(process.execPath, [CLI, '--diff', join(dir, '.git', 'upward.diff'), '--step', '1', '--attempt', '1', '--cwd', cwd], { encoding: 'utf8', ...(env ? { env } : {}) });
} }
// Parked outside the work tree, in the git dir, so a later `git ls-files --others` never lists them. // Parked outside the work tree, in the git dir, so a later `git ls-files --others` never lists them.
@ -399,8 +402,7 @@ test('[M7] a failing HEAD read exits 2 and moves nothing', async () => {
const ctl = spawnSync('git', ['ls-tree', 'HEAD'], { cwd: dir, encoding: 'utf8', env }); const ctl = spawnSync('git', ['ls-tree', 'HEAD'], { cwd: dir, encoding: 'utf8', env });
assert.equal(ctl.status, 128, `control: the fake git must fail ls-tree: ${ctl.stderr}`); assert.equal(ctl.status, 128, `control: the fake git must fail ls-tree: ${ctl.stderr}`);
assert.equal(spawnSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8', env }).status, 0, 'control: the fake git runs every other call'); assert.equal(spawnSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8', env }).status, 0, 'control: the fake git runs every other call');
writeFileSync(join(dir, '.git', 'upward.diff'), DIFF); const r = run(dir, DIFF, dir, env);
const r = spawnSync(process.execPath, [CLI, '--diff', join(dir, '.git', 'upward.diff'), '--step', '1', '--attempt', '1', '--cwd', dir], { encoding: 'utf8', env });
assert.equal(r.status, 2, `a HEAD read that failed must be exit 2: ${r.stdout}${r.stderr}`); assert.equal(r.status, 2, `a HEAD read that failed must be exit 2: ${r.stdout}${r.stderr}`);
assert.match(r.stderr, /ls-tree/, 'the message does not name the git call that failed'); assert.match(r.stderr, /ls-tree/, 'the message does not name the git call that failed');
const listed = git(dir, 'ls-files').split('\n'); const listed = git(dir, 'ls-files').split('\n');
@ -437,12 +439,6 @@ test('[M7] a HEAD path missing from the index exits 2 and moves nothing', () =>
} }
}); });
/** The CLI run on `diff` written into the repo's git dir, its child given childEnv(). */
function runIsolated(dir, diff = DIFF) {
writeFileSync(join(dir, '.git', 'upward.diff'), diff);
return spawnSync(process.execPath, [CLI, '--diff', join(dir, '.git', 'upward.diff'), '--step', '1', '--attempt', '1', '--cwd', dir], { encoding: 'utf8', env: childEnv() });
}
test('[M7] a corrupt branch ref is a failed HEAD read: exit 2, nothing unindexed or moved', () => { test('[M7] a corrupt branch ref is a failed HEAD read: exit 2, nothing unindexed or moved', () => {
const dir = repo(); const dir = repo();
try { try {
@ -454,7 +450,7 @@ test('[M7] a corrupt branch ref is a failed HEAD read: exit 2, nothing unindexed
const ctl = spawnSync('git', ['rev-parse', '--verify', '--quiet', 'HEAD'], { cwd: dir, encoding: 'utf8', env: childEnv() }); const ctl = spawnSync('git', ['rev-parse', '--verify', '--quiet', 'HEAD'], { cwd: dir, encoding: 'utf8', env: childEnv() });
assert.equal(ctl.status, 1, `control: the corrupt ref reads like an unborn HEAD (exit 1): ${ctl.stderr}`); assert.equal(ctl.status, 1, `control: the corrupt ref reads like an unborn HEAD (exit 1): ${ctl.stderr}`);
assert.equal(ctl.stdout.trim(), '', 'control: and prints nothing'); assert.equal(ctl.stdout.trim(), '', 'control: and prints nothing');
const r = runIsolated(dir); const r = run(dir, DIFF, dir, childEnv());
assert.equal(r.status, 2, `a corrupt branch ref must be exit 2: ${r.stdout}${r.stderr}`); assert.equal(r.status, 2, `a corrupt branch ref must be exit 2: ${r.stdout}${r.stderr}`);
const listed = git(dir, 'ls-files').split('\n'); const listed = git(dir, 'ls-files').split('\n');
assert.ok(listed.includes('tests/batch.sh'), 'the committed file was taken out of the index although HEAD could not be read'); assert.ok(listed.includes('tests/batch.sh'), 'the committed file was taken out of the index although HEAD could not be read');
@ -476,7 +472,7 @@ test('[M7r] an unborn HEAD parks the attempt\'s files', () => {
assert.match(git(dir, 'ls-files'), /tests\/batch\.sh/, 'control: the intent-to-add file is in the index'); assert.match(git(dir, 'ls-files'), /tests\/batch\.sh/, 'control: the intent-to-add file is in the index');
const ctl = spawnSync('git', ['rev-parse', '--verify', '--quiet', 'HEAD'], { cwd: dir, encoding: 'utf8', env: childEnv() }); const ctl = spawnSync('git', ['rev-parse', '--verify', '--quiet', 'HEAD'], { cwd: dir, encoding: 'utf8', env: childEnv() });
assert.equal(ctl.status, 1, 'control: HEAD is unborn'); assert.equal(ctl.status, 1, 'control: HEAD is unborn');
const r = runIsolated(dir); const r = run(dir, DIFF, dir, childEnv());
assert.equal(r.status, 0, `an unborn HEAD must park: ${r.stdout}${r.stderr}`); assert.equal(r.status, 0, `an unborn HEAD must park: ${r.stdout}${r.stderr}`);
assert.deepEqual(JSON.parse(r.stdout).parked, ['tests/batch.sh', 'tests/cli-stdin.test.mjs']); assert.deepEqual(JSON.parse(r.stdout).parked, ['tests/batch.sh', 'tests/cli-stdin.test.mjs']);
assert.doesNotMatch(git(dir, 'ls-files'), /tests\/batch\.sh/, 'the intent-to-add entry is still in the index'); assert.doesNotMatch(git(dir, 'ls-files'), /tests\/batch\.sh/, 'the intent-to-add entry is still in the index');

View file

@ -70,11 +70,7 @@ function finding(id, rule_key, extra = {}) {
function repoWith(reviews) { function repoWith(reviews) {
const dir = mkdtempSync(join(tmpdir(), 'recurrence-')); const dir = mkdtempSync(join(tmpdir(), 'recurrence-'));
reviews.forEach((r, i) => { reviewsInto(join(dir, '.claude'), reviews);
const p = join(dir, '.claude', 'projects', `2026-10-0${i + 1}-${r.slug}`);
mkdirSync(p, { recursive: true });
writeFileSync(join(p, 'review.md'), reviewText(r));
});
return dir; return dir;
} }

View file

@ -323,10 +323,11 @@ function gateRepo({ sub = '', claude = '# Project\n', claudeLink = false, object
g(repo, 'add', '-A'); g(repo, 'add', '-A');
g(repo, 'commit', '-q', '-m', msg); g(repo, 'commit', '-q', '-m', msg);
}; };
const gate = ({ verdict = 'ALLOW', sha = reviewed } = {}) => { // `env` is added to the child's environment; `repo` is the --repo given (the project dir by default).
const gate = ({ verdict = 'ALLOW', sha = reviewed, env = {}, repo: at = dir } = {}) => {
const p = join(base, 'review.md'); const p = join(base, 'review.md');
writeFileSync(p, review({ verdict, severities: [verdict === 'BLOCK' ? 'BLOCKER' : 'MINOR'], scopeShaEnd: sha })); writeFileSync(p, review({ verdict, severities: [verdict === 'BLOCK' ? 'BLOCKER' : 'MINOR'], scopeShaEnd: sha }));
return spawnSync(process.execPath, [GATE, '--json', p, '--repo', dir], { encoding: 'utf-8', env: childEnv() }); return spawnSync(process.execPath, [GATE, '--json', p, '--repo', at], { encoding: 'utf-8', env: childEnv(env) });
}; };
return { return {
base, repo, dir, reviewed, put, commit, gate, base, repo, dir, reviewed, put, commit, gate,
@ -715,13 +716,6 @@ function failingGit(sub) {
}; };
} }
/** The gate on an ALLOW review of the fixture's reviewed commit, with `env` and `repo` given. */
function gateWith(fx, { env = {}, repo = fx.dir } = {}) {
const p = join(fx.base, 'review.md');
writeFileSync(p, review({ verdict: 'ALLOW', severities: ['MINOR'], scopeShaEnd: fx.reviewed }));
return spawnSync(process.execPath, [GATE, '--json', p, '--repo', repo], { encoding: 'utf-8', env: childEnv(env) });
}
function assertNotMeasured(r, re) { function assertNotMeasured(r, re) {
assert.equal(r.status, 2, r.stdout + r.stderr); assert.equal(r.status, 2, r.stdout + r.stderr);
assert.doesNotMatch(r.stdout, /review-stale/); assert.doesNotMatch(r.stdout, /review-stale/);
@ -734,7 +728,7 @@ test('[M4] a git that cannot run rev-parse is not measured', () => {
const fx = gateRepo(); const fx = gateRepo();
const git = failingGit('rev-parse'); const git = failingGit('rev-parse');
try { try {
assertNotMeasured(gateWith(fx, { env: { PATH: git.path } }), /rev-parse/); assertNotMeasured(fx.gate({ env: { PATH: git.path } }), /rev-parse/);
} finally { git.cleanup(); fx.cleanup(); } } finally { git.cleanup(); fx.cleanup(); }
}); });
@ -744,9 +738,9 @@ test('[M4] a git that fails rev-list after the commit resolves is not measured',
try { try {
lessonsCommit(fx); lessonsCommit(fx);
// Control: with the real git the same review opens the gate, so the run below gets past rev-parse. // Control: with the real git the same review opens the gate, so the run below gets past rev-parse.
const control = gateWith(fx); const control = fx.gate();
assert.equal(control.status, 0, control.stdout + control.stderr); assert.equal(control.status, 0, control.stdout + control.stderr);
assertNotMeasured(gateWith(fx, { env: { PATH: git.path } }), /rev-list/); assertNotMeasured(fx.gate({ env: { PATH: git.path } }), /rev-list/);
} finally { git.cleanup(); fx.cleanup(); } } finally { git.cleanup(); fx.cleanup(); }
}); });
@ -755,16 +749,16 @@ test('[M4] a git that fails diff after the commit resolves is not measured', ()
const git = failingGit('diff'); const git = failingGit('diff');
try { try {
lessonsCommit(fx); lessonsCommit(fx);
const control = gateWith(fx); const control = fx.gate();
assert.equal(control.status, 0, control.stdout + control.stderr); assert.equal(control.status, 0, control.stdout + control.stderr);
assertNotMeasured(gateWith(fx, { env: { PATH: git.path } }), /diff/); assertNotMeasured(fx.gate({ env: { PATH: git.path } }), /diff/);
} finally { git.cleanup(); fx.cleanup(); } } finally { git.cleanup(); fx.cleanup(); }
}); });
test('[M4] --repo with a missing path is not measured', () => { test('[M4] --repo with a missing path is not measured', () => {
const fx = gateRepo(); const fx = gateRepo();
try { try {
assertNotMeasured(gateWith(fx, { repo: join(fx.base, 'no-such-dir') }), /no-such-dir/); assertNotMeasured(fx.gate({ repo: join(fx.base, 'no-such-dir') }), /no-such-dir/);
} finally { fx.cleanup(); } } finally { fx.cleanup(); }
}); });
@ -774,7 +768,7 @@ test('[M4] --repo that is no repository is not measured', () => {
const plain = join(fx.base, 'plain'); const plain = join(fx.base, 'plain');
mkdirSync(plain); mkdirSync(plain);
// The ceiling stops git from finding a repository above the fixture. // The ceiling stops git from finding a repository above the fixture.
assertNotMeasured(gateWith(fx, { repo: plain, env: { GIT_CEILING_DIRECTORIES: fx.base } }), /not a git repository/); assertNotMeasured(fx.gate({ repo: plain, env: { GIT_CEILING_DIRECTORIES: fx.base } }), /not a git repository/);
} finally { fx.cleanup(); } } finally { fx.cleanup(); }
}); });