fix(responses): compact and the Kiro inner retries join the request send budget (#4546) #4611
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import { readBoundedResponseBody } from "../lib/bounded-body"; | |
| import { resolveClientRetryAfter } from "../lib/retry-after"; | ||
| import { parseRetryAfterMs } from "../combos"; | ||
| import { | ||
| SendBudgetExhaustedError, | ||
| abortError, | ||
| cancelResponseBodyBestEffort, | ||
| fetchWithAttemptDeadline, | ||
|
|
@@ -162,6 +163,13 @@ async function fetchWithResetRecovery( | |
| let lastError: unknown; | ||
| for (let attempt = 0; attempt < RESET_ATTEMPTS; attempt++) { | ||
| if (ctx.abortSignal?.aborted) throw abortError(ctx.abortSignal); | ||
| // Every physical send is admitted, not just the adapter entry. Kiro nests a throttle loop | ||
| // over this ladder and can run the ladder twice per throttle round, so counting one entry | ||
| // as one send hid up to eighteen upstream requests from the per-request cap (#4546). | ||
| const decision = ctx.sendBudget?.reserveDispatch({ sendClass: "transient", targetKey: url }); | ||
| if (decision && (!decision.allowed || !decision.permit.use())) { | ||
| throw new SendBudgetExhaustedError(url); | ||
|
Comment on lines
+169
to
+171
There was a problem hiding this comment.
When the canonical endpoint returns an endpoint-specific failure and the legacy endpoint then returns a transient 429, these URL-based target keys consume the policy's sole target transition ( AGENTS.md reference: src/AGENTS.md:L17-L19 Useful? React with 👍 / 👎. |
||
| } | ||
| try { | ||
| const headers = new Headers(request.headers); | ||
| const recovered = attempt > 0; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,8 +76,14 @@ import { | |
| fetchWithResetRetry, | ||
| fetchWithTransientRetry, | ||
| applyUpstreamRecoveryInit, | ||
| SendBudgetExhaustedError, | ||
| TRANSIENT_RETRY_MAX_ATTEMPTS, | ||
| type UpstreamSendRecovery, | ||
| } from "../../lib/upstream-retry"; | ||
| import { | ||
| createRequestExecutionBudget, | ||
| type RequestExecutionBudget, | ||
| } from "../../lib/request-execution-budget"; | ||
| import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; | ||
| import { | ||
| acquireUpstreamHostAdmission, | ||
|
|
@@ -224,6 +230,14 @@ export interface HandleResponsesCompactOptions { | |
| nativeMainRefreshDependencies?: NativeMainRefreshDependencies; | ||
| /** Release the listener's idle guard only after the complete request body is accepted. */ | ||
| onRequestBodyRead?: () => void; | ||
| /** | ||
| * The logical request's send budget (#4546). Compact used to hold its own: the normal send | ||
| * took a fresh transient allowance of three, the 401 replay and the 429 alternate each added | ||
| * one -- and the guard that was supposed to make those two mutually exclusive keys on | ||
| * `kind === "pool"`, so a main-pool credential could spend all five. The recursive handoff | ||
| * child then started over, so one compact could reach ten. | ||
| */ | ||
| sendBudget?: RequestExecutionBudget; | ||
| } | ||
|
|
||
| export function compactResponseTooLargeError(): Response { | ||
|
|
@@ -760,6 +774,10 @@ export async function handleResponsesCompact( | |
| // so routed-model reasoning items (reasoning_text content) don't 400 the ChatGPT backend. | ||
| const compactBody = sanitizeReasoningInputContent(compactBodyRaw) as typeof compactBodyRaw; | ||
| const compactUrl = `${base}/responses/compact`; | ||
| // One holder for this logical compact, inherited by the handoff child so a second model | ||
| // does not start over with a fresh four. | ||
| const sendBudget: RequestExecutionBudget = options.sendBudget ?? createRequestExecutionBudget(); | ||
| const compactTargetKey = `${route.providerName}|${route.modelId}|compact`; | ||
|
Comment on lines
+777
to
+780
There was a problem hiding this comment.
Introducing a request-wide adapter transport contract and changing Kiro/compact retry ownership modifies shared adapters and server transport behavior, but the commit leaves all mapped structure documents untouched, including AGENTS.md reference: src/AGENTS.md:L10-L11 Useful? React with 👍 / 👎. |
||
| const actualCompactHostKey = upstreamHostHealthKey( | ||
| route.providerName, | ||
| safeOriginLabel(compactUrl), | ||
|
|
@@ -834,6 +852,27 @@ export async function handleResponsesCompact( | |
| // wrapping reset retry — because those retries happen before any alternate is even | ||
| // considered. The alternate is one bounded send: a second ladder would multiply the | ||
| // work an already-rejecting pool is doing. | ||
| // | ||
| // Both modes now draw one shared budget. The comment below used to say the 401 replay | ||
| // spends the account budget so the 429 alternate is skipped, but that guard is keyed on | ||
| // `kind === "pool"` and a main-pool credential left it false -- so 401 then 429 really did | ||
| // reach five. The single sends spend the base allowance first and then the one shared | ||
| // final-recovery reserve, which is the same rule the Responses path follows. | ||
| const sendSingleCompactAttempt = ( | ||
| doFetch: () => Promise<Response>, | ||
| ): Promise<Response> => { | ||
| if (sendBudget.remainingBaseSends(TRANSIENT_RETRY_MAX_ATTEMPTS) > 0) { | ||
| sendBudget.used += 1; | ||
| return doFetch(); | ||
| } | ||
| const decision = sendBudget.reserveDispatch({ | ||
| sendClass: "auth-recovery", | ||
| targetKey: compactTargetKey, | ||
| }); | ||
| if (!decision.allowed) return Promise.reject(new SendBudgetExhaustedError(safeHostLabel(compactUrl))); | ||
| if (!decision.permit.use()) return Promise.reject(new SendBudgetExhaustedError(safeHostLabel(compactUrl))); | ||
|
Comment on lines
+872
to
+873
There was a problem hiding this comment.
When the initial three-send ladder ends in a 401 for a AGENTS.md reference: src/AGENTS.md:L17-L17 Useful? React with 👍 / 👎. |
||
| return doFetch(); | ||
| }; | ||
| const sendCompactAttempt = ( | ||
| sendProvider: OcxProviderConfig, | ||
| sendHeaders: Headers, | ||
|
|
@@ -866,8 +905,15 @@ export async function handleResponsesCompact( | |
| return res; | ||
| }); | ||
| return recovery === "single" | ||
| ? sendSingleCompactAttempt(doFetch) | ||
| : fetchWithTransientRetry(doFetch, { | ||
| abortSignal: req.signal, | ||
| label: safeHostLabel(compactUrl), | ||
| // Draws the shared remainder instead of a fresh three. Compact is a native endpoint | ||
| // of the same logical turn, so its sends belong to the same cap. | ||
| attempts: sendBudget.remainingBaseSends(TRANSIENT_RETRY_MAX_ATTEMPTS), | ||
| onSendsConsumed: (used: number) => { sendBudget.used += Math.max(0, used); }, | ||
| }); | ||
| }; | ||
|
|
||
| // The account each outcome belongs to. Reassigned only when the alternate send below | ||
|
|
@@ -1133,7 +1179,9 @@ export async function handleResponsesCompact( | |
| logCtx, | ||
| turnAdmissionLease, | ||
| admission, | ||
| // The handoff child is the same logical compact on a second model, so it inherits | ||
| // the holder. Forwarding `options` alone was not enough: the child minted its own. | ||
| { ...options, sendBudget }, | ||
| ); | ||
| if (fallback.ok || fallback.status === 499) return fallback; | ||
| await fallback.body?.cancel().catch(() => undefined); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit changes physical-send admission in both Kiro's nested retry ladder and native compact's refresh, alternate-account, and recursive-handoff paths, but it adds no tests. Existing Kiro retry tests omit
sendBudget, so they continue exercising the explicitly unlimited compatibility path and cannot detect regressions such as legacy-endpoint throttling or compact budget exhaustion; add focused budgeted cases to the existing Kiro and compaction test files.AGENTS.md reference: src/AGENTS.md:L22-L26
Useful? React with 👍 / 👎.