docs(providers): say where transientRetryOn5xx does not apply - #4894
Conversation
The reference entry opens with "Key-auth openai-chat providers only", which an operator reasonably reads as "every other adapter gets no transient retry". That is not what happens. A provider whose adapter is openai-responses never reaches transientRetryPolicyFor at all. createResponsesPassthroughAdapter declares passthrough: true, and core.ts returns into executePassthroughResponse on that flag before the adapter dispatch path is built. The passthrough lane then applies its own transient ladder from a fixed constant, so such a provider gets replay the setting can neither enable nor tune. Both halves of that mismatch mislead: the option looks broader than it is, and the untouched lane looks quieter than it is. One sentence in each locale states the boundary. Documentation only. No runtime behaviour changes.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedNext included review available in 38 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
리뷰 · 우선순위 76 / 80이 PR은 문서만 고친다. 고치는 곳은 왜 고치느냐면, 지금 문장이 두 쪽으로 사람을 속이기 때문이다. 표는 "키 인증 근거는 현재 추가된 문장은 그 경계를 한 줄로 못 박는다. 영어: 어댑터가 호스트 CI는 지금 exact-head( docs-site/.../providers.md 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc41176fee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `webSearchBridge?` | `{ enabled?: boolean; backend?: "ollama" \| "openai" \| "anthropic" \| "xai" \| "gemini" \| "exa"; maxSearches?: number; timeoutMs?: number; endpoint?: string }` | Key-auth `openai-responses` passthrough providers only. Off by default. Codex always declares the hosted `web_search` tool, and the passthrough relays it on the assumption the destination executes it. A gateway that does not run hosted search answers with a `function_call` named `web_search` that nothing runs, and the undeclared-tool guard ends the turn. With `enabled: true` and an explicit `backend` OpenCodex intercepts that call, runs the search itself, feeds the result back to the same upstream, and shows Codex a hosted `web_search_call` cell. Never armed for `authMode: "forward"` (ChatGPT already searches) or for a provider that executes hosted search upstream. `backend` is required; there is no implicit default and a missing credential for the named backend leaves the bridge disarmed rather than falling through to another paid search. `ollama` reuses this provider's own API key on `POST <origin>/api/web_search`, so the origin must be `https://ollama.com` unless the operator names `endpoint` explicitly. `openai` / `anthropic` / `xai` / `gemini` / `exa` reuse the matching sidecar executor and that executor's own credential (`webSearchSidecar.exaApiKey` for Exa). The search model comes from `webSearchSidecar.model` only when `webSearchSidecar.backend` resolves to the same backend this bridge names; otherwise the bridge runs that backend's own default, because a model chosen for one vendor is rejected by another. An unset `webSearchSidecar.backend` resolves to `openai`, so an unset-backend model reaches an `openai` bridge and no other. There is no per-provider bridge model override. Streaming turns only. A turn that mixes `web_search` with another client tool call still fails closed rather than dropping the client's call. Assistant text such as XML-like `<web_search>` prose is not executed. Defaults: `maxSearches: 3` (1..10), `timeoutMs: 60000` (1000..600000). | | ||
| | `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | API-key providers only (`authMode: "key"`). Opt-in same-target 429 retry: when `retryOn429` is absent the feature is off; object presence enables it unless `enabled: false`. On 429 the proxy waits (upstream `Retry-After` or the fixed interval) and replays the identical request on the same key before any key failover — across the main text-turn recovery loop, the Responses passthrough wire, the image/video bridge, the web-search sidecar, and terminal continuations. Only pre-stream HTTP 429 responses are eligible for replay; custom `runTurn` transports are outside the HTTP retry loop. `attempts` counts same-key replays after the first 429 (total sends = `attempts` + 1) and is one request-wide budget shared by the main recovery loop, the terminal-guard continuation, and bridge retries. Exhausting `attempts` only stops further same-key replays: normal key failover or final-error handling then applies per the available targets — on the key-auth passthrough wire there is no failover, so the exhausted 429 surfaces as-is. Codex itself never retries 429, so this is the only defense for single-key providers. Defaults: `enabled: true`, `attempts: 3`, `intervalMs: 5000`, `maxIntervalMs: 60000` (any single wait is capped at `maxIntervalMs`, itself capped at 600000), `respectRetryAfter: true`. | | ||
| | `transientRetryOn5xx?` | `{ enabled?: boolean; attempts?: number }` | Key-auth `openai-chat` providers only. Opt-in retry for pre-stream transient upstream statuses (500, 502, 503, 504, 520, 521, 522): absent means off, object presence enables it unless `enabled: false`. Covers the initial Responses request, the terminal-guard continuation, and native `/v1/chat/completions`. `attempts` is the TOTAL number of upstream sends allowed for one request including the first (1..10, default 3) — it is one budget shared with connection-reset recovery, so `3` means at most three real requests reach the provider. Waits use a fixed 400 ms exponential backoff capped at 5 s and honor `Retry-After`. Separate from `retryOn429`, which handles rate limiting; mid-stream failures are never replayed. | | ||
| | `transientRetryOn5xx?` | `{ enabled?: boolean; attempts?: number }` | Key-auth `openai-chat` providers only — a provider whose `adapter` is `openai-responses` goes through the Responses passthrough path instead, which applies its own fixed transient ladder and never reads this option. Opt-in retry for pre-stream transient upstream statuses (500, 502, 503, 504, 520, 521, 522): absent means off, object presence enables it unless `enabled: false`. Covers the initial Responses request, the terminal-guard continuation, and native `/v1/chat/completions`. `attempts` is the TOTAL number of upstream sends allowed for one request including the first (1..10, default 3) — it is one budget shared with connection-reset recovery, so `3` means at most three real requests reach the provider. Waits use a fixed 400 ms exponential backoff capped at 5 s and honor `Retry-After`. Separate from `retryOn429`, which handles rate limiting; mid-stream failures are never replayed. | |
There was a problem hiding this comment.
For mixed-wire providers, modelAdapters and registry defaults are resolved before dispatch, so a provider configured with adapter: "openai-chat" can still use the Responses passthrough path for a specific model, while an openai-responses provider can resolve a model to Chat. The new wording keys the retry boundary only to the provider-level adapter, which can therefore mislead operators about whether transientRetryOn5xx.attempts controls a selected model; describe this in terms of the effective adapter after per-model resolution and update the translations consistently.
AGENTS.md reference: docs-site/AGENTS.md:L8-L10
Useful? React with 👍 / 👎.
github-actions
Bot
added
the
documentation
|
✅ Deterministic PR hygiene checks passed. |
1 participant
Summary
The
transientRetryOn5xxreference entry opens with "Key-authopenai-chatproviders only", and an operator reasonably reads that as "every other adapter gets no transient retry". That is not what happens, and the gap sent at least one contributor after the wrong fix.A provider whose
adapterisopenai-responsesnever reachestransientRetryPolicyFor.createResponsesPassthroughAdapterdeclarespassthrough: true as const(src/adapters/openai-responses/passthrough.ts:190-193), andsrc/server/responses/core.ts:116returns intoexecutePassthroughResponseon exactly that flag, before the adapter dispatch path is constructed. That lane then applies its own transient ladder from a fixed constant (src/server/responses/passthrough-dispatch.ts:760,TRANSIENT_RETRY_MAX_ATTEMPTSatsrc/lib/upstream-retry.ts:106).So both halves of the current sentence mislead. The option looks broader than it is, and the lane it does not cover looks quieter than it is. This adds one sentence naming that boundary, in the English source and each of the seven translations, so no locale contradicts another.
The underlying defect — that a provider policy cannot reach the passthrough lane at all — is tracked separately in issue #4893 and is not addressed here.
Verification
transientRetryPolicyForhas exactly four call sites insrc/, and three of them (adapter-dispatch.ts:305,adapter-continuation.ts:207and:411) sit past the early return incore.ts:116. The fourth,chat-native.ts:301, serves/v1/chat/completionsand is unaffected by this wording.src/adapters/openai-chat.tsdeclares nopassthroughflag, so the existing sentence stays correct for that adapter.docs-sitemarkdown files, one line each. No runtime, test, script, workflow orstructure/path is touched, so the ownership gate has nothing to bind and the file-size ratchet has no source file to measure.changesjob scopes the expensive legs out for a docs-only diff and still publishes the aggregate check.Checklist