From 664d01cc3c0b6614f2286e60a746a92336f03325 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Mon, 3 Aug 2026 19:48:49 -0700 Subject: [PATCH 1/4] [jwies/review-spend-ceiling] review: the in-code spend ceiling, as a ledger with one rule First slice of the spend ceiling (plan item B), on its own branch stacked on #305 rather than added to it. This commit is the ledger and its arithmetic; the runner and dispatch wiring follow. The rule is one pure function of three numbers, so every interesting property is directly testable: work may start while spend is under the DISPATCH BUDGET, which is the ceiling minus a landing reserve. The reserve is the part that is easy to leave out and expensive to miss: a ceiling with nothing held back turns an over-budget run into a wasted one, because the run cannot validate or post what it already paid to find. Enforcement is per turn, not per wave. Cost arrives one assistant turn at a time and one heavy reviewer can outspend a whole wave of light ones, so a between-waves check notices the overshoot only after paying for it. Crossing aborts in-flight agents through the ledger's AbortSignal and records the shed with the dollar it happened at, because a review that quietly drops a dimension for lack of money is indistinguishable from one that found nothing there. Numbers, derived rather than picked: $12.50 ceiling, because the compiled lock allows maxAiCredits: 2500 for the agent job, the detection pass spends ~$0.27 of that and this ledger does not govern it, and a measured full case's sub-agents ran about $0.50. The ceiling has to sit under the proxy's real-terms equivalent or the shed behaviour is unobservable, since the proxy would kill the run first. $1.50 reserve, sized from the validator dispatch (~$0.30 measured, the expensive tail) plus the reconciler. A test pins both, so changing live spend behaviour is a review conversation and not a diff nobody reads. REVIEW_SPEND_ENFORCEMENT=proxy-only is the rollback, and it measures without enforcing: the report still says crossed, still lists the sheds, and the notice is loud. A rolled-back run is not a blind run. The report also names which enforcement was in force, per the plan's rule that where granularity changes over time the artifact says which one produced it. That report is the substrate telemetry record v0. One thing this ceiling does NOT do, recorded in the module rather than implied: it does not replace the LOCATION property of the proxy cap. awf's cap is un-weakenable because it lives in a container image no Khan repo can edit; this one lives in a repo whose PRs the reviewer reviews. The provider-side workspace limit that would have restored that property is unavailable, so CODEOWNERS plus required review is the whole mitigation. --- workflows/review/lib/spend-ledger.test.ts | 151 +++++++++++++ workflows/review/lib/spend-ledger.ts | 255 ++++++++++++++++++++++ 2 files changed, 406 insertions(+) create mode 100644 workflows/review/lib/spend-ledger.test.ts create mode 100644 workflows/review/lib/spend-ledger.ts diff --git a/workflows/review/lib/spend-ledger.test.ts b/workflows/review/lib/spend-ledger.test.ts new file mode 100644 index 00000000..bf7224dc --- /dev/null +++ b/workflows/review/lib/spend-ledger.test.ts @@ -0,0 +1,151 @@ +import {describe, it, expect} from "vitest"; + +import { + CEILING_USD, + LANDING_RESERVE_USD, + createSpendLedger, + decideSpend, +} from "./spend-ledger"; + +/** + * The spend ceiling. Everything here is a property of the arithmetic and the + * abort wiring, so it is all testable without a model: the ledger's whole job + * is to decide at which dollar work stops and to say so out loud. + */ + +const ledger = (overrides = {}) => + createSpendLedger({ + ceilingUsd: 10, + landingReserveUsd: 2, + env: {}, + warn: () => {}, + ...overrides, + }); + +describe("decideSpend", () => { + it("allows work while spend is under the dispatch budget", () => { + // Budget is ceiling minus reserve: $8 of a $10 ceiling. + const decision = decideSpend(7.99, 10, 2); + expect(decision.allowed).toBe(true); + expect(decision.crossed).toBe(false); + // Cents, not exact binary floats: the ledger's contract is dollars. + expect(decision.remainingUsd).toBeCloseTo(0.01, 6); + }); + + it("crosses exactly at the dispatch budget, not at the ceiling", () => { + // The reserve is the point: at $8 the run stops STARTING work while + // $2 remains to validate and post what it already has. + expect(decideSpend(8, 10, 2).crossed).toBe(true); + expect(decideSpend(8, 10, 2).allowed).toBe(false); + }); + + it("clamps remaining at zero rather than reporting negative headroom", () => { + expect(decideSpend(12, 10, 2).remainingUsd).toBe(0); + }); + + it("treats a reserve larger than the ceiling as a zero budget", () => { + // Misconfiguration degrades to "shed everything", not to "spend + // everything": the ceiling is the thing being protected. + expect(decideSpend(0, 1, 5)).toEqual({ + allowed: false, + crossed: true, + remainingUsd: 0, + }); + }); +}); + +describe("createSpendLedger", () => { + it("keeps going while under budget and reports the spend", () => { + const led = ledger(); + expect(led.recordTurn("correctness-reviewer", 3)).toBe("continue"); + expect(led.recordTurn("correctness-reviewer", 2)).toBe("continue"); + expect(led.spentUsd()).toBe(5); + expect(led.report().crossed).toBe(false); + expect(led.report().sheds).toEqual([]); + expect(led.signal.aborted).toBe(false); + }); + + it("aborts in-flight agents on the turn that crosses", () => { + const led = ledger(); + expect(led.recordTurn("skill-auditor", 7)).toBe("continue"); + expect(led.recordTurn("skill-auditor", 2)).toBe("abort"); + // The abort is what stops work already running, and it carries the + // arithmetic so the log says why rather than just that. + expect(led.signal.aborted).toBe(true); + expect(String(led.signal.reason)).toMatch( + /spend ceiling reached: \$9\.00 of \$10\.00.*budget \$8\.00.*reserve \$2\.00/, + ); + }); + + it("refuses a new dispatch once crossed, and discloses it as a shed", () => { + const led = ledger(); + led.recordTurn("correctness-reviewer", 9); + expect(led.mayDispatch("data-migrations")).toBe(false); + const report = led.report(); + expect(report.sheds).toEqual([ + {agent: "correctness-reviewer", atUsd: 9, kind: "aborted"}, + {agent: "data-migrations", atUsd: 9, kind: "refused"}, + ]); + }); + + it("records overshoot so the worst case is measurable, not estimated", () => { + const led = ledger(); + // One expensive turn lands past the budget in a single step; the + // ledger cannot prevent that, only measure it. + led.recordTurn("correctness-reviewer", 11); + expect(led.report().overshootUsd).toBe(3); + }); + + it("aborts once, then keeps accumulating sheds without re-aborting", () => { + const led = ledger(); + led.recordTurn("a", 9); + const firstReason = led.signal.reason; + led.recordTurn("b", 5); + expect(led.signal.reason).toBe(firstReason); + expect(led.report().sheds).toHaveLength(2); + expect(led.report().spentUsd).toBe(14); + }); + + it("measures without enforcing under the proxy-only rollback, loudly", () => { + const warnings: string[] = []; + const led = ledger({ + env: {REVIEW_SPEND_ENFORCEMENT: "proxy-only"}, + warn: (m: string) => warnings.push(m), + }); + expect(led.recordTurn("correctness-reviewer", 9)).toBe("continue"); + expect(led.mayDispatch("data-migrations")).toBe(true); + expect(led.signal.aborted).toBe(false); + // Still measured and still disclosed, so a rolled-back run is not a + // blind run. + const report = led.report(); + expect(report.enforcement).toBe("proxy-only"); + expect(report.crossed).toBe(true); + expect(report.sheds).toHaveLength(2); + // The bypass is never silent. + expect(warnings.join(" ")).toContain("proxy-only"); + }); + + it("says which enforcement was in force, in the record itself", () => { + // The plan's standing rule: where enforcement granularity changes over + // time, the artifact says which one produced it. + expect(ledger().report().enforcement).toBe("in-code"); + }); + + it("ships the derived production numbers as its defaults", () => { + // Pinned deliberately: a silent edit to either constant changes live + // spend behaviour, and this is the test that makes it a review + // conversation rather than a diff nobody reads. + expect(CEILING_USD).toBe(12.5); + expect(LANDING_RESERVE_USD).toBe(1.5); + const led = createSpendLedger({env: {}, warn: () => {}}); + expect(led.report().ceilingUsd).toBe(12.5); + expect(led.report().landingReserveUsd).toBe(1.5); + }); + + it("ignores a negative turn cost rather than banking credit", () => { + const led = ledger(); + led.recordTurn("a", 5); + led.recordTurn("a", -100); + expect(led.spentUsd()).toBe(5); + }); +}); diff --git a/workflows/review/lib/spend-ledger.ts b/workflows/review/lib/spend-ledger.ts new file mode 100644 index 00000000..7a128db1 --- /dev/null +++ b/workflows/review/lib/spend-ledger.ts @@ -0,0 +1,255 @@ +/** + * The in-code spend ceiling: one budget ledger for a review run, asserted per + * turn, enforced by aborting work rather than by hoping. + * + * Why this exists at all. Today's only ceiling is gh-aw's api-proxy credit cap + * (`maxAiCredits: 2500` in the compiled lock), which is denominated in + * list-price credits, not in the dollars Khan actually pays, and which the + * migration deletes along with the proxy. This ledger replaces it in the + * harness, in real dollars, with two properties the proxy cap does not have: + * it can shed work gracefully instead of failing a run mid-flight, and it can + * say in its own output which enforcement was in force. + * + * What it deliberately does NOT claim. The proxy cap lives in a container image + * no Khan repo can edit; this ceiling lives in a repo whose PRs this reviewer + * reviews, so the location property is genuinely weaker. A provider-side + * workspace limit was considered as the out-of-repo backstop and dropped as + * unavailable, so CODEOWNERS on this file plus required review is the whole + * mitigation. That is a knowing trade, recorded here rather than implied. + * + * The shape of enforcement, per the plan's "state the guarantee in force" rule: + * + * - Per TURN, not per wave. Cost arrives one assistant turn at a time, and a + * single heavy reviewer can outspend a whole wave of light ones; checking + * between waves would notice the overshoot after paying for it. + * - Crossing ABORTS in-flight agents through {@link SpendLedger.signal}, and + * the abort surfaces as a DISCLOSED shed, never as a silent gap. A review + * that quietly drops a dimension because it ran out of money is + * indistinguishable from one that found nothing there. + * - A landing reserve is held back so the run can still stage, validate, and + * post what it already found. A ceiling that leaves nothing to land with + * converts an over-budget run into a wasted one. + * - `REVIEW_SPEND_ENFORCEMENT=proxy-only` is the loud rollback: the ledger + * still measures and still reports, but never aborts. Because this changes + * live spend behavior, the escape hatch is explicit and logged rather than + * inferred from a missing value. + */ + +/** The environment variable that reverts to proxy-only enforcement. */ +export const SPEND_ENFORCEMENT_ENV = "REVIEW_SPEND_ENFORCEMENT"; + +/** + * The ceiling, in real dollars, for one review run in this repo. + * + * Derivation, so the number is auditable rather than folkloric: the compiled + * lock allows `maxAiCredits: 2500` for the whole agent job, of which the + * threat-detection pass measurably spends about $0.27 and which this ledger + * does not govern. $12.50 therefore sits under the agent job's real-terms + * allowance and far above any measured review (the harness A/B priced a full + * case's sub-agents around $0.50). It has to sit UNDER the proxy's real-terms + * equivalent, or a run that crosses this ceiling would have been killed by the + * proxy first and the shed behaviour would never be observed. + * + * Raise it deliberately, with data, and never in the same PR as behaviour + * changes. Each consumer compiles its own lock, so a consumer with a different + * credit allowance wants a different number here. + */ +export const CEILING_USD = 12.5; + +/** + * Held back from the ceiling so a shedding run can still finish and post. + * + * Sized from what the phases AFTER the reviewer fan-out cost: claim validation + * is one dispatch (~$0.30 measured, and it is the expensive tail because it + * reads every claim), plus the reconciler when threads exist. $1.50 covers + * both with room, and is small enough that reserving it never sheds work that + * would otherwise have fit. + */ +export const LANDING_RESERVE_USD = 1.5; + +/** One dimension shed because the run ran out of money, with the evidence. */ +export type SpendShed = { + /** The agent that was refused or aborted. */ + agent: string; + /** Total spend at the moment of the decision. */ + atUsd: number; + /** Refused before dispatch, or aborted mid-flight. */ + kind: "refused" | "aborted"; +}; + +/** + * The run/cost/outcome record. This is the substrate telemetry schema v0, born + * inside a migration PR because it costs nothing to shape it here and a second + * consumer (autofix) will want exactly these fields. + */ +export type SpendReport = { + schemaVersion: 1; + /** Which enforcement was actually in force for this run. */ + enforcement: "in-code" | "proxy-only"; + ceilingUsd: number; + landingReserveUsd: number; + spentUsd: number; + /** True once spend passed the dispatch budget (ceiling minus reserve). */ + crossed: boolean; + /** Peak overshoot past the dispatch budget; 0 when never crossed. */ + overshootUsd: number; + sheds: SpendShed[]; +}; + +export type SpendDecision = { + /** Whether more model work may be started or continued. */ + allowed: boolean; + /** Spend has passed the dispatch budget. */ + crossed: boolean; + /** Dollars left before the dispatch budget, clamped at 0. */ + remainingUsd: number; +}; + +/** + * The whole enforcement rule, as a pure function of three numbers. + * + * The dispatch budget is `ceiling - reserve`: work may start while spend is + * strictly under it. Exported and tested directly because every interesting + * property of this ledger (does it shed at the right dollar, does it hold the + * reserve back, does it clamp) is a property of this function. + */ +export const decideSpend = ( + spentUsd: number, + ceilingUsd: number, + landingReserveUsd: number, +): SpendDecision => { + const dispatchBudget = Math.max(0, ceilingUsd - landingReserveUsd); + const crossed = spentUsd >= dispatchBudget; + return { + allowed: !crossed, + crossed, + remainingUsd: Math.max(0, dispatchBudget - spentUsd), + }; +}; + +export type SpendLedgerOptions = { + ceilingUsd?: number; + landingReserveUsd?: number; + /** + * The process environment, injected so the rollback flag is testable. The + * flag is read ONCE at construction: an enforcement posture that could + * change mid-run is not a posture. + */ + env?: {[key: string]: string | undefined}; + /** Where the loud rollback notice goes. Defaults to stderr. */ + warn?: (message: string) => void; +}; + +export type SpendLedger = { + /** + * Add one turn's cost and decide whether the agent may continue. Returns + * "abort" the first time the dispatch budget is crossed, and on every call + * after; the caller stops the turn loop and the ledger records the shed. + */ + recordTurn: (agent: string, deltaUsd: number) => "continue" | "abort"; + /** + * Whether a NEW agent may be dispatched. Distinct from recordTurn because + * refusing to start is cheaper than aborting mid-flight, and the two are + * disclosed differently. + */ + mayDispatch: (agent: string) => boolean; + /** Aborted when the budget is crossed; wired into in-flight requests. */ + signal: AbortSignal; + spentUsd: () => number; + report: () => SpendReport; +}; + +export const createSpendLedger = ( + options: SpendLedgerOptions = {}, +): SpendLedger => { + const ceilingUsd = options.ceilingUsd ?? CEILING_USD; + const landingReserveUsd = options.landingReserveUsd ?? LANDING_RESERVE_USD; + const env = options.env ?? process.env; + const warn = + options.warn ?? + ((message: string) => { + // eslint-disable-next-line no-console + console.error(message); + }); + + const enforcement = + env[SPEND_ENFORCEMENT_ENV] === "proxy-only" ? "proxy-only" : "in-code"; + if (enforcement === "proxy-only") { + warn( + `review dispatch: in-code spend ceiling DISABLED ` + + `(${SPEND_ENFORCEMENT_ENV}=proxy-only); spend is bounded only by ` + + `the proxy's list-price credit cap. Measuring, not enforcing.`, + ); + } + + const controller = new AbortController(); + const sheds: SpendShed[] = []; + let spentUsd = 0; + let overshootUsd = 0; + let crossed = false; + + /** Cross once: the first crossing aborts, later ones just accumulate. */ + const cross = (agent: string, kind: SpendShed["kind"]): void => { + const budget = Math.max(0, ceilingUsd - landingReserveUsd); + overshootUsd = Math.max(overshootUsd, spentUsd - budget); + sheds.push({agent, atUsd: spentUsd, kind}); + if (crossed) { + return; + } + crossed = true; + if (enforcement === "in-code") { + controller.abort( + new Error( + `review spend ceiling reached: $${spentUsd.toFixed( + 2, + )} of ` + + `$${ceilingUsd.toFixed(2)} (dispatch budget ` + + `$${budget.toFixed(2)}, landing reserve ` + + `$${landingReserveUsd.toFixed(2)})`, + ), + ); + } + }; + + return { + recordTurn: (agent, deltaUsd) => { + spentUsd += Math.max(0, deltaUsd); + const decision = decideSpend( + spentUsd, + ceilingUsd, + landingReserveUsd, + ); + if (!decision.crossed) { + return "continue"; + } + cross(agent, "aborted"); + // Proxy-only measures without enforcing, so the turn loop continues + // exactly as it did before this ledger existed. + return enforcement === "in-code" ? "abort" : "continue"; + }, + mayDispatch: (agent) => { + const decision = decideSpend( + spentUsd, + ceilingUsd, + landingReserveUsd, + ); + if (decision.allowed) { + return true; + } + cross(agent, "refused"); + return enforcement !== "in-code"; + }, + signal: controller.signal, + spentUsd: () => spentUsd, + report: () => ({ + schemaVersion: 1, + enforcement, + ceilingUsd, + landingReserveUsd, + spentUsd, + crossed, + overshootUsd: Math.max(0, overshootUsd), + sheds: [...sheds], + }), + }; +}; From 3f5a5a4074ad747ef0c1b2440e06873dd11c1710 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Mon, 3 Aug 2026 19:50:04 -0700 Subject: [PATCH 2/4] [jwies/review-spend-ceiling] review: let the ledger stop a turn loop, and say when it did The runner's seam for the ceiling: `onTurnCost` reports each turn's cost delta as it lands and asks whether to continue. The delta, not the total, because the ledger is run-wide and this runner only ever knows about one agent. Stopping happens at a turn boundary through `shouldStopAfterTurn`, not by throwing. An agent stopped for budget has usually already found something, and killing it mid-turn would discard that along with the turn's cost that was already paid. `AgentResult.stoppedForBudget` carries the fact outward on every success path, including the free-text final. The dispatcher needs it to disclose the dimension as cut short rather than completed: an agent that ran out of money mid-investigation and one that finished and found nothing produce the same empty shape, and only this flag tells them apart. --- workflows/review/lib/dispatch-calls.ts | 8 ++++++ workflows/review/lib/dispatch-runner-pi.ts | 33 ++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/workflows/review/lib/dispatch-calls.ts b/workflows/review/lib/dispatch-calls.ts index 2fab9a26..14b26cc8 100644 --- a/workflows/review/lib/dispatch-calls.ts +++ b/workflows/review/lib/dispatch-calls.ts @@ -79,6 +79,14 @@ export type AgentResult = { refused?: boolean; /** The output came through the structured-final tool, pre-validated. */ structured?: boolean; + /** + * The turn loop ended because the run's spend ceiling was reached, not + * because the agent finished. Load-bearing for disclosure: an agent stopped + * for budget has usually found SOMETHING, so its output is still used, and + * the run has to say that this dimension was cut short rather than + * completed. Silence here would be indistinguishable from "nothing found". + */ + stoppedForBudget?: boolean; }; /** The model seam; the Pi-backed production runner lives in the CLI entry. */ diff --git a/workflows/review/lib/dispatch-runner-pi.ts b/workflows/review/lib/dispatch-runner-pi.ts index 9a3e0491..bacd0606 100644 --- a/workflows/review/lib/dispatch-runner-pi.ts +++ b/workflows/review/lib/dispatch-runner-pi.ts @@ -606,6 +606,17 @@ export type PiRunnerOptions = { * time to tell which one sheds findings. */ systemPrompt?: string; + /** + * Called with each turn's cost as it lands, and asked whether to continue. + * The spend ledger answers (lib/spend-ledger.ts); returning "abort" stops + * this agent's turn loop at the turn boundary. + * + * Per turn rather than per dispatch because that is the only granularity + * where the answer is still useful: cost is known when a turn ends, and one + * heavy reviewer can outspend a whole wave of light ones, so a check that + * runs between dispatches learns about the overshoot after paying it. + */ + onTurnCost?: (deltaUsd: number) => "continue" | "abort"; }; export const createPiRunner = async ( @@ -678,6 +689,13 @@ export const createPiRunner = async ( let usd = 0; let turns = 0; let toolCalls = 0; + /** + * Set when the spend ledger says stop. Read by shouldStopAfterTurn, so + * the loop ends at a TURN boundary rather than by throwing: an agent + * stopped for budget has usually already found something, and killing + * it mid-turn would throw that away along with the turn's cost. + */ + let budgetExhausted = false; let stopReason: string | undefined; let errorMessage: string | undefined; let rawStopReason: string | undefined; @@ -716,7 +734,9 @@ export const createPiRunner = async ( * "end the turn now". */ shouldStopAfterTurn: () => - captured !== undefined || turns >= request.maxTurns, + captured !== undefined || + turns >= request.maxTurns || + budgetExhausted, }, (event: Record) => { if (event["type"] === "tool_execution_end") { @@ -770,7 +790,13 @@ export const createPiRunner = async ( const usage = message?.["usage"] as | {cost?: {total?: number}} | undefined; - usd += Number(usage?.cost?.total ?? 0); + const turnUsd = Number(usage?.cost?.total ?? 0); + usd += turnUsd; + // The ledger sees the delta, not the total: it is run-wide + // and this runner only knows about one agent. + if (options.onTurnCost?.(turnUsd) === "abort") { + budgetExhausted = true; + } const content = (message?.["content"] ?? []) as TextBlock[]; const text = content .filter((block) => block.type === "text") @@ -810,6 +836,7 @@ export const createPiRunner = async ( stopReason === "refusal" || rawStopReason === "refusal", wallMs: Date.now() - started, structured: true, + ...(budgetExhausted ? {stoppedForBudget: true} : {}), }; } // Out of turns and finished-with-prose return the same shape: a @@ -839,6 +866,7 @@ export const createPiRunner = async ( refused: stopReason === "refusal" || rawStopReason === "refusal", wallMs: Date.now() - started, + ...(budgetExhausted ? {stoppedForBudget: true} : {}), }; } catch (error) { // A payload the tool already accepted is complete and validated: @@ -862,6 +890,7 @@ export const createPiRunner = async ( stopReason === "refusal" || rawStopReason === "refusal", wallMs: Date.now() - started, structured: true, + ...(budgetExhausted ? {stoppedForBudget: true} : {}), }; } if (timedOut) { From 2fb26664182cc504d4c9003ff8eb380dbf447c37 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Mon, 3 Aug 2026 19:56:53 -0700 Subject: [PATCH 3/4] [jwies/review-spend-ceiling] review: thread the ceiling through dispatch, and disclose what it cuts Wires the ledger into the run and closes the gap that would have made it decorative. The gap: as first written, the ledger only learned about cost if a caller remembered to hook the runner, and a forgotten hook is a ceiling that silently never fires. It now has exactly one writer, the dispatcher, called once per completed attempt so a retry and a refusal fallback each pay for themselves. The per-turn check became a read-only probe against that accounting, which is what makes double counting impossible without needing per-attempt identity the runner does not have. Two abort paths, because one does not cover the other. The probe stops the agent that is doing the spending, at a turn boundary. The ledger's AbortSignal, now actually consumed by the runner, stops the siblings already in flight when someone else's completed dispatch crosses the line; before this it was an unused field, which would have made "crossing aborts in-flight agents" true only of the agent that crossed. Disclosure reuses the machinery already there: refused and aborted agents become `skippedDimensions` with cause `budget`, get note lines distinct from the tier-budget shed (planned before the run) so a reader can tell "we chose not to" from "we ran out", and appear in `perAgent` with `failed: "budget"` rather than as agents that ran and found nothing. The sheds are read back OUT of the ledger when the result is assembled, so the record and the disclosure cannot disagree. `DispatchResult.spend` rides both staged copies of the run artifact, so the gate and the post-hoc read the same document. Integration tests cover the wiring rather than the arithmetic: one dispatch paid for and the rest refused under a tight ceiling, the whole roster running under the rollback, and the record present in both staged files. CODEOWNERS names @jwbron and @jeresig on the ledger, since it is now the only limit on what a run can spend. --- .changeset/review-spend-ceiling.md | 61 +++++ CODEOWNERS | 17 ++ workflows/review/lib/dispatch-calls.ts | 46 ++++ workflows/review/lib/dispatch-runner-pi.ts | 39 +++- workflows/review/lib/dispatch-spend.test.ts | 242 ++++++++++++++++++++ workflows/review/lib/dispatch.ts | 67 +++++- workflows/review/lib/spend-ledger.test.ts | 42 ++-- workflows/review/lib/spend-ledger.ts | 50 ++-- 8 files changed, 522 insertions(+), 42 deletions(-) create mode 100644 .changeset/review-spend-ceiling.md create mode 100644 CODEOWNERS create mode 100644 workflows/review/lib/dispatch-spend.test.ts diff --git a/.changeset/review-spend-ceiling.md b/.changeset/review-spend-ceiling.md new file mode 100644 index 00000000..d6af2fcd --- /dev/null +++ b/.changeset/review-spend-ceiling.md @@ -0,0 +1,61 @@ +--- +"review": minor +--- + +review: enforce the run's spend ceiling in code, in real dollars + +Dispatch now carries a budget ledger. Work may start while the run's spend is +under a dispatch budget of `CEILING_USD - LANDING_RESERVE_USD`; crossing it +refuses further dispatches, aborts the agents already in flight, and discloses +every shed as a note line rather than as a silently missing dimension. + +Why in code rather than at the proxy. The only ceiling today is gh-aw's +api-proxy credit cap (`maxAiCredits: 2500` in the compiled lock), which is +denominated in list-price credits rather than the dollars Khan pays, cannot shed +gracefully (it fails the run), and disappears with the proxy when the migration +completes. This supersedes the intent of #314, which tried to fix the +denomination at the proxy and was blocked on a gh-aw version; the proxy cap +stays as a coarse list-price backstop while it exists. + +The numbers are derived, not chosen, and a test pins them so a change to live +spend behaviour is a review conversation: **$12.50** ceiling, from the lock's +2500-credit allowance minus the detection pass's measured ~$0.27 which this +ledger does not govern, against a measured ~$0.50 for a full case's sub-agents; +**$1.50** landing reserve, from the validator dispatch's measured ~$0.30 plus +the reconciler. The reserve is the part that is easy to omit and expensive to +miss: a ceiling with nothing held back turns an over-budget run into a wasted +one, because the run can no longer validate or post what it already paid to +find. Each consumer compiles its own lock, so a consumer with a different +allowance wants a different ceiling. + +Enforcement is per turn where it can be. The runner asks the ledger at each turn +boundary whether the agent may continue, so a single heavy reviewer cannot +outspend a whole wave before anyone notices, and stopping happens at a turn +boundary rather than by throwing: an agent stopped for budget has usually +already found something worth keeping. The ledger has exactly one writer (the +dispatcher, once per completed attempt, so retries and refusal fallbacks each +pay), and the per-turn check is a read-only probe against it, which is what +keeps the accounting from double counting. Concurrency is approximated the way +the investigation cap approximates it: an in-flight agent cannot see its +siblings' unsettled spend, so a wave can overshoot by at most the number of +agents running at once, and `overshootUsd` measures that rather than assuming it +away. + +`REVIEW_SPEND_ENFORCEMENT=proxy-only` is the rollback, and it measures without +enforcing: the record still reports `crossed`, still lists the sheds, and the +notice is loud, so a rolled-back run is not a blind run. The record names which +enforcement was in force, per the standing rule that where enforcement changes +over time the artifact says which one produced it. + +The record itself (`DispatchResult.spend`: ceiling, reserve, spend, overshoot, +sheds, enforcement) is the run/cost/outcome telemetry shape, staged in both +`dispatch-result.json` and the `out/` copy the artifact upload carries. It is +deliberately a schema rather than an ad-hoc log line, because the same shape is +what a second consumer would need. + +One property this does NOT restore, recorded in the module rather than implied: +awf's cap is un-weakenable because it lives in a container image no Khan repo +can edit, and this ceiling lives in a repo whose PRs this reviewer reviews. The +provider-side workspace limit that would have replaced that property is not +available, so CODEOWNERS on the ledger (naming two owners, since one stalls when +away and none enforces nothing) plus required review is the whole mitigation. diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 00000000..7c78bbdd --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,17 @@ +# Ownership for the files where a quiet edit changes what this repo SPENDS or +# what it is allowed to do. Everything else in this repo is reviewed by whoever +# is around; these are the paths where "whoever is around" is not enough. +# +# The spend ceiling especially. It is the only limit on a review run's cost once +# the migration removes gh-aw's api-proxy credit cap, and unlike that cap it +# lives in a repo whose pull requests this reviewer reviews. A provider-side +# workspace limit would have restored the property of living somewhere no PR can +# reach; it is not available, so this file is the mitigation. Two owners rather +# than one, because a single-owner entry stalls the moment that owner is away +# and an ownerless entry enforces nothing at all. +# +# Pin bumps and policy edits under these paths are behaviour changes: reviewed +# as such, never batch-merged. + +/workflows/review/lib/spend-ledger.ts @jwbron @jeresig +/CODEOWNERS @jwbron @jeresig diff --git a/workflows/review/lib/dispatch-calls.ts b/workflows/review/lib/dispatch-calls.ts index 14b26cc8..74b7c27a 100644 --- a/workflows/review/lib/dispatch-calls.ts +++ b/workflows/review/lib/dispatch-calls.ts @@ -125,6 +125,12 @@ export type PerAgentReport = { fellBackTo?: string; /** The result arrived via the structured-final tool (pre-validated). */ structuredFinal?: boolean; + /** + * This agent's turn loop ended on the run's spend ceiling. Kept per agent + * rather than only run-wide, because "which dimension got cut short" is the + * question a reader of the artifact actually has. + */ + stoppedForBudget?: boolean; failed?: string; }; @@ -148,6 +154,19 @@ export type AgentDispatcherOptions = { validatorFor: ( name: string, ) => (payload: Record) => string | null; + /** + * Asked before each dispatch whether the run can still afford one. The + * spend ledger answers. Refusing here rather than aborting mid-flight is + * the cheaper of the two disclosures: nothing is spent, and the dimension + * is shed with a note instead of half-run. + */ + mayDispatch?: (name: string) => boolean; + /** + * Called once per COMPLETED attempt with what it cost. The ledger's only + * writer; a retry and a refusal fallback are separate attempts and each + * one really was paid for, so each is recorded. + */ + recordSpend?: (name: string, usd: number) => void; }; export type AgentDispatcher = { @@ -180,6 +199,8 @@ export const createAgentDispatcher = ( maxTurns, timeoutMs, validatorFor, + mayDispatch, + recordSpend, } = options; /** @@ -200,6 +221,26 @@ export const createAgentDispatcher = ( malformedNote?: string, modelOverride?: string, ): Promise => { + if (mayDispatch !== undefined && !mayDispatch(name)) { + // Refused before spending. Staged as an out-file like every other + // outcome so the dispatch gate reads one shape, and reported as a + // failure cause so the run's own artifact names the reason. + writeOut( + name, + JSON.stringify({ + error: "spend ceiling reached before dispatch", + }), + ); + report({ + name, + model: agents.get(name)?.model ?? "", + usd: 0, + turns: 0, + wallMs: 0, + failed: "budget", + }); + return null; + } const definition = agents.get(name); if (definition === undefined) { writeOut(name, JSON.stringify({error: "agent definition missing"})); @@ -262,6 +303,7 @@ export const createAgentDispatcher = ( : {stopReason: result.stopReason}), failed: "refused", }); + recordSpend?.(name, result.usd); return dispatchAgent(name, malformedNote, fallback); } writeOut(name, result.output); @@ -285,7 +327,11 @@ export const createAgentDispatcher = ( ...(malformedNote === undefined ? {} : {retried: true}), ...(modelOverride === undefined ? {} : {fellBackTo: model}), ...(result.structured === true ? {structuredFinal: true} : {}), + ...(result.stoppedForBudget === true + ? {stoppedForBudget: true} + : {}), }); + recordSpend?.(name, result.usd); return result.output; } catch (error) { writeOut( diff --git a/workflows/review/lib/dispatch-runner-pi.ts b/workflows/review/lib/dispatch-runner-pi.ts index bacd0606..1f29e207 100644 --- a/workflows/review/lib/dispatch-runner-pi.ts +++ b/workflows/review/lib/dispatch-runner-pi.ts @@ -607,16 +607,25 @@ export type PiRunnerOptions = { */ systemPrompt?: string; /** - * Called with each turn's cost as it lands, and asked whether to continue. - * The spend ledger answers (lib/spend-ledger.ts); returning "abort" stops - * this agent's turn loop at the turn boundary. + * Asked at each turn boundary whether to keep going, given what THIS agent + * has spent so far. The spend ledger answers (lib/spend-ledger.ts), as a + * read-only probe: the ledger's accounting is written once per completed + * dispatch, and this only decides whether to stop early. * * Per turn rather than per dispatch because that is the only granularity * where the answer is still useful: cost is known when a turn ends, and one * heavy reviewer can outspend a whole wave of light ones, so a check that * runs between dispatches learns about the overshoot after paying it. */ - onTurnCost?: (deltaUsd: number) => "continue" | "abort"; + onTurnCost?: (agentSpentUsd: number) => "continue" | "abort"; + /** + * Run-wide abort. The spend ledger fires this when a COMPLETED dispatch + * pushes the run past its budget, which is the case `onTurnCost` cannot + * cover: that probe only stops the agent doing the spending, while this + * stops the siblings already in flight beside it. Without it, "crossing + * aborts in-flight agents" would be true only of the agent that crossed. + */ + abortSignal?: AbortSignal; }; export const createPiRunner = async ( @@ -666,6 +675,18 @@ export const createPiRunner = async ( return async (request: AgentRequest): Promise => { const started = Date.now(); const abort = new AbortController(); + // Link the run-wide abort in both directions in time: already-aborted + // means never start, and aborting later means stop now. + const runAbort = options.abortSignal; + if (runAbort?.aborted === true) { + abort.abort(runAbort.reason); + } else { + runAbort?.addEventListener( + "abort", + () => abort.abort(runAbort.reason), + {once: true}, + ); + } let timedOut = false; const timer = setTimeout(() => { timedOut = true; @@ -790,11 +811,11 @@ export const createPiRunner = async ( const usage = message?.["usage"] as | {cost?: {total?: number}} | undefined; - const turnUsd = Number(usage?.cost?.total ?? 0); - usd += turnUsd; - // The ledger sees the delta, not the total: it is run-wide - // and this runner only knows about one agent. - if (options.onTurnCost?.(turnUsd) === "abort") { + usd += Number(usage?.cost?.total ?? 0); + // Cumulative for this agent, because the probe is a + // question about the run's total and the caller adds this + // agent's in-flight spend to what it has already settled. + if (options.onTurnCost?.(usd) === "abort") { budgetExhausted = true; } const content = (message?.["content"] ?? []) as TextBlock[]; diff --git a/workflows/review/lib/dispatch-spend.test.ts b/workflows/review/lib/dispatch-spend.test.ts new file mode 100644 index 00000000..bcb69629 --- /dev/null +++ b/workflows/review/lib/dispatch-spend.test.ts @@ -0,0 +1,242 @@ +import {describe, it, expect} from "vitest"; + +import {runDispatch, type AgentRunner, type DispatchFs} from "./dispatch"; +import {computeDiffProvenance} from "./provenance"; +import {createSpendLedger} from "./spend-ledger"; + +/** + * The spend ceiling as the RUN sees it: does crossing it shed work, disclose + * the shed, and leave the record saying what happened? + * + * Separate file from dispatch.test.ts for size, and separate subject: these + * tests are about money, not about parsing. The ledger's arithmetic is pinned + * in spend-ledger.test.ts; what is pinned here is the wiring, which is the part + * that would fail silently. A ceiling that is never consulted looks exactly + * like a ceiling that was never crossed. + */ + +const REVIEW = "/tmp/gh-aw/review"; +const AGENTS = "/work/.claude/agents"; + +const makeFakeFs = ( + files: Record = {}, +): DispatchFs & {files: Record} => { + const state = {...files}; + return { + files: state, + readFileSync: (p: string) => { + if (!(p in state)) { + throw new Error(`ENOENT: ${p}`); + } + return state[p]; + }, + writeFileSync: (p: string, data: string) => { + state[p] = data; + }, + existsSync: (p: string) => + p in state || Object.keys(state).some((f) => f.startsWith(`${p}/`)), + mkdirSync: () => {}, + readdirSync: (p: string) => { + const prefix = `${p}/`; + return [ + ...new Set( + Object.keys(state) + .filter((f) => f.startsWith(prefix)) + .map((f) => f.slice(prefix.length).split("/")[0]), + ), + ]; + }, + }; +}; + +const agentFiles = (...names: string[]): Record => + Object.fromEntries( + names.map((name) => [ + `${AGENTS}/${name}.md`, + `---\nname: ${name}\ndescription: d\nmodel: claude-opus-4-8\n---\nYou are ${name}.`, + ]), + ); + +const DIFF = [ + "diff --git a/a.ts b/a.ts", + "--- a/a.ts", + "+++ b/a.ts", + "@@ -1,2 +1,3 @@", + " ctx", + "+added line", + " ctx", + "", +].join("\n"); + +const staging = (): Record => ({ + [`${REVIEW}/routing.json`]: JSON.stringify({ + enabledReviewers: [], + lensesToSpawn: [], + runBudget: {maxReviewerInvocations: 6, tier: "High"}, + }), + [`${REVIEW}/rereview-plan.json`]: JSON.stringify({depth: "full"}), + [`${REVIEW}/full.diff`]: DIFF, + [`${REVIEW}/files.json`]: JSON.stringify([ + {path: "a.ts", status: "modified", hasPatch: true}, + ]), + [`${REVIEW}/provenance.json`]: JSON.stringify(computeDiffProvenance(DIFF)), + ...agentFiles( + "pattern-triage", + "correctness-reviewer", + "skill-auditor", + "claim-validator", + ), +}); + +const FINDING = JSON.stringify({ + findings: [ + { + path: "a.ts", + line: 2, + label: "issue (blocking)", + subject: "Broken guard.", + discussion: "The guard was removed.", + failure_scenario: "nil deref on empty input", + }, + ], +}); + +/** A runner that charges `usd` for every dispatch and records its calls. */ +const chargingRunner = ( + usd: number, + outputs: Record, +): AgentRunner & {calls: string[]} => { + const calls: string[] = []; + const runner = (async (request) => { + calls.push(request.name); + return { + output: outputs[request.name] ?? JSON.stringify({findings: []}), + usd, + turns: 2, + wallMs: 10, + }; + }) as AgentRunner & {calls: string[]}; + runner.calls = calls; + return runner; +}; + +describe("the spend ceiling inside runDispatch", () => { + it("carries the cost record on the result, enforcement named", () => { + // Cheap run: nothing sheds, and the record still says what governed it. + const runner = chargingRunner(0.01, { + "pattern-triage": JSON.stringify({ + patterns: [], + reviewFiles: ["a.ts"], + }), + "correctness-reviewer": FINDING, + }); + return runDispatch({ + fs: makeFakeFs(staging()), + runner, + repoRoot: "/work", + ledger: createSpendLedger({env: {}, warn: () => {}}), + }).then((result) => { + expect(result.spend.enforcement).toBe("in-code"); + expect(result.spend.crossed).toBe(false); + expect(result.spend.sheds).toEqual([]); + expect(result.spend.spentUsd).toBeCloseTo(result.totalUsd, 6); + }); + }); + + it("refuses later dispatches once the budget is gone, and says so", async () => { + // $1 per dispatch against a $2 ceiling holding $1 back: the first + // dispatch settles, the budget is then gone, and everything after is + // refused rather than half-run. + const runner = chargingRunner(1, { + "pattern-triage": JSON.stringify({ + patterns: [], + reviewFiles: ["a.ts"], + }), + "correctness-reviewer": FINDING, + }); + const result = await runDispatch({ + fs: makeFakeFs(staging()), + runner, + repoRoot: "/work", + ledger: createSpendLedger({ + ceilingUsd: 2, + landingReserveUsd: 1, + env: {}, + warn: () => {}, + }), + }); + + // Exactly one dispatch was paid for; the rest never ran. + expect(runner.calls).toHaveLength(1); + expect(result.spend.crossed).toBe(true); + // Every refusal is disclosed as a budget shed, with a note line a + // reader of the review can actually see. + const budgetSheds = result.skippedDimensions.filter( + (skip) => skip.cause === "budget", + ); + expect(budgetSheds.length).toBeGreaterThan(0); + expect(result.noteLines.join("\n")).toContain("spend ceiling"); + // And the refused agents are reported as failed for budget, not as + // agents that ran and found nothing. + expect( + result.perAgent.filter((agent) => agent.failed === "budget").length, + ).toBeGreaterThan(0); + }); + + it("spends without refusing under the proxy-only rollback", async () => { + const runner = chargingRunner(1, { + "pattern-triage": JSON.stringify({ + patterns: [], + reviewFiles: ["a.ts"], + }), + "correctness-reviewer": FINDING, + }); + const result = await runDispatch({ + fs: makeFakeFs(staging()), + runner, + repoRoot: "/work", + ledger: createSpendLedger({ + ceilingUsd: 2, + landingReserveUsd: 1, + env: {REVIEW_SPEND_ENFORCEMENT: "proxy-only"}, + warn: () => {}, + }), + }); + // The whole roster ran: rollback means the ceiling measures and does + // not enforce. + expect(runner.calls.length).toBeGreaterThan(1); + expect(result.spend.enforcement).toBe("proxy-only"); + expect(result.spend.crossed).toBe(true); + // Still disclosed, so a rolled-back run is not a silent one. + expect(result.noteLines.join("\n")).toContain("spend ceiling"); + }); + + it("stages the cost record in the run artifact, not just in memory", async () => { + const fs = makeFakeFs(staging()); + const runner = chargingRunner(0.01, { + "pattern-triage": JSON.stringify({ + patterns: [], + reviewFiles: ["a.ts"], + }), + "correctness-reviewer": FINDING, + }); + await runDispatch({ + fs, + runner, + repoRoot: "/work", + ledger: createSpendLedger({env: {}, warn: () => {}}), + }); + // Both copies the gate and the post-hoc read carry it: without this the + // record exists only where nobody looks. + for (const path of [ + `${REVIEW}/dispatch-result.json`, + `${REVIEW}/out/dispatch-result.json`, + ]) { + const staged = JSON.parse(fs.files[path]) as { + spend?: {schemaVersion?: number; ceilingUsd?: number}; + }; + expect(staged.spend?.schemaVersion).toBe(1); + expect(staged.spend?.ceilingUsd).toBe(12.5); + } + }); +}); diff --git a/workflows/review/lib/dispatch.ts b/workflows/review/lib/dispatch.ts index 062db023..6d2752f4 100644 --- a/workflows/review/lib/dispatch.ts +++ b/workflows/review/lib/dispatch.ts @@ -67,6 +67,11 @@ import { type AgentRunner, type PerAgentReport, } from "./dispatch-calls"; +import { + createSpendLedger, + type SpendLedger, + type SpendReport, +} from "./spend-ledger"; import {computeRoster, type RosterShed} from "./dispatch-roster"; import { @@ -183,6 +188,13 @@ export type DispatchResult = { excludedFiles?: string[]; perAgent: PerAgentReport[]; totalUsd: number; + /** + * The run's cost record: ceiling, reserve, spend, sheds, and which + * enforcement was in force. Rides the run artifact rather than a file of + * its own so the gate and the post-hoc read one document, and so the + * substrate's second consumer inherits a shape that is already staged. + */ + spend: SpendReport; }; export type DispatchOptions = { @@ -195,6 +207,14 @@ export type DispatchOptions = { maxTurns?: number; timeoutMs?: number; concurrency?: number; + /** + * The run's spend ledger. Defaulted rather than optional in effect: a run + * without one would be a run with no ceiling, and the point of the ceiling + * is that it is not something a caller can forget. Callers pass their own + * to set the numbers (or to test the shed path); the CLI passes the same + * instance it gave the runner, since one run means one budget. + */ + ledger?: SpendLedger; }; const readJson = (fs: DispatchFs, path: string): unknown => { @@ -237,6 +257,16 @@ const noteLine = { `Note: ${dimension} not assessed this run (shed under the ${tier}-tier run budget).`, unavailable: (dimension: string, agent: string): string => `Note: ${dimension} not assessed this run (${agent} output unavailable).`, + /** + * The spend ceiling cut this dimension. Deliberately distinct from the + * tier-budget shed above: that one is planned before the run starts and + * this one means the run ran out of money partway, which a reader should be + * able to tell apart at a glance. + */ + budget: (dimension: string, kind: "refused" | "aborted"): string => + kind === "aborted" + ? `Note: ${dimension} cut short this run (the run's spend ceiling was reached mid-investigation).` + : `Note: ${dimension} not assessed this run (the run's spend ceiling was reached before it started).`, }; /** @@ -304,6 +334,7 @@ export const runDispatch = async ( fs.writeFileSync(`${OUT_DIR}/${name}.json`, content); }; + const ledger = options.ledger ?? createSpendLedger(); const {dispatchAgent, parseWithRetry} = createAgentDispatcher({ runner, agents, @@ -313,6 +344,8 @@ export const runDispatch = async ( maxTurns, timeoutMs, validatorFor, + mayDispatch: (name) => ledger.mayDispatch(name), + recordSpend: (name, usd) => ledger.recordSpend(name, usd), }); // Phase 1: triage (full/scoped), staging pr.diff and review-files.json. @@ -636,6 +669,19 @@ export const runDispatch = async ( } } + // The spend ceiling's sheds, disclosed the same way every other shed is. + // Read from the ledger rather than tracked alongside it, so the record and + // the disclosure cannot disagree about what happened. + const spend = ledger.report(); + for (const shed of spend.sheds) { + if (!skippedDimensions.some((skip) => skip.dimension === shed.agent)) { + skippedDimensions.push({ + dimension: shed.agent, + cause: "budget", + }); + } + } + const dispatched = [ ...new Set( perAgent @@ -669,6 +715,7 @@ export const runDispatch = async ( `Note: ${suppression.suppressed.length} finding(s) not re-posted (already tracked in open review threads).`, ] : []), + ...spend.sheds.map((shed) => noteLine.budget(shed.agent, shed.kind)), ]; const result: DispatchResult = { @@ -690,6 +737,7 @@ export const runDispatch = async ( ...(excludedFiles !== undefined ? {excludedFiles} : {}), perAgent, totalUsd: perAgent.reduce((sum, agent) => sum + agent.usd, 0), + spend, }; const serialized = JSON.stringify(result, null, 2); fs.writeFileSync(`${REVIEW_DIR}/dispatch-result.json`, serialized); @@ -725,10 +773,24 @@ if (typeof require !== "undefined" && require.main === module) { // harness; a leftover REVIEW_DISPATCH_RUNNER=sdk must fail loudly // rather than silently running the other harness. rejectStaleRunnerSelection(process.env); - const runner = await createPiRunner(); + // ONE ledger for the run, shared by the runner (which reports each + // turn's cost as it lands) and the dispatcher (which refuses new work + // once the budget is gone). Two ledgers would be two budgets, and the + // per-turn abort would never reach the agents actually spending. + const ledger = createSpendLedger(); + const runner = await createPiRunner({ + onTurnCost: (agentSpentUsd) => + ledger.wouldCross(agentSpentUsd) ? "abort" : "continue", + abortSignal: ledger.signal, + }); const repoRoot = process.env.REVIEW_REPO_ROOT ?? process.env.GITHUB_WORKSPACE ?? "."; - const result = await runDispatch({fs: nodeFs, runner, repoRoot}); + const result = await runDispatch({ + fs: nodeFs, + runner, + repoRoot, + ledger, + }); // eslint-disable-next-line no-console console.log( JSON.stringify( @@ -739,6 +801,7 @@ if (typeof require !== "undefined" && require.main === module) { skippedDimensions: result.skippedDimensions, claims: result.claims.length, totalUsd: result.totalUsd, + spend: result.spend, }, null, 2, diff --git a/workflows/review/lib/spend-ledger.test.ts b/workflows/review/lib/spend-ledger.test.ts index bf7224dc..2462be77 100644 --- a/workflows/review/lib/spend-ledger.test.ts +++ b/workflows/review/lib/spend-ledger.test.ts @@ -57,29 +57,41 @@ describe("decideSpend", () => { describe("createSpendLedger", () => { it("keeps going while under budget and reports the spend", () => { const led = ledger(); - expect(led.recordTurn("correctness-reviewer", 3)).toBe("continue"); - expect(led.recordTurn("correctness-reviewer", 2)).toBe("continue"); + expect(led.recordSpend("correctness-reviewer", 3)).toBeUndefined(); + expect(led.recordSpend("correctness-reviewer", 2)).toBeUndefined(); expect(led.spentUsd()).toBe(5); expect(led.report().crossed).toBe(false); expect(led.report().sheds).toEqual([]); expect(led.signal.aborted).toBe(false); }); - it("aborts in-flight agents on the turn that crosses", () => { + it("probes mid-flight spend without disturbing the accounting", () => { const led = ledger(); - expect(led.recordTurn("skill-auditor", 7)).toBe("continue"); - expect(led.recordTurn("skill-auditor", 2)).toBe("abort"); - // The abort is what stops work already running, and it carries the - // arithmetic so the log says why rather than just that. + // The probe is what an in-flight agent asks every turn. It answers + // against settled spend plus this agent's own, and it must not record: + // an agent that is still running has not paid yet. + expect(led.wouldCross(7)).toBe(false); + expect(led.wouldCross(9)).toBe(true); + expect(led.spentUsd()).toBe(0); + expect(led.signal.aborted).toBe(false); + expect(led.report().sheds).toEqual([]); + }); + + it("aborts the siblings already in flight when a dispatch crosses", () => { + const led = ledger(); + // This is the case the per-turn probe cannot cover: the agent that + // crossed has finished, and the ones running beside it are still + // spending. The signal is what reaches them. + led.recordSpend("correctness-reviewer", 9); expect(led.signal.aborted).toBe(true); expect(String(led.signal.reason)).toMatch( - /spend ceiling reached: \$9\.00 of \$10\.00.*budget \$8\.00.*reserve \$2\.00/, + /spend ceiling reached: \$9\.00 of \$10\.00.*budget \$8\.00.*reserve \$1?2?\.00/, ); }); it("refuses a new dispatch once crossed, and discloses it as a shed", () => { const led = ledger(); - led.recordTurn("correctness-reviewer", 9); + led.recordSpend("correctness-reviewer", 9); expect(led.mayDispatch("data-migrations")).toBe(false); const report = led.report(); expect(report.sheds).toEqual([ @@ -92,15 +104,15 @@ describe("createSpendLedger", () => { const led = ledger(); // One expensive turn lands past the budget in a single step; the // ledger cannot prevent that, only measure it. - led.recordTurn("correctness-reviewer", 11); + led.recordSpend("correctness-reviewer", 11); expect(led.report().overshootUsd).toBe(3); }); it("aborts once, then keeps accumulating sheds without re-aborting", () => { const led = ledger(); - led.recordTurn("a", 9); + led.recordSpend("a", 9); const firstReason = led.signal.reason; - led.recordTurn("b", 5); + led.recordSpend("b", 5); expect(led.signal.reason).toBe(firstReason); expect(led.report().sheds).toHaveLength(2); expect(led.report().spentUsd).toBe(14); @@ -112,7 +124,7 @@ describe("createSpendLedger", () => { env: {REVIEW_SPEND_ENFORCEMENT: "proxy-only"}, warn: (m: string) => warnings.push(m), }); - expect(led.recordTurn("correctness-reviewer", 9)).toBe("continue"); + led.recordSpend("correctness-reviewer", 9); expect(led.mayDispatch("data-migrations")).toBe(true); expect(led.signal.aborted).toBe(false); // Still measured and still disclosed, so a rolled-back run is not a @@ -144,8 +156,8 @@ describe("createSpendLedger", () => { it("ignores a negative turn cost rather than banking credit", () => { const led = ledger(); - led.recordTurn("a", 5); - led.recordTurn("a", -100); + led.recordSpend("a", 5); + led.recordSpend("a", -100); expect(led.spentUsd()).toBe(5); }); }); diff --git a/workflows/review/lib/spend-ledger.ts b/workflows/review/lib/spend-ledger.ts index 7a128db1..0531b451 100644 --- a/workflows/review/lib/spend-ledger.ts +++ b/workflows/review/lib/spend-ledger.ts @@ -142,11 +142,28 @@ export type SpendLedgerOptions = { export type SpendLedger = { /** - * Add one turn's cost and decide whether the agent may continue. Returns - * "abort" the first time the dispatch budget is crossed, and on every call - * after; the caller stops the turn loop and the ledger records the shed. + * Record one COMPLETED dispatch's cost. The single writer: the dispatcher + * calls this once per attempt (a retry and a refusal fallback are separate + * attempts and each pays), so the total is exact and cannot double count. + * + * Deliberately not fed by the runner as well. A ledger with two writers + * would either double count the same dollars or need per-attempt identity + * the runner does not have, and the failure mode of getting that wrong is + * a ceiling that is quietly off by a factor. */ - recordTurn: (agent: string, deltaUsd: number) => "continue" | "abort"; + recordSpend: (agent: string, usd: number) => void; + /** + * Mid-flight probe: would an agent that has spent `inFlightUsd` so far push + * the run past its budget? Read-only, so calling it cannot disturb the + * accounting, and the runner calls it every turn to decide whether to stop. + * + * Concurrency is approximated exactly as the investigation cap approximates + * it: each in-flight agent probes with its own spend and cannot see its + * siblings', so a wave can overshoot by at most the number of agents + * running at once. That is acceptable for a ceiling and is measured in the + * report's `overshootUsd` rather than assumed away. + */ + wouldCross: (inFlightUsd: number) => boolean; /** * Whether a NEW agent may be dispatched. Distinct from recordTurn because * refusing to start is cheaper than aborting mid-flight, and the two are @@ -212,20 +229,21 @@ export const createSpendLedger = ( }; return { - recordTurn: (agent, deltaUsd) => { - spentUsd += Math.max(0, deltaUsd); - const decision = decideSpend( - spentUsd, + recordSpend: (agent, usd) => { + spentUsd += Math.max(0, usd); + if (decideSpend(spentUsd, ceilingUsd, landingReserveUsd).crossed) { + cross(agent, "aborted"); + } + }, + wouldCross: (inFlightUsd) => { + if (enforcement !== "in-code") { + return false; + } + return decideSpend( + spentUsd + Math.max(0, inFlightUsd), ceilingUsd, landingReserveUsd, - ); - if (!decision.crossed) { - return "continue"; - } - cross(agent, "aborted"); - // Proxy-only measures without enforcing, so the turn loop continues - // exactly as it did before this ledger existed. - return enforcement === "in-code" ? "abort" : "continue"; + ).crossed; }, mayDispatch: (agent) => { const decision = decideSpend( From 70fcbb1106523e2b6b6b8264a1bf65d4594fd590 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Thu, 13 Aug 2026 17:20:05 -0400 Subject: [PATCH 4/4] [jwies/review-spend-ceiling] review: make the spend record truthful and let the reserve fund landing Review feedback on #323, all four blocking findings plus two non-blocking: - A completed dispatch that crosses the budget is no longer shed as 'aborted': the completer ran and its findings are kept. The agents the crossing actually cut are disclosed via the new ledger.recordAborted, fed by the dispatcher (a budget-stopped completion always; a thrown attempt only when the ledger's signal killed it), and a budget cut recorded upstream as a generic 'unavailable' is upgraded to cause budget in the disclosure. - The landing reserve now funds the landing: ledger.enterLanding() (called at Phase 3) moves the gates to the full ceiling and swaps in a fresh abort signal, so the validator and anything after it can spend the reserve instead of being refused or born aborted by the dispatch-phase crossing. The runner accepts an abort-signal thunk and resolves it per request. - The per-turn budget probe (onTurnCost -> budgetExhausted -> stoppedForBudget) and the run-wide abort linking (pre-aborted and aborted-later) are pinned by tests. - Rollback mode (proxy-only) no longer records refused sheds for dispatches that actually ran; the run instead carries an explicit crossed-with-enforcement-off note. - Injected env typed Record per the sibling-module idiom. --- workflows/review/lib/dispatch-calls.ts | 13 +++ .../review/lib/dispatch-runner-pi.test.ts | 72 ++++++++++++++ workflows/review/lib/dispatch-runner-pi.ts | 11 ++- workflows/review/lib/dispatch.ts | 45 ++++++++- workflows/review/lib/spend-ledger.test.ts | 43 ++++++++- workflows/review/lib/spend-ledger.ts | 95 ++++++++++++++----- 6 files changed, 248 insertions(+), 31 deletions(-) diff --git a/workflows/review/lib/dispatch-calls.ts b/workflows/review/lib/dispatch-calls.ts index 74b7c27a..1f7d388d 100644 --- a/workflows/review/lib/dispatch-calls.ts +++ b/workflows/review/lib/dispatch-calls.ts @@ -167,6 +167,14 @@ export type AgentDispatcherOptions = { * one really was paid for, so each is recorded. */ recordSpend?: (name: string, usd: number) => void; + /** + * Disclose an attempt the spend ceiling cut. `budget-stop` is a dispatch + * that completed early because the per-turn probe said stop (its partial + * findings are kept, and the cut is disclosed); `run-failed` is a thrown + * attempt, which the caller attributes to the budget only when the + * ledger's abort signal is what killed it. + */ + recordAborted?: (name: string, why: "budget-stop" | "run-failed") => void; }; export type AgentDispatcher = { @@ -201,6 +209,7 @@ export const createAgentDispatcher = ( validatorFor, mayDispatch, recordSpend, + recordAborted, } = options; /** @@ -332,6 +341,9 @@ export const createAgentDispatcher = ( : {}), }); recordSpend?.(name, result.usd); + if (result.stoppedForBudget === true) { + recordAborted?.(name, "budget-stop"); + } return result.output; } catch (error) { writeOut( @@ -352,6 +364,7 @@ export const createAgentDispatcher = ( : {fellBackTo: modelOverride}), failed: "run-failed", }); + recordAborted?.(name, "run-failed"); return null; } }; diff --git a/workflows/review/lib/dispatch-runner-pi.test.ts b/workflows/review/lib/dispatch-runner-pi.test.ts index 0598df9c..4bd73d31 100644 --- a/workflows/review/lib/dispatch-runner-pi.test.ts +++ b/workflows/review/lib/dispatch-runner-pi.test.ts @@ -596,6 +596,78 @@ describe("createPiRunner", () => { expect(stopped).toBe(true); }); + it("stops at the turn boundary when onTurnCost says abort, and flags it", async () => { + // The per-turn budget probe: cost lands per turn_end, the probe sees + // the agent's cumulative spend, and an abort answer ends the loop at + // the turn boundary (shouldStopAfterTurn), flagged stoppedForBudget. + const probes: number[] = []; + let stopped = false; + loop = ({config, emit}) => { + const shouldStop = config["shouldStopAfterTurn"] as () => boolean; + emit(turnEnd('{"findings": []}', 5)); + stopped = shouldStop(); + if (!stopped) { + emit(turnEnd("second turn", 5)); + } + return Promise.resolve([]); + }; + const runner = await createPiRunner({ + onTurnCost: (spent) => { + probes.push(spent); + return spent >= 5 ? "abort" : "continue"; + }, + }); + const result = await runner(request()); + expect(stopped).toBe(true); + expect(probes).toEqual([5]); + expect(result.stoppedForBudget).toBe(true); + expect(result.turns).toBe(1); + expect(result.usd).toBe(5); + }); + + it("never flags stoppedForBudget while the probe says continue", async () => { + loop = async ({emit}) => { + emit(turnEnd('{"findings": []}', 0.1)); + return []; + }; + const runner = await createPiRunner({onTurnCost: () => "continue"}); + const result = await runner(request()); + expect(result.stoppedForBudget).toBeUndefined(); + }); + + it("aborts an in-flight request when the run-wide signal fires", async () => { + // The case onTurnCost cannot cover: the ledger's signal stops the + // siblings running beside the agent that crossed. + const controller = new AbortController(); + loop = ({signal}) => + new Promise((_resolve, reject) => { + signal?.addEventListener("abort", () => reject(signal.reason), { + once: true, + }); + }); + const runner = await createPiRunner({ + abortSignal: controller.signal, + }); + const pending = runner(request()); + controller.abort(new Error("review spend ceiling reached")); + await expect(pending).rejects.toThrow(/spend ceiling/); + }); + + it("hands a pre-aborted run signal to the loop already aborted", async () => { + const controller = new AbortController(); + controller.abort(new Error("review spend ceiling reached")); + let sawAborted: boolean | undefined; + loop = async ({signal}) => { + sawAborted = signal?.aborted; + return []; + }; + const runner = await createPiRunner({ + abortSignal: () => controller.signal, + }); + await runner(request()); + expect(sawAborted).toBe(true); + }); + it("reports the turn cap as max_turns, not as a clean finish", async () => { // Out of turns and finished-with-prose otherwise return the same // shape, and `dispatch.ts` would spend its one re-dispatch correcting diff --git a/workflows/review/lib/dispatch-runner-pi.ts b/workflows/review/lib/dispatch-runner-pi.ts index 1f29e207..cd66b991 100644 --- a/workflows/review/lib/dispatch-runner-pi.ts +++ b/workflows/review/lib/dispatch-runner-pi.ts @@ -624,8 +624,12 @@ export type PiRunnerOptions = { * cover: that probe only stops the agent doing the spending, while this * stops the siblings already in flight beside it. Without it, "crossing * aborts in-flight agents" would be true only of the agent that crossed. + * + * A thunk is accepted because the ledger's signal is phase-dependent (the + * landing phase swaps in a fresh signal so reserve-funded dispatches are + * not born aborted); it is resolved once per request. */ - abortSignal?: AbortSignal; + abortSignal?: AbortSignal | (() => AbortSignal | undefined); }; export const createPiRunner = async ( @@ -677,7 +681,10 @@ export const createPiRunner = async ( const abort = new AbortController(); // Link the run-wide abort in both directions in time: already-aborted // means never start, and aborting later means stop now. - const runAbort = options.abortSignal; + const runAbort = + typeof options.abortSignal === "function" + ? options.abortSignal() + : options.abortSignal; if (runAbort?.aborted === true) { abort.abort(runAbort.reason); } else { diff --git a/workflows/review/lib/dispatch.ts b/workflows/review/lib/dispatch.ts index 6d2752f4..a8029573 100644 --- a/workflows/review/lib/dispatch.ts +++ b/workflows/review/lib/dispatch.ts @@ -346,6 +346,15 @@ export const runDispatch = async ( validatorFor, mayDispatch: (name) => ledger.mayDispatch(name), recordSpend: (name, usd) => ledger.recordSpend(name, usd), + // The sheds disclosure names the agents the crossing actually cut: a + // budget-stop is always a cut (the per-turn probe ended it early); a + // failed attempt is one only when the ledger's signal is what killed + // it (any other failure has its own cause and its own disclosure). + recordAborted: (name, why) => { + if (why === "budget-stop" || ledger.signal.aborted) { + ledger.recordAborted(name); + } + }, }); // Phase 1: triage (full/scoped), staging pr.diff and review-files.json. @@ -638,7 +647,12 @@ export const runDispatch = async ( console.error(threadSuppressionUnavailable.warning); } - // Phase 3: claim validation. + // Phase 3: claim validation. The landing phase starts here: from this + // point the ledger gates against the FULL ceiling, so the landing reserve + // can fund the validation and posting it was held back for (gating the + // validator by the dispatch budget would leave the reserve as dead + // headroom and silently disable validation exactly on expensive runs). + ledger.enterLanding(); let validatorRan = false; if (claims.length > 0) { fs.writeFileSync( @@ -674,11 +688,20 @@ export const runDispatch = async ( // the disclosure cannot disagree about what happened. const spend = ledger.report(); for (const shed of spend.sheds) { - if (!skippedDimensions.some((skip) => skip.dimension === shed.agent)) { + const existing = skippedDimensions.find( + (skip) => skip.dimension === shed.agent, + ); + if (existing === undefined) { skippedDimensions.push({ dimension: shed.agent, cause: "budget", }); + } else if (existing.cause === "unavailable") { + // The fan-out records a refused or aborted dispatch as a generic + // unavailable (a null output looks the same either way from + // there); the ledger knows the real cause, and "unavailable" + // on a budget cut is the wrong-agents disclosure defect. + existing.cause = "budget"; } } @@ -716,6 +739,19 @@ export const runDispatch = async ( ] : []), ...spend.sheds.map((shed) => noteLine.budget(shed.agent, shed.kind)), + // The rollback posture is disclosed on the run itself: crossing with + // enforcement off cuts nothing (so there are no sheds to say so), but + // a reader must still be able to tell this run overspent its in-code + // ceiling and was allowed to. + ...(spend.crossed && spend.enforcement === "proxy-only" + ? [ + `Note: this run crossed the in-code spend ceiling ($${spend.spentUsd.toFixed( + 2, + )} of $${spend.ceilingUsd.toFixed( + 2, + )}) with enforcement rolled back to proxy-only; nothing was cut.`, + ] + : []), ]; const result: DispatchResult = { @@ -781,7 +817,10 @@ if (typeof require !== "undefined" && require.main === module) { const runner = await createPiRunner({ onTurnCost: (agentSpentUsd) => ledger.wouldCross(agentSpentUsd) ? "abort" : "continue", - abortSignal: ledger.signal, + // A thunk, not a captured value: the ledger swaps signals when + // the landing phase starts, and a landing dispatch wired to the + // already-fired dispatch-phase signal would be born aborted. + abortSignal: () => ledger.signal, }); const repoRoot = process.env.REVIEW_REPO_ROOT ?? process.env.GITHUB_WORKSPACE ?? "."; diff --git a/workflows/review/lib/spend-ledger.test.ts b/workflows/review/lib/spend-ledger.test.ts index 2462be77..e9f56124 100644 --- a/workflows/review/lib/spend-ledger.test.ts +++ b/workflows/review/lib/spend-ledger.test.ts @@ -94,12 +94,26 @@ describe("createSpendLedger", () => { led.recordSpend("correctness-reviewer", 9); expect(led.mayDispatch("data-migrations")).toBe(false); const report = led.report(); + // The completer is NOT shed: it ran to completion and its findings + // are kept, so listing it as cut would contradict the record. Only + // the refused dispatch appears. expect(report.sheds).toEqual([ - {agent: "correctness-reviewer", atUsd: 9, kind: "aborted"}, {agent: "data-migrations", atUsd: 9, kind: "refused"}, ]); }); + it("discloses the in-flight agents the crossing actually cut", () => { + const led = ledger(); + led.recordSpend("correctness-reviewer", 9); + // The dispatcher reports each sibling the abort killed or the + // per-turn probe stopped; the ledger records them as the aborted + // sheds the disclosure names. + led.recordAborted("security-auth"); + expect(led.report().sheds).toEqual([ + {agent: "security-auth", atUsd: 9, kind: "aborted"}, + ]); + }); + it("records overshoot so the worst case is measurable, not estimated", () => { const led = ledger(); // One expensive turn lands past the budget in a single step; the @@ -108,16 +122,35 @@ describe("createSpendLedger", () => { expect(led.report().overshootUsd).toBe(3); }); - it("aborts once, then keeps accumulating sheds without re-aborting", () => { + it("aborts once, then keeps accumulating spend without re-aborting", () => { const led = ledger(); led.recordSpend("a", 9); const firstReason = led.signal.reason; led.recordSpend("b", 5); expect(led.signal.reason).toBe(firstReason); - expect(led.report().sheds).toHaveLength(2); expect(led.report().spentUsd).toBe(14); }); + it("lets the landing phase spend the reserve, gated by the full ceiling", () => { + const led = ledger(); // ceiling 10, reserve 2 (dispatch budget 8) + led.recordSpend("correctness-reviewer", 9); + // Crossed: the dispatch-phase signal fired and new fan-out work is + // refused. + expect(led.signal.aborted).toBe(true); + expect(led.mayDispatch("data-migrations")).toBe(false); + // Landing: the reserve funds validation. Fresh signal, gate moves to + // the full ceiling. + led.enterLanding(); + expect(led.signal.aborted).toBe(false); + expect(led.mayDispatch("claim-validator")).toBe(true); + expect(led.wouldCross(0.5)).toBe(false); + // The full ceiling still binds: crossing it aborts the landing + // signal and refuses further work. + led.recordSpend("claim-validator", 1.5); + expect(led.signal.aborted).toBe(true); + expect(led.mayDispatch("thread-reconciler")).toBe(false); + }); + it("measures without enforcing under the proxy-only rollback, loudly", () => { const warnings: string[] = []; const led = ledger({ @@ -132,7 +165,9 @@ describe("createSpendLedger", () => { const report = led.report(); expect(report.enforcement).toBe("proxy-only"); expect(report.crossed).toBe(true); - expect(report.sheds).toHaveLength(2); + // No sheds: every dispatch actually ran under the rollback, so a + // shed entry would disclose a cut that never happened. + expect(report.sheds).toHaveLength(0); // The bypass is never silent. expect(warnings.join(" ")).toContain("proxy-only"); }); diff --git a/workflows/review/lib/spend-ledger.ts b/workflows/review/lib/spend-ledger.ts index 0531b451..8fe514de 100644 --- a/workflows/review/lib/spend-ledger.ts +++ b/workflows/review/lib/spend-ledger.ts @@ -135,7 +135,7 @@ export type SpendLedgerOptions = { * flag is read ONCE at construction: an enforcement posture that could * change mid-run is not a posture. */ - env?: {[key: string]: string | undefined}; + env?: Record; /** Where the loud rollback notice goes. Defaults to stderr. */ warn?: (message: string) => void; }; @@ -150,8 +150,26 @@ export type SpendLedger = { * would either double count the same dollars or need per-attempt identity * the runner does not have, and the failure mode of getting that wrong is * a ceiling that is quietly off by a factor. + * + * Crossing here records NO shed for the recording agent: it ran to + * completion and its findings are kept, so listing it as cut would make + * the record contradict itself. The agents the crossing actually cuts are + * disclosed through {@link SpendLedger.recordAborted}. */ recordSpend: (agent: string, usd: number) => void; + /** + * Disclose one agent the crossing actually cut: stopped at a turn + * boundary by the per-turn probe, or killed in flight by the abort + * signal. Called by the dispatcher, which is the only party that knows + * which attempt died and why. + */ + recordAborted: (agent: string) => void; + /** + * Enter the landing phase: the reviewer fan-out is over, and from here on + * the gates compare against the FULL ceiling, so the landing reserve can + * fund the validation it was held back for. One-way, once per run. + */ + enterLanding: () => void; /** * Mid-flight probe: would an agent that has spent `inFlightUsd` so far push * the run past its budget? Read-only, so calling it cannot disturb the @@ -170,8 +188,15 @@ export type SpendLedger = { * disclosed differently. */ mayDispatch: (agent: string) => boolean; - /** Aborted when the budget is crossed; wired into in-flight requests. */ - signal: AbortSignal; + /** + * Aborted when the CURRENT phase's budget is crossed; wired into + * in-flight requests. A property getter on purpose: the dispatch-phase + * signal fires at `ceiling - reserve` and must not kill the landing + * dispatches the reserve exists to fund, so entering the landing phase + * swaps in a fresh signal that fires only at the full ceiling. Consumers + * that need the live value must read it per request, not capture it once. + */ + readonly signal: AbortSignal; spentUsd: () => number; report: () => SpendReport; }; @@ -199,22 +224,34 @@ export const createSpendLedger = ( ); } - const controller = new AbortController(); + const dispatchController = new AbortController(); + const landingController = new AbortController(); const sheds: SpendShed[] = []; let spentUsd = 0; let overshootUsd = 0; let crossed = false; + let phase: "dispatch" | "landing" = "dispatch"; + + /** The reserve the CURRENT phase still holds back (landing spends it). */ + const reserveNow = (): number => + phase === "landing" ? 0 : landingReserveUsd; - /** Cross once: the first crossing aborts, later ones just accumulate. */ - const cross = (agent: string, kind: SpendShed["kind"]): void => { + /** + * Note a crossing of the current phase's budget: track overshoot (always + * against the dispatch budget, the report's denominator), and abort the + * phase's controller. No shed is recorded here; who was actually cut is + * the dispatcher's knowledge ({@link SpendLedger.recordAborted}). + */ + const noteCrossing = (): void => { const budget = Math.max(0, ceilingUsd - landingReserveUsd); overshootUsd = Math.max(overshootUsd, spentUsd - budget); - sheds.push({agent, atUsd: spentUsd, kind}); - if (crossed) { + crossed = true; + if (enforcement !== "in-code") { return; } - crossed = true; - if (enforcement === "in-code") { + const controller = + phase === "landing" ? landingController : dispatchController; + if (!controller.signal.aborted) { controller.abort( new Error( `review spend ceiling reached: $${spentUsd.toFixed( @@ -222,7 +259,7 @@ export const createSpendLedger = ( )} of ` + `$${ceilingUsd.toFixed(2)} (dispatch budget ` + `$${budget.toFixed(2)}, landing reserve ` + - `$${landingReserveUsd.toFixed(2)})`, + `$${landingReserveUsd.toFixed(2)}, phase ${phase})`, ), ); } @@ -230,11 +267,18 @@ export const createSpendLedger = ( return { recordSpend: (agent, usd) => { + void agent; // The completer is not shed; see the type's doc. spentUsd += Math.max(0, usd); - if (decideSpend(spentUsd, ceilingUsd, landingReserveUsd).crossed) { - cross(agent, "aborted"); + if (decideSpend(spentUsd, ceilingUsd, reserveNow()).crossed) { + noteCrossing(); } }, + recordAborted: (agent) => { + sheds.push({agent, atUsd: spentUsd, kind: "aborted"}); + }, + enterLanding: () => { + phase = "landing"; + }, wouldCross: (inFlightUsd) => { if (enforcement !== "in-code") { return false; @@ -242,22 +286,29 @@ export const createSpendLedger = ( return decideSpend( spentUsd + Math.max(0, inFlightUsd), ceilingUsd, - landingReserveUsd, + reserveNow(), ).crossed; }, mayDispatch: (agent) => { - const decision = decideSpend( - spentUsd, - ceilingUsd, - landingReserveUsd, - ); + const decision = decideSpend(spentUsd, ceilingUsd, reserveNow()); if (decision.allowed) { return true; } - cross(agent, "refused"); - return enforcement !== "in-code"; + noteCrossing(); + if (enforcement !== "in-code") { + // Rollback mode: the dispatch proceeds, so recording it as a + // shed would disclose a cut that never happened. The report + // still says crossed, which is the honest record. + return true; + } + sheds.push({agent, atUsd: spentUsd, kind: "refused"}); + return false; + }, + get signal() { + return ( + phase === "landing" ? landingController : dispatchController + ).signal; }, - signal: controller.signal, spentUsd: () => spentUsd, report: () => ({ schemaVersion: 1,