fix(combos): preserve declared targets under the send budget

src/lib/request-execution-budget.ts

@@ -168,35 +168,53 @@ const RESERVE_FUNDED_CLASSES: ReadonlySet<SendClass> = new Set<SendClass>([
 
 let logicalRequestSeq = 0;
 
-export function createRequestExecutionBudget(
-  policy: RequestExecutionBudgetPolicy = CODEX_TEXT_GUARDED_BUDGET_POLICY,
-  logicalRequestId?: string,
-  observer?: RequestSendObserver,
+/**
+ * One request's physical-send ledger, held apart from the budget object so a derived policy
+ * scope can share the exact same one.
+ *
+ * `spent` and `pendingExternalSends` belong together: a pending booking is a send that is
+ * already counted in `spent` and awaiting its reporter, so a scope that shared one without the
+ * other would either charge that send twice or never charge it at all.
+ *
+ * The durable-spend observer belongs here for the same reason. It books one entry per physical
+ * send by watching this counter move, so a derived scope that spent the counter without
+ * carrying the observer would move it without booking, and a combo child's sends would go
+ * missing from the ledger (#4707).
+ */
+interface SharedSendLedger {
+  spent: number;
+  pendingExternalSends: number;
+  readonly observer?: RequestSendObserver;
+}
+
+const sharedSendLedgers = new WeakMap<RequestExecutionBudget, SharedSendLedger>();
+
+function createRequestExecutionBudgetWithLedger(
+  policy: RequestExecutionBudgetPolicy,
+  logicalRequestId: string | undefined,
+  counter: SharedSendLedger,
 ): RequestExecutionBudget {
-  let spent = 0;
-  // Reservations whose physical send is reported by a retry helper rather than by the permit.
-  // They are already charged; the reporter's first send settles one instead of charging again.
-  let pendingExternalSends = 0;
+  const observer = counter.observer;
   let reserveSpent = false;
   let alternateTargetSends = 0;
   let targetTransitions = 0;
   let lastTargetKey: string | undefined;
 
   const budget: RequestExecutionBudget = {
-    get used(): number { return spent; },
+    get used(): number { return counter.spent; },
     set used(next: number) {
       // The retry helpers report their real send count by assigning through this field. A
       // reservation taken with `countedExternally` has already booked one of those sends, so
       // the report settles the pending booking first and only the surplus is charged.
-      const delta = next - spent;
+      const delta = next - counter.spent;
       if (delta <= 0) {
-        spent = Math.max(0, next);
+        counter.spent = Math.max(0, next);
         return;
       }
-      const settled = Math.min(delta, pendingExternalSends);
-      pendingExternalSends -= settled;
+      const settled = Math.min(delta, counter.pendingExternalSends);
+      counter.pendingExternalSends -= settled;
       const charged = delta - settled;
-      spent += charged;
+      counter.spent += charged;
       // These sends have already left. The ledger records them even past a ceiling it would
       // have refused, because refusing after the fact only hides spend that was really
       // incurred -- the refusal has to happen at the reservation below, or not at all.
@@ -211,11 +229,11 @@ export function createRequestExecutionBudget(
     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 - spent));
+      return Math.max(0, Math.min(capped, policy.baseSendAllowance - counter.spent));
     },
     reserveDispatch(intent: DispatchIntent): DispatchDecision {
       if (intent.replaySafe === false) return { allowed: false, reason: "not-replay-safe" };
-      if (spent >= policy.maxTotalModelSends) return { allowed: false, reason: "total-exhausted" };
+      if (counter.spent >= policy.maxTotalModelSends) return { allowed: false, reason: "total-exhausted" };
 
       const changesTarget = lastTargetKey !== undefined && lastTargetKey !== intent.targetKey;
       const isAlternateTarget = changesTarget || intent.sendClass === "account-failover"
@@ -230,7 +248,7 @@ export function createRequestExecutionBudget(
       // 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 = policy.baseSendAllowance - spent <= 0;
+      const drawsReserve = policy.baseSendAllowance - counter.spent <= 0;
       if (drawsReserve) {
         if (!RESERVE_FUNDED_CLASSES.has(intent.sendClass)) {
           return { allowed: false, reason: "base-allowance-exhausted" };
@@ -250,8 +268,8 @@ export function createRequestExecutionBudget(
       // one remaining send admitted two physical sends, which is the per-request multiplication
       // this budget exists to stop. Everything is booked now; `release()` is the way back.
       const previousTargetKey = lastTargetKey;
-      spent += 1;
-      if (intent.countedExternally === true) pendingExternalSends += 1;
+      counter.spent += 1;
+      if (intent.countedExternally === true) counter.pendingExternalSends += 1;
       if (drawsReserve) reserveSpent = true;
       if (isAlternateTarget) alternateTargetSends += 1;
       if (changesTarget) targetTransitions += 1;
@@ -273,8 +291,8 @@ export function createRequestExecutionBudget(
             // The booking this reservation made for an external reporter is now owned by the
             // caller. Leaving it pending is not harmless: the next `used` report of this request
             // would settle against it and one real send would go uncharged.
-            if (intent.countedExternally === true && pendingExternalSends > 0) {
-              pendingExternalSends -= 1;
+            if (intent.countedExternally === true && counter.pendingExternalSends > 0) {
+              counter.pendingExternalSends -= 1;
             }
             return true;
           },
@@ -284,10 +302,10 @@ export function createRequestExecutionBudget(
             // An externally counted reservation the reporter already settled paid for a send
             // that physically happened. Refunding it would hand the request a free send back.
             if (intent.countedExternally === true) {
-              if (pendingExternalSends === 0) return;
-              pendingExternalSends -= 1;
+              if (counter.pendingExternalSends === 0) return;
+              counter.pendingExternalSends -= 1;
             }
-            spent -= 1;
+            counter.spent -= 1;
             observer?.refund();
             if (drawsReserve) reserveSpent = false;
             if (isAlternateTarget) alternateTargetSends -= 1;
@@ -298,9 +316,60 @@ export function createRequestExecutionBudget(
       };
     },
   };
+  sharedSendLedgers.set(budget, counter);
   return budget;
 }
 
+export function createRequestExecutionBudget(
+  policy: RequestExecutionBudgetPolicy = CODEX_TEXT_GUARDED_BUDGET_POLICY,
+  logicalRequestId?: string,
+  observer?: RequestSendObserver,
+): RequestExecutionBudget {
+  return createRequestExecutionBudgetWithLedger(policy, logicalRequestId, {
+    spent: 0,
+    pendingExternalSends: 0,
+    ...(observer ? { observer } : {}),
+  });
+}
+
+/**
+ * A budget that applies its own policy and keeps its own recovery ledgers while spending the
+ * parent's exact physical-send ledger.
+ *
+ * Aliasing the public `used` property was not enough, and that is the whole defect. The factory
+ * reads its own private counter back in `remainingBaseSends`, in the total check, and in the
+ * reserve test, so an aliased scope answered every admission question from a counter that only
+ * ever saw its own reservations. A combo's per-target holdback is computed from
+ * `maxTotalModelSends` and is therefore unenforceable unless the scope actually observes what
+ * the request has already spent.
+ */
+export function deriveRequestExecutionBudget(
+  parent: RequestExecutionBudget,
+  policy: RequestExecutionBudgetPolicy,
+): RequestExecutionBudget {
+  return createRequestExecutionBudgetWithLedger(policy, parent.logicalRequestId, ledgerFor(parent));
+}
+
+/**
+ * A budget that did not come from this factory still honors the public `used` contract, so
+ * bridge onto it rather than failing the request. `isRequestExecutionBudget` is a shape test,
+ * so a stub can reach here; turning that into a thrown error would convert a routing request
+ * into a 500 to report a condition production never produces. Only a factory-backed parent can
+ * share pending external bookings and a durable-spend observer, which are private by
+ * construction; a bridged scope keeps the parent's spend accurate and books nothing of its own.
+ */
+function ledgerFor(parent: RequestExecutionBudget): SharedSendLedger {
+  const existing = sharedSendLedgers.get(parent);
+  if (existing) return existing;
+  let pendingExternalSends = 0;
+  return {
+    get spent(): number { return parent.used; },
+    set spent(next: number) { parent.used = next; },
+    get pendingExternalSends(): number { return pendingExternalSends; },
+    set pendingExternalSends(next: number) { pendingExternalSends = next; },
+  };
+}
+
 export function isRequestExecutionBudget(
   value: TransientSendBudget | undefined,
 ): value is RequestExecutionBudget {

src/server/responses/core-combo.ts

@@ -2,7 +2,7 @@ import { isDeclaredReasoningEffort } from "../../reasoning-effort";
 import { recordAttemptRequestedEffort } from "../request-log";
 import {
   CODEX_TEXT_GUARDED_BUDGET_POLICY,
-  createRequestExecutionBudget,
+  deriveRequestExecutionBudget,
   isRequestExecutionBudget,
 } from "../../lib/request-execution-budget";
 import type {
@@ -107,25 +107,27 @@ export function comboExecutionBudgetPolicy(declaredTargets: number): RequestExec
 /**
  * A budget scope that keeps its own recovery ledgers but spends the SAME request-wide counter.
  *
- * `used` is redefined as an accessor onto the parent because the factory reads it back off this
- * object -- `remainingBaseSends` and the total check both do -- so a copied number would let a
- * combo target run its ladder against a stale total, which is precisely the per-layer counting
- * this work exists to remove. The reserve, alternate-target and transition ledgers stay
- * per-scope on purpose: a combo target's account failover is its own recovery decision, while
- * the request total still bounds every target together.
+ * The sharing has to happen inside the factory. Redefining `used` as an accessor onto the parent
+ * only shared what callers read from the outside: `remainingBaseSends`, the total check and the
+ * reserve test all consult the factory's own private counter, which an overridden property
+ * cannot reach. Each derived scope therefore admitted dispatches as though the request had spent
+ * nothing, and the per-target holdback below -- expressed against `maxTotalModelSends` -- had
+ * nothing to hold back from.
+ *
+ * `deriveRequestExecutionBudget` binds the scope to the parent's real ledger, including pending
+ * externally-counted bookings and the durable-spend observer, all of which must travel together.
+ * A pending booking is a send already counted in the total and waiting for its reporter, and the
+ * observer books by watching that same counter move (#4707) -- so a scope that spent the counter
+ * without carrying the observer would move it without booking, and this combo's child sends
+ * would go missing from the spend ledger. The reserve, alternate-target and transition ledgers
+ * stay per-scope on purpose: a combo target's account failover is its own recovery decision,
+ * while the request total still bounds every target together.
  */
 export function deriveSendBudgetScope(
   parent: RequestExecutionBudget,
   policy: RequestExecutionBudgetPolicy,
 ): RequestExecutionBudget {
-  const scope = createRequestExecutionBudget(policy, parent.logicalRequestId);
-  Object.defineProperty(scope, "used", {
-    get: () => parent.used,
-    set: (value: number) => { parent.used = value; },
-    enumerable: true,
-    configurable: true,
-  });
-  return scope;
+  return deriveRequestExecutionBudget(parent, policy);
 }
 
 

structure/transports/responses.md

@@ -779,6 +779,27 @@ later recovery in the same request then cannot have. `tests/lib/execution-budget
 pins the settlement rule and every ladder shape against exactly that, and
 `tests/responses/responses-core-modules.test.ts` pins the adapter view's live delegation.
 
+A combo derives a policy scope per target, and that derivation has to happen inside the budget
+factory. Overriding the public `used` property shares only what callers read from outside:
+`remainingBaseSends`, the total check and the reserve test all consult the factory's own private
+counter, which an overridden property cannot reach. Each derived scope therefore admitted
+dispatches as though the request had spent nothing, and the per-target holdback in
+`comboTargetSendBudget` — expressed against `maxTotalModelSends` — had nothing to hold back from,
+so a long failover combo could exhaust the allowance before its later declared targets were ever
+attempted. `deriveRequestExecutionBudget` binds the scope to the parent's real ledger instead.
+
+Three things travel on that shared ledger and have to travel together. The spend and the pending
+externally-counted bookings, because a pending booking is a send already counted in the total and
+waiting for its reporter, so sharing one without the other would either charge that send twice or
+never charge it. And the durable-spend observer below, because it books by watching this counter
+move: a derived scope that spent the counter without carrying the observer would move it without
+booking, and a combo child's sends would go missing from the ledger. `permit.assumeCharge()`
+closes its booking on the same shared ledger, so the adapter handoff above and the combo
+derivation agree rather than each settling against a counter the other cannot see.
+
+What stays per-scope is deliberate: the reserve, alternate-target and transition ledgers are each
+target's own recovery decision, while the physical-send total is what binds every target together.
+
 ## Durable spend reservations
 
 The request's send budget bounds how many times it may reach upstream; the spend ledger bounds