chore: merge train 260821 — land #2270 (apply_patch on routed Responses destinations) - #2327
Merged
Conversation
`ROUTED_CUSTOM_TOOL_PASSTHROUGH` exempted `apply_patch` from routed
custom-tool lowering unconditionally, so it reached every routed destination
as a `type: "custom"` tool with `custom_tool_call` items. xAI's Responses
endpoint rejects that item type:
422 Failed to deserialize the JSON body into the target type:
input[5]: invalid "custom_tool_call" item: missing field `id`
The message is misleading — the id is present. Instrumenting the adapter
showed the item leaving as
`{"type":"custom_tool_call","id":"ctc_abc123","call_id":"c1",...}`; xAI
reports the first field its own parser cannot satisfy rather than the real
problem, which is that it does not accept the item type. Same class as its
"Could not decode the compaction blob" message for a reasoning field, so the
fix is not to generate or preserve ids.
Live A/B against the endpoint — identical body, identical id, only the tool
name differs:
apply_patch (exempt from lowering) -> 422
my_custom_thing (lowered to a function) -> 200
Lowering is what makes it work; the exemption is what breaks it. It surfaces
on Codex's compact turn because a real session always contains apply_patch
calls, but a plain replay reproduces it too.
The exemption is not wrong everywhere — the canonical ChatGPT surface speaks
custom_tool_call natively and lowering there would regress it. The defect is
that one unconditional rule about "routed providers" encoded a claim about a
single destination's capability. Add `supportsResponsesCustomTools`,
following the existing `supportsOpenAiWebSearchToolFields` shape: declared on
the registry row and the provider config, filled only when unset, and
consumed as an explicit denial. Absent or true keeps today's behaviour
byte-identical; only xAI declares false.
The response path needed no special case: it is name-generic, so once
apply_patch joins the converted set the existing repair restores the
function_call and its streaming argument events to a custom_tool_call with
the original call id.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every routed lowering step derives its plan from the tool declarations, and
the compaction body build deletes them. It ran first, so on a compaction turn
the plan was empty and replayed call items reached the wire in their private
shapes. Against xAI:
422 Failed to deserialize the JSON body into the target type:
input[5]: invalid "custom_tool_call" item: missing field `id`
The id is present; xAI reports the first field its own parser cannot satisfy
rather than the real problem, which is that it does not accept the item type.
Instrumented the adapter to pin the mechanism: with declarations present the
call item is converted; with them absent, or on a compaction turn, it goes out
raw. Reordering locally produced `function_call` / `function_call_output` with
`tools` still absent and the compact prompt still appended.
This is the second time this exact shape has been fixed here — a replayed
namespace key survived for the same reason. That fix taught one lowering step
to cope; this one fixes the pipeline, so the next private field added does not
need its own workaround. The invariant is now stated at the call site: the
compaction body build removes the tool surface and must be the last routed
transform.
Two effects beyond the call items, both improvements: `promoteClientLoadedTools`
could previously reintroduce top-level `tools` after compaction had removed
them, which running compaction last now prevents; and namespace-collision
validation runs before the declarations are deleted. Non-compaction output is
byte-identical, pinned by an exact comparison test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 59d0cde7f75f0e645a12ec44a388609dfba50ce6)
The namespace-replay restore test verified the restored custom_tool_call events but never checked that the stream still ends with data: [DONE], so a regression that drops the terminal marker would have passed. The sibling lowering test already asserts it; match that. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zpLCh4eEms6un3VjapRgL
Forward auth is not an OpenAI-destination identity. A noncanonical forward provider that denies native custom tools must still convert apply_patch. Pin the adapter serialization and the handleResponses path.
…t explicit denial Adversarial review P2: the lowering boundary rested on code reading only — add a negative pin proving the exact canonical Codex forward surface ignores supportsResponsesCustomTools: false and keeps custom tools verbatim.
Contributor
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe change adds provider capability routing for Responses custom tools. It gates ChangesResponses custom-tool capability routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant routeModel
participant openaiResponses
participant Upstream
Client->>routeModel: submit Responses request
routeModel->>openaiResponses: resolve provider capability
openaiResponses->>openaiResponses: lower unsupported custom tools
openaiResponses->>Upstream: send transformed request
Upstream-->>openaiResponses: stream function-call events
openaiResponses-->>Client: restore custom-tool events
Suggested reviewers: ✨ Finishing Touches📝 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 |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fifth leg of the merge train: lands #2270 — fix(responses): make apply_patch work on routed Responses destinations plus a boundary hardening test.
Review chain:
supportsResponsesCustomTools: falseand keeps custom tools verbatimVerification
Checklist
Summary by CodeRabbit
New Features
apply_patch, to providers with different tool capabilities.Bug Fixes