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:
parent
2844400a00
commit
8c60d2079e
4 changed files with 80 additions and 3 deletions
|
|
@ -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
|
||||
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
|
||||
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
|
||||
# Removal — idempotent, safe to repeat. Same root, same absolute-path guard as
|
||||
|
|
|
|||
|
|
@ -135,7 +135,7 @@ Fail-open and fail-closed are split deliberately:
|
|||
| Condition | Outcome | Why |
|
||||
|---|---|---|
|
||||
| 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_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` |
|
||||
|
|
|
|||
|
|
@ -29,6 +29,18 @@
|
|||
// 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.
|
||||
//
|
||||
// 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:
|
||||
// - Out of scope (no marker, no session_id, unparsable stdin, stale marker,
|
||||
// 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'));
|
||||
|
||||
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;
|
||||
|
||||
|
|
@ -143,6 +161,16 @@ try {
|
|||
const toolLine =
|
||||
` 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.
|
||||
//
|
||||
// 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` +
|
||||
toolLine +
|
||||
` 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.`,
|
||||
);
|
||||
}
|
||||
|
|
@ -174,6 +203,7 @@ if (ledger.granted > budget) {
|
|||
` Run ${marker.runId} shows ${ledger.granted} granted turns against a budget of ${budget}.\n` +
|
||||
toolLine +
|
||||
` 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.`,
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
|
||||
// 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
|
||||
// that cannot count must not grant. The unreadable-ledger branch returned 0
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue