fix(cap): move the enforcement boundary to a denial tombstone, not the turn count
allowTurn() appends BEFORE the turn runs, so during granted turn N the
ledger holds N records. The hook denied at `used >= budget`, which blocked
every tool call of the FINAL granted turn: the primitive granted B turns
and the harness permitted B-1. Worse, an exhausted run therefore always
terminated through an exit-2 tool denial instead of the graceful "cap
exhausted" exit at commands/trekresearch.md - and the prose says in as
many words that exit 2 is not exit 1, so the model was pushed out through
the one exit it is told NOT to treat as a cap.
The review recommended denying at `used > budget`. Taken alone that fixes
the count and breaks the hook: once the O_EXCL claim (previous commit)
makes a breached ledger impossible, `granted > budget` can no longer fire,
and the case this hook exists for - the loop consults the gate, is denied,
and issues the tool call anyway - would be allowed. A deny branch that
cannot be reached is a dead security claim, which is the same thing S82
removed two of rather than leave standing.
So the denial itself became a record. allowTurn() appends a tombstone
{runId, exhausted: true} when it denies for budget, and the hook denies on
the tombstone. Both properties now hold at once:
granted == budget, no tombstone -> turn B is in flight -> ALLOW
tombstone present -> the gate already said no -> DENY
granted > budget -> breached, any cause -> DENY
A tombstone is not a turn: readLedger reports {granted, exhausted}
separately so it can never consume budget. allowTurn short-circuits on an
existing tombstone, so a hammered gate neither re-walks every slot nor
grows the ledger. The tombstone write is best effort on purpose - the
denial is already the correct answer, so a ledger that cannot take the
record must not turn a denial into a grant.
The parallel-boundary test now asserts GRANTED turns rather than raw
ledger lines, because the denied callers legitimately add tombstones.
Review finding 8eb53458ac3efec778094f9f03b09e1cc1077a09 (MINOR).
Operator decision: tombstone over the literal recommended_action.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LuGhWAbWyRFBFeemfhxoVv
This commit is contained in:
parent
156539204a
commit
32e20fc0dc
6 changed files with 180 additions and 33 deletions
|
|
@ -33,17 +33,20 @@ const RUN_ID = 'run-xyz789';
|
|||
* Build a throwaway CLAUDE_PLUGIN_DATA dir holding a scope marker for
|
||||
* `sessionId` and `turns` ledger entries for RUN_ID.
|
||||
*/
|
||||
function fixture({ turns = 0, sessionId = SESSION, startedAt = new Date() } = {}) {
|
||||
function fixture({ turns = 0, sessionId = SESSION, startedAt = new Date(), exhausted = false } = {}) {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'voyage-cap-'));
|
||||
mkdirSync(join(dir, 'trekresearch-loop-scope'), { recursive: true });
|
||||
writeFileSync(
|
||||
join(dir, 'trekresearch-loop-scope', `${sessionId}.json`),
|
||||
JSON.stringify({ runId: RUN_ID, startedAt: startedAt.toISOString() }),
|
||||
);
|
||||
const ledger = Array.from({ length: turns }, (_, i) =>
|
||||
JSON.stringify({ ts: new Date().toISOString(), runId: RUN_ID, dimension: `d${i}`, effort: 'high' }),
|
||||
).join('\n');
|
||||
writeFileSync(join(dir, 'trekresearch-loop-ledger.jsonl'), ledger ? ledger + '\n' : '');
|
||||
const lines = Array.from({ length: turns }, (_, i) =>
|
||||
JSON.stringify({ ts: new Date().toISOString(), runId: RUN_ID, dimension: `d${i}`, effort: 'high', slot: i + 1 }),
|
||||
);
|
||||
// The tombstone research-loop-cap.mjs appends when it denies a turn for
|
||||
// budget. Its presence is what tells this hook "the gate already said no".
|
||||
if (exhausted) lines.push(JSON.stringify({ ts: new Date().toISOString(), runId: RUN_ID, exhausted: true }));
|
||||
writeFileSync(join(dir, 'trekresearch-loop-ledger.jsonl'), lines.length ? lines.join('\n') + '\n' : '');
|
||||
return dir;
|
||||
}
|
||||
|
||||
|
|
@ -65,8 +68,8 @@ function searchInput(sessionId = SESSION) {
|
|||
// -----------------------------------------------------------------------
|
||||
// DENY — budget spent
|
||||
// -----------------------------------------------------------------------
|
||||
test('pre-agent-cap DENIES a loop turn once the ledger shows the budget spent', async () => {
|
||||
const dir = fixture({ turns: BUDGET });
|
||||
test('pre-agent-cap DENIES once the budget gate has denied a turn (tombstone present)', async () => {
|
||||
const dir = fixture({ turns: BUDGET, exhausted: true });
|
||||
const { code, stderr } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: dir,
|
||||
|
|
@ -76,7 +79,7 @@ test('pre-agent-cap DENIES a loop turn once the ledger shows the budget spent',
|
|||
assert.match(stderr, new RegExp(`${BUDGET}`), 'stderr must state the budget');
|
||||
});
|
||||
|
||||
test('pre-agent-cap DENIES above the budget too (used > budget)', async () => {
|
||||
test('pre-agent-cap DENIES above the budget too (a breached ledger, whatever caused it)', async () => {
|
||||
const dir = fixture({ turns: BUDGET + 5 });
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
|
|
@ -86,7 +89,15 @@ test('pre-agent-cap DENIES above the budget too (used > budget)', async () => {
|
|||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// ALLOW — under the cap
|
||||
// ALLOW — under the cap, and ON the cap.
|
||||
//
|
||||
// allowTurn appends BEFORE the turn runs, so during the FINAL granted turn the
|
||||
// ledger already holds `budget` records. Denying at `used >= budget` therefore
|
||||
// blocked that turn's own tool calls: the primitive granted B turns, the
|
||||
// harness permitted B-1, and an exhausted run always ended through an exit-2
|
||||
// denial rather than the graceful "cap exhausted" exit the prose defines. The
|
||||
// boundary belongs one turn later, and the tombstone above — not the count —
|
||||
// is what marks a run actually finished.
|
||||
// -----------------------------------------------------------------------
|
||||
test('pre-agent-cap ALLOWS a loop turn under the budget', async () => {
|
||||
const dir = fixture({ turns: BUDGET - 1 });
|
||||
|
|
@ -97,6 +108,18 @@ test('pre-agent-cap ALLOWS a loop turn under the budget', async () => {
|
|||
assert.strictEqual(code, 0);
|
||||
});
|
||||
|
||||
test('pre-agent-cap ALLOWS the FINAL granted turn — its own record is already on the ledger', async () => {
|
||||
const dir = fixture({ turns: BUDGET });
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
...CAPPED_ENV,
|
||||
CLAUDE_PLUGIN_DATA: dir,
|
||||
});
|
||||
assert.strictEqual(
|
||||
code, 0,
|
||||
'turn B is granted and in flight; denying it makes the harness permit B-1 turns and forces the wrong exit',
|
||||
);
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// DOES NOT OVER-BLOCK — the property that keeps this hook safe to wire
|
||||
// globally. A broken PreToolUse hook would brick every session on the box.
|
||||
|
|
@ -223,9 +246,11 @@ test('pre-agent-cap enforces via the fallback root when CLAUDE_PLUGIN_DATA is ab
|
|||
);
|
||||
writeFileSync(
|
||||
join(root, 'trekresearch-loop-ledger.jsonl'),
|
||||
Array.from({ length: BUDGET }, (_, i) =>
|
||||
JSON.stringify({ ts: new Date().toISOString(), runId: RUN_ID, dimension: `d${i}`, effort: 'high' }),
|
||||
).join('\n') + '\n',
|
||||
[
|
||||
...Array.from({ length: BUDGET }, (_, i) =>
|
||||
JSON.stringify({ ts: new Date().toISOString(), runId: RUN_ID, dimension: `d${i}`, effort: 'high', slot: i + 1 })),
|
||||
JSON.stringify({ ts: new Date().toISOString(), runId: RUN_ID, exhausted: true }),
|
||||
].join('\n') + '\n',
|
||||
);
|
||||
|
||||
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue