fix(guards): a step whose test is seen green may stop while it waits for its controller
voyage-lekerepo trial (2026-10-06) point 7, operator decision 2026-10-08 («grønt merke på låsen»): the stop guard blocked Stop three times while the executor waited for the controller, with the step's test already green. - test-lock.mjs green --cmd "<Verify>" runs the step's test command (same allowlist + denylist as done-line) and records green_at only on exit 0; pending clears it; a new lock starts without it. - stop-test-lock lets a green-marked lock stop; the write guard still refuses the locked test files. - trekexecute: D1 marks green after a PASS; each fix round marks pending. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
81dc5af680
commit
393b8e5a0a
5 changed files with 163 additions and 10 deletions
|
|
@ -1486,9 +1486,20 @@ status = `blocked` and apply `On failure: escalate`. Skip D2 manifest
|
|||
verification (nothing to verify for a read-only sandbox test). Commit none —
|
||||
jump straight to Phase 7 with a structured "sandbox-blocked" reason.
|
||||
|
||||
If **PASS**: proceed to Sub-step D2 (manifest verification). If **FAIL**: the
|
||||
failure output is a finding for Sub-step F; still run D2 and D3, so the
|
||||
controller and the next fix round see every failure at once.
|
||||
If **PASS**: mark the step's test lock green, then proceed to Sub-step D2
|
||||
(manifest verification). The mark is what lets the session wait for its
|
||||
controller (Sub-step E) without the stop guard refusing every turn — the
|
||||
test files stay locked against edits (operator decision 2026-10-08):
|
||||
|
||||
```bash
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/util/test-lock.mjs green --cmd "{Verify command}"
|
||||
```
|
||||
|
||||
It runs the test command itself and marks only on exit 0; exit 1 (the test
|
||||
did not pass this time) is a finding for Sub-step F like a FAIL, exit 2 (could
|
||||
not run) is never green. A step that is `"not-locked"` skips it.
|
||||
If **FAIL**: the failure output is a finding for Sub-step F; still run D2 and
|
||||
D3, so the controller and the next fix round see every failure at once.
|
||||
|
||||
#### Sub-step D2 — Manifest verification
|
||||
|
||||
|
|
@ -1629,7 +1640,7 @@ session's effort, Sub-step E).
|
|||
|
||||
| `fix_rounds` | Goes to | What happens |
|
||||
|---|---|---|
|
||||
| 1, 2, 3 | a fresh helper | Launch `voyage:step-implementer` again (no `name`, no `isolation`), `mode: fix`, with the findings (file, line, problem, fix) and the mechanical failures. It fixes only those; the tests stay locked (except in a `not-red-at-base` round, Sub-step B). Then B (both ways), D1, D2, D3 run again and a fresh controller re-controls only what was fixed (Sub-step E). `attempts` +1; `iterations_remaining` −1 |
|
||||
| 1, 2, 3 | a fresh helper | First `node ${CLAUDE_PLUGIN_ROOT}/lib/util/test-lock.mjs pending` (the step is no longer seen green; D1 marks it again). Launch `voyage:step-implementer` again (no `name`, no `isolation`), `mode: fix`, with the findings (file, line, problem, fix) and the mechanical failures. It fixes only those; the tests stay locked (except in a `not-red-at-base` round, Sub-step B). Then B (both ways), D1, D2, D3 run again and a fresh controller re-controls only what was fixed (Sub-step E). `attempts` +1; `iterations_remaining` −1 |
|
||||
| 4 | the plan | **After three failed fix rounds the question is the architecture, not a fourth fix.** The step goes back to planning (below). Stop class 4 |
|
||||
| 5 | the operator | A step already sent to the plan (`fix_rounds` 4) that fails its control again after re-planning goes to the operator. The executor never builds this step again: on a later `--resume` it only runs the checks (B, D1, D2, D3, E) on whatever the operator changed. If they fail again the step stays at 5 — there is no sixth round, `fix_rounds` never exceeds 5 — and the run stops again. Stop class 4 |
|
||||
|
||||
|
|
|
|||
|
|
@ -38,6 +38,10 @@ let lock = readLock(input?.cwd || process.cwd());
|
|||
if (lock.state !== 'locked' || lock.lock.session !== input?.session_id) lock = lockForSession(input?.session_id);
|
||||
if (lock.state !== 'locked') process.exit(0);
|
||||
if (!lock.lock.session || lock.lock.session !== input?.session_id) process.exit(0);
|
||||
// The step's test has been seen passing by a command (`test-lock.mjs green`):
|
||||
// the session may stop -- e.g. to wait for its controller -- while the write
|
||||
// guard keeps the test files locked (operator decision 2026-10-08).
|
||||
if (lock.lock.green_at) process.exit(0);
|
||||
|
||||
const files = lock.lock.files.join(', ');
|
||||
process.stdout.write(JSON.stringify({
|
||||
|
|
|
|||
|
|
@ -13,8 +13,20 @@
|
|||
// node lib/util/test-lock.mjs lock --files <a,b> --step <N> [--cwd <dir>]
|
||||
// node lib/util/test-lock.mjs unlock [--cwd <dir>]
|
||||
// node lib/util/test-lock.mjs status [--cwd <dir>] (JSON on stdout)
|
||||
// Exit: 0 = done, 1 = refused (another step holds the lock), 2 = could not
|
||||
// run (not a repository, bad usage, unreadable lock). 2 is never "unlocked".
|
||||
// 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
|
||||
// repository, bad usage, unreadable lock, a command outside the test-runner
|
||||
// allowlist). 2 is never "unlocked" and never green.
|
||||
//
|
||||
// The green mark (operator decision 2026-10-08). The stop guard blocked Stop
|
||||
// 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
|
||||
// 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).
|
||||
// The session is taken from CLAUDE_CODE_SESSION_ID, which Claude Code sets in
|
||||
// Bash tool subprocesses (code.claude.com/docs/en/env-vars).
|
||||
|
|
@ -27,7 +39,10 @@ import { join, resolve, dirname } from 'node:path';
|
|||
import { fileURLToPath } from 'node:url';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { resolveRepo, canonicalPath } from '../verification/repo-tools.mjs';
|
||||
import { allowedCommand, screenCommand } from '../verification/criteria-runner.mjs';
|
||||
import { commandEnv, DEFAULT_TIMEOUT_MS } from '../verification/red-proof.mjs';
|
||||
|
||||
export const LOCK_VERSION = 1;
|
||||
|
||||
|
|
@ -113,7 +128,7 @@ function parseArgs(argv) {
|
|||
const out = { _: [] };
|
||||
for (let i = 0; i < argv.length; i += 1) {
|
||||
const a = argv[i];
|
||||
if (a === '--files' || a === '--step' || a === '--cwd') { out[a.slice(2)] = argv[i + 1]; i += 1; }
|
||||
if (a === '--files' || a === '--step' || a === '--cwd' || a === '--cmd') { out[a.slice(2)] = argv[i + 1]; i += 1; }
|
||||
else out._.push(a);
|
||||
}
|
||||
return out;
|
||||
|
|
@ -127,7 +142,9 @@ function fail(code, msg) {
|
|||
function main(argv) {
|
||||
const args = parseArgs(argv);
|
||||
const cmd = args._[0];
|
||||
if (!['lock', 'unlock', 'status'].includes(cmd)) fail(2, 'usage: test-lock.mjs lock --files <a,b> --step <N> | unlock | status [--cwd <dir>]');
|
||||
if (!['lock', 'unlock', 'status', 'green', 'pending'].includes(cmd)) {
|
||||
fail(2, 'usage: test-lock.mjs lock --files <a,b> --step <N> | unlock | status | green --cmd "<test command>" | pending [--cwd <dir>]');
|
||||
}
|
||||
const cwd = resolve(args.cwd || process.cwd());
|
||||
const cur = readLock(cwd);
|
||||
if (cur.state === 'no-repo') fail(2, `not a git repository: ${cwd}`);
|
||||
|
|
@ -140,11 +157,36 @@ 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, lock_path: cur.path,
|
||||
locked_at: l.locked_at ?? null, green_at: l.green_at ?? null, lock_path: cur.path,
|
||||
}, null, 2) + '\n');
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
if (cmd === 'green' || cmd === 'pending') {
|
||||
if (cur.state === 'corrupt') fail(2, `unreadable lock ${cur.path} (${cur.error})`);
|
||||
if (cur.state !== 'locked') fail(1, 'no lock to mark');
|
||||
const lock = { ...cur.lock };
|
||||
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 });
|
||||
if (!allowed.allowed) fail(2, `command not run: ${allowed.reason}`);
|
||||
const screen = screenCommand(args.cmd);
|
||||
if (!screen.allowed) fail(2, `command blocked: ${screen.rule}`);
|
||||
const r = spawnSync('/bin/sh', ['-c', args.cmd], {
|
||||
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) {
|
||||
fail(2, `the test command did not run to an exit code (${r.error?.message ?? r.signal ?? `exit ${r.status}`})`);
|
||||
}
|
||||
if (r.status !== 0) fail(1, `the test command exited ${r.status}; the lock stays unmarked`);
|
||||
lock.green_at = new Date().toISOString();
|
||||
}
|
||||
writeFileSync(cur.path, JSON.stringify(lock, null, 2) + '\n');
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
if (cmd === 'unlock') {
|
||||
if (cur.state === 'corrupt') fail(2, `unreadable lock ${cur.path} (${cur.error}); inspect it before removing it`);
|
||||
removeSessionPointer(cur.lock?.session);
|
||||
|
|
|
|||
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:1124-1139 | 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:2330-2352 | 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:2341-2363 | 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 |
|
||||
|
|
|
|||
96
tests/hooks/test-lock-green.test.mjs
Normal file
96
tests/hooks/test-lock-green.test.mjs
Normal file
|
|
@ -0,0 +1,96 @@
|
|||
// tests/hooks/test-lock-green.test.mjs
|
||||
// The test lock's green mark (operator decision 2026-10-08, voyage-lekerepo
|
||||
// trial point 7). The stop guard blocked Stop three times while the executor
|
||||
// waited for the controller in the background, though the step's test was
|
||||
// already green. Now:
|
||||
// - `test-lock.mjs green --cmd "<Verify>"` RUNS the step's test command and
|
||||
// marks the lock "seen green" only on exit 0 (a command proves it, never a
|
||||
// word);
|
||||
// - the stop guard lets a green-marked lock stop; the write guard still
|
||||
// refuses the locked files — the lock still protects the test;
|
||||
// - `test-lock.mjs pending` clears the mark when a fix round starts.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { writeFileSync, mkdirSync, readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { runHook } from '../helpers/hook-helper.mjs';
|
||||
import { ROOT, makeRepo, parseJsonOut } from './guard-helpers.mjs';
|
||||
|
||||
const CLI = join(ROOT, 'lib', 'util', 'test-lock.mjs');
|
||||
const STOP = join(ROOT, 'hooks', 'scripts', 'stop-test-lock.mjs');
|
||||
const PRE_WRITE = join(ROOT, 'hooks', 'scripts', 'pre-write-executor.mjs');
|
||||
const SESS = `sess-green-${process.pid}-${Date.now()}`;
|
||||
|
||||
function cli(repo, args) {
|
||||
const r = spawnSync(process.execPath, [CLI, ...args], { cwd: repo, encoding: 'utf8', env: { ...process.env, CLAUDE_CODE_SESSION_ID: SESS } });
|
||||
return { code: r.status, stdout: r.stdout, stderr: r.stderr };
|
||||
}
|
||||
|
||||
function lockedRepo(testBody) {
|
||||
const repo = makeRepo('main');
|
||||
mkdirSync(join(repo, 'tests'), { recursive: true });
|
||||
writeFileSync(join(repo, 'tests', 'step.test.mjs'), [
|
||||
"import { test } from 'node:test';",
|
||||
"import { strict as assert } from 'node:assert';",
|
||||
`test('step', () => { ${testBody} });`,
|
||||
'',
|
||||
].join('\n'));
|
||||
assert.equal(cli(repo, ['lock', '--files', 'tests/step.test.mjs', '--step', '3']).code, 0);
|
||||
return repo;
|
||||
}
|
||||
|
||||
const CMD = 'node --test tests/step.test.mjs';
|
||||
const stop = (repo) => ({ hook_event_name: 'Stop', session_id: SESS, cwd: repo, stop_hook_active: false });
|
||||
const edit = (file_path) => ({ hook_event_name: 'PreToolUse', tool_name: 'Edit', tool_input: { file_path, old_string: 'x', new_string: 'z' } });
|
||||
|
||||
test('green: the step\'s test passes -> the lock is marked green, Stop goes through, the test file stays locked', async () => {
|
||||
const repo = lockedRepo('assert.equal(1, 1);');
|
||||
const g = cli(repo, ['green', '--cmd', CMD]);
|
||||
assert.equal(g.code, 0, g.stderr);
|
||||
assert.ok(JSON.parse(cli(repo, ['status']).stdout).green_at, 'status shows green_at');
|
||||
assert.equal(parseJsonOut((await runHook(STOP, stop(repo))).stdout), null, 'a green lock does not block Stop');
|
||||
const w = await runHook(PRE_WRITE, edit(join(repo, 'tests', 'step.test.mjs')));
|
||||
assert.equal(w.code, 2, 'the test file is still locked against edits');
|
||||
});
|
||||
|
||||
test('green: the test fails -> exit 1, no mark, Stop is still refused', async () => {
|
||||
const repo = lockedRepo('assert.equal(1, 2);');
|
||||
assert.equal(cli(repo, ['green', '--cmd', CMD]).code, 1);
|
||||
assert.equal(JSON.parse(cli(repo, ['status']).stdout).green_at, null);
|
||||
assert.equal(parseJsonOut((await runHook(STOP, stop(repo))).stdout)?.decision, 'block');
|
||||
});
|
||||
|
||||
test('green: a command outside the test-runner allowlist is never run -> exit 2, no mark', () => {
|
||||
const repo = lockedRepo('assert.equal(1, 1);');
|
||||
const g = cli(repo, ['green', '--cmd', 'echo ok']);
|
||||
assert.equal(g.code, 2, g.stderr);
|
||||
assert.equal(JSON.parse(cli(repo, ['status']).stdout).green_at, null);
|
||||
});
|
||||
|
||||
test('green: no lock to mark -> exit 1', () => {
|
||||
const repo = makeRepo('main');
|
||||
assert.equal(cli(repo, ['green', '--cmd', CMD]).code, 1);
|
||||
});
|
||||
|
||||
test('pending: a fix round clears the mark, and Stop is refused again', async () => {
|
||||
const repo = lockedRepo('assert.equal(1, 1);');
|
||||
assert.equal(cli(repo, ['green', '--cmd', CMD]).code, 0);
|
||||
assert.equal(cli(repo, ['pending']).code, 0);
|
||||
assert.equal(JSON.parse(cli(repo, ['status']).stdout).green_at, null);
|
||||
assert.equal(parseJsonOut((await runHook(STOP, stop(repo))).stdout)?.decision, 'block');
|
||||
});
|
||||
|
||||
test('re-lock of the same step starts without a mark', () => {
|
||||
const repo = lockedRepo('assert.equal(1, 1);');
|
||||
assert.equal(cli(repo, ['green', '--cmd', CMD]).code, 0);
|
||||
assert.equal(cli(repo, ['lock', '--files', 'tests/step.test.mjs', '--step', '3']).code, 0);
|
||||
assert.equal(JSON.parse(cli(repo, ['status']).stdout).green_at, null);
|
||||
});
|
||||
|
||||
test('trekexecute: D1 marks the lock green through the command, a fix round marks it pending', () => {
|
||||
const md = readFileSync(join(ROOT, 'commands', 'trekexecute.md'), 'utf8');
|
||||
assert.match(md, /test-lock\.mjs green --cmd "\{Verify command\}"/);
|
||||
assert.match(md, /test-lock\.mjs pending/);
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue