Repository navigation
fix(responses): relay undeclared passthrough tool calls on chat and anthropic inbound - #6732
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughPassthrough delivery now limits undeclared tool-name refusal to Responses inbound requests. The SSE guard accepts an enforcement flag, and integration tests cover Anthropic, Chat Completions, and Responses requests across streaming and upstream response formats. ChangesPassthrough guard behavior
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to No merge-blocking issue is identified in the reviewed changes; normal checks can proceed. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: a4889429d3
ℹ️ 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".
| const { commitReasoningReplayServingRoute, recordTerminalOutcomes } = responseEffects; | ||
| const { parsed, route, subagentQuotaFailureModel, clientRequestedStream, translatorBudget } = requestState; | ||
| const { parsed, route, subagentQuotaFailureModel, clientRequestedStream, translatorBudget, inboundWire } = requestState; | ||
| const enforceDeclaredToolNames = inboundWire !== "chat" && inboundWire !== "anthropic"; |
There was a problem hiding this comment.
When a Responses upstream emits an undeclared tool for a Chat inbound request, this line now disables the refusal, but the five cases in tests/responses/chat-sse-framing-guard.test.ts:93-121 still require an undeclared client tool error and forbid tool_calls. Those tests exercise this exact passthrough path with both streaming modes and CR-fragmented SSE, so the changed-source test suite will fail consistently; update them to assert successful relay while retaining their framing coverage.
AGENTS.md reference: src/AGENTS.md:L24-L26
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
Merge b1e1735 additively after the specified dev c0f8165. The four newer upstream commits are 18a8cda (lidge-jun#6731), 13348ce (lidge-jun#6729), 8d30945 (lidge-jun#6713) and b1e1735 (lidge-jun#6732); their history is preserved unchanged. The only content conflict is src/combos/jev.ts, where lidge-jun#6731 edited the endpoint resolver that the P0 exchange extraction moved to src/combos/jev-service-exchange.ts. This merge keeps the P0 side of jev.ts byte for byte. The exchange resolver still applies the pre-lidge-jun#6731 trailing-slash normalization after this commit; carrying the shared endpoint authority into it is a separate follow-up commit. Co-authored-by: SeongwoongCho <35558061+SeongwoongCho@users.noreply.github.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
1 participant
Summary
Closes #6648.
Claude Code (and other
/v1/messagesor/v1/chat/completionsclients) routed to a provider on the Responses passthrough path had their turn killed withrouted provider emitted undeclared client tool "Write"; only request-declared tools may be calledwhenever the model called a tool the request had not declared, for example a deferred or runtime-discovered tool.The documented contract (#4799,
structure/adapters/compatibility-contracts.md"Undeclared-tool refusal is an inbound-protocol claim") is that only theresponsesinbound wire refuses an undeclared call (#1700);chatandanthropicrelay it to the client's own tool runner.run-turn-execution.tsandadapter-delivery.tsalready scope it that way. The passthrough path did not:passthrough-delivery.tsrefused on every wire.This change scopes only the two passthrough refusal points (the SSE block rewrite and the buffered JSON 502) by
inboundWire.undeclaredToolGuardActiveis kept on every wire because it also gates namespace normalization and continuation suppression, so:responsesinbound refusal is unchanged (stream, buffered JSON, reframed SSE).chat/anthropicis still never written toprevious_response_idreplay state.default.Read→Read) still runs on every wire.responseswire.inboundWireis set server-side by each endpoint handler, so a/v1/responsescaller cannot opt out of #1700 enforcement.Verification
tests/responses/passthrough-deferred-tools.test.tsfailed on unmodifieddev(22 pass / 8 fail; every Anthropic and Chat stream/buffered case refusedWritewith the message above) and passes after the fix (30 pass / 0 fail). It drives/v1/messagesand/v1/chat/completionsthrough a Responses-adapter passthrough provider, covers SSE and JSON upstreams, undeclared and declared names, namespace normalization, and asserts/v1/responsesstill refuses.bun test tests/responses/passthrough-deferred-tools.test.ts tests/responses/responses-undeclared-tool-guard.test.ts tests/responses/chat-completions-deferred-tools.test.ts— 133 pass / 0 fail (independent reviewer run).tests/adapters/bridge.test.ts, layout and file-size ratchet tests (27 pass),bun run typecheck,bun run structure:check,bun run privacy:scan.a4889429d3: approved, no blocking findings. Probes confirmed byte-identicalresponsesrefusal output against the parent commit, no wire spoofing through body or headers, and replay suppression on both SSE and JSON paths.Checklist
devstructure/transports/responses-wire-shapes.mdupdated for the owned source changeExisting framing test updated
tests/responses/chat-sse-framing-guard.test.ts(from the #6461 carry, 2753ef9) protects SSE framing alignment: the passthrough guard must read the same CR/LF/CRLF event boundaries as the client projection, so framing cannot hide a call from validation. It used chat-inbound refusal as its observable, which this PR intentionally removes. The refusal-under-CR-framing cases now run against/v1/responses, where refusal still applies, with the same stream/fragmented/live-before-EOF fixtures. The chat cases now assert the same CR-framed call is relayed intact astool_calls. A mutation check by the reviewer confirmed the coverage holds: making only the guard blind to CR delimiters fails all five Responses refusal cases.The branch also merges
devand joins onelayout.jsonregistration onto an existing line, becausedevat 1a8377d (#6721) had reached 2,000 lines and tripped the file-size ratchet (NEW_OVERSIZED) for every PR.Additional verification
bun test $(rg -l 'undeclared client tool|undeclaredTool|enforceDeclaredToolNames' tests)plussse-rewrite-line-endings, layout and ratchet tests: 604 pass / 0 fail across 23 files (independent reviewer run onc392328689).c3923286897b8b96d5e6c283bdc7f441129723bf: approved, no blockers.