feat(research-loop-cap): give the discovery ceiling a reader, not just a sentence

The bounded-cost NFR asks for explicit ceilings on BOTH axes - max
conversation turns and max discovered dimensions. The turn axis got
MAX_CONV_TURNS, a ledger-backed reader and a PreToolUse enforcer. The
discovery axis got one sentence in Phase 4.5 prose ("append candidates only
while the whole list stays at or below maxDimensions: 8") with no constant
of its own, no reader, and no test that a run exceeding it is caught. That
is the brief_reviewer_iter_cap shape the operator decision warned about: a
cap nothing reads.

checkDimensionCeiling() is the reader, exposed on the CLI as
--check-dimensions N (exit 0 within, exit 1 rejected), and Phase 4.5 step 3
now calls it once the final list is settled instead of merely describing the
bound.

Three deliberate choices:

- The ceiling IS MAX_TOTAL_DIMENSIONS, the constant that sizes the turn
  budget. Both axes read one settings.json:16 value, so they cannot end up
  enforcing different numbers - a second constant is how that drift starts.
- An unreadable count is REJECTED ('abc', null, undefined, {}, -1, NaN,
  non-integers). A cost ceiling that waves through what it cannot measure is
  not a ceiling.
- --check-dimensions requires no run id, effort or VOYAGE_STORM_ENABLED.
  Phase 4.5 never calls the budget gate - that is why its skip-guard reads
  the flag directly - so the ceiling check must not inherit the gate's
  preconditions.

The mitigation the review already verified still holds and is unchanged:
MAX_TOTAL_DIMENSIONS bounds actual retrieval cost regardless of how many
dimensions discovery appends. What was missing was anything that FAILS on a
list over the bound, and now a run over it is rejected by exit code.

Review finding 96a3ee51152dfe72aca703f771843f2f3639e7b6 (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:09:11 +02:00
commit 84612b2641
4 changed files with 137 additions and 2 deletions

View file

@ -418,6 +418,23 @@ agent surfaced that no interview dimension claims.
raised here, so the documented 38 dimension range stays true and the raised here, so the documented 38 dimension range stays true and the
README prose about it stays untouched. If the interview already produced 8 README prose about it stays untouched. If the interview already produced 8
dimensions, this phase discovers nothing and says so. dimensions, this phase discovers nothing and says so.
**The ceiling has a reader — use it.** Once the final list is settled, run
the check below. Exit 1 means the list exceeded the ceiling: drop discovered
dimensions until it passes. Do not proceed to Phase 5 on a rejected list —
the turn budget is sized against this same ceiling, so a list over it spends
a budget that was never approved for it.
```bash
# Same VOYAGE_ROOT resolution as the per-turn protocol in Phase 5. Exit 0 =
# within the ceiling, exit 1 = rejected. JSON on stdout: {ok, count, ceiling, reason?}
node "$VOYAGE_ROOT/lib/util/research-loop-cap.mjs" --check-dimensions {final dimension count}
```
The ceiling constant is `MAX_TOTAL_DIMENSIONS` in
`lib/util/research-loop-cap.mjs` — deliberately the same constant that sizes
the Phase 5 turn budget, so the two axes of the bounded-cost NFR cannot end
up enforcing different numbers for one `settings.json:16` value.
4. **Record the baseline.** Keep the interview-derived count as 4. **Record the baseline.** Keep the interview-derived count as
`dimensions_baseline` so the discovered delta is machine-readable against `dimensions_baseline` so the discovered delta is machine-readable against
the final `dimensions` (Phase 8 stats). the final `dimensions` (Phase 8 stats).

View file

@ -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/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/{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/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. Each grant first claims a turn **slot** with `O_EXCL` under `trekresearch-loop-claims/`, so the bound survives several callers deciding at once — counting the ledger and then appending is read-then-write, and Phase 4.5/5 can spawn several agents in one message. Budget = `TREKRESEARCH_MAX_CONV_TURNS` (default `3`, invalid values fall back to `3`) × `maxDimensions` (8, `settings.json:16`). **Default-off:** 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/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. Each grant first claims a turn **slot** with `O_EXCL` under `trekresearch-loop-claims/`, so the bound survives several callers deciding at once — counting the ledger and then appending is read-then-write, and Phase 4.5/5 can spawn several agents in one message. Budget = `TREKRESEARCH_MAX_CONV_TURNS` (default `3`, invalid values fall back to `3`) × `maxDimensions` (8, `settings.json:16`). **Default-off:** 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. `checkDimensionCeiling()` is the reader for the OTHER axis of the bounded-cost NFR — the size of the whole dimension list after Phase 4.5 discovery — against the same `MAX_TOTAL_DIMENSIONS`, so the two axes cannot enforce different numbers for one `settings.json:16` value; an unreadable count is rejected, not waved through. CLI shim, two modes: `--run-id ID --dimension D --effort E` (budget gate) and `--check-dimensions N` (ceiling, no run id/effort/flag required since Phase 4.5 never calls the budget gate)
- `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>"` - `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): Wiring points (replaces previous prose-grep instructions):

View file

@ -29,9 +29,15 @@
// the legitimate first-turn state. This module is a budget control, not // the legitimate first-turn state. This module is a budget control, not
// telemetry — the opposite of lib/stats/event-emit.mjs's fail-open. // telemetry — the opposite of lib/stats/event-emit.mjs's fail-open.
// //
// CLI shim: // CLI shim, two modes:
// node lib/util/research-loop-cap.mjs --run-id ID --dimension D --effort E // node lib/util/research-loop-cap.mjs --run-id ID --dimension D --effort E
// → JSON: { ok, used, budget, reason? } (exit 0 = granted, exit 1 = denied) // → JSON: { ok, used, budget, reason? } (exit 0 = granted, exit 1 = denied)
//
// node lib/util/research-loop-cap.mjs --check-dimensions N
// → JSON: { ok, count, ceiling, reason? } (exit 0 = within, exit 1 = rejected)
// The second axis of the bounded-cost NFR. Phase 4.5 never calls the budget
// gate, so this mode requires no run id, effort or STORM flag — but it reads
// the SAME MAX_TOTAL_DIMENSIONS the budget is sized against.
import { existsSync, mkdirSync, appendFileSync, readFileSync, writeFileSync } from 'node:fs'; import { existsSync, mkdirSync, appendFileSync, readFileSync, writeFileSync } from 'node:fs';
import { dirname, join } from 'node:path'; import { dirname, join } from 'node:path';
@ -47,6 +53,40 @@ export function isStormEnabled(env = process.env) {
return env.VOYAGE_STORM_ENABLED === '1'; return env.VOYAGE_STORM_ENABLED === '1';
} }
/**
* The OTHER cost ceiling: how large the whole dimension list may get after
* Phase 4.5 discovery has appended to it.
*
* The bounded-cost NFR asks for explicit ceilings on both axes. The turn axis
* had a constant, a ledger-backed reader and a PreToolUse enforcer; the
* discovery axis had only a sentence in Phase 4.5 prose a cap nothing reads,
* which is the failure mode the operator decision on brief_reviewer_iter_cap
* warned about. This is the reader.
*
* The ceiling is MAX_TOTAL_DIMENSIONS on purpose: the value that sizes the turn
* budget IS settings.json:16's maxDimensions, and a second constant for the same
* number is how two readers end up enforcing different bounds.
*
* Accepts the dimension list or its count, because Phase 4.5 has the list and
* the CLI has a number. A count that cannot be read is REJECTED a cost ceiling
* that waves through what it cannot measure is not a ceiling.
*
* @param {string[]|number|string} dimensions
* @param {{ceiling?: number}} [opts]
* @returns {{ok: boolean, count: number|null, ceiling: number, reason?: string}}
*/
export function checkDimensionCeiling(dimensions, opts = {}) {
const ceiling = Number.isFinite(opts.ceiling) ? opts.ceiling : MAX_TOTAL_DIMENSIONS;
const count = Array.isArray(dimensions) ? dimensions.length : Number(dimensions);
if (dimensions === null || dimensions === undefined || !Number.isInteger(count) || count < 0) {
return { ok: false, count: null, ceiling, reason: 'unreadable_dimension_count' };
}
if (count > ceiling) {
return { ok: false, count, ceiling, reason: 'ceiling_exceeded' };
}
return { ok: true, count, ceiling };
}
/** /**
* Coerce TREKRESEARCH_MAX_CONV_TURNS. NaN, empty, negative, zero, Infinity, or * Coerce TREKRESEARCH_MAX_CONV_TURNS. NaN, empty, negative, zero, Infinity, or
* any fraction that floors below 1 all fall back to MAX_CONV_TURNS never to * any fraction that floors below 1 all fall back to MAX_CONV_TURNS never to
@ -280,12 +320,23 @@ function parseArgs(argv) {
if (a === '--run-id') out.runId = argv[++i]; if (a === '--run-id') out.runId = argv[++i];
else if (a === '--dimension') out.dimension = argv[++i]; else if (a === '--dimension') out.dimension = argv[++i];
else if (a === '--effort') out.effort = argv[++i]; else if (a === '--effort') out.effort = argv[++i];
else if (a === '--check-dimensions') out.checkDimensions = argv[++i];
} }
return out; return out;
} }
if (import.meta.url === `file://${process.argv[1]}`) { if (import.meta.url === `file://${process.argv[1]}`) {
const args = parseArgs(process.argv.slice(2)); const args = parseArgs(process.argv.slice(2));
// The dimension ceiling is a Phase 4.5 concern, and Phase 4.5 never calls the
// budget gate — so this branch must not inherit the gate's preconditions
// (run id, effort, STORM flag). It is a pure bound on list size.
if (args.checkDimensions !== undefined) {
const result = checkDimensionCeiling(args.checkDimensions);
process.stdout.write(JSON.stringify(result) + '\n');
process.exit(result.ok ? 0 : 1);
}
if (!args.runId || !args.dimension || !args.effort) { if (!args.runId || !args.dimension || !args.effort) {
process.stdout.write(JSON.stringify({ process.stdout.write(JSON.stringify({
ok: false, ok: false,

View file

@ -18,6 +18,7 @@ import {
resolveLedgerPath, resolveLedgerPath,
resolveDataRoot, resolveDataRoot,
readLedger, readLedger,
checkDimensionCeiling,
MAX_CONV_TURNS, MAX_CONV_TURNS,
MAX_TOTAL_DIMENSIONS, MAX_TOTAL_DIMENSIONS,
} from '../../lib/util/research-loop-cap.mjs'; } from '../../lib/util/research-loop-cap.mjs';
@ -486,6 +487,72 @@ test('readLedger — a tombstone is reported separately and never as a granted t
}); });
}); });
// ---- the discovery ceiling has a reader, not just a sentence ----------------
//
// The bounded-cost NFR asks for explicit ceilings on BOTH axes: max conversation
// turns and max discovered dimensions. The turn axis got MAX_CONV_TURNS, a
// ledger-backed reader and a PreToolUse enforcer. The discovery axis got a
// sentence in Phase 4.5 — "append candidates only while the whole list stays at
// or below maxDimensions: 8" — with no constant of its own, no reader, and no
// test that a run exceeding it is caught. That is the same shape as the
// brief_reviewer_iter_cap failure the operator decision warned about: a cap
// nothing reads.
//
// The ceiling is deliberately the SAME constant that sizes the turn budget. Two
// constants for one settings.json:16 value is how the two drift apart.
test('checkDimensionCeiling — a list at the ceiling is accepted', () => {
const r = checkDimensionCeiling(Array.from({ length: MAX_TOTAL_DIMENSIONS }, (_, i) => `d${i}`));
assert.equal(r.ok, true);
assert.equal(r.count, MAX_TOTAL_DIMENSIONS);
assert.equal(r.ceiling, MAX_TOTAL_DIMENSIONS);
});
test('checkDimensionCeiling — one dimension over the ceiling is REJECTED', () => {
const r = checkDimensionCeiling(Array.from({ length: MAX_TOTAL_DIMENSIONS + 1 }, (_, i) => `d${i}`));
assert.equal(r.ok, false, 'a ceiling that accepts ceiling+1 is not a ceiling');
assert.equal(r.reason, 'ceiling_exceeded');
assert.equal(r.count, MAX_TOTAL_DIMENSIONS + 1);
});
test('checkDimensionCeiling — a plain count works as well as a list', () => {
assert.equal(checkDimensionCeiling(8).ok, true);
assert.equal(checkDimensionCeiling(9).ok, false);
assert.equal(checkDimensionCeiling('8').ok, true);
});
test('checkDimensionCeiling — an unreadable count is rejected, never waved through', () => {
for (const bad of ['abc', null, undefined, {}, -1, NaN]) {
const r = checkDimensionCeiling(bad);
assert.equal(r.ok, false, `${JSON.stringify(bad)} must not pass a cost ceiling`);
assert.equal(r.reason, 'unreadable_dimension_count');
}
});
test('checkDimensionCeiling — the ceiling is the same constant that sizes the turn budget', () => {
// Phase 4.5 and the Phase 5 budget must not be able to disagree about 8.
assert.equal(checkDimensionCeiling(0).ceiling, MAX_TOTAL_DIMENSIONS);
});
test('CLI shim — --check-dimensions exits 0 at the ceiling and 1 above it', () => {
const at = runShim(['--check-dimensions', String(MAX_TOTAL_DIMENSIONS)], {});
assert.equal(at.code, 0, `at the ceiling must exit 0; got ${at.out}`);
assert.equal(JSON.parse(at.out.trim()).ok, true);
const over = runShim(['--check-dimensions', String(MAX_TOTAL_DIMENSIONS + 1)], {});
assert.equal(over.code, 1, 'a run over the ceiling must be rejected by exit code, not by prose');
const parsed = JSON.parse(over.out.trim());
assert.equal(parsed.ok, false);
assert.equal(parsed.reason, 'ceiling_exceeded');
});
test('CLI shim — --check-dimensions needs no runId, effort or STORM flag', () => {
// It is a cost ceiling on Phase 4.5, which never calls the budget gate, so it
// must not inherit the budget gate's preconditions.
const r = runShimStripped(['--check-dimensions', '3'], {});
assert.equal(r.code, 0, `should not require --run-id/--effort; got ${r.out}`);
});
// ---- (g) shim contract -------------------------------------------------------- // ---- (g) shim contract --------------------------------------------------------
test('CLI shim — grants and exits 0 when enabled + high effort + budget available', () => { test('CLI shim — grants and exits 0 when enabled + high effort + budget available', () => {