From 7044f8ca9e4ae6e6f681c42226efc9a3e1ee184d Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 14 Sep 2026 17:34:13 +0900 Subject: [PATCH] fix(responses): carry the transient send budget across combo children (#4546) The budget was a counter local to one handleResponsesInner frame, and a combo parent runs a separate child turn per target, so a three-target fan-out took three fresh allowances. It is now a holder on HandleResponsesOptions, minted at genuine ingress and inherited by children through the options spread that already carries comboAttempt and translatorBudget. --- src/lib/upstream-retry.ts | 15 ++++++++++ src/server/responses/core.ts | 28 +++++++++++++------ .../lib/transient-budget-scope-source.test.ts | 10 +++++-- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/lib/upstream-retry.ts b/src/lib/upstream-retry.ts index dbeb846da3..6fd57eb406 100644 --- a/src/lib/upstream-retry.ts +++ b/src/lib/upstream-retry.ts @@ -57,6 +57,21 @@ const RESET_RETRY_MAX_DELAY_MS = 1_000; // Transient-5xx status retry layer (pre-stream only; devlog/_plan/260716_claudecode_hardening/010). /** Total sends one transient-retry helper call may make: 1 initial + 2 retries. */ export const TRANSIENT_RETRY_MAX_ATTEMPTS = 3; + +/** + * Transient sends already spent by one LOGICAL request. + * + * A mutable holder rather than a counter local to one call frame, because the thing that has to + * share it spans frames: a combo parent runs a separate child turn per target, and a per-child + * counter is what let one logical request reach upstream three times per target (#4546). + */ +export interface TransientSendBudget { + used: number; +} + +export function createTransientSendBudget(): TransientSendBudget { + return { used: 0 }; +} const TRANSIENT_RETRY_BASE_DELAY_MS = 400; const TRANSIENT_RETRY_MAX_DELAY_MS = 5_000; // A failed attempt slower than this is the "slow 502" incident shape (191s observed on diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index aa441e6b7d..40e7f5bd50 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -224,6 +224,8 @@ import { prepareSameTarget429Wait, sleepWithAbort, TRANSIENT_RETRY_MAX_ATTEMPTS, + createTransientSendBudget, + type TransientSendBudget, } from "../../lib/upstream-retry"; import { ForwardAdmissionCredentialError, @@ -1902,6 +1904,12 @@ export interface HandleResponsesOptions { onStoredPool401ReplayDispatched?: () => void; /** Caller-owned for Chat/Claude replay; omitted only at genuine Responses ingress. */ translatorBudget?: TranslatorBudget; + /** + * Transient sends already spent by this logical request. Combo children inherit the parent's + * holder through the options spread, so a fan-out shares one allowance instead of taking a + * fresh one per target (#4546). + */ + sendBudget?: TransientSendBudget; /** * Terminal vision-describe marker (roadmap 180): true when the inbound * request IS the vision sidecar's own loopback describe call. The plan site @@ -3448,6 +3456,9 @@ export async function handleResponses( visionDescribeTerminal: options.visionDescribeTerminal === true || req.headers.get("x-opencodex-vision-describe") === "1", translatorBudget, + // Created once at genuine ingress; a combo child arrives with the parent's holder already + // in options and must not start a fresh allowance. + sendBudget: options.sendBudget ?? createTransientSendBudget(), }); return ownsBudget ? finalizeOwnedTranslatorBudget(response, translatorBudget) : response; } catch (error) { @@ -4972,15 +4983,16 @@ async function handleResponsesInner( routedMuseToolNameAliases = builtRequest.convertedMuseToolNameAliases ?? new Map(); }; - // One request-scoped transient-retry budget owner, declared ABOVE the passthrough branch so - // that branch shares it too. It used to sit below, which put it in the temporal dead zone for - // the passthrough sends and left each recovery leg taking the helper's fresh default of 3 -- - // the source of the measured amplification in #4546. A per-leg budget lets a request that - // recovers several times multiply upstream load. - let transientSendsUsed = 0; - const noteTransientSends = (used: number): void => { transientSendsUsed += Math.max(0, used); }; + // One transient-retry budget for the whole LOGICAL request, read ABOVE the passthrough branch + // so that branch shares it too. It used to be a local declared below, which put it in the + // temporal dead zone for the passthrough sends and left each recovery leg taking the helper's + // fresh default of 3. It is now a holder carried on options, so a combo child inherits the + // parent's spend instead of starting over per target -- both halves of the measured + // amplification in #4546. + const sendBudget = options.sendBudget ?? createTransientSendBudget(); + const noteTransientSends = (used: number): void => { sendBudget.used += Math.max(0, used); }; const remainingTransientSendBudget = (budget: number): number => - Math.max(1, budget - transientSendsUsed); + Math.max(1, budget - sendBudget.used); if ("passthrough" in adapter && adapter.passthrough && !routedCompaction) { let hostAdmissionLease = pendingHostAdmissionLease; diff --git a/tests/lib/transient-budget-scope-source.test.ts b/tests/lib/transient-budget-scope-source.test.ts index 66ee919b66..9fb26bac23 100644 --- a/tests/lib/transient-budget-scope-source.test.ts +++ b/tests/lib/transient-budget-scope-source.test.ts @@ -25,8 +25,14 @@ describe("transient send budget stays request-scoped", () => { test("every transient-retry call site draws from the shared counter", () => { const core = source("server/responses/core.ts"); - // One owner per request, declared before any leg can send. - expect(core.match(/let transientSendsUsed = 0;/g)).toHaveLength(1); + // One holder per LOGICAL request, read before any leg can send and inherited by combo + // children through the options spread rather than recreated per child turn. + expect(core.match(/const sendBudget = options\.sendBudget \?\? createTransientSendBudget\(\);/g)) + .toHaveLength(1); + // Genuine ingress mints it; a child arrives with the parent's and must not replace it. + expect(core).toContain("sendBudget: options.sendBudget ?? createTransientSendBudget(),"); + // The regressed shape: a counter local to one call frame, which a combo child restarts. + expect(core).not.toContain("let transientSendsUsed = 0;"); expect(core.match(/const remainingTransientSendBudget = \(budget: number\): number =>/g)).toHaveLength(1); // Seven legs report into the same counter: the adapter initial send, the 429/rotation