fix(cap): fail closed when the ledger cannot be READ, in both modules

An unreadable ledger returned 0 from countTurns in BOTH the primitive and
the hook, so a run whose ledger existed but could not be read (EISDIR,
EACCES, EIO) was handed the full budget again on every call - unbounded.
research-loop-cap.mjs argues against exactly that three lines above the
code that did it, and its missing-DIRECTORY case already failed closed.
The unreadable-FILE case now agrees with it.

Only ENOENT still counts as zero turns spent: that is the legitimate
first-turn state, and the reason this cannot just throw on any read
failure.

The hook no longer carries its own countTurns. It imports the primitive's
exported readLedger(), the same way it already resolves the data root
through resolveDataRoot() - a reader and a writer with private copies of
the counting rule is how a hook ends up enforcing a different bound than
the gate it backs. In scope + cannot count now exits 2 with a message
that says counting failed, not that the budget is spent.

Fail-closed stays scoped to the loop: a test pins that an unreadable
ledger in an OUT-of-scope session still exits 0, because a PreToolUse
hook that over-blocks bricks every session on the box.

Also dropped the existsSync pre-check before the read - readFileSync's
own ENOENT carries the same information without a second syscall that
can disagree with the read that follows it.

Review finding 5e1c6230f48ead38fa77cd8f4b06bfdc2b5b7bbf (MINOR).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LuGhWAbWyRFBFeemfhxoVv

docs/architecture.md

@@ -12,7 +12,7 @@ Imported from `CLAUDE.md` via pointer.
 - `lib/stats/event-emit.mjs` — single-source stats event emitter for autonomy-gate transitions and main-merge-gate (v3.4.0)
 - `lib/validators/{brief,research,plan,progress,session-state}-validator.mjs` — schema validators with CLI shims (`node lib/validators/X.mjs --json <path>`)
 - `lib/validators/architecture-discovery.mjs` — drift-WARN external-contract discovery for `architecture/overview.md`
-- `lib/util/research-loop-cap.mjs` — stateful, **default-off** turn budget for the `/trekresearch` bounded conversation loop. `allowTurn()` derives the used-turn count from its own append-only JSONL ledger; it never asks the caller how many turns it has spent, because a cap that does is not a cap. Budget = `TREKRESEARCH_MAX_CONV_TURNS` (default `3`, invalid values fall back to `3`) × `maxDimensions` (8, `settings.json:16`). Grants 0 unless `VOYAGE_STORM_ENABLED=1`, which is also the second condition on Phase 4.5's skip-guard (that phase does not call this module): unset, **both** STORM phases are inert. `resolveDataRoot()` is the single root for everything the loop writes — `CLAUDE_PLUGIN_DATA` when the harness sets it, `~/.claude/voyage` when it does not (it is empty in the Bash tool's process env, which is where the loop actually runs); the cap hook resolves through the same function, so writer and reader cannot disagree. A ledger that cannot be **written** still denies the turn (fail-closed — the opposite of `event-emit.mjs`, which is telemetry and must never block). CLI shim: `node lib/util/research-loop-cap.mjs --run-id ID --dimension D --effort E`
+- `lib/util/research-loop-cap.mjs` — stateful, **default-off** turn budget for the `/trekresearch` bounded conversation loop. `allowTurn()` derives the used-turn count from its own append-only JSONL ledger; it never asks the caller how many turns it has spent, because a cap that does is not a cap. Budget = `TREKRESEARCH_MAX_CONV_TURNS` (default `3`, invalid values fall back to `3`) × `maxDimensions` (8, `settings.json:16`). Grants 0 unless `VOYAGE_STORM_ENABLED=1`, which is also the second condition on Phase 4.5's skip-guard (that phase does not call this module): unset, **both** STORM phases are inert. `resolveDataRoot()` is the single root for everything the loop writes — `CLAUDE_PLUGIN_DATA` when the harness sets it, `~/.claude/voyage` when it does not (it is empty in the Bash tool's process env, which is where the loop actually runs); the cap hook resolves through the same function, so writer and reader cannot disagree. A ledger that cannot be **written** denies the turn, and one that exists but cannot be **read** denies it too — only `ENOENT` counts as zero turns spent, that being the legitimate first-turn state (fail-closed — the opposite of `event-emit.mjs`, which is telemetry and must never block). The exported `readLedger()` is the single counting rule; the cap hook calls it rather than keeping a private copy. CLI shim: `node lib/util/research-loop-cap.mjs --run-id ID --dimension D --effort E`
 - `lib/validators/query-privacy-gate.mjs` — gates **every** outbound research query before it leaves the machine; the hard-block tier (secret-shaped strings) is not operator-overridable, so a query that trips it must be reformulated rather than forced through. CLI shim: `node lib/validators/query-privacy-gate.mjs "<query>"`
 
 Wiring points (replaces previous prose-grep instructions):

hooks/scripts/pre-agent-cap.mjs

@@ -36,6 +36,12 @@
 //     research-loop-cap.mjs's own stance: a budget control that cannot count
 //     must not grant. (The former "CLAUDE_PLUGIN_DATA absent" deny is gone —
 //     the root now always resolves, so that branch could no longer fire.)
+//   - In scope and the ledger cannot be counted (EISDIR, EACCES, EIO — anything
+//     but ENOENT) => exit 2, same reason. This branch used to ALLOW: the hook
+//     carried a private countTurns() whose catch returned 0, so an unreadable
+//     ledger read as "no turns spent". Counting now goes through the
+//     primitive's exported readLedger(), so reader and writer cannot hold
+//     different rules about what an unreadable ledger means.
 //
 // Counting is read-only. The ledger is append-only and written solely by
 // research-loop-cap.mjs's allowTurn(); if this hook appended, the cap would
@@ -48,7 +54,7 @@ import { join, dirname } from 'node:path';
 import { fileURLToPath } from 'node:url';
 
 const HERE = dirname(fileURLToPath(import.meta.url));
-const { resolveLedgerPath, resolveDataRoot, resolveMaxConvTurns, isStormEnabled, MAX_TOTAL_DIMENSIONS } =
+const { resolveLedgerPath, resolveDataRoot, resolveMaxConvTurns, isStormEnabled, readLedger, MAX_TOTAL_DIMENSIONS } =
   await import(join(HERE, '..', '..', 'lib', 'util', 'research-loop-cap.mjs'));
 
 const SCOPE_DIRNAME = 'trekresearch-loop-scope';
@@ -111,27 +117,28 @@ if (!Number.isFinite(startedAt) || Date.now() - startedAt > ttlMs) {
 
 // --- In scope from here on. ---
 
-// 6. The ledger is the only source of truth for turns spent.
+// 6. The ledger is the only source of truth for turns spent, and it is counted
+//    through the primitive's OWN readLedger(). This hook used to carry a
+//    private copy of the counting rule whose read error returned 0 — so an
+//    unreadable ledger read as "no turns spent" and ALLOWED, in the one branch
+//    where this hook is supposed to fail closed.
 const ledgerPath = resolveLedgerPath(env);
 
-function countTurns(path, runId) {
-  if (!existsSync(path)) return 0;
-  let text;
-  try { text = readFileSync(path, 'utf-8'); } catch { return 0; }
-  let count = 0;
-  for (const line of text.split('\n')) {
-    if (!line) continue;
-    try {
-      if (JSON.parse(line).runId === runId) count++;
-    } catch { /* skip malformed lines */ }
-  }
-  return count;
-}
-
 // 7. Same bound the primitive uses: turns-per-dimension × the whole dimension
 //    list under settings.json:16's maxDimensions ceiling.
 const budget = resolveMaxConvTurns(env) * MAX_TOTAL_DIMENSIONS;
-const used = countTurns(ledgerPath, marker.runId);
+
+let used;
+try {
+  used = readLedger(ledgerPath, marker.runId).granted;
+} catch (e) {
+  deny(
+    `  Run ${marker.runId} is in scope, but its turn ledger could not be read:\n` +
+    `    ${e.message}\n` +
+    `  A budget control that cannot count must not grant. Fix or remove the\n` +
+    `  ledger, or set VOYAGE_DISABLE_CAP_HOOK=1 to disable enforcement.`,
+  );
+}
 
 if (used >= budget) {
   deny(

lib/util/research-loop-cap.mjs

@@ -21,9 +21,11 @@
 // pre-agent-cap.mjs resolves it through the SAME function, so the writer and
 // the reader can never disagree about where the ledger lives.
 //
-// The fail-closed stance stays where it is still real: a ledger that cannot be
-// written denies the turn. This module is a budget control, not telemetry —
-// the opposite of lib/stats/event-emit.mjs's fail-open.
+// The fail-closed stance covers both directions of ledger IO: a ledger that
+// cannot be WRITTEN denies the turn, and a ledger that exists but cannot be
+// READ denies it too. Only ENOENT counts as zero turns spent, because that is
+// the legitimate first-turn state. This module is a budget control, not
+// telemetry — the opposite of lib/stats/event-emit.mjs's fail-open.
 //
 // CLI shim:
 //   node lib/util/research-loop-cap.mjs --run-id ID --dimension D --effort E
@@ -78,20 +80,48 @@ export function resolveLedgerPath(env = process.env) {
   return join(resolveDataRoot(env), LEDGER_FILENAME);
 }
 
-function countTurns(ledgerPath, runId) {
-  if (!existsSync(ledgerPath)) return 0;
+/**
+ * Read one run's turn count off the append-only ledger.
+ *
+ * ENOENT is 0 turns spent — the legitimate first-turn state, and the reason
+ * this cannot simply throw on every read failure. Every OTHER read error
+ * (EISDIR, EACCES, EIO) THROWS, because returning 0 from an unreadable ledger
+ * re-granted the full budget on every call: unbounded, and the exact
+ * silently-grant-unlimited failure this module's header argues against three
+ * lines above the code that did it. The missing-directory case already failed
+ * closed; this makes the unreadable-file case agree with it.
+ *
+ * The `existsSync` pre-check is deliberately gone: readFileSync's own ENOENT
+ * carries the same information without a second syscall that can disagree with
+ * the read that follows it.
+ *
+ * Exported so hooks/scripts/pre-agent-cap.mjs counts through this exact
+ * function. A reader and a writer with private copies of the counting rule are
+ * how a hook ends up enforcing a different bound than the gate it backs.
+ *
+ * @param {string} ledgerPath
+ * @param {string} runId
+ * @returns {{granted: number}}
+ * @throws when the ledger exists but cannot be read
+ */
+export function readLedger(ledgerPath, runId) {
   let text;
-  try { text = readFileSync(ledgerPath, 'utf-8'); }
-  catch { return 0; }
-  let count = 0;
+  try {
+    text = readFileSync(ledgerPath, 'utf-8');
+  } catch (e) {
+    if (e && e.code === 'ENOENT') return { granted: 0 };
+    const err = new Error(`ledger unreadable at ${ledgerPath}: ${e.message}`);
+    err.code = 'VOYAGE_LEDGER_UNREADABLE';
+    throw err;
+  }
+  let granted = 0;
   for (const line of text.split('\n')) {
     if (!line) continue;
     try {
-      const rec = JSON.parse(line);
-      if (rec.runId === runId) count++;
+      if (JSON.parse(line).runId === runId) granted++;
     } catch { /* skip malformed lines */ }
   }
-  return count;
+  return { granted };
 }
 
 /**
@@ -122,7 +152,12 @@ export function allowTurn({ runId, dimension, effort } = {}, opts = {}) {
   const budget = maxConvTurns * MAX_TOTAL_DIMENSIONS;
 
   const ledgerPath = resolveLedgerPath(env);
-  const used = countTurns(ledgerPath, runId);
+  let used;
+  try {
+    used = readLedger(ledgerPath, runId).granted;
+  } catch (e) {
+    return { ok: false, used: 0, budget, reason: `ledger-read-failed: ${e.message}` };
+  }
   if (used >= budget) {
     return { ok: false, used, budget, reason: 'budget_exhausted' };
   }

tests/hooks/agent-cap.test.mjs

@@ -17,7 +17,7 @@ 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 } from 'node:fs';
+import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { runHookWithEnv } from '../helpers/hook-helper.mjs';
 
@@ -153,6 +153,37 @@ 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');
 });
 
+// -----------------------------------------------------------------------
+// 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
+// and therefore ALLOWED, which is the opposite. A directory standing where
+// the ledger file belongs reproduces it portably (EISDIR).
+// -----------------------------------------------------------------------
+test('pre-agent-cap DENIES when the ledger cannot be read at all (fail closed)', async () => {
+  const dir = fixture({ turns: 0 });
+  const ledgerPath = join(dir, 'trekresearch-loop-ledger.jsonl');
+  rmSync(ledgerPath, { force: true });
+  mkdirSync(ledgerPath, { recursive: true });
+  const { code, stderr } = await runHookWithEnv(CAP_HOOK, searchInput(), {
+    ...CAPPED_ENV,
+    CLAUDE_PLUGIN_DATA: dir,
+  });
+  assert.strictEqual(code, 2, 'an in-scope run whose ledger cannot be counted must not be granted');
+  assert.match(stderr, /could not be read|unreadable/i, 'stderr must say counting failed, not that the budget is spent');
+});
+
+test('pre-agent-cap ALLOWS an unreadable ledger when the session is OUT of scope', async () => {
+  const dir = fixture({ turns: 0, sessionId: 'a-different-session' });
+  const ledgerPath = join(dir, 'trekresearch-loop-ledger.jsonl');
+  rmSync(ledgerPath, { force: true });
+  mkdirSync(ledgerPath, { recursive: true });
+  const { code } = await runHookWithEnv(CAP_HOOK, searchInput(), {
+    ...CAPPED_ENV,
+    CLAUDE_PLUGIN_DATA: dir,
+  });
+  assert.strictEqual(code, 0, 'fail-closed is scoped to the loop, it must not brick unrelated sessions');
+});
+
 // -----------------------------------------------------------------------
 // Kill switch + default-off
 // -----------------------------------------------------------------------

tests/lib/research-loop-cap.test.mjs

@@ -7,7 +7,7 @@
 import { test } from 'node:test';
 import { strict as assert } from 'node:assert';
 import { execFileSync } from 'node:child_process';
-import { mkdtempSync, rmSync, existsSync } from 'node:fs';
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync, existsSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { dirname, join } from 'node:path';
 import { fileURLToPath } from 'node:url';
@@ -17,6 +17,7 @@ import {
   resolveMaxConvTurns,
   resolveLedgerPath,
   resolveDataRoot,
+  readLedger,
   MAX_CONV_TURNS,
   MAX_TOTAL_DIMENSIONS,
 } from '../../lib/util/research-loop-cap.mjs';
@@ -268,6 +269,61 @@ test('allowTurn — missing runId or dimension denies with missing_args', () =>
   });
 });
 
+// ---- fail-closed on an unreadable ledger ------------------------------------
+//
+// The module's own header states that a budget control "must never silently
+// grant unlimited turns just because the data dir is missing". The
+// missing-DIRECTORY case already failed closed; the unreadable-FILE case
+// returned 0 from countTurns and therefore re-granted the full budget on every
+// call, unbounded — a fail-open in the same module that argues against one.
+// A directory standing where the ledger file belongs reproduces it portably
+// (EISDIR), with no chmod that a root test runner would ignore.
+
+function withUnreadableLedger(fn) {
+  return withTmpDataDir((dir) => {
+    mkdirSync(join(dir, 'trekresearch-loop-ledger.jsonl'), { recursive: true });
+    return fn(dir);
+  });
+}
+
+test('readLedger — a missing ledger is 0 turns, not an error (turn 1 must be grantable)', () => {
+  withTmpDataDir((dir) => {
+    assert.equal(readLedger(join(dir, 'nope.jsonl'), 'r1').granted, 0);
+  });
+});
+
+test('readLedger — an unreadable ledger throws rather than reporting 0 turns spent', () => {
+  withUnreadableLedger((dir) => {
+    assert.throws(
+      () => readLedger(join(dir, 'trekresearch-loop-ledger.jsonl'), 'r1'),
+      /unreadable/i,
+    );
+  });
+});
+
+test('readLedger — malformed lines are skipped, well-formed ones for the run still count', () => {
+  withTmpDataDir((dir) => {
+    const p = join(dir, 'trekresearch-loop-ledger.jsonl');
+    writeFileSync(p, [
+      JSON.stringify({ runId: 'r1', dimension: 'd1' }),
+      '{ not json',
+      '',
+      JSON.stringify({ runId: 'other', dimension: 'd1' }),
+      JSON.stringify({ runId: 'r1', dimension: 'd2' }),
+    ].join('\n') + '\n');
+    assert.equal(readLedger(p, 'r1').granted, 2);
+  });
+});
+
+test('allowTurn — an unreadable ledger DENIES the turn instead of granting a fresh budget', () => {
+  withUnreadableLedger((dir) => {
+    const env = { CLAUDE_PLUGIN_DATA: dir, VOYAGE_STORM_ENABLED: '1' };
+    const r = allowTurn({ runId: 'r-unreadable', dimension: 'd1', effort: 'high' }, { env });
+    assert.equal(r.ok, false, 'a budget control that cannot count must not grant');
+    assert.match(r.reason, /ledger-read-failed/);
+  });
+});
+
 // ---- (g) shim contract --------------------------------------------------------
 
 test('CLI shim — grants and exits 0 when enabled + high effort + budget available', () => {