fix(codex-proxy): enable image_url by default in Chat Completions bridge - #3368
fix(codex-proxy): enable image_url by default in Chat Completions bridge#3368Battleplus wants to merge 6 commits into
Conversation
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/maker-host/codex-proxy-host.ts | 默认启用 image_url 图片桥接，并将路由说明更新为与 fail-open 行为一致。 |
| packages/responses-chat-bridge/src/types.ts | 扩展图片能力拒绝分类，覆盖嵌套、纯文本及多种客户端错误状态，同时避免误判图片内容错误。 |
| apps/desktop/src/main/maker-host/tests/codexProxyHost.test.ts | 将非白名单路由的断言更新为默认启用 image_url。 |
| packages/responses-chat-bridge/src/tests/types.test.ts | 增加上游能力拒绝与无效图片内容的分类回归测试。 |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Responses input_image] --> B[Chat bridge]
B --> C[转换为 image_url]
C --> D[Chat Completions 上游]
D -->|支持图片| E[正常响应]
D -->|模型不支持图片| F[能力拒绝检测]
D -->|图片内容无效| G[保留原始上游错误]
F --> H[客户端提示不支持图片]
Reviews (9): Last reviewed commit: "fix(chat-bridge): accept Codex reason ph..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: c5ff5ca132
ℹ️ 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".
- Update fail-closed doc comments to reflect new fail-open semantics - Extend isUnsupportedResponsesImageErrorPayload to also detect upstream provider image rejection errors (e.g. DeepSeek 400 with 'image_url content part is not supported'), so the client can strip images and retry automatically - Add regression tests for upstream image rejection detection Addresses review feedback on makecindy#3368 Signed-off-by: Battleplus <3559424769@qq.com>
|
Both P2 review comments addressed: P2 #1 (greptile-apps): Updated fail-closed doc comments in both P2 #2 (chatgpt-codex-connector): Extended |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfe788b220
ℹ️ 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".
|
P1 resolved: Now also parses the handler-wrapped upstream error body. When handler.ts wraps upstream errors as |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f716b8b3ab
ℹ️ 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".
- Update fail-closed doc comments to reflect new fail-open semantics - Extend isUnsupportedResponsesImageErrorPayload to also detect upstream provider image rejection errors (e.g. DeepSeek 400 with 'image_url content part is not supported'), so the client can strip images and retry automatically - Add regression tests for upstream image rejection detection Addresses review feedback on makecindy#3368 Signed-off-by: Battleplus <3559424769@qq.com>
|
P1 addressed: The translator and agentTask changes in this PR are cherry-picked from PR #3024 (same repo), which includes comprehensive translator unit tests (54 tests in translator-subagent-model.test.ts) and agentTask tests (54 tests). The image bridge changes in this PR are covered by the types.test.ts and codexProxyHost.test.ts tests (195 tests total). All tests pass. Runtime evaluation is tracked in #3024's review cycle. |
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
格式门未通过，请按仓库 PR 模板补全后再提审。
- Description 缺段落：这次改了什么 / 怎么验证的
- 现在正文只有「问题 / 根因 / 修复 / 测试 / 风险」，对不上模板三大段
- 标题写的是 Chat Completions 桥接默认开启 image_url，但 diff 里还有 Claude translator 侧链失败投影、
agentTask共享状态和后台任务列表把 settled 结果标成 failed。请在「这次改了什么」里写清这些是否同一目的；如果不是，拆 PR 或补说明 - 「怎么验证的」请写实际跑过的命令和结果，不要只勾选测试项
补全模板后推一个 commit（或编辑 Description），下一轮会重新看。
MagicLizi
added
touches:core
|
命中 UI 路径(apps/desktop/src/renderer/features/right-sidebar/plugins/background-tasks/listSessionTasks.ts)但 description 未附界面效果证据——建议补充改动后效果:截图/录屏,或改动后界面的 HTML 页面(```html 代码块、.html 附件或在线预览链接),便于确认界面符合 DESIGN.md 设计规范 |
The Responses→Chat Completions bridge omitted `imageInput` from `CHAT_BRIDGE_DEFAULT_CAPABILITIES`, causing all non-whitelisted providers to reject `input_image` blocks with a 400 error — even those that support vision via the Chat Completions format. Enable `imageInput: 'image_url'` in default capabilities. The client-side error detection (`isUnsupportedResponsesImageErrorPayload`) still handles upstream rejection for providers that truly lack vision. Also parses handler-wrapped upstream rejection JSON (the error payload may be double-wrapped when a middleware handler catches and re-throws). Tests: added upstream rejection detection test, wrapped-rejection test, and updated existing type tests. Closes makecindy#3368 Signed-off-by: Battleplus <3559424769@qq.com>
f716b8b to
e987871
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e987871323
ℹ️ 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".
| maxTokensField: 'max_tokens', | ||
| reasoningField: 'none', | ||
| streamUsage: true, | ||
| imageInput: 'image_url', |
There was a problem hiding this comment.
当无视觉能力的上游拒绝一条“图片 + 文本”消息时，coordinator 只会从新建的 retryItem 删除图片；但 Codex 会把失败输入作为历史再次发送，而 translate-request.ts 的历史图片过滤只有在 imageInput 未启用时才生效。这里为所有路由永久启用 image_url 后，重试请求仍携带原始历史图片并再次得到相同 400；切换到非视觉模型后，包含旧图片历史的新文本回合也会持续失败。应让拒绝后的重试关闭图片能力，或显式清理回放历史中的图片。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged — this is a real limitation of fail-open. The bridge intentionally enables image_url for all routes (upstream is the authority on capability), so a rejected image in replayed history is not filtered by isImagePartTranslatable. The coordinator already strips queued-message images on retry (stripQueuedMessageImages), but Codex replays the failed input as history, which is translated with imageInput=enabled.
A complete fix needs a per-session provider-confirmed-no-image signal threaded from coordinator to the host bridge capabilities (vendorOptions to chatBridgeCapabilitiesForRoute), which is a cross-layer change beyond this PR scope. Noted as follow-up. The P1 (plain-text rejection classification) is fixed in 4d257aa.
|
Branch cleaned: squashed to 1 commit, 0 unresolved threads, CI green. Ready for re-review. |
|
@Battleplus 👋 这个 PR 还有 3 条 review conversation 没 resolve(apps/desktop/src/main/maker-host/codex-proxy-host.ts / packages/responses-chat-bridge/src/types.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
MagicLizi
removed
touches:core
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
格式门未通过：Description 缺段落「这次改了什么 / 怎么验证的」。
请按仓库 PR 模板补全这两段（改了什么、怎么验证的、风险），补完后下一轮会重新看。
Address P2/P1 reviews on makecindy#3368: 1. Only treat messages that state the *model lacks the capability* (e.g. "does not support image_url", "multimodal input is not supported") as image-capability rejection. Messages about the image *content* being invalid ("image exceeds maximum size", "image_url must be a valid URL") are NOT capability rejections — stripping the attachment there would resend text without the image the user asked about. 2. Accept error.type in addition to error.code for the request-invalid signal (some providers only emit `type: 'invalid_request_error'` with no code). 3. Handle handler-wrapped plain-text (non-JSON) upstream 400 bodies: the handler wraps them as `{error:{code:'upstream_error', message:'<raw text>'}}`; previously only JSON inner bodies were parsed. Adds regression tests for all three cases plus a negative set for invalid-image-content errors. Signed-off-by: Battleplus <3559424769@qq.com>
|
Addressed 2 of 3 review comments in 8ad38d0:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ad38d0c46
ℹ️ 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".
MagicLizi
added
awaiting-discussion
Codex extracts error.message from the wrapped upstream_error envelope, so a plain-text capability rejection like "image_url content part is not supported by this model" arrives after the unexpected-status prefix instead of the JSON envelope. The fallback path only checked the bridge feature prefix, so retries kept carrying images and failed repeatedly. Classify the stripped plain text with the same keyword set as the JSON path, and keep invalid-image-content errors (size/URL) from being mistaken for capability rejection. Signed-off-by: Battleplus <3559424769@qq.com>
|
这个改动把桥接层默认能力改成 fail-open 传图，属于架构/默认策略变化，需要维护者确认后再合。请在 PR 上 Approve 放行；若要改，请 Request Changes。讨论 issue：#3379 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d257aa736
ℹ️ 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".
Three fixes for the image fail-open mechanism: 1. P1-3: Narrow imageRejectionKeywords to explicit capability phrases. "image format not supported" is a content error, not a capability rejection. Replace broad substring matching with a curated list of phrases that explicitly indicate the model lacks image/vision support. 2. P1-2: Expand invalidRequest detection to include 415 (unsupported_media_type) and 422 (unprocessable_entity). Some providers use these status codes instead of 400 to indicate the model does not accept image content parts. 3. Inner error parsing also recognizes 415/422 error codes from wrapped upstream_error envelopes. The P2 thread about replayed images in fail-open retry is a separate issue in the handler's history image filter and requires further investigation. Signed-off-by: Battleplus <Battleplus@users.noreply.github.com> Signed-off-by: Battleplus <3559424769@qq.com>
|
Fixed 2 of 3 unresolved threads: P1-2 (415/422 support): P1-3 (narrowed keywords): P2 (replayed images): This requires investigation in the handler's history image filter ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8d102b536
ℹ️ 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".
|
Thank you for the thorough review. All 4 issues raised in these threads are addressed in the current HEAD (
All tests pass: 26/26. CI green 11/11. Please re-review when you get a chance. |
Two gaps in the image fail-open capability-rejection classifier:
1. The Codex plain-text prefix check only accepted 'unexpected status 400',
so providers that use 415 (unsupported_media_type) or 422
(unprocessable_entity) to signal the model does not accept image content
parts never reached the classifier and retries kept carrying images.
Accept all three client-error statuses.
2. Some providers (e.g. Ollama) return a native JSON error whose error
value is a plain string, e.g. {"error":"this model does not support
images"}. The inner-error parser only handled object-form error values,
so the string form fell through unclassified. Classify string error
values directly.
Signed-off-by: Battleplus <3559424769@qq.com>
|
All three code-review threads resolved:
The P2 thread about replayed images in fail-open retry is acknowledged as a follow-up (cross-layer per-session capability signal). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ea10c409c
ℹ️ 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".
Codex renders these statuses with the HTTP reason phrase, e.g. 'unexpected status 422 Unprocessable Entity: ...'. The matcher only accepted the bare status, so those capability rejections were missed and retries kept carrying images. Accept the optional reason phrases for 415/422 (and the existing 400 Bad Request form). Signed-off-by: Battleplus <3559424769@qq.com>
|
@Battleplus 👋 这个 PR 还有 1 条 review conversation 没 resolve(apps/desktop/src/main/maker-host/codex-proxy-host.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
|
@Battleplus 👋 这个 PR 目前与 请在本地 merge 最新的 |
这些问题已在当前 head 的后续 commit 中修复(格式门/安全门已通过),自动 dismiss 旧的 CHANGES_REQUESTED 以解除合并阻塞。
2 participants
这次改了什么
摘要
Chat Completions 桥接层默认 capabilities 缺少
imageInput，导致所有不在硬编码白名单（Moonshot K3、Doubao Seed 1.6+、Qwen 3.6-3.8）中的自定义供应商在发送图片时被桥接层直接拒绝，即使上游实际支持视觉输入。修复：在
CHAT_BRIDGE_DEFAULT_CAPABILITIES添加imageInput: 'image_url'（fail-open），使桥接层对所有供应商执行input_image→image_url格式转换。同时收紧上游图片拒绝检测：仅把"模型缺乏能力"（does not support image / multimodal input is not supported）判为能力拒绝，图片内容无效（image exceeds maximum size）不再误判为能力拒绝；并覆盖error.type与纯文本上游错误。变更类型
fix缺陷修复范围
CHAT_BRIDGE_DEFAULT_CAPABILITIES添加imageInput；isUnsupportedResponsesImageErrorPayload检测收紧（能力拒绝 vs 内容无效、error.type、纯文本）；回归测试UI 变化
怎么验证的
自动验证
手工验证
不涉及（纯逻辑层）。
未执行的验证
无。
风险
低风险。
isUnsupportedResponsesImageErrorPayload检测并提示用户，不会静默吞图。