fix(translator): round-trip Codex custom tool calls - #4417
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a request-local tool catalog and a per-call stream state tracker to preserve and reconstruct Codex custom tool calls (such as Cursor's ApplyPatch) through Chat Completions streaming and non-streaming responses. It also adds comprehensive integration and regression tests for both HTTP and WebSocket executors. The review feedback highlights a critical issue in codex_openai_tool_stream.go where the template slice is assigned directly without a deep copy, which can lead to data corruption or race conditions when modified in-place by sjson. It is recommended to perform a deep copy of the template slice before applying modifications.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| toolCall, _ = sjson.SetBytes(toolCall, "function.name", state.name) | ||
| toolCall, _ = sjson.SetBytes(toolCall, "function.arguments", input) | ||
|
|
||
| chunk := template |
There was a problem hiding this comment.
In Go, slice assignment chunk := template copies the slice header but shares the underlying array. Since sjson.SetBytes and sjson.SetRawBytes may modify the underlying array in-place if there is spare capacity, reusing the same template slice across multiple tool calls (e.g., in emitCompletedToolCalls) or returning it as the terminal chunk can lead to data corruption or race conditions. To prevent this, create a deep copy of the template slice.
| chunk := template | |
| chunk := make([]byte, len(template)) | |
| copy(chunk, template) |
| toolCall, _ = sjson.SetBytes(toolCall, "index", state.chatIndex) | ||
| toolCall, _ = sjson.SetBytes(toolCall, "function.arguments", delta) | ||
|
|
||
| chunk := template |
There was a problem hiding this comment.
Reusing the shared template slice header without copying the underlying array can lead to data corruption if sjson writes to the spare capacity of the shared array. Create a deep copy of the template slice to ensure isolation.
| chunk := template | |
| chunk := make([]byte, len(template)) | |
| copy(chunk, template) |
…ply-patch # Conflicts: # internal/translator/codex/openai/chat-completions/codex_openai_request.go
…ply-patch # Conflicts: # internal/translator/codex/openai/chat-completions/codex_openai_request.go # internal/translator/codex/openai/chat-completions/codex_openai_response.go
What changed
custom_tool_callresponse events into Chat Completionstool_callsfor streaming and non-streaming responsesfunctionenvelope is restored tocustom_tool_callonly when its name unambiguously matches a custom tool declared by that requestcustom_tool_call_outputwhile preserving ordinary function-call behaviorWhy
Cursor exposes ApplyPatch through a Chat Completions-compatible function envelope. Codex returns it as a Responses API custom tool call. The translator previously ignored custom-tool response events and later classified history only from the envelope type, so the tool call or its follow-up output was lost and the agent stopped before its final continuation.
Root cause
The response translator only handled
function_call/response.function_call_arguments.*. The request translator also treated every standardtype: "function"history item as a normal function call, without consulting the custom tools declared in the current request.Validation
Regression tests were committed failing before implementation, then passed after the fix.
gofmt -w .go test ./internal/translator/codex/openai/chat-completions/...go test ./...go build -o test-output ./cmd/server && rm test-outputThe executor tests exercise both HTTP SSE and Codex WebSocket streaming through the same translator.