review: clamp the run budget to the effective credit cap; land before the cap

.changeset/review-budget-cap-clamp.md

@@ -0,0 +1,5 @@
+---
+"review": patch
+---
+
+Clamp the router's run budget to the effective per-run AI-credits cap, and make the cap visible to the run. The tier budget table is sized inside the workflow's assumed $10 ceiling, but the cap is enforced runner-side by the firewall api-proxy and was invisible in-container, so a consumer with a tighter `max-ai-credits` got soft targets that promised more work than the cap could pay for; the graceful-degradation path never fired and the run died at the api-proxy with findings in hand (observed on a 400-credit behavior test: three finders completed, ~390 credits spent, killed mid-persist before validation, nothing posted). The frontmatter now sets `max-ai-credits: 1000` explicitly and mirrors it into the agent environment as `REVIEW_MAX_AI_CREDITS` (kept in sync by consumers that override the cap); the router resolves the cap from that mirror (falling back to `GH_AW_MAX_AI_CREDITS`, a visible awf config, then gh-aw's 1000-credit default) and scales the selected tier's soft targets proportionally, floored at the trivial tier's values, marking `runBudget.capClamped` and `runBudget.effectiveCreditCap` in routing.json. The budget prompt gains a matching estimated-credits proxy (sum of in-band `subagent_tokens` over completed sub-agents, divided by 5,000) and two mandatory proxy checkpoints, the critical one immediately after the last finder returns and before claim validation, so a tight run sheds and lands a partial review instead of dying at the cap.

workflows/review/eval/judge-live-model.ts

@@ -45,7 +45,9 @@ const scoreOne = async (request: JudgeRequest): Promise<JudgeScore> => {
         "",
         `Evidence trace:\n${request.evidenceTrace.join("\n")}`,
     ].join("\n");
-    let response: Response | undefined;
+    // Typed via the fetch signature: the repo's eslint no-undef does not
+    // know the fetch-API globals in type position.
+    let response: Awaited<ReturnType<typeof fetch>> | undefined;
     for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt += 1) {
         try {
             response = await fetch(API_URL, {

workflows/review/lib/credit-cap.ts

@@ -0,0 +1,145 @@
+/**
+ * The effective per-run AI-credit cap: discovery from the environment and the
+ * clamp that resizes a tier budget to fit inside it. Split from `router.ts`
+ * by concern (and its max-lines budget); the router imports both functions
+ * and remains the single CLI entry point.
+ */
+
+import type {RunBudget} from "./router";
+import {DEFAULT_TIER_BUDGETS} from "./router";
+
+/** The read-only slice of the filesystem surface cap discovery needs. */
+export type CreditCapFs = {
+    readFileSync: (p: string, enc: "utf8") => string;
+    existsSync: (p: string) => boolean;
+};
+
+/**
+ * gh-aw's baked-in per-run AI-credits default (credits; 1 credit = $0.01),
+ * assumed when no cap is discoverable from the environment.
+ */
+export const DEFAULT_MAX_AI_CREDITS = 1000;
+
+/**
+ * The landing reserve: a clamped budget's soft targets aim at this fraction
+ * of the effective cap, never the cap itself. Spend is unobservable mid-run
+ * (the orchestrator works from proxies), and requests already in flight when
+ * a checkpoint passes still bill after it, so a soft target equal to the hard
+ * cap leaves no room to land: the second budget-shed acceptance run engaged
+ * the clamp and still died at 416/400 credits mid-skill-audit. A quarter of
+ * the cap is sized to absorb both the proxy estimation error and the
+ * in-flight overshoot observed there.
+ */
+export const LANDING_RESERVE_RATIO = 0.75;
+
+/**
+ * Clamp a tier budget to the effective per-run credit cap. The tier table is
+ * sized inside the workflow's assumed $10 ceiling; when a consumer sets a
+ * tighter `max-ai-credits`, the un-clamped soft targets promise more work
+ * than the hard cap can pay for, and the run dies at the api-proxy with
+ * findings in hand instead of shedding early (observed: a 400-credit run
+ * planned against the high tier's $10 targets and was killed mid-validation).
+ * The clamped soft dollar target is {@link LANDING_RESERVE_RATIO} of the cap,
+ * not the cap itself, so a run that sheds against its targets lands inside
+ * the hard ceiling. Scaling is proportional to spend, floored at the trivial
+ * tier's values so even a tiny cap yields the smallest designed review rather
+ * than zero work. A zero/negative cap means explicitly uncapped; undefined
+ * means unknown.
+ */
+export const clampBudgetToCreditCap = (
+    budget: RunBudget,
+    capCredits: number | undefined,
+): RunBudget => {
+    if (
+        capCredits === undefined ||
+        !Number.isFinite(capCredits) ||
+        capCredits <= 0
+    ) {
+        return budget;
+    }
+    const capUsd = capCredits / 100;
+    const softCapUsd = capUsd * LANDING_RESERVE_RATIO;
+    if (softCapUsd >= budget.maxUsd) {
+        // The tier's own soft target already leaves at least the reserve
+        // under the hard cap; record the cap and change nothing.
+        return {...budget, effectiveCreditCap: capCredits};
+    }
+    const ratio = softCapUsd / budget.maxUsd;
+    const floor = DEFAULT_TIER_BUDGETS.trivial;
+    const scale = (value: number, min: number): number =>
+        Math.max(min, Math.floor(value * ratio));
+    return {
+        ...budget,
+        maxReviewerInvocations: scale(
+            budget.maxReviewerInvocations,
+            floor.maxReviewerInvocations,
+        ),
+        maxToolCallsPerFinding: scale(
+            budget.maxToolCallsPerFinding,
+            floor.maxToolCallsPerFinding,
+        ),
+        maxTotalToolCalls: scale(
+            budget.maxTotalToolCalls,
+            floor.maxTotalToolCalls,
+        ),
+        maxWallClockMinutes: scale(
+            budget.maxWallClockMinutes,
+            floor.maxWallClockMinutes,
+        ),
+        maxUsd: softCapUsd,
+        effectiveCreditCap: capCredits,
+        capClamped: true,
+    };
+};
+
+/**
+ * Resolve the effective per-run AI-credit cap from the environment, most
+ * explicit source first:
+ *
+ *   1. `REVIEW_MAX_AI_CREDITS` — the frontmatter `env:` mirror of
+ *      `max-ai-credits` (the cap itself is enforced runner-side by the
+ *      firewall api-proxy and is not otherwise exported to the agent).
+ *   2. `GH_AW_MAX_AI_CREDITS` — in case a future gh-aw exports it directly.
+ *   3. The awf firewall config (`apiProxy.maxAiCredits`), when its file is
+ *      visible from the agent container.
+ *   4. gh-aw's baked-in default ({@link DEFAULT_MAX_AI_CREDITS}).
+ *
+ * Returns the cap in credits; may be zero/negative when a source explicitly
+ * disables the cap (the clamp treats that as uncapped).
+ */
+export const resolveCreditCap = (
+    env: Record<string, string | undefined>,
+    fs: CreditCapFs,
+): number => {
+    for (const name of ["REVIEW_MAX_AI_CREDITS", "GH_AW_MAX_AI_CREDITS"]) {
+        const raw = env[name];
+        if (raw !== undefined && raw.trim() !== "") {
+            const parsed = Number(raw);
+            if (Number.isFinite(parsed)) {
+                return parsed;
+            }
+        }
+    }
+    const runnerTemp = env["RUNNER_TEMP"];
+    const candidates = [
+        ...(runnerTemp ? [`${runnerTemp}/gh-aw/awf-config.json`] : []),
+        "/tmp/gh-aw/awf-config.json",
+    ];
+    for (const path of candidates) {
+        if (!fs.existsSync(path)) {
+            continue;
+        }
+        try {
+            const parsed = JSON.parse(fs.readFileSync(path, "utf8")) as {
+                apiProxy?: {maxAiCredits?: unknown};
+            };
+            const cap = parsed.apiProxy?.maxAiCredits;
+            if (typeof cap === "number" && Number.isFinite(cap)) {
+                return cap;
+            }
+        } catch {
+            // Unreadable or unparseable candidate: fall through to the next.
+        }
+    }
+    return DEFAULT_MAX_AI_CREDITS;
+};

workflows/review/lib/router.test.ts

@@ -1,5 +1,11 @@
 import {describe, it, expect} from "vitest";
 
+import {
+    clampBudgetToCreditCap,
+    DEFAULT_MAX_AI_CREDITS,
+    resolveCreditCap,
+} from "./credit-cap";
+
 import {
     ALWAYS_ON_LENSES,
     computeRunBudget,
@@ -386,6 +392,151 @@ describe("computeRunBudget: scaling", () => {
     });
 });
 
+describe("clampBudgetToCreditCap", () => {
+    it("records but does not clamp a cap that covers the tier budget", () => {
+        const clamped = clampBudgetToCreditCap(DEFAULT_TIER_BUDGETS.high, 2500);
+        expect(clamped).toEqual({
+            ...DEFAULT_TIER_BUDGETS.high,
+            effectiveCreditCap: 2500,
+        });
+        expect(clamped.capClamped).toBeUndefined();
+    });
+
+    it("scales every soft target to the reserve-adjusted cap, not the cap", () => {
+        // The budget-shed incident shape: the high tier ($10) planned inside a
+        // 400-credit ($4) cap, so the run died at the api-proxy mid-validation.
+        // The soft dollar target is 75% of the cap (the landing reserve): the
+        // acceptance re-run showed a soft target equal to the hard cap still
+        // dies at it (416/400), because spend is unobservable mid-run and
+        // in-flight work bills after the last observable checkpoint.
+        const clamped = clampBudgetToCreditCap(DEFAULT_TIER_BUDGETS.high, 400);
+        expect(clamped).toEqual({
+            ...DEFAULT_TIER_BUDGETS.high,
+            maxReviewerInvocations: 3, // floor(12 * 0.3)
+            maxToolCallsPerFinding: 2, // floor(8 * 0.3)
+            maxTotalToolCalls: 36, // floor(120 * 0.3)
+            maxWallClockMinutes: 6, // floor(20 * 0.3)
+            maxUsd: 3, // 4 * 0.75
+            effectiveCreditCap: 400,
+            capClamped: true,
+        });
+    });
+
+    it("leaves a tier alone only when its target clears the reserve", () => {
+        // $10 tier under a 1400-credit ($14) cap: 75% of the cap is $10.50,
+        // above the tier's own $10 target, so nothing needs resizing.
+        const clamped = clampBudgetToCreditCap(DEFAULT_TIER_BUDGETS.high, 1400);
+        expect(clamped).toEqual({
+            ...DEFAULT_TIER_BUDGETS.high,
+            effectiveCreditCap: 1400,
+        });
+        // $10 tier under a 1200-credit ($12) cap: the reserve-adjusted target
+        // ($9) is below the tier's $10, so the clamp engages.
+        const tight = clampBudgetToCreditCap(DEFAULT_TIER_BUDGETS.high, 1200);
+        expect(tight.capClamped).toBe(true);
+        expect(tight.maxUsd).toBe(9);
+    });
+
+    it("never drops below the trivial tier's floors", () => {
+        const floor = DEFAULT_TIER_BUDGETS.trivial;
+        const clamped = clampBudgetToCreditCap(DEFAULT_TIER_BUDGETS.high, 10);
+        expect(clamped.maxReviewerInvocations).toBe(
+            floor.maxReviewerInvocations,
+        );
+        expect(clamped.maxToolCallsPerFinding).toBe(
+            floor.maxToolCallsPerFinding,
+        );
+        expect(clamped.maxTotalToolCalls).toBe(floor.maxTotalToolCalls);
+        expect(clamped.maxWallClockMinutes).toBe(floor.maxWallClockMinutes);
+        // The dollar target has no floor: a floor above the cap would defeat
+        // the cap. 75% of the $0.10 cap, within float precision.
+        expect(clamped.maxUsd).toBeCloseTo(0.075, 10);
+        expect(clamped.capClamped).toBe(true);
+    });
+
+    it("treats a zero or negative cap as explicitly uncapped", () => {
+        expect(clampBudgetToCreditCap(DEFAULT_TIER_BUDGETS.high, -1)).toEqual(
+            DEFAULT_TIER_BUDGETS.high,
+        );
+        expect(clampBudgetToCreditCap(DEFAULT_TIER_BUDGETS.high, 0)).toEqual(
+            DEFAULT_TIER_BUDGETS.high,
+        );
+    });
+
+    it("applies through computeRunBudget via config.maxAiCredits", () => {
+        const budget = computeRunBudget("high", false, {
+            ...baseConfig,
+            maxAiCredits: 400,
+        });
+        expect(budget.tier).toBe("high");
+        expect(budget.capClamped).toBe(true);
+        expect(budget.maxUsd).toBe(3);
+    });
+});
+
+describe("resolveCreditCap", () => {
+    const noFs = {
+        existsSync: (): boolean => false,
+        readFileSync: (): string => {
+            throw new Error("unexpected read");
+        },
+    };
+
+    it("prefers the REVIEW_MAX_AI_CREDITS frontmatter mirror", () => {
+        expect(
+            resolveCreditCap(
+                {REVIEW_MAX_AI_CREDITS: "2500", GH_AW_MAX_AI_CREDITS: "400"},
+                noFs,
+            ),
+        ).toBe(2500);
+    });
+
+    it("falls back to GH_AW_MAX_AI_CREDITS", () => {
+        expect(resolveCreditCap({GH_AW_MAX_AI_CREDITS: "400"}, noFs)).toBe(400);
+    });
+
+    it("ignores non-numeric and empty env values and keeps looking", () => {
+        expect(
+            resolveCreditCap(
+                {REVIEW_MAX_AI_CREDITS: "lots", GH_AW_MAX_AI_CREDITS: ""},
+                noFs,
+            ),
+        ).toBe(DEFAULT_MAX_AI_CREDITS);
+    });
+
+    it("reads apiProxy.maxAiCredits from a visible awf config", () => {
+        const {fs} = fakeFs({
+            "/tmp/gh-aw/awf-config.json": JSON.stringify({
+                apiProxy: {maxAiCredits: 400},
+            }),
+        });
+        expect(resolveCreditCap({}, fs)).toBe(400);
+    });
+
+    it("prefers the RUNNER_TEMP awf config over the /tmp fallback", () => {
+        const {fs} = fakeFs({
+            "/rt/gh-aw/awf-config.json": JSON.stringify({
+                apiProxy: {maxAiCredits: 250},
+            }),
+            "/tmp/gh-aw/awf-config.json": JSON.stringify({
+                apiProxy: {maxAiCredits: 400},
+            }),
+        });
+        expect(resolveCreditCap({RUNNER_TEMP: "/rt"}, fs)).toBe(250);
+    });
+
+    it("survives an unparseable awf config and falls through", () => {
+        const {fs} = fakeFs({
+            "/tmp/gh-aw/awf-config.json": "not json",
+        });
+        expect(resolveCreditCap({}, fs)).toBe(DEFAULT_MAX_AI_CREDITS);
+    });
+
+    it("defaults to gh-aw's baked-in cap when nothing is discoverable", () => {
+        expect(resolveCreditCap({}, noFs)).toBe(DEFAULT_MAX_AI_CREDITS);
+    });
+});
+
 /* -------------------------------------------------------------------------- */
 /* Misrouted floor                                                            */
 /* -------------------------------------------------------------------------- */
@@ -606,6 +757,21 @@ describe("runCli", () => {
         expect(JSON.parse(written[ROUTING_OUT])).toEqual(json);
     });
 
+    it("clamps the run budget to the credit cap from the environment", () => {
+        const {fs} = fakeFs({
+            [FILES_PATH]: JSON.stringify([
+                {path: "db/migrations/0001.sql", status: "added"},
+            ]),
+            [ROUTING_CONFIG_PATH]:
+                "**/migrations/** tier=high lens=data-migrations",
+        });
+        const json = runCli(fs, ".", {REVIEW_MAX_AI_CREDITS: "400"});
+        expect(json.runBudget.tier).toBe("high");
+        expect(json.runBudget.capClamped).toBe(true);
+        expect(json.runBudget.maxUsd).toBe(3);
+        expect(json.runBudget.effectiveCreditCap).toBe(400);
+    });
+
     it("accepts the {files:[...]} wrapper and degrades safely without a ROUTING config", () => {
         const {fs, written} = fakeFs({
             [FILES_PATH]: JSON.stringify({