fix(codex): single-flight token refresh — concurrent expiry race clears freshly rotated tokens#42
Open
olegbrok wants to merge 4 commits into
Open
fix(codex): single-flight token refresh — concurrent expiry race clears freshly rotated tokens#42olegbrok wants to merge 4 commits into
olegbrok wants to merge 4 commits into
Conversation
The Responses Lite lane used for gpt-5.6-{sol,terra,luna} moves every tool
into an AdditionalTools developer prefix and sends top-level tools: null.
Upstream only supports function/custom tools on that lane, so a hosted
web_search tool is never registered:
- a forced {type: tool, name: web_search} tool_choice translates to a
top-level web_search tool_choice that references no registered tool,
which upstream rejects with 502
"Tool choice 'web_search_preview' not found in 'tools' parameter" (raine#26)
- unforced searches never engage real web search, so Claude Code's
WebSearch reports zero results
Registering the web_search tool top-level on the lite lane is not an
option — upstream rejects it: "X-OpenAI-Internal-Codex-Responses-Lite
only supports function tools, custom tools, and client-executed tool
search."
Fix: requests carrying a hosted web_search tool use the full Responses
API instead of the lite lane, where web_search registers top-level and
searches execute (verified live: real results through gpt-5.6-sol).
Also downgrade a web_search tool_choice to auto whenever no web_search
tool ended up registered, so no request shape can reproduce the 502.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live A/B on gpt-5.6-sol: with false, searches often miss the target (unrelated results); with true the same query returns the correct repo with utm_source=openai live-search markers. Local deploy only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ults with them) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… on refresh 401 Concurrent requests hitting the expiry margin each launched their own refresh with the same single-use rotating refresh token: the first rotates it, the losers get 401 from the token endpoint and clear_auth() destroys the winner's freshly saved tokens. In production this shows as minutes-long windows where every request 401s (agent fan-outs at token expiry), self-recovering only when a late flight wins the race. - refresh_now now holds a dedicated flight lock; racing callers re-check the store under the lock and return the winner's tokens without touching the token endpoint - on refresh 401/403, if the stored refresh token rotated since our read (another process sharing the store), return the rotated tokens instead of clearing auth The grok provider already guards this exact race (concurrent_stale_401_refreshes_reuse_the_rotated_access_token); this brings codex to parity.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CodexAuthManager::refresh_nowisn't serialized: thecachedmutex guards only cache reads/writes. N concurrent requests that hit the expiry margin each POST/oauth/tokenwith the same (single-use, rotating) refresh token. The first rotates it; the rest get 401/403 — and the 401 path callsclear_auth(), destroying the tokens the winner just saved.Production signature (observed 2026-07-12 on a machine running agent fan-outs through the proxy): minutes-long windows where every request 401s, starting exactly when many parallel requests coincide with token expiry, self-recovering only when a late refresh flight happens to win. Between windows everything is fine, which makes it easy to misattribute (we spent a night blaming per-model flakiness — the correlation was just which traffic bursts during a bad window).
Fix
refresh_nowholds a dedicatedrefresh_flightmutex for the whole refresh; racing callers block, then re-check the store under the lock and return the winner's tokens without touching the token endpoint.Notes
The grok provider already guards this exact race (see
concurrent_stale_401_refreshes_reuse_the_rotated_access_token) — this brings codex to parity. Two unit tests added mirroring that pattern (the re-check path is provable without an HTTP mock: reaching the network would fail the test).Tests: full suite green (415 passing incl. 2 new).