fix(responses): complete and persist Codex image outputs - #5014
fix(responses): complete and persist Codex image outputs#5014markchenlife wants to merge 1 commit into
Conversation
|
This pull request targeted The base branch has been automatically changed to |
![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: 42bb970aae
ℹ️ 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".
| failureEvent = "response.failed" | ||
| } | ||
| framer := &responsesSSEFramer{failureEvent: failureEvent} | ||
| framer := &responsesSSEFramer{failureEvent: failureEvent, imageOutputDir: h.codexImageOutputDir()} |
There was a problem hiding this comment.
When a client upgrades /v1/responses to WebSocket, this SSE framer is bypassed: forwardResponsesWebsocket parses each payload and writes it directly in openai_responses_websocket_forward.go. Consequently, Codex WebSocket clients still receive no synthesized response.image_generation_call.completed event, and codex-image-output-dir neither persists nor links the image. Route WebSocket payloads through equivalent image lifecycle and persistence handling.
Useful? React with 👍 / 👎.
| item := gjson.GetBytes(payload, "item") | ||
| if item.Get("type").String() != "image_generation_call" { | ||
| return | ||
| } |
There was a problem hiding this comment.
When stream is false or omitted, handleNonStreamingResponse writes the final response directly, so this event-specific function is never invoked; moreover, a non-streaming image is under output[], not item. Thus enabling codex-image-output-dir has no effect for valid non-streaming Responses requests even though the new configuration describes completed Responses results generally. Process final response output items before writing the response as well.
Useful? React with 👍 / 👎.
42bb970 to
a171261
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a17126138e
ℹ️ 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".
| case "response.content_part.added", "response.content_part.done": | ||
| if gjson.GetBytes(payload, "part.type").String() == "output_text" { | ||
| return appendImageMarkdownAtPath(payload, "part.text", markdown) |
There was a problem hiding this comment.
When codex-image-output-dir is enabled and the next assistant message follows the normal response.content_part.added → response.output_text.delta sequence, this branch appends the Markdown link to part.text, and the delta branch appends it again because imageMarkdownInjected is still false. A client that initializes content from the added part and then accumulates deltas therefore renders the same generated image twice; reserve the injection for the delta and terminal snapshots, or consume the pending preview consistently after this event.
Useful? React with 👍 / 👎.
| case "response.output_text.delta": | ||
| if f.imageMarkdownInjected { | ||
| return payload | ||
| } |
There was a problem hiding this comment.
When one streamed response interleaves two image items with assistant text after each, this response-wide boolean is set by the first message and is never reset when the second image is persisted. The first text delta after the second image is consequently forwarded unchanged, so delta-driven clients never receive the second file path; track unannounced image paths and consume them per following assistant message rather than using a permanent stream-wide flag.
Useful? React with 👍 / 👎.
1 participant
Summary
response.image_generation_call.completedevent when Codex upstream finishes an image item with a resultcompleted, preserve monotonic SSE sequence numbers, and avoid duplicate completion eventscodex-image-output-dirsetting that persists PNG, JPEG, or WebP results and appends a renderable absolute Markdown image path for local Codex clientsRoot cause
Codex upstream can emit a result-bearing
response.output_item.donefor animage_generation_callwhile leaving the item status asgeneratingand omittingresponse.image_generation_call.completed. CLIProxy forwarded that incomplete lifecycle unchanged. Codex Desktop retained the Base64 result in its session data, but had no completed image event or local file reference it could render.The output directory remains disabled by default, so existing remote and general-purpose deployments keep their current behavior.
Tests
go test ./internal/config ./internal/translator/codex/openai/responses ./sdk/api/handlers/openai