Repository navigation
fix(codex): raise max_output_tokens below the Codex minimum of 16 - #7288
RyoKusnadi wants to merge 2 commits into
Conversation
Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
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. |
![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: fcce8efb07
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if errSet := doc.Set("max_output_tokens", codexMinMaxOutputTokens); errSet != nil { | ||
| return body | ||
| } | ||
| return doc.Bytes() |
There was a problem hiding this comment.
Whenever a sub-16 value triggers this path, doc.Bytes() serializes the request at this helper boundary, but every production caller then reparses body into another ojson.Doc (for example Execute in codex_executor_execute.go:60 and WebSocket preparation in codex_websockets_stream.go:893) before applying the remaining normalization and finalization stages. Move this clamp onto those existing request documents before payload finalization, or pass the document onward, so the hot request path retains the required single-document mutation/serialization pipeline.
AGENTS.md reference: AGENTS.md:L53-L53
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good point. Moved it to clampCodexMaxOutputTokensDoc(doc), called on the existing request document in each Codex executor path right after ensureWebSearchToolForHistoryDoc, so there's no extra serialize/reparse. Done in 471f086, description updated.
Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
Problem
In Claude Code,
/model gpt-5.6-solfails with400 Invalid 'max_output_tokens': integer below minimum value. Expected a value >= 16, but got 1 instead, and the model is not changed. It worked on v8.0.51.How it happens
Claude Code checks a model with a probe request that has
max_tokens: 1. Since 01b24f1 (#7230) the Codex request translators forward the client's token limit asmax_output_tokens, so the 1 reaches the Codex Responses API, which rejects anything below 16. The OpenAI chat, Gemini, Interactions and Responses translators forward it the same way.Steps to reproduce
POST /v1/messageswith{"model":"gpt-5.6-sol","max_tokens":1,"messages":[{"role":"user","content":"Hi"}]}.Expected: the request goes through. Actual: upstream 400 as above.
Cause
internal/translator/codex/*copy the client limit tomax_output_tokensunchanged. Codex has a minimum of 16 that the translators don't know about.Fix
A new
clampCodexMaxOutputTokensDocraises amax_output_tokensbelow 16 to 16 on the request document. Each Codex executor path (HTTP execute, stream, compact, websocket execute and stream) calls it right afterensureWebSearchToolForHistoryDoc, on the same*ojson.Doc, before the payload finalizer, so user payload rules still apply last. It lives in the executor becauseAGENTS.mdasks for no standaloneinternal/translator/changes, and a per-translator fix would need four copies.Scope
Larger values, a missing field and
nullare untouched. #7249 also hits this minimum, from credential verification; with this change that path sends 16 as well. I updated the existing verification test, which expected the 1 to be kept. Other providers (xAI, Claude) are not changed.Testing
New tests in
codex_executor_translate_test.go:TestClampCodexMaxOutputTokensDoc(1, 0, 15, 16, 4096, absent, null) andTestClampCodexMaxOutputTokensDocClaudeProbe(real Claude to Codex translator gives 1, the clamp makes it 16). The existingTestCodexVerifyInferenceTranslatesPayloadAndRequiresCompletedEvent_Issue5786covers the executor end to end: before the fix the upstream body hasmax_output_tokens: 1, after it 16.Ran
go test ./internal/runtime/executor/(4735 passed),go veton the executor andgo build ./.... Not run: the full suite and a live Codex request.Environment: fix/codex-min-max-output-tokens @ 471f086 (on dev), go1.27.1, darwin/arm64
Fixes #7282