fix(responses): one send budget per logical request, and zero means zero (#4546)

devlog/_plan/260914_cost_guard_stabilization/030_move_reason_evidence.md

@@ -34,3 +34,26 @@ wp3 lands the reason at the decision point and the record, because that is what
 makes the wp2 and wp3 rules auditable in the field rather than only in tests. The
 dashboard rendering and the amplification metric (sends per logical request) belong
 with wp4, where the send budget gives them a denominator that means something.
+
+## Outcome
+
+Closed. `resolveCodexAccountForThreadDetailed` now returns a `CodexAffinityDecision` on every
+selection path, the pool auth context carries it, and `logCtx.affinity` / `logCtx.affinityReason`
+are assigned in `core.ts` (`849f3c9ccf`). A release recorded by the outcome path -- a 429
+clearing the pin -- is held per thread, bounded at 4096 entries, and consumed by that thread's
+next resolve.
+
+Two audit rounds changed the shape, and both corrections are worth keeping:
+
+The reason was being synthesized at the call site instead of read from the guard that actually
+refused the account. It now comes from `codexAccountBlockReason`, and a release survives a
+resolve that finds no account at all (`b8d90ba3a8`, closing #4598).
+
+`appendUsageEntry` builds the persisted entry from an explicit field whitelist, so the affinity
+fields the writer set were dropped silently by the normalizer and the whole feature was a no-op
+end to end. `ab6fd697c1` adds them to the whitelist and surfaces the decision in the route
+explanation. The general lesson for anything downstream of the usage log: a field the writer
+sets but the normalizer does not name does not exist.
+
+What wp3 deliberately did not do: render the reason in the dashboard, and count sends per
+logical request. Both wait for wp4's budget to give them a denominator.

devlog/_plan/260914_cost_guard_stabilization/040_send_budget.md

@@ -143,3 +143,113 @@ That is observable today on the Codex, passthrough and combo paths --
 `logCtx.attempts[].sendCount` across combo children. It is **not** observable for the
 Kiro and Cursor inner retries, which call `noteAttemptSend` once before dispatching,
 so those need instrumentation before their counts can be pinned.
+
+## Step 1 status, and six corrections the next audit round produced
+
+Step 1 landed (`7f9284ab1e`): `sendBudget` rides `HandleResponsesOptions`, is minted once at
+ingress (`core.ts:3461`) and inherited by a combo child through the existing options spread
+(`core.ts:3113`). Six findings from the follow-up audit change what comes next, so they are
+recorded rather than quietly folded in.
+
+**The combo fix is half a fix.** A child inherits the *counter* but the adapter initial send
+never reads it as a *limit*: `core.ts:7656` passes `attempts: transientPolicy.attempts` raw.
+The oracle's own comment justifies that with "nothing has been spent yet", which is true for a
+first turn and false for combo target 2. So target 1 can spend the budget and target 2 still
+draws a fresh full policy allowance. Until `:7656` draws the remainder like every other leg,
+the measured 12 does not come down.
+
+**The cross-account move is not merely unbudgeted, it is unbounded per request.**
+`retryCodexPoolOnAlternateAccount` is at `core.ts:1434` (not `:1645`), and it sends directly
+with `fetchWithHeaderTimeout` at `:1626` inside a loop whose `maxRetrySends` is 1 for a real
+alternate but **7** for the same-account gated-400 ladder. The important part is the caller:
+it sits inside `passthroughRecovery: for (;;)` (`:5628`), `excludeAccountId` excludes only the
+account that just failed (`:1492`), and no per-request flag records that a move already
+happened. Sequential account moves are bounded today by pool exhaustion and cooldowns, by
+nothing else. A flat `used` counter does not close that; a separate move counter does.
+
+**`fetchWithResetRetry` has no counting seam at all.** `onSendsConsumed` lives only on
+`TransientRetryOptions` (`upstream-retry.ts:304`) and fires only from `fetchWithTransientRetry`
+(`:479`). Every leg that falls back to reset-only retry -- the non-policy adapter initial send
+and every `rebuildAndRefetch` recovery kind when `refetchTransientPolicy` is null -- is
+*uncountable*, not just uncounted. Step 2 therefore starts by giving `ResetRetryOptions` the
+same callback, not by adding call-site wiring.
+
+**There is a fourth floor.** Besides `core.ts:4995` and `upstream-retry.ts:374, 420`, the
+inner `remaining = () => Math.max(1, budget - sent)` at `upstream-retry.ts:439` re-floors the
+reset call. Removing the three named sites still lets a spent budget send once.
+
+**The exhaustion contract is already decided by the codebase, twice.** `fetchWithTransientRetry`
+returns the last response with its body intact when the budget runs out (`:476`), and the
+reachable native-Chat path preserves the terminal 429 (pinned at
+`tests/responses/chat-completions-endpoint.test.ts:1553, 1597`). The synthetic throw at
+`chat-native.ts:308` is an unreachable backstop, not the policy. Return-the-last-answer is the
+contract; a throw would hide the status, the `Retry-After` header and any quota body -- exactly
+the evidence #3294/#3606 said to preserve. The throw stays only as a typed backstop for a
+caller that forgot to check.
+
+**`sendCount` already reaches the wire.** The claim above that it "never reaches /api/usage or
+the GUI" is wrong. It is a required persisted field (`src/usage/log.ts:100`), it survives the
+whitelist normalizer (`:465`), `/api/logs` spreads it (`src/server/management/shared.ts:222`)
+and the GUI already types it (`gui/src/pages/Logs.tsx:126`). What is missing is rendering (the
+attempts table has no column) and aggregation (`summarizeUsage` counts attempts, never sends).
+
+## Delivery slices
+
+Steps 2-5 are not one diff. Verification here is hosted CI only, so a slice that breaks forty
+pinned counts at once is undiagnosable. They ship in this order, one PR each:
+
+- **Slice A (this cycle).** Split the budget and close the two holes that need no new plumbing:
+  `TransientSendBudget` gains `accountMoves` with `CROSS_ACCOUNT_MAX_SENDS = 1`;
+  `retryCodexPoolOnAlternateAccount` charges a move and refuses a second one with the existing
+  `{ kind: "no-alternate" }` path after `recordUnmovedTransientOutcome()`; the adapter initial
+  send at `:7656` draws `remainingTransientSendBudget(transientPolicy.attempts)`. The split has
+  to come first because step 2 without it collapses the working 3 same-account + 1 alternate
+  shape that `tests/responses/responses-compaction-routing.test.ts:1346` pins.
+- **Slice B.** `onSendsConsumed` on `ResetRetryOptions`, unconditional wiring at `:7652` and
+  `:7775`, `sendBudget` on `HandleResponsesCompactOptions`, and the empty-completion /
+  `runTurnAttempt` charge at `core.ts:7346`.
+- **Slice C.** All four floors to `Math.max(0, ...)` plus the refusal contract above, with the
+  pinned counts in `responses-opaque-blob-recovery.test.ts` rewritten to the refusal shape.
+- **Slice D.** The pool-wide retry ratio cap and `sendCount` aggregation.
+
+Kiro (up to ~18 sends per call, ~36 with the text fallback) and Cursor ride
+`AdapterFetchContext`; that field must be optional and unlimited by default or every adapter
+unit test that calls the transport context-free breaks.
+
+## Slice A landed, and the four counterexamples that shaped it
+
+PR #4609 carries the guarded profile from the PRD: four model sends per logical request, a base
+allowance of three, and one final-recovery reserve that an account move and a validated rebuild
+share. An adversarial audit round found four things that would have shipped as defects.
+
+**Charging the same send twice.** `permit.use()` increments `used`, and `onSendsConsumed`
+increments it again for anything routed through the retry helper. A four-send cap would have
+behaved as a two-send cap and every acceptance row would have been off by a factor of two. The
+intent now carries `countedExternally`, so a helper-routed permit books the reserve and the
+alternate-target ledgers but leaves `used` to the reporter.
+
+**Removing the floor kills a recovery the PRD wants kept.** The pinned sanitized-rebuild case
+at `responses-opaque-blob-recovery.test.ts:600` is three 502s plus one rebuild, and its own
+comment says the rebuild "draws on what is LEFT of that same budget" -- which is the floor. With
+the floor gone the rebuild gets zero and the request dies at three. `recoverySendAllowance`
+spends the base allowance first and only then draws the reserve, which is what keeps that fourth
+send alive for the right reason instead of by accident.
+
+**The exhaustion contract is a call-site problem.** A typed throw inside the helper cannot
+restore a body the caller already cancelled, and every catch on these paths launders a rejection
+into 502 `upstream_error`. So the OAuth 401 replay and the same-target 429 wait check the
+remainder in their own conditions, before the cancel, and an exhausted request returns the real
+401 or 429 with its `Retry-After`. The typed error stays only as the backstop for a leg that
+never had a prior response.
+
+**Reserving too early burns the slot on a request that never moved.** The same-account
+gated-model 400 ladder runs through the same function and is bounded at eight sends by
+`maxRetrySends`. Reserving before `retrySameConfirmedAccount` is known would have spent the
+single failover slot on it. The reservation is guarded on `!retryAuthCtx`, which the ladder has
+already set.
+
+Residual, accepted rather than hidden: `maxTargetTransitions` and `maxAlternateTargetSends`
+would refuse the pinned three-target combo hop, so combo hops are not wired to
+`reserveDispatch` in this slice and those fields are exercised only by the account-failover
+path. Wiring combo needs a per-target policy, not a per-request transition cap. Compact, Kiro,
+Cursor and the generic OAuth hops still hold their own allowances.

src/lib/request-execution-budget.ts

@@ -0,0 +1,202 @@
+/**
+ * One logical request, one execution budget (#4546).
+ *
+ * The amplification behind #4546 was never a single missing limit. Every layer that can
+ * re-send a request -- transport retry, adapter retry, auth recovery, account failover, combo
+ * failover, repair -- counted its own allowance, so a per-layer 3 composed into a per-request
+ * 12. #4605 and #4608 gave the transient layers one shared counter; this module is the policy
+ * that counter answers to.
+ *
+ * The policy is an INTERSECTION of constraints, not four independent counters. A request that
+ * still has total allowance left is not thereby entitled to a second account move, and a
+ * request that changed credentials does not get its target-transition allowance back. The
+ * default profile keeps the recovery shape that actually works today -- three same-account
+ * sends plus one alternate -- by funding the alternate from a reserve that a validated
+ * sanitized repair can spend instead, but never both.
+ */
+import type { TransientSendBudget } from "./upstream-retry";
+
+export type SendClass =
+  | "initial"
+  | "transient"
+  | "auth-recovery"
+  | "repair"
+  | "account-failover"
+  | "combo-failover"
+  | "prewarm";
+
+export interface RequestExecutionBudgetPolicy {
+  /** Every model send of one logical request, including the reserve. */
+  readonly maxTotalModelSends: number;
+  /** Shared by the initial send, same-target transient retries, and refresh/repair legs. */
+  readonly baseSendAllowance: number;
+  /** ONE final recovery, shared by an account move and a validated rebuild. Not one each. */
+  readonly finalRecoveryAllowance: number;
+  readonly maxAlternateTargetSends: number;
+  readonly maxTargetTransitions: number;
+}
+
+/**
+ * Text Codex guarded profile. Three same-account sends plus one alternate is the recovery
+ * shape that live traffic depends on, so a flat ceiling of 3 would break a working path.
+ */
+export const CODEX_TEXT_GUARDED_BUDGET_POLICY: RequestExecutionBudgetPolicy = {
+  maxTotalModelSends: 4,
+  baseSendAllowance: 3,
+  finalRecoveryAllowance: 1,
+  maxAlternateTargetSends: 1,
+  maxTargetTransitions: 1,
+};
+
+export const REQUEST_BUDGET_POLICY_VERSION = "guarded-v1";
+
+export type BudgetDenial =
+  | "total-exhausted"
+  | "base-allowance-exhausted"
+  | "final-recovery-spent"
+  | "alternate-target-exhausted"
+  | "target-transition-exhausted"
+  | "not-replay-safe";
+
+export interface DispatchIntent {
+  readonly sendClass: SendClass;
+  /**
+   * (provider route, endpoint, model lane, upstream credential identity). A quota domain is a
+   * different thing and must not be folded in here.
+   */
+  readonly targetKey: string;
+  /**
+   * False refuses the dispatch outright. A request whose execution state upstream is unknown
+   * is not replayable just because budget remains (RFC 9110 9.2.2).
+   */
+  readonly replaySafe?: boolean;
+  /**
+   * True when the physical send is already reported through another counter -- the retry
+   * helpers' `onSendsConsumed` hook. The permit then books the reserve, alternate-target and
+   * transition ledgers but leaves `used` to that reporter, because charging both is how a
+   * four-send cap silently becomes a two-send cap.
+   */
+  readonly countedExternally?: boolean;
+}
+
+export interface SingleUseDispatchPermit {
+  readonly sendClass: SendClass;
+  /** Consume exactly once. A second call returns false and charges nothing. */
+  use(): boolean;
+}
+
+export type DispatchDecision =
+  | { allowed: true; permit: SingleUseDispatchPermit }
+  | { allowed: false; reason: BudgetDenial };
+
+/**
+ * Carried on HandleResponsesOptions so a combo child, a rebuild and an alternate-account leg
+ * all decrement the same holder. `used` is the existing #4605 counter and still counts every
+ * model send; the reserve is what the fourth send draws on once the base allowance is gone.
+ */
+export interface RequestExecutionBudget extends TransientSendBudget {
+  readonly logicalRequestId: string;
+  readonly policyVersion: string;
+  readonly policy: RequestExecutionBudgetPolicy;
+  reserveDispatch(intent: DispatchIntent): DispatchDecision;
+  /**
+   * Sends still available from the base allowance, capped by a layer's own maximum.
+   * Returns 0 when the allowance is gone -- it never floors to 1, because a floor of 1 is
+   * what let every recovery leg send one more time forever.
+   */
+  remainingBaseSends(cap: number): number;
+  readonly reserveSpent: boolean;
+  readonly alternateTargetSends: number;
+  readonly targetTransitions: number;
+  readonly lastTargetKey: string | undefined;
+}
+
+const RESERVE_FUNDED_CLASSES: ReadonlySet<SendClass> = new Set<SendClass>([
+  "account-failover",
+  "combo-failover",
+  "repair",
+  "auth-recovery",
+]);
+
+let logicalRequestSeq = 0;
+
+export function createRequestExecutionBudget(
+  policy: RequestExecutionBudgetPolicy = CODEX_TEXT_GUARDED_BUDGET_POLICY,
+  logicalRequestId?: string,
+): RequestExecutionBudget {
+  let reserveSpent = false;
+  let alternateTargetSends = 0;
+  let targetTransitions = 0;
+  let lastTargetKey: string | undefined;
+
+  const budget: RequestExecutionBudget = {
+    used: 0,
+    logicalRequestId: logicalRequestId ?? `lr-${Date.now().toString(36)}-${(logicalRequestSeq += 1).toString(36)}`,
+    policyVersion: REQUEST_BUDGET_POLICY_VERSION,
+    policy,
+    get reserveSpent() { return reserveSpent; },
+    get alternateTargetSends() { return alternateTargetSends; },
+    get targetTransitions() { return targetTransitions; },
+    get lastTargetKey() { return lastTargetKey; },
+    remainingBaseSends(cap: number): number {
+      const capped = Number.isFinite(cap) ? Math.trunc(cap) : 0;
+      return Math.max(0, Math.min(capped, policy.baseSendAllowance - budget.used));
+    },
+    reserveDispatch(intent: DispatchIntent): DispatchDecision {
+      if (intent.replaySafe === false) return { allowed: false, reason: "not-replay-safe" };
+      if (budget.used >= policy.maxTotalModelSends) return { allowed: false, reason: "total-exhausted" };
+
+      const changesTarget = lastTargetKey !== undefined && lastTargetKey !== intent.targetKey;
+      const isAlternateTarget = changesTarget || intent.sendClass === "account-failover"
+        || intent.sendClass === "combo-failover";
+      if (isAlternateTarget && changesTarget && targetTransitions >= policy.maxTargetTransitions) {
+        return { allowed: false, reason: "target-transition-exhausted" };
+      }
+      if (isAlternateTarget && alternateTargetSends >= policy.maxAlternateTargetSends) {
+        return { allowed: false, reason: "alternate-target-exhausted" };
+      }
+
+      // The base allowance is spent first. Only once it is gone does a recovery class reach
+      // for the single shared reserve -- an account move and a validated rebuild cannot each
+      // take one.
+      const drawsReserve = budget.remainingBaseSends(policy.baseSendAllowance) === 0;
+      if (drawsReserve) {
+        if (!RESERVE_FUNDED_CLASSES.has(intent.sendClass)) {
+          return { allowed: false, reason: "base-allowance-exhausted" };
+        }
+        if (reserveSpent || policy.finalRecoveryAllowance <= 0) {
+          return { allowed: false, reason: "final-recovery-spent" };
+        }
+      }
+
+      let consumed = false;
+      return {
+        allowed: true,
+        permit: {
+          sendClass: intent.sendClass,
+          use(): boolean {
+            if (consumed) return false;
+            consumed = true;
+            // Charged here, immediately before the physical send, rather than reported after
+            // the helper returns: a counter that is only reconciled afterwards cannot stop two
+            // concurrent legs that both read the same remainder.
+            if (intent.countedExternally !== true) budget.used += 1;
+            if (drawsReserve) reserveSpent = true;
+            if (isAlternateTarget) alternateTargetSends += 1;
+            if (changesTarget) targetTransitions += 1;
+            lastTargetKey = intent.targetKey;
+            return true;
+          },
+        },
+      };
+    },
+  };
+  if (lastTargetKey === undefined) lastTargetKey = undefined;
+  return budget;
+}
+
+export function isRequestExecutionBudget(
+  value: TransientSendBudget | undefined,
+): value is RequestExecutionBudget {
+  return typeof (value as RequestExecutionBudget | undefined)?.reserveDispatch === "function";
+}