test(cap-hook): cover the crash-time marker branches and EXECUTE the marker snippet
Two gaps, both test-side. No production behaviour changes here: the
branches were already correct, they were simply unpinned, so a later edit
could have broken them silently.
1. Crash-time states the hook header's TTL discussion anticipates and no
test exercised: a partially written marker (valid prefix, no close - what
an interrupted printf leaves), a marker with no runId, a marker whose
runId is empty or not a string, and malformed ledger lines. All must
ALLOW, because a marker we cannot read cannot say which run we are in,
and a half-written ledger line is not a spent turn. One test pins the
other direction too: skipping bad lines must not mean skipping the run's
tombstone.
2. The marker snippet is now RUN, not asserted about. Every existing pin on
the marker lifecycle (tests/lib/doc-consistency.test.mjs) is a substring
assertion on prose, so a snippet emitting invalid JSON or writing to a
path the hook never reads would keep the whole suite green while the hook
allowed everything - the failure S82 had to find by hand with a
scratchpad probe. Three tests now extract the real ```bash blocks from
commands/trekresearch.md and execute them with CLAUDE_PLUGIN_DATA
stripped and HOME sandboxed:
- the write snippet lands parseable JSON at the exact path the hook
looks up, with runId and a Date.parse-able startedAt;
- an empty CLAUDE_CODE_SESSION_ID produces NO `.json` marker and says
the cap stays inert;
- write snippet -> real hook denies -> removal snippet -> real hook
allows, which is the writer/reader agreement end to end.
On the non-string runId: it clears the falsy guard and then matches no
ledger record, so the run reads as 0 turns and is allowed. Allow is correct
either way and no writer emits one, so that stays a pin on the outcome
rather than an argument for a type guard on an unreachable state.
Review finding 823d8c28d7c993717606433fd6711d5a67877967 (MINOR).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LuGhWAbWyRFBFeemfhxoVv
This commit is contained in:
parent
dc245af408
commit
2844400a00
1 changed files with 192 additions and 1 deletions
|
|
@ -17,7 +17,8 @@ import { test } from 'node:test';
|
|||
import { strict as assert } from 'node:assert';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from 'node:fs';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from 'node:fs';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { runHookWithEnv } from '../helpers/hook-helper.mjs';
|
||||
|
||||
|
|
@ -291,6 +292,196 @@ test('pre-agent-cap never writes to the ledger', async () => {
|
|||
assert.strictEqual(readFileSync(ledgerPath, 'utf-8'), before);
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Crash-time marker states. These are the states the TTL discussion in the
|
||||
// hook header anticipates, and none of them had a test: a marker written
|
||||
// half-way, and a marker whose runId never made it. Both must ALLOW — a
|
||||
// marker we cannot read cannot tell us which run we are in, and guessing
|
||||
// would deny tool calls in a session we know nothing about.
|
||||
// -----------------------------------------------------------------------
|
||||
test('pre-agent-cap ALLOWS on a partially written (corrupt) scope marker', async () => {
|
||||
const dir = fixture({ turns: BUDGET, exhausted: true });
|
||||
// Exactly what an interrupted printf leaves behind: valid prefix, no close.
|
||||
writeFileSync(join(dir, 'trekresearch-loop-scope', `${SESSION}.json`), '{"runId":"run-xyz789","star');
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: dir,
|
||||
});
|
||||
assert.strictEqual(code, 0, 'an unparsable marker is not evidence of a loop turn');
|
||||
});
|
||||
|
||||
test('pre-agent-cap ALLOWS a marker that carries no runId', async () => {
|
||||
const dir = fixture({ turns: BUDGET, exhausted: true });
|
||||
writeFileSync(
|
||||
join(dir, 'trekresearch-loop-scope', `${SESSION}.json`),
|
||||
JSON.stringify({ startedAt: new Date().toISOString() }),
|
||||
);
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: dir,
|
||||
});
|
||||
assert.strictEqual(code, 0, 'without a runId there are no ledger lines to count against');
|
||||
});
|
||||
|
||||
test('pre-agent-cap ALLOWS a marker whose runId is not a string', async () => {
|
||||
const dir = fixture({ turns: BUDGET, exhausted: true });
|
||||
// Truthy, so it clears the `!marker?.runId` guard and the session counts as
|
||||
// in scope — but readLedger compares runId with ===, so a number matches no
|
||||
// record and the run reads as 0 turns spent. Allow is the right answer either
|
||||
// way, which is why this stays a pin on the OUTCOME and not an argument for a
|
||||
// type guard: no writer emits a non-string runId, and the two routes are
|
||||
// indistinguishable from outside.
|
||||
writeFileSync(
|
||||
join(dir, 'trekresearch-loop-scope', `${SESSION}.json`),
|
||||
JSON.stringify({ runId: 5, startedAt: new Date().toISOString() }),
|
||||
);
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: dir,
|
||||
});
|
||||
assert.strictEqual(code, 0);
|
||||
});
|
||||
|
||||
test('pre-agent-cap ALLOWS a marker whose runId is the empty string', async () => {
|
||||
const dir = fixture({ turns: BUDGET, exhausted: true });
|
||||
writeFileSync(
|
||||
join(dir, 'trekresearch-loop-scope', `${SESSION}.json`),
|
||||
JSON.stringify({ runId: '', startedAt: new Date().toISOString() }),
|
||||
);
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: dir,
|
||||
});
|
||||
assert.strictEqual(code, 0);
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Malformed ledger lines — a truncated final write must not be counted as a
|
||||
// turn, and must not stop the well-formed lines around it from counting.
|
||||
// -----------------------------------------------------------------------
|
||||
test('pre-agent-cap does not count a malformed ledger line as a turn', async () => {
|
||||
const dir = fixture({ turns: BUDGET - 1 });
|
||||
const ledgerPath = join(dir, 'trekresearch-loop-ledger.jsonl');
|
||||
writeFileSync(ledgerPath, readFileSync(ledgerPath, 'utf-8') + '{"runId":"run-xyz789","dimen\n');
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: dir,
|
||||
});
|
||||
assert.strictEqual(code, 0, 'a half-written line is not a spent turn');
|
||||
});
|
||||
|
||||
test('pre-agent-cap still finds the tombstone with malformed lines around it', async () => {
|
||||
const dir = fixture({ turns: BUDGET, exhausted: true });
|
||||
const ledgerPath = join(dir, 'trekresearch-loop-ledger.jsonl');
|
||||
writeFileSync(ledgerPath, '{ garbage\n' + readFileSync(ledgerPath, 'utf-8') + 'also garbage\n');
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: dir,
|
||||
});
|
||||
assert.strictEqual(code, 2, 'skipping bad lines must not mean skipping the run’s denial record');
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// The marker snippet is EXECUTED, not asserted about.
|
||||
//
|
||||
// Every existing pin on the marker lifecycle is a substring assertion on the
|
||||
// prose in commands/trekresearch.md. A snippet that emitted invalid JSON, or
|
||||
// wrote to a path the hook never looks at, would keep the whole suite green
|
||||
// while the hook silently allowed everything — which is the exact failure S82
|
||||
// found by hand. So these tests run the real shell blocks out of the command
|
||||
// file, with CLAUDE_PLUGIN_DATA stripped and HOME sandboxed, and then run the
|
||||
// real hook against what they produced.
|
||||
// -----------------------------------------------------------------------
|
||||
const CMD_FILE = join(ROOT, 'commands', 'trekresearch.md');
|
||||
|
||||
/** Pull the ```bash block that contains `needle` out of the command file. */
|
||||
function bashBlockContaining(needle) {
|
||||
const text = readFileSync(CMD_FILE, 'utf-8');
|
||||
const at = text.indexOf(needle);
|
||||
assert.ok(at > -1, `commands/trekresearch.md no longer contains ${JSON.stringify(needle)}`);
|
||||
const open = text.lastIndexOf('```bash', at);
|
||||
assert.ok(open > -1, `no \`\`\`bash fence opens before ${JSON.stringify(needle)}`);
|
||||
const bodyStart = text.indexOf('\n', open) + 1;
|
||||
const close = text.indexOf('```', bodyStart);
|
||||
assert.ok(close > bodyStart, 'unterminated bash fence');
|
||||
return text.slice(bodyStart, close);
|
||||
}
|
||||
|
||||
function runSnippet(snippet, env) {
|
||||
return execFileSync('bash', ['-c', snippet], {
|
||||
encoding: 'utf-8',
|
||||
env: { PATH: process.env.PATH, ...env },
|
||||
});
|
||||
}
|
||||
|
||||
test('the marker WRITE snippet lands valid JSON exactly where the hook looks for it', () => {
|
||||
const home = mkdtempSync(join(tmpdir(), 'voyage-snippet-'));
|
||||
const sessionId = 'snippet-session-1';
|
||||
const snippet = bashBlockContaining('Arms the PreToolUse cap').replace(/\{run_id\}/g, 'snippet-run-1');
|
||||
|
||||
runSnippet(snippet, { HOME: home, CLAUDE_CODE_SESSION_ID: sessionId });
|
||||
|
||||
const markerPath = join(home, '.claude', 'voyage', 'trekresearch-loop-scope', `${sessionId}.json`);
|
||||
assert.ok(existsSync(markerPath), `snippet wrote no marker at ${markerPath}`);
|
||||
const marker = JSON.parse(readFileSync(markerPath, 'utf-8')); // throws if the printf emits bad JSON
|
||||
assert.strictEqual(marker.runId, 'snippet-run-1', 'runId must be the same id passed to --run-id');
|
||||
assert.ok(Number.isFinite(Date.parse(marker.startedAt)), `startedAt must parse, got ${marker.startedAt}`);
|
||||
});
|
||||
|
||||
test('the marker snippet writes NO file when CLAUDE_CODE_SESSION_ID is empty', () => {
|
||||
const home = mkdtempSync(join(tmpdir(), 'voyage-snippet-'));
|
||||
const snippet = bashBlockContaining('Arms the PreToolUse cap').replace(/\{run_id\}/g, 'snippet-run-2');
|
||||
|
||||
const out = runSnippet(snippet, { HOME: home });
|
||||
|
||||
const scopeDir = join(home, '.claude', 'voyage', 'trekresearch-loop-scope');
|
||||
assert.ok(!existsSync(join(scopeDir, '.json')), 'an empty session id must not produce a `.json` marker');
|
||||
assert.match(out, /stays inert/i, 'the snippet must say the harness cap is inert, not fail silently');
|
||||
});
|
||||
|
||||
test('write snippet then real hook: the loop’s own writer arms the enforcement end to end', async () => {
|
||||
const home = mkdtempSync(join(tmpdir(), 'voyage-snippet-'));
|
||||
const sessionId = 'snippet-session-3';
|
||||
const runId = 'snippet-run-3';
|
||||
|
||||
runSnippet(
|
||||
bashBlockContaining('Arms the PreToolUse cap').replace(/\{run_id\}/g, runId),
|
||||
{ HOME: home, CLAUDE_CODE_SESSION_ID: sessionId },
|
||||
);
|
||||
// A spent, tombstoned ledger for that same runId, under the same resolved root.
|
||||
writeFileSync(
|
||||
join(home, '.claude', 'voyage', 'trekresearch-loop-ledger.jsonl'),
|
||||
[
|
||||
...Array.from({ length: BUDGET }, (_, i) =>
|
||||
JSON.stringify({ ts: new Date().toISOString(), runId, dimension: `d${i}`, effort: 'high', slot: i + 1 })),
|
||||
JSON.stringify({ ts: new Date().toISOString(), runId, exhausted: true }),
|
||||
].join('\n') + '\n',
|
||||
);
|
||||
|
||||
const denied = await runHookWithEnv(CAP_HOOK, searchInput(sessionId), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: '',
|
||||
HOME: home,
|
||||
});
|
||||
assert.strictEqual(denied.code, 2, 'the hook must find the snippet’s marker and enforce against it');
|
||||
|
||||
// And the removal snippet must disarm it again — same root, same guard.
|
||||
runSnippet(
|
||||
bashBlockContaining('Removal — idempotent').replace(/\{run_id\}/g, runId),
|
||||
{ HOME: home, CLAUDE_CODE_SESSION_ID: sessionId },
|
||||
);
|
||||
assert.ok(
|
||||
!existsSync(join(home, '.claude', 'voyage', 'trekresearch-loop-scope', `${sessionId}.json`)),
|
||||
'the removal snippet must delete the marker the write snippet created',
|
||||
);
|
||||
const allowed = await runHookWithEnv(CAP_HOOK, searchInput(sessionId), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: '',
|
||||
HOME: home,
|
||||
});
|
||||
assert.strictEqual(allowed.code, 0, 'a removed marker must take the session back out of scope');
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Wiring — pattern from tests/hooks/hooks-json-stop-wired.test.mjs
|
||||
// -----------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue