fix(runtime): send no output-token limit to the OpenAI Codex subscription backend - #5738
Conversation
…tion backend The ChatGPT Codex backend rejects max_output_tokens with HTTP 400 "Unsupported parameter". Two paths still sent it on openai-codex: a per-model output limit, which made every turn on that model fail, and the context-overflow recovery cap, which made any recovered turn fail even with no limit configured. providerAcceptsOutputTokenLimit states the fact once in the provider registry. ModelAdapter.acceptsOutputTokenLimit() reads it: maxOutputTokens() yields no limit there, and overflow recovery adds no cap. Settings show the model's output limit as read-only on such a connection and say why, with a warning when a saved value is not applied. Fixes apache#5736 Generated-by: Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Pass the connection's provider type to CapabilityEditor and resolve providerAcceptsOutputTokenLimit there, so the settings page takes no new dependency and the renderer architecture ledger is unchanged. Generated-by: Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
hqhq1025
left a comment
There was a problem hiding this comment.
I found one P1 issue in the main-turn request path (inline). This PR adds a provider-level output-limit capability, suppresses the Codex override and overflow-recovery cap in the caller, and disables the unsupported settings field. The remaining startStream fallback still reintroduces a configured limit, so the advertised main-turn fix is not complete.
I checked the Runtime request path, provider catalog, settings call sites, and the relationship to #5723. On this head, Node 24 npm ci, build:test, 109 focused tests, Biome on all changed files, git diff --check, and merge-tree against current main passed. A direct ModelAdapter.startStream probe with a Codex connection and a 4096 override observed maxOutputTokens: 4096 at doStream, despite maxOutputTokensForInput() returning undefined. I did not run a live Codex backend or packaged Desktop UI. The current-head CI test is red on an apparently unrelated opencli-chrome.test.js Windows-store-page assertion; it also needs resolution before merging.
#5723 changes the auxiliary/non-streaming Codex fetch path; this PR targets streamed main turns. They are complementary, not an atomic code dependency: landing only #5723 leaves configured main turns broken, while landing only this PR leaves auxiliary calls unaddressed. Their branches merge cleanly in a local merge-tree check, but the combined behavior still needs verification after this P1 is fixed.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| } | ||
|
|
||
| maxOutputTokens(): number | undefined { | ||
| if (!this.acceptsOutputTokenLimit()) return undefined; |
There was a problem hiding this comment.
P1: This guard does not protect the actual streamed request. startStream() at lines 288-295 uses input.maxOutputTokens ?? selectedModelMaxOutputTokens(...) instead of this guarded method. When AiSdkTurn omits the limit for openai-codex, that fallback recalculates the configured override and passes it to streamText (line 382). On this head, a Codex adapter with a 4096 override reports maxOutputTokensForInput() === undefined, but a direct startStream() probe observes doStream.maxOutputTokens === 4096. The streamed Codex request therefore still carries the backend-rejected limit. Gate this fallback as well, and test the final doStream/wire request rather than only the helper return value.
There was a problem hiding this comment.
Confirmed, thanks. When its caller passed no limit, ModelAdapter.startStream fell back to selectedModelMaxOutputTokens(...) directly. The guard in maxOutputTokens() / maxOutputTokensForInput() never saw that path, so a Codex turn still sent the configured 4096.
Fixed in cad717b. The check now sits at the one place a main-turn limit reaches the wire: startStream sends no maxOutputTokens when acceptsOutputTokenLimit() is false, whether the value came from the caller or from the model configuration. Because of that, overflow recovery needs no special case, so I reverted the ai-sdk-turn.ts change; this PR no longer touches that file.
Tests:
- New
model-adaptercase that goes throughstartStreaminto aMockLanguageModelV4. It asserts what reachesdoStream: onopenai-codexwith a 4096 override, nothing, both with no caller limit and with a caller-supplied 8000. Onopenai, 4096 and 8000 respectively. - The existing Codex overflow-recovery case now passes through the adapter alone.
- Both fail when the gate is removed, and pass with it.
model-adapter38/38,overflow-reactive-recovery60/60,mid-turn-capacity-backend68/68; runtimetscpasses.
On the red CI: I can't read the job log from here (the log download is blocked on my network), so thanks for naming opencli-chrome.test.js. That test covers the Windows Chrome Web Store launch path, which this PR does not touch. Other unrelated PRs went red around the same time, so I'll see whether it reproduces on this push.
On #5723: agreed with your reading. Neither PR alone fixes both call paths.
… reject it startStream fell back to the configured per-model limit whenever its caller passed none, so a Codex turn still sent the override even though maxOutputTokens() and maxOutputTokensForInput() returned undefined. Gate the one place the limit reaches the wire instead: a provider that rejects any limit gets none, whether it came from the caller or from the model configuration. Overflow recovery needs no special case of its own, so its change is reverted. Generated-by: Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
hqhq1025
left a comment
There was a problem hiding this comment.
On this head, the previously reported P1 is fixed. ModelAdapter.startStream now checks provider support at the final output-limit boundary, so a configured Codex limit and a caller-supplied recovery cap are both omitted before streamText. The new doStream test covers both sources and preserves limits for the regular OpenAI provider. I also exercised a Codex connection with a 4096 override through the actual SDK model and a stubbed subscription fetch: the streamed request body had no max_output_tokens. I found no further substantiated P0–P3 issue in the changed code.
The PR changes the provider capability in core, the Runtime request path, and the model-settings field and copy; it adds core and Runtime tests. Node 24 build:test, 178 focused tests, Biome on the changed files, git diff --check, and merge-tree checks against current main and #5723 passed. Current-head CI test is green. I did not independently run the live Codex backend, the packaged Desktop UI, or cross-platform visual checks.
#5723 is still needed to handle auxiliary/non-streaming Codex calls; this PR covers streamed main turns. The branches merge cleanly and have no atomic code dependency, but both need to land for the two user-facing request paths to be fixed. Final merge and product acceptance remain for maintainers.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
likun666661
left a comment
There was a problem hiding this comment.
Reviewed current head cad717b. The original P1 in ModelAdapter.startStream is fixed at the final request boundary: configured limits and caller-supplied overflow-recovery caps are omitted for openai-codex, while regular OpenAI behavior remains unchanged. The doStream tests cover both paths. No blocking issue found within this PR's streamed main-turn scope. Auxiliary/non-streaming Codex calls remain covered by #5723.
…ckend A main turn on a Codex subscription connection carried the model's `max_output_tokens`, which that backend answers with HTTP 400 "Unsupported parameter". The model adapter now sends no limit for a provider whose runtime adapter is `openai-codex` (`providerAcceptsOutputTokenLimit`), whatever the model is configured with. Lead: apache#5738 (7f25f27). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Watermark bfb315a. Done: apache#5573/apache#5600/apache#5601, apache#5521, apache#4875, apache#5723, apache#5738, apache#5742. Not applicable: apache#5737, apache#5593. Deferred: apache#5730. Consider: apache#5599, apache#5120, apache#5693. Diverged: apache#5740. Skipped: ACP, WorkHub, upstream renderer and packages/ui, one refactor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
The ChatGPT Codex backend rejects
max_output_tokenswithHTTP 400 {"detail":"Unsupported parameter: max_output_tokens"}. Two paths onmainstill send it on anopenai-codexconnection:modelOverrides[model].maxOutputTokensis set from the connection's model settings. Every main turn on that model fails, and the settings page accepts and saves the value without a word (bug(runtime): a per-model output limit makes every OpenAI Codex subscription turn fail with HTTP 400 #5736).min(limit ?? 8K, 8K). So on Codex, even with no limit configured, a turn that reaches overflow recovery sendsmax_output_tokens: 8000and fails with the same 400.The fix states the fact once and lets both Runtime and settings read it:
providerAcceptsOutputTokenLimit(core/provider-registry.ts). It is false for theopenai-codexruntime adapter.ModelAdapter.acceptsOutputTokenLimit(). When it is false,maxOutputTokens()returnsundefined, andstartStream, the one place a main-turn limit reaches the wire, sends none: not a caller-supplied one such as overflow recovery's 8K cap, not the configured model limit, and not the capacity-derived one.A limit cannot be honoured on this backend at all, so sending it only turns a working turn into a failed one. The explicit part is in settings, where the user sees that the field does not apply, rather than in a request that the backend rejects.
Fixes #5736
Verification
Real backend. I ran a Runtime Host on a copy of a Desktop workspace with a ChatGPT-subscription connection, set
modelOverrides['gpt-6-astra'] = { maxOutputTokens: 4096 }, and ran one streamed main turn:main(87fc9f69c)failed,request_rejected:HTTP 400 {"detail":"Unsupported parameter: max_output_tokens"}completedWith this branch, a capped Codex turn sends the same request body as the uncapped turn that completed: no
max_output_tokens. Themodel-adaptertest pins that. I'll post the real-backend result here once the route is stable.Tests. Each fails without the change:
provider-catalog-contract:providerAcceptsOutputTokenLimitis false only foropenai-codex, and true for an unknown provider.model-adapter: a Codex connection with a configured 4096 limit yields no limit (maxOutputTokens()andmaxOutputTokensForInput()both returnundefined), while the same override onopenaistill yields 4096.model-adapter/startStream: what reachesdoStreamonopenai-codexwith a 4096 override is no limit, both with no caller limit and with a caller-supplied 8000. Onopenaiit is 4096 and 8000 respectively.overflow-reactive-recovery: on a Codex connection with a 128K-capacity model, none of the three requests (including the post-recovery retry) carriesmaxOutputTokens.startStreamgate removed.Results: core 903/903 (full suite),
model-adapter38/38,overflow-reactive-recovery60/60,mid-turn-capacity-backend68/68, ui 662/662.Checks:
tsc --noEmitfor@maka/runtimeandapps/desktop,biome checkon the changed files, andcheck-locale-hygiene --base upstream/mainall pass.UI. Storybook
Product/Settings/Providers › O Auth Connections Disambiguated, Codex connection → Configure model parameters: GPT-6 Astra, with the field hovered.The pre-commit hook cannot run on this Windows machine:
biome.cmdfails underspawnSyncwithEINVAL. I ran its steps manually.Review focus
max_output_tokensinside the Codex fetch adapter, but only for auxiliary (generateText) calls. Those calls pass their cap directly rather than throughModelAdapter. It forwards main turns unchanged. This PR stops main turns from producing the cap at all. The two do not overlap. Once both land, auxiliary calls could readproviderAcceptsOutputTokenLimittoo, so the rule has a single site. I've left that as a follow-up rather than coupling the two PRs.AI use
Tool(s) and scope: Claude Code (Claude Opus) reproduced the failure against the real backend, implemented the change and tests, and ran the verification above; liugddx reviewed. The commit carries a
Generated-bytrailer.Checklist
Does this PR entail a change in behavior?
🤖 Generated with Claude Code