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:
parent
8c60d2079e
commit
7dc0add768
4 changed files with 137 additions and 2 deletions
|
|
@ -418,6 +418,23 @@ agent surfaced that no interview dimension claims.
|
|||
raised here, so the documented 3–8 dimension range stays true and the
|
||||
README prose about it stays untouched. If the interview already produced 8
|
||||
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
|
||||
`dimensions_baseline` so the discovered delta is machine-readable against
|
||||
the final `dimensions` (Phase 8 stats).
|
||||
|
|
|
|||
|
|
@ -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. 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>"`
|
||||
|
||||
Wiring points (replaces previous prose-grep instructions):
|
||||
|
|
|
|||
|
|
@ -29,9 +29,15 @@
|
|||
// 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:
|
||||
// CLI shim, two modes:
|
||||
// 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)
|
||||
//
|
||||
// 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 { dirname, join } from 'node:path';
|
||||
|
|
@ -47,6 +53,40 @@ export function isStormEnabled(env = process.env) {
|
|||
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
|
||||
* 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];
|
||||
else if (a === '--dimension') out.dimension = argv[++i];
|
||||
else if (a === '--effort') out.effort = argv[++i];
|
||||
else if (a === '--check-dimensions') out.checkDimensions = argv[++i];
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||
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) {
|
||||
process.stdout.write(JSON.stringify({
|
||||
ok: false,
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import {
|
|||
resolveLedgerPath,
|
||||
resolveDataRoot,
|
||||
readLedger,
|
||||
checkDimensionCeiling,
|
||||
MAX_CONV_TURNS,
|
||||
MAX_TOTAL_DIMENSIONS,
|
||||
} 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 --------------------------------------------------------
|
||||
|
||||
test('CLI shim — grants and exits 0 when enabled + high effort + budget available', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue