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

github-actions · 2026-07-10T20:09:32Z

note (non-blocking): LANDING_RESERVE_RATIO = 0.75 reads as the reserve held back, but it is the fraction of the cap the soft target aims at (softCapUsd = capUsd * LANDING_RESERVE_RATIO, line 78); the reserve is the complementary 0.25, as the "A quarter of the cap" docstring says. No behavioral defect — the tests pin 0.75 — but the inverted name invites a future "fix" toward 0.25 that would quietly quarter every clamped budget. A name like LANDING_TARGET_RATIO would match the arithmetic.

Lower-confidence observations (non-blocking)

credit-cap.ts:114 — resolveCreditCap checks the hand-synced REVIEW_MAX_AI_CREDITS mirror above the machine-enforced awf-config. No defect today (the in-repo consumer omits the mirror and falls through to the awf-config's true cap), but a consumer that set a stale mirror would clamp to the wrong cap; a min-across-sources or awf-first order would be desync-safe.

review.md:244 — the in-repo consumer .github/workflows/review.md overrides max-ai-credits: 2500 with no REVIEW_MAX_AI_CREDITS mirror, so the KEEP-IN-SYNC contract is already unheld in-repo; it resolves correctly only via the awf-config fall-through.

review.md:807 — the 0.75 clamp reserve, the ~0.75 shed trigger, and the ÷5,000 token proxy compound to roughly 3× conservatism, so a tight-cap run may start shedding at ~1⁄3 of the cap. Consider carrying the margin in one place.

github-actions · 2026-07-10T20:09:32Z

nitpick (non-blocking): The !Number.isFinite(capCredits) guard is untested (the zero/negative branch on the same condition is). Low value in practice since resolveCreditCap filters non-finite values before the clamp, so this is only reachable via a direct call to the exported function.

github-actions · 2026-07-10T20:09:32Z

suggestion (non-blocking): No test covers a valid-JSON awf config whose apiProxy.maxAiCredits is absent or non-numeric — the exact case this typeof cap === "number" guard defends. The suite covers unparseable and numeric configs but not this fall-through, so a regression that dropped the type check wouldn't be caught. A small addition would close it:

it("falls through when a valid awf config lacks a numeric maxAiCredits", () => { const {fs} = fakeFs({"/tmp/gh-aw/awf-config.json": JSON.stringify({apiProxy: {}})}); expect(resolveCreditCap({}, fs)).toBe(DEFAULT_MAX_AI_CREDITS); });

-Original file line number
+Diff line change
@@ -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.

-Original file line number
+Diff line change
@@ Expand Up / @@ -72,7 +72,19 @@ export type LiveDefectSpec = { @@
      */
     export type CaseLive = {
         prContext: LivePrContext;
-        /** Case-dir-relative path to the post-change tree (default `tree`). */
+        /**
+         * Case-dir-relative path to the post-change tree (default `tree`).
+         *
+         * Trees are deliberately minimal: validation requires only the changed
+         * files to exist. The staged copy of this tree is the sub-agent's whole
+         * world (its cwd, with no network), so the agent WILL often try to read
+         * an imported module or caller that is not there; that read returns an
+         * ordinary not-found tool error the agent tolerates and works around,
+         * not a run failure. The cost of a missing file is realism, not a crash:
+         * include the context files whose absence would change what a reviewer
+         * concludes (e.g. the decorator module a sibling handler imports), and
+         * nothing more.
+         */
         tree: string;
         /** Labeled defects a live run must catch. */
         mustCatchSpecs?: LiveDefectSpec[];
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -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;
+    };

-Original file line number
+Diff line change
@@ -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,
@@ Expand Down Expand Up / @@ -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                                                            */
     /* -------------------------------------------------------------------------- */
@@ Expand Down Expand Up / @@ -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({
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

Uh oh!

Diff view

Diff view

There are no files selected for viewing

github-actions Bot Jul 10, 2026

Uh oh!

github-actions Bot Jul 10, 2026

Uh oh!

github-actions Bot Jul 10, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Are you sure you want to change the base?

Uh oh!

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

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

github-actions Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!