fix(ai): honor Retry-After when retry-after-ms is unparseable#111353
Conversation
clawsweeper
Bot
added
proof: sufficient
|
The single red check ( It fails in That suite passes locally on the same commit (8/8, Happy to rebase if you'd prefer a fresh run, but I did not want to churn the branch over an unrelated flake. |
|
Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 7:36 AM ET / 11:36 UTC. Summary PR surface: Source 0, Tests +58. Total +58 across 2 files. Reproducibility: yes. source-reproducible with high confidence: a retryable 429 carrying Review metrics: none identified. Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused ordered-header fallback after the final merge-result check, preserving the existing clamp and Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible with high confidence: a retryable 429 carrying Is this the best way to solve the issue? Yes. Falling through only when the preferred millisecond header cannot be parsed is the narrowest fix and matches the ordered parser behavior used by the official OpenAI client. (github.com) AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c95a8e3df1e6. Label changesLabel justifications:
Evidence reviewedPR surface: Source 0, Tests +58. Total +58 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles) |
|
Refreshed against current On the earlier red check, for the record: One correction on the "stored data model" note: no persistent data-model change is present. The flagged path is Post-refresh verification on this head: @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Update on the checks after refreshing against They all trace to one error:
So the pre-existing |
The ChatGPT Responses retry path returned as soon as `retry-after-ms` was present, so a malformed or empty value discarded a valid `Retry-After` header sent alongside it and the client fell back to blind exponential backoff. The sibling parser in `provider-transport-fetch.ts` treats the two headers as ordered preferences and falls through on a parse failure; this aligns the Responses path with that contract.
20fdd76 to
fe205ac
Compare
|
Merged via squash.
|
…aw#111353) * fix(ai): honor Retry-After when retry-after-ms is unparseable The ChatGPT Responses retry path returned as soon as `retry-after-ms` was present, so a malformed or empty value discarded a valid `Retry-After` header sent alongside it and the client fell back to blind exponential backoff. The sibling parser in `provider-transport-fetch.ts` treats the two headers as ordered preferences and falls through on a parse failure; this aligns the Responses path with that contract. * test(ai): deduplicate retry header coverage --------- Co-authored-by: Peter Steinberger <[email protected]>
2 participants
What Problem This Solves
Fixes an issue where users on ChatGPT (Codex) models would hit repeated rate-limit
failures when the server asked them to wait a specific amount of time. When a 429
response carries a
retry-after-msvalue the client cannot parse, the server'svalid
Retry-Afterheader sitting next to it is discarded, and the client retrieson its own short exponential schedule instead. It gets rate-limited again, burns
its remaining attempts well inside the server's cooldown window, and surfaces a
rate-limit error for a turn the server had told it to simply wait out.
Why This Change Was Made
retry-after-msandRetry-Afterare ordered preferences, not exclusivealternatives: the millisecond header is preferred when usable, otherwise the
seconds header applies. The retry path treated the mere presence of
retry-after-msas the decision point and returned before ever readingRetry-After, so an unparseable value — or an empty string, which is present butcarries no information — silently downgraded the wait to blind backoff.
This aligns the path with
parseRetryAfterSecondsinsrc/agents/provider-transport-fetch.ts:465, which handles the same two-headerprotocol and falls through to
Retry-Afterwhenretry-after-msfails to parse.The fix adopts that shape exactly: return only on a successful parse.
Scope is deliberately narrow. Precedence is unchanged, a parseable
retry-after-msstill wins, and the existing clamp and HTTP-date handling areuntouched. The behavior predates the extraction of this helper in #110655; the
refactor preserved it faithfully rather than introducing it.
User Impact
On rate-limited ChatGPT Responses turns, the client now waits as long as the
server asked whenever the server communicated that in either header. Sessions
that previously failed with a rate-limit error during a provider cooldown now
recover on their own. Users who never receive a malformed
retry-after-mssee nochange.
Evidence
Real-behavior proof — no mocks, no fake timers, real socket, real wall clock.
A genuine
node:httpserver answers the first request with429,retry-after-ms: not-a-number,retry-after: 3, and the realstreamOpenAICodexResponsesis driven against it viatsx. The measured gap isthe actual elapsed time between the two inbound requests observed by the server:
Before (this branch with the source change reverted):
After:
1016 ms is
BASE_DELAY_MS * 2 ** 0— the blind fallback. 3018 ms is the server'sinstruction.
Regression tests. Three cases added to the existing suite: an unparseable
retry-after-ms, an empty one, and a guard that a parseableretry-after-msstill takes precedence over a conflicting
Retry-After.The new tests are load-bearing. Reverting only the source change (test file
untouched) turns them red with exactly the symptom described above:
Local checks.
openai-chatgpt-responses.retry.test.tsandopenai-chatgpt-responses.test.ts: 42 passed.oxlintclean,oxfmt --checkclean,
check-max-lines-ratchetclean (the helper's line count is unchanged).AI-assisted.