fix(responses-bridge): custom tool round-trip and allowlist preservation for Codex CLI - #32258
Conversation
…ion for Codex CLI Convert Responses API custom tools to Chat Completions function tools and map function_call responses back to custom_tool_call output items so Codex CLI gets the apply_patch round-trip it expects. Preserve and validate allowed_callers during the custom->function conversion so the Anthropic adapter's caller allowlist is not silently dropped, which would let a tool meant to be callable only by another tool be invoked directly by the model. Use modern type annotations (list/dict/set/X | None) throughout to keep the ruff strict budget within its ratcheted ceilings.
|
Generated by Claude Code |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes the Responses → Chat Completions bridge so
Confidence Score: 5/5Safe to merge; all changes are scoped to the responses bridge and carry comprehensive test coverage. The custom tool round-trip, allowlist enforcement, streaming latch, and disconnect-metadata fixes are all well-implemented and well-tested. The logic is consistent across the streaming and non-streaming paths, the security-relevant allowlist enforcement is correct, and the multi-turn history reconstruction for custom_tool_call items is properly scoped to avoid corrupting plain function_call items. litellm/responses/litellm_completion_transformation/custom_tools.py — the allowed_callers placement in ChatCompletionToolParam could affect strict OpenAI-compatible providers, but this is limited to the unusual case where a custom tool carries allowed_callers.
|
| Filename | Overview |
|---|---|
| litellm/responses/litellm_completion_transformation/custom_tools.py | New module for forward/reverse conversion between Responses API custom tools and Chat Completions function tools. |
| litellm/responses/litellm_completion_transformation/transformation.py | Extended to convert custom tools, drop unsupported types with warning, and recover custom_tool_call payloads in multi-turn history. |
| litellm/responses/litellm_completion_transformation/streaming_iterator.py | Uses build_tool_call_item_kwargs for custom_tool_call streaming events and latches completed_response. |
| litellm/router.py | FallbackResponsesStreamWrapper now copies source_iterator.completed_response on StopAsyncIteration. |
| litellm/proxy/common_request_processing.py | Guards _apply_client_disconnect_metadata against None and uses explicit get+reassign pattern. |
| litellm/llms/openai/responses/guardrail_translation/handler.py | extract_request_tool_names includes type: custom tools for allowlist enforcement. |
| litellm/types/responses/main.py | Adds CustomToolCallOutputItem Pydantic model. |
| litellm/types/llms/openai.py | Registers CustomToolCallOutputItem in ResponsesAPIResponse union and adds allowed_callers to ChatCompletionToolParam. |
Reviews (6): Last reviewed commit: "fix(responses-bridge): default missing f..." | Re-trigger Greptile
Greptile SummaryThis PR backports a fix (originally #31571 by
Confidence Score: 4/5The core custom tool round-trip logic is well-tested and the three targeted bug fixes are low-risk; two non-blocking gaps in observability and return-type clarity are worth addressing before merge. The custom tool conversion, streaming latch, and client-disconnect fixes are correct and covered by new tests. Two concerns keep this from a fully clean assessment: unsupported tool types are silently discarded with no warning log, and model_dump() on line 1693 converts every pre-existing output item to a plain dict, broader than the new CustomToolCallOutputItem alone required. transformation.py (silent tool drop at lines 1125-1138 and broad model_dump at line 1693); custom_tools.py (unused build_custom_tool_call_item)
|
| Filename | Overview |
|---|---|
| litellm/responses/litellm_completion_transformation/custom_tools.py | New module implementing custom→function tool conversion and function_call→custom_tool_call reverse conversion; build_custom_tool_call_item is defined but never called from production code |
| litellm/responses/litellm_completion_transformation/transformation.py | Custom tool conversion integrated into the non-streaming path; model_dump() now applied to all output items; computer_use/image_generation/namespace/shell tools silently dropped without a warning |
| litellm/responses/litellm_completion_transformation/streaming_iterator.py | Streaming path updated to emit custom_tool_call items via the shared helper; completed_response latched on the response.completed event; mostly type-annotation modernisation |
| litellm/proxy/common_request_processing.py | Correctly fixes crash when metadata is explicitly None during a streaming client disconnect |
| litellm/router.py | FallbackResponsesStreamWrapper falls back to source_iterator.completed_response on StopAsyncIteration |
| litellm/types/responses/main.py | Adds CustomToolCallOutputItem Pydantic model |
| litellm/types/llms/openai.py | Adds CustomToolCallOutputItem to the ResponsesAPIResponse.output union type |
| tests/test_litellm/responses/test_custom_tool_call.py | New mock-only test file covering the custom tool round-trip |
| tests/test_litellm/responses/litellm_completion_transformation/test_litellm_completion_responses.py | Existing computer_use test expectation updated to reflect drop behaviour; new tests cover custom tool conversion and allowed_callers preservation |
| tests/test_litellm/proxy/test_common_request_processing.py | New mock-only tests verify the None-metadata guard |
Reviews (2): Last reviewed commit: "fix(responses-bridge): custom tool round..." | Re-trigger Greptile
Type convert_custom_tool_to_function_tool against Mapping/ChatCompletionToolParam and validate allowed_callers with a strict TypeAdapter so the two new cast() calls that tripped the LIT006 ceiling are gone. Warn when dropping Responses-only tool types (computer_use, image_generation, namespace, shell) instead of discarding them silently. Return output items as Pydantic models instead of model_dump()ing every item to a dict, matching the declared return type. Apply the same None-safe metadata pattern to the request_data paths that still used setdefault, and drop the unused build_custom_tool_call_item helper.
|
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Empty arguments drop custom input
- Changed the recovery guard from
is Noneto a falsy check so an emptyargumentsstring still falls back to reconstructing the payload frominput.
- Changed the recovery guard from
You can send follow-ups to the cloud agent here.
…ponses route The Responses guardrail translation handler only extracted function and mcp tool names, so a key or team restricted by metadata.allowed_tools could invoke a disallowed tool by declaring it with type custom now that the bridge converts custom tools into callable Chat Completions function tools. Extract custom tool names through the same path so check_tools_allowlist rejects them.
910c211 to
65db265
Compare
|
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Empty arguments reuse input field
- Gated the input-based arguments rebuild on
type == "custom_tool_call"so regularfunction_callitems with empty-string arguments are no longer wrapped into a{"content": ...}envelope.
- Gated the input-based arguments rebuild on
You can send follow-ups to the cloud agent here.
…ll items
Recovering tool arguments from the input field on any falsy arguments value
made plain function_call input items with empty arguments and a stray input
key get rewritten into a {"content": ...} envelope, corrupting multi-turn
replay for normal function tools. Gate the recovery on the item type so it
only applies to custom_tool_call items, which are the ones that store their
payload in input.
4a5b764 to
07ba468
Compare
|
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missing arguments become literal None
- Changed
str(raw_arguments)tostr(raw_arguments or "")so a missing/Noneargumentskey produces an empty string instead of the literal "None".
- Changed
You can send follow-ups to the cloud agent here.
…ty string With input recovery scoped to custom_tool_call items, a plain function_call input item without an arguments key left raw_arguments as None and the downstream str() turned it into the literal string None. Coerce to an empty string instead, matching the pre-bridge behavior.
5f87711 to
87347a6
Compare
|
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 87347a6. Configure here.
2f0cdb3
into
litellm_internal_staging
Relevant issues
Copy of #31571 by @duanhongyi, recreated on an internal branch so CircleCI can run. All credit for the original fix goes to @duanhongyi; the first commit preserves his authorship
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
See the original PR #31571 for context; this PR carries the same commit, rebased onto the latest litellm_internal_staging, plus review-feedback commits
Type
🐛 Bug Fix
Changes
When the bridge routes
/responsesrequests to providers that only support Chat Completions, tools withtype: "custom"(for example Codex CLI'sapply_patch) were silently dropped or treated as plain function tools. The model then returned afunction_calloutput item that the client could not map back to acustom_tool_call, breaking the round-trip. The conversion also discardedallowed_callers, so a tool meant to be callable only by another tool could be invoked directly by the modelThe new
litellm/responses/litellm_completion_transformation/custom_tools.pymodule forward-converts custom tools to function tools (with the grammar embedded in the description so the model produces correctly formatted output), reverse-convertsfunction_callresponses back tocustom_tool_calloutput items (unwrapping the{"content": ...}JSON envelope), and validates and preservesallowed_callers.transformation.pyandstreaming_iterator.pyshare this module so both the non-streaming and streaming paths emitcustom_tool_callitems when the call targets a tool that was originally customrouter.pynow falls back to the source iterator'scompleted_responsewhen the inner streaming generator is exhausted instead of logging a spurious "no completed_response" warning, andcommon_request_processing.pyguards_apply_client_disconnect_metadataagainst aNonetarget dict during streaming disconnects.CustomToolCallOutputItemis added inlitellm/types/responses/main.pyand registered in theResponsesAPIResponseoutput item unionFollow-up commits address review feedback on the copy. From Greptile:
convert_custom_tool_to_function_toolis now typed againstMapping/ChatCompletionToolParamwithallowed_callersvalidated by a strictTypeAdapter(removing the twocast()calls that tripped the LIT006 ceiling in lint); dropping Responses-only tool types (computer_use,image_generation,namespace,shell) now logs a warning instead of being silent; output items are returned as Pydantic models instead ofmodel_dump()ing every item to a dict; therequest_datametadata paths that still usedsetdefaultnow use the same None-safe pattern as thelogging_objpaths; and the unusedbuild_custom_tool_call_itemhelper is removed. From Bugbot: payload recovery frominputalso triggers whenargumentsis an empty string, that recovery is scoped tocustom_tool_callitems so a plainfunction_callwith empty arguments is never rewritten into a{"content": ...}envelope, and afunction_callitem with noargumentskey now yields an empty arguments string instead of the literal string "None". From Veria: the Responses guardrail translation handler now extractscustomtool names too, socheck_tools_allowlist(metadata.allowed_tools) rejects a disallowed tool declared astype: "custom"instead of letting the bridge convert it into a callable function toolTests cover the custom to function to
custom_tool_callround-trip,allowed_callerspreservation, the transformation integration, both client disconnect metadata fixes, allowlist enforcement for custom tools on the responses route, and the scopedinputpayload recovery including missing and emptyargumentsfor both item types