fix(cap-hook): shrink the inherited deny window and print the way out of it

DEFAULT_TTL_MS was 6h, measured from marker.startedAt rather than last
activity, and `claude --resume` keeps the same session_id - so a run that
died holding its marker handed the resumed session the remainder of that
window, denying every WebSearch/WebFetch/Task including work unrelated to
research. The header's design goal ("An unrelated session must never be
denied") held across sessions and read as broader than it was: by this
scope key a resume IS the same session.

Three changes, none of which pretends to close it:

- The tombstone boundary (32e20fc) already removed the common case. A run
  that crashed MID-loop leaves no denial record, so the resume is allowed;
  only a crash AFTER the cap denied a turn opens a window at all. Pinned by
  a test with a part-spent ledger and no tombstone.
- TTL 6h -> 2h. A 24-turn loop at a couple of minutes per turn is under an
  hour, so nothing needed six, and debris no longer owns the rest of the
  working day.
- Every denial now prints the marker path with "if this loop is not
  running, delete it", plus the auto-reset horizon. The window existed
  before with no stated remedy, which is what made it expensive.

A liveness check would close it properly. This hook has nothing
trustworthy to check liveness against - the marker's writer is a shell
snippet whose $$ is a subshell, not the session - so the limit is written
into the header as a limit instead of being papered over.

Review finding d913d1b655012fe206ea925b9fc77b401566a39e (MINOR).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LuGhWAbWyRFBFeemfhxoVv
This commit is contained in:
Kjell Tore Guttormsen 2026-08-12 23:06:47 +02:00
commit 22cb7df403
4 changed files with 80 additions and 3 deletions

View file

@ -510,7 +510,10 @@ through a blocked tool call. Once denied, the hook keeps denying for as long as
the marker is there — including Phase 6, which spawns agents. A marker that outlives the loop turns a bound on this loop into a brick the marker is there — including Phase 6, which spawns agents. A marker that outlives the loop turns a bound on this loop into a brick
on the rest of the session. Cleanup covers the three exits and nothing else: a on the rest of the session. Cleanup covers the three exits and nothing else: a
crashed session runs no cleanup at all, and is covered instead by the hook's crashed session runs no cleanup at all, and is covered instead by the hook's
TTL (default 6h, `VOYAGE_CAP_SCOPE_TTL_MS`), which auto-resets a stale marker. TTL (default 2h, `VOYAGE_CAP_SCOPE_TTL_MS`), which auto-resets a stale marker.
A crash mid-loop leaves no denial record, so a resumed session is not blocked by
it; only a crash AFTER the cap denied a turn hands the resume a deny window, and
every denial prints the marker path to delete.
```bash ```bash
# Removal — idempotent, safe to repeat. Same root, same absolute-path guard as # Removal — idempotent, safe to repeat. Same root, same absolute-path guard as

View file

@ -135,7 +135,7 @@ Fail-open and fail-closed are split deliberately:
| Condition | Outcome | Why | | Condition | Outcome | Why |
|---|---|---| |---|---|---|
| No marker / no `session_id` / unparsable stdin | allow | Not evidence of a loop turn | | No marker / no `session_id` / unparsable stdin | allow | Not evidence of a loop turn |
| Marker older than TTL (default 6h, `VOYAGE_CAP_SCOPE_TTL_MS`) | allow + auto-reset | A crashed run must not deny tool calls forever | | Marker older than TTL (default 2h, `VOYAGE_CAP_SCOPE_TTL_MS`) | allow + auto-reset | A crashed run must not deny tool calls forever, and `--resume` keeps the same `session_id` |
| `VOYAGE_DISABLE_CAP_HOOK=1` | allow | Kill switch | | `VOYAGE_DISABLE_CAP_HOOK=1` | allow | Kill switch |
| `VOYAGE_STORM_ENABLED``1` | allow | Default-off: no loop runs, nothing to enforce | | `VOYAGE_STORM_ENABLED``1` | allow | Default-off: no loop runs, nothing to enforce |
| In scope, `CLAUDE_PLUGIN_DATA` absent | **deny** | A budget control that cannot count must not grant — same stance as `research-loop-cap.mjs` | | In scope, `CLAUDE_PLUGIN_DATA` absent | **deny** | A budget control that cannot count must not grant — same stance as `research-loop-cap.mjs` |

View file

@ -29,6 +29,18 @@
// unrelated session must never be denied because some other run spent its // unrelated session must never be denied because some other run spent its
// budget; a PreToolUse hook that over-blocks breaks every session on the box. // budget; a PreToolUse hook that over-blocks breaks every session on the box.
// //
// Stated limit, because the guarantee above is about OTHER sessions and reads
// as broader than it is: `claude --resume` keeps the same session_id, so a
// resumed session is the same session by this key. If a run reached its cap
// and then died before removing the marker, the resume inherits the remainder
// of the TTL, for any WebSearch/WebFetch/Task — research or not. Three things
// bound it rather than close it: only an EXHAUSTED run denies at all (a
// part-spent crash leaves no tombstone and is allowed), the TTL is 2h rather
// than a working day, and every denial prints the marker path to delete. A
// liveness check would close it properly, but a PreToolUse hook has nothing
// trustworthy to check liveness against — the marker's writer is a shell
// snippet whose $$ is a subshell, not the session.
//
// Fail-open vs fail-closed, deliberately split: // Fail-open vs fail-closed, deliberately split:
// - Out of scope (no marker, no session_id, unparsable stdin, stale marker, // - Out of scope (no marker, no session_id, unparsable stdin, stale marker,
// kill switch, STORM off) => exit 0. Fail OPEN. // kill switch, STORM off) => exit 0. Fail OPEN.
@ -58,7 +70,13 @@ const { resolveLedgerPath, resolveDataRoot, resolveMaxConvTurns, isStormEnabled,
await import(join(HERE, '..', '..', 'lib', 'util', 'research-loop-cap.mjs')); await import(join(HERE, '..', '..', 'lib', 'util', 'research-loop-cap.mjs'));
const SCOPE_DIRNAME = 'trekresearch-loop-scope'; const SCOPE_DIRNAME = 'trekresearch-loop-scope';
const DEFAULT_TTL_MS = 6 * 60 * 60 * 1000; // 6h — longer than any real research run // 2h — comfortably longer than any real research run (a 24-turn loop at a couple
// of minutes a turn is under an hour), and short enough that debris does not own
// the rest of the working day. The TTL is measured from marker.startedAt rather
// than from last activity, and `claude --resume` keeps the same session_id, so
// this window is what a resumed session can inherit from a run that died holding
// the marker. It was 6h; nothing needed six.
const DEFAULT_TTL_MS = 2 * 60 * 60 * 1000;
const env = process.env; const env = process.env;
@ -143,6 +161,16 @@ try {
const toolLine = const toolLine =
` Tool: ${input?.tool_name ?? 'unknown'}${input?.agent_type ? ` (agent: ${input.agent_type})` : ''}\n`; ` Tool: ${input?.tool_name ?? 'unknown'}${input?.agent_type ? ` (agent: ${input.agent_type})` : ''}\n`;
// Every denial names the marker. If this run is over and the marker outlived it
// — the loop's own cleanup covers its three exits, but a crash between the
// exhaustion record and the removal runs no cleanup at all — deleting this file
// is the remedy, and a resumed session (same session_id) would otherwise sit out
// the remaining TTL for work that has nothing to do with research.
const remedyLines =
` If this loop is not running, the marker is debris — delete it:\n` +
` ${markerPath}\n` +
` It also auto-resets ${Math.round(ttlMs / 3600000)}h after the run started (VOYAGE_CAP_SCOPE_TTL_MS).\n`;
// 8. The boundary is the TOMBSTONE, not the count. // 8. The boundary is the TOMBSTONE, not the count.
// //
// allowTurn() appends before the turn runs, so during the final granted turn the // allowTurn() appends before the turn runs, so during the final granted turn the
@ -164,6 +192,7 @@ if (ledger.exhausted > 0) {
` (${ledger.granted}/${budget} loop turns spent), and this call came after it.\n` + ` (${ledger.granted}/${budget} loop turns spent), and this call came after it.\n` +
toolLine + toolLine +
` Remaining gaps belong in the brief as open questions, not in another turn.\n` + ` Remaining gaps belong in the brief as open questions, not in another turn.\n` +
remedyLines +
` Raise TREKRESEARCH_MAX_CONV_TURNS deliberately, or set VOYAGE_DISABLE_CAP_HOOK=1.`, ` Raise TREKRESEARCH_MAX_CONV_TURNS deliberately, or set VOYAGE_DISABLE_CAP_HOOK=1.`,
); );
} }
@ -174,6 +203,7 @@ if (ledger.granted > budget) {
` Run ${marker.runId} shows ${ledger.granted} granted turns against a budget of ${budget}.\n` + ` Run ${marker.runId} shows ${ledger.granted} granted turns against a budget of ${budget}.\n` +
toolLine + toolLine +
` The ledger has been breached; the loop is over regardless of cause.\n` + ` The ledger has been breached; the loop is over regardless of cause.\n` +
remedyLines +
` Raise TREKRESEARCH_MAX_CONV_TURNS deliberately, or set VOYAGE_DISABLE_CAP_HOOK=1.`, ` Raise TREKRESEARCH_MAX_CONV_TURNS deliberately, or set VOYAGE_DISABLE_CAP_HOOK=1.`,
); );
} }

View file

@ -177,6 +177,50 @@ test('pre-agent-cap ALLOWS when the scope marker is older than the TTL', async (
assert.strictEqual(code, 0, 'a stale marker must auto-reset, not deny forever'); assert.strictEqual(code, 0, 'a stale marker must auto-reset, not deny forever');
}); });
// The TTL runs from marker.startedAt, not from last activity, and `claude
// --resume` keeps the same session_id — so a run that died leaving its marker
// behind hands the resumed session whatever deny window is left. Two things
// bound that: the window is hours, not the machine's life (below), and it only
// opens at all once the budget gate has actually denied a turn.
test('pre-agent-cap ALLOWS a resumed session whose crashed run never exhausted its budget', async () => {
// Marker still fresh, ledger part-spent, no tombstone: the run died mid-loop.
const dir = fixture({ turns: 5 });
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
...CAPPED_ENV,
CLAUDE_PLUGIN_DATA: dir,
});
assert.strictEqual(
code, 0,
'a part-spent run leaves no denial record, so resuming its session must not brick unrelated work',
);
});
test('pre-agent-cap uses a default TTL of hours, not a day — a 3h-old marker auto-resets', async () => {
const dir = fixture({
turns: BUDGET,
exhausted: true,
startedAt: new Date(Date.now() - 3 * 3600 * 1000),
});
const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
...CAPPED_ENV,
CLAUDE_PLUGIN_DATA: dir,
}); // no VOYAGE_CAP_SCOPE_TTL_MS — this is the built-in default
assert.strictEqual(code, 0, 'no real research run lasts 3h; a marker that old is debris');
});
test('pre-agent-cap names the marker path when it denies, so the operator has a remedy', async () => {
const dir = fixture({ turns: BUDGET, exhausted: true });
const { code, stderr } = await runHookWithEnv(CAP_HOOK, searchInput(), {
...CAPPED_ENV,
CLAUDE_PLUGIN_DATA: dir,
});
assert.strictEqual(code, 2);
assert.ok(
stderr.includes(join(dir, 'trekresearch-loop-scope', `${SESSION}.json`)),
`stderr must name the marker to delete; got:\n${stderr}`,
);
});
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// Fail CLOSED once in scope — the hook's own header says a budget control // Fail CLOSED once in scope — the hook's own header says a budget control
// that cannot count must not grant. The unreadable-ledger branch returned 0 // that cannot count must not grant. The unreadable-ledger branch returned 0