refactor: 去除 Codex CLI 伪装并简化 Codex 兼容逻辑#1166
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 本次拉取请求旨在简化 AxonHub 与 Codex 服务的集成方式。通过移除对 Codex CLI 的特定伪装行为,AxonHub 现在采取“最小转换”策略,减少了不必要的复杂性,并允许调用方对请求头有更大的控制权。这一变更使得系统更加清晰和可维护,同时确保了核心功能的兼容性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the Codex outbound transformer and related components to improve flexibility and maintainability. Key changes include decoupling the transformer from hardcoded Codex CLI user agents, versions, and system instructions, instead using a more generic 'axonhub' identity by default while allowing specific inbound headers to be preserved. The CodexQuotaChecker now utilizes a standardized httpclient.NewRequestBuilder for streamlined HTTP request handling. The LLM request parameter handling has been refined, making the transformer more selective about modifications and ensuring compatibility with the Codex Responses API. Additionally, session ID management has been improved, prioritizing inbound headers, then context, and generating a UUID if neither is present. The changes are supported by enhanced testing for header handling, session ID precedence, and the customized executor's behavior for both streaming and non-streaming requests.
There was a problem hiding this comment.
Pull request overview
该 PR 将 Codex 通道从“伪装 Codex CLI”调整为“最小转换/最小兼容面”,减少默认注入的 CLI 专用身份、指令与请求头,改为尽量透传调用方信息并使用 AxonHub 默认身份;同时同步更新 quota、trace 与 Codex 出站相关测试以匹配新契约。
Changes:
- 移除 Codex CLI 伪装逻辑(默认 UA/Originator/Version/系统指令注入与相关判定),收敛 Codex 出站转换规则。
- Codex quota 检查改用统一
httpclient发起请求(减少 CLI 专用 header 依赖),并新增相应单测。 - 更新/新增 Codex、trace、OAuth 相关测试,明确“透传 + AxonHub 默认身份 + 最小必要转换”的新行为。
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| llm/transformer/openai/codex/token_provider_test.go | 断言 OAuth 刷新请求默认使用 AxonHub UA。 |
| llm/transformer/openai/codex/token.go | TokenProvider 不再显式注入 Codex CLI UA。 |
| llm/transformer/openai/codex/outbound_test.go | 删除与 CLI 指令注入/判定相关的旧测试。 |
| llm/transformer/openai/codex/outbound_executor_test.go | 新增/重写 Codex 出站行为测试(SSE、身份透传覆盖、聚合非流式请求、最小转换契约)。 |
| llm/transformer/openai/codex/outbound.go | 移除 CLI 伪装/指令注入逻辑,保留必要的 Codex Responses 适配与会话/鉴权处理。 |
| llm/transformer/openai/codex/constants.go | 移除 Codex CLI 常量与指令文本,新增 AxonHub 默认 Originator 常量。 |
| llm/transformer/openai/codex/codex_simulator_test.go | 调整模拟器测试以验证“最小身份/透传”与 Session_id 优先级。 |
| llm/httpclient/utils.go | header blocklist 格式化调整(无语义变化)。 |
| internal/server/middleware/trace_test.go | Codex trace 行为测试增强:同时验证 Session_id 写入 context;新增缺失 Session 不设置 trace 的用例。 |
| internal/server/biz/provider_quota/codex_checker_test.go | 新增 Codex quota checker 的最小 header 行为测试。 |
| internal/server/biz/provider_quota/codex_checker.go | quota checker 改为使用 httpclient 请求构建/执行,移除 CLI 专用 header 注入与 account_id header 依赖。 |
| internal/server/api/codex_test.go | 新增 StartOAuth 返回的 AuthURL 不包含 originator 参数的测试。 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| resp, err := hc.Do(ctx, httpRequest) | ||
| if err != nil { | ||
| return QuotaData{}, fmt.Errorf("quota request failed: %w", err) | ||
| } | ||
|
|
||
| defer func() { _ = resp.Body.Close() }() | ||
|
|
||
| // Read body | ||
| body, err := io.ReadAll(resp.Body) | ||
| if err != nil { | ||
| return QuotaData{}, fmt.Errorf("failed to read response body: %w", err) | ||
| } | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| return QuotaData{}, fmt.Errorf("quota request failed with status %d: %s", resp.StatusCode, string(body)) | ||
| } | ||
|
|
||
| return c.parseResponse(body) | ||
| return c.parseResponse(resp.Body) |
There was a problem hiding this comment.
After switching to hc.Do(ctx, httpRequest), non-2xx responses now return an *httpclient.Error whose Error() string does not include the response body. Previously this code read the body and included it in the returned error, which is helpful for diagnosing quota failures. Consider detecting *httpclient.Error here and wrapping the error to include err.Body (similar to oauth.wrapHttpError), so callers still get actionable error details.
| assert.Equal(t, codexAPIURL, finalReq.URL.String()) | ||
| assert.Equal(t, "text/plain", finalReq.Header.Get("Accept")) | ||
| assert.Equal(t, "application/json", finalReq.Header.Get("Content-Type")) |
There was a problem hiding this comment.
Simulator.Simulate() ends by calling httpclient.BuildHttpRequest, but real Codex traffic goes through HttpClient.DoStream, which always overwrites Accept to text/event-stream (and adds other stream headers). As a result, asserting finalReq.Header.Get("Accept") == "text/plain" here is misleading vs the actual executed request. Consider either removing the Accept assertion, or updating the simulator/test to mimic the DoStream header injection when validating outbound headers.
| reqCopy.TransformerMetadata["include"] = []string{"reasoning.encrypted_content"} | ||
| } | ||
|
|
||
| if reqCopy.ReasoningSummary == nil || *reqCopy.ReasoningSummary == "" { |
| reqCopy.Store = lo.ToPtr(false) | ||
|
|
||
| // Codex recommends parallel tool calls. | ||
| reqCopy.ParallelToolCalls = lo.ToPtr(true) |
| reqCopy.MaxTokens = nil | ||
|
|
||
| // Strip sampling params and tier. | ||
| reqCopy.ServiceTier = nil |
|
我确认下,这个改动较大。 有些字段的设置都是有原因的,需要比较完善的真实测试覆盖。 |
我用codex cli和opencode都测试过了没问题。 上面提到的几个删除的内容,我是考虑希望能够尽量透传客户端的参数,尤其是 |
|
codex 应该是不支持 metadata 的 |
|
这次修改的目标是明确收敛 AxonHub 在 Codex 通道里的职责:去掉客户端伪装,只保留必要的协议转换,不再替调用方模拟 Codex CLI 的请求行为。 具体来说:
这点我也额外对了下 opencode 和 @ai-sdk/openai 的实现:
所以从这个角度看,当前这版实现已经符合预期设计:
|
|
reasoningSummary="auto"、include=["reasoning.encrypted_content"] 这两个建议加上默认值。 如果没有默认值,那么在 cc 里面使用,就不会传这些值,就会导致返回里面没有 encrypted_content,interleave thinking 失效。 |
|
已按要求修复 |
|
感谢 PR |
* refactor: 去除 Codex CLI 伪装逻辑 * fix: filter metadata from codex outbound * fix: restore codex reasoning defaults * refactor: align codex defaults with previous structure * test: align codex simulator header assertions
摘要
变更说明
User-Agent、Originator、Version、指令注入及相关判定逻辑。store=false、鉴权和Session_id回退。影响范围