Repository navigation
fix(openai): preserve tool calls across the messages bridge - #9
Conversation
Function calls and their outputs are top-level Responses input items, while assistant text uses output_text. The reverse bridge now emits Anthropic tool_use blocks for JSON and SSE responses and preserves call IDs across the tool-result turn.
VictorMinemu
left a comment
VictorMinemu
left a comment
There was a problem hiding this comment.
Thank you for the contribution, the clear reproduction, and the comprehensive happy-path tests. The overall direction is good: keeping function_call and function_call_output as top-level Responses items matches the API contract.
Before merging, please address these blocking safety and protocol issues:
-
src/protocol/openai-response-to-anthropic.ts:32-45treats every non-messageoutput item as afunction_call. Responses can also returnreasoningand other item types. A valid reasoning item currently becomes an Anthropictool_usewith missingidandname, and changesstop_reasontotool_use. -
src/proxy/messages-cross-route.ts:106-178can materialize a tool call afterresponse.output_item.addedeven if the SSE fails, is truncated, or ends beforeoutput_item.done/response.completed. Combined withparseToolInputfalling back to{}, a malformed or incomplete call can look executable and may invoke a tool with unsafe defaults. Please fail closed: require finalized items and a successful terminal response, and propagate failed/incomplete/premature EOF as an error. -
JSON
refusalcontent is currently turned into a text block withouttext, while streaming refusal events are dropped. Please preserve or explicitly map refusals so the model safety signal is not lost.
Please also require non-empty call_id and name, do not substitute item.id for call_id, and validate that parsed arguments are an object rather than silently synthesizing {}.
Suggested regression coverage: reasoning before text/tool output; JSON and SSE refusals; missing IDs; malformed, scalar, or partial arguments; and failed/incomplete/prematurely closed SSE streams.
I verified the current head with npm test (30 files, 204 tests), npm run build, and npm run lint; all pass. Once the guards and negative-path tests are added, we can re-review promptly. Thanks again for improving CC-Router.
VictorMinemu
left a comment
VictorMinemu
left a comment
There was a problem hiding this comment.
Thanks again for the original contribution. I added the fail-closed protocol guards and negative-path coverage discussed in the review, including reasoning items, refusals, invalid metadata and arguments, and incomplete SSE streams. The published head passes build, lint, and all 221 tests. Approved for merge.
This takes upstream VictorMinemu/CC-Router d770488..555980d (v0.6.2 through the tool-call bridge) as the point this fork diverges, WITHOUT taking its tree. Nothing in the working tree changes here, so read this merge as an acknowledgement, not an integration. The reason it cannot be a content merge: upstream's 04e5dd1 and our 9194727 are the same work by the same author on the same day, diverged in form after PR VictorMinemu#9 was reshaped in review. A three-way merge therefore conflicts across ten files - 50 hunks - between two editions of one change, and resolving it yields a tree that is neither side and that no reviewer can check. Upstream's two genuine improvements on top of that shared base - 08c69cb (atomic tool arguments) and the maintainer's 0c91943 (fail closed on invalid tool streams) - are real and we want them. They are ported deliberately in the commits that follow this one, with their tests, keeping original authorship. The remaining upstream delta is the reshaping of our own commit and is deliberately not taken. From here upstream is read-only: a source to cherry-pick from, not a branch to track.
Ports the response-side half of upstream's 0c91943 (VictorMinemu) together with its shared helper, and takes upstream's edition of the two files whose only local commit was 9194727 — the same work as upstream's 04e5dd1, by the same author, reshaped in review on PR VictorMinemu#9. Nothing of ours is lost there. The behaviour this changes: unparseable tool-call arguments used to degrade to `input: {}`. That is worse than failing. A response truncated mid-JSON by max_output_tokens then reached the client as a well-formed tool call with arguments the model never produced — a search with no query, a delete with no filter. It now throws, and because our cross-route catch treats any throw before the first byte as a failed attempt, the request degrades to Anthropic and the caller still gets a real answer. Upstream answers 502 here; having a second provider, we can do better. A refusal is now carried as a refusal rather than reported as a completed turn, and a function call naming no tool is refused outright. One upstream simplification is deliberately not taken. Its `messageContentToAnthropic` reads `block.text` off every content block, which is sound for what we emit — function calls belong at the top level of `input` — but this is an inbound endpoint and a client may still nest them. That read yields undefined and fails the whole request with a TypeError, so the nested form is decoded here instead, as it was before. The narrow emit-side type is kept; the defence is at runtime, where the malformed input actually arrives.
2 participants
What this PR does
Preserves function tool calls across the Claude Code Messages ↔ OpenAI Responses bridge. Calls and tool outputs are sent as top-level Responses items, assistant text uses
output_text, and JSON or SSE function-call responses return as Anthropictool_useblocks with the originalcall_idand block order.Type of change
Testing
npm run buildpassesnpm testpasses (30 files, 204 tests)Related issues
Closes #8