Repository navigation
fix(responses): lift additional_tools input items into tools on the chat bridge - #38388
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
f0a87cd to
54bd965
Compare
PR overviewThis pull request updates the chat bridge to lift One issue has been addressed, but nested tool filtering remains ineffective when a guardrail removes a tool. An attacker could preserve a rejected tool through the bridge and make it available for use, bypassing the intended guardrail with a limited, request-level blast radius. Open issues (1)
Fixed/addressed: 1 · PR risk: 6/10 |
Greptile SummaryThis PR updates the Responses-to-Chat bridge to extract tools from Codex-style additional_tools input items before message conversion.
Confidence Score: 4/5The PR appears safe to merge functionally, with a non-blocking repository-convention issue around duplicated parsing logic and test placement. The changed bridge preserves surviving messages and sends lifted tools through the established converter; the remaining accepted concern is maintainability drift between parallel additional_tools implementations. Files Needing Attention: litellm/responses/litellm_completion_transformation/transformation.py; tests/test_litellm/responses/litellm_completion_transformation/test_additional_tools_lifting.py
|
| Filename | Overview |
|---|---|
| litellm/responses/litellm_completion_transformation/transformation.py | Correctly routes nested tools into the existing converter, but duplicates an existing wire-format parser rather than sharing it. |
| tests/test_litellm/responses/litellm_completion_transformation/test_additional_tools_lifting.py | Provides meaningful bridge regression coverage, though repository guidance calls for adding bug-fix cases to the existing mapped test module. |
Reviews (1): Last reviewed commit: "fix(responses): lift additional_tools in..." | Re-trigger Greptile
54bd965 to
aa44c80
Compare
9b9e670 to
5d3dd54
Compare
…hat bridge An additional_tools input item carries tool definitions but no content, so the Responses -> Chat Completions conversion dropped it silently and the model was offered no tools at all. Lift the nested tools into the converted tool list. Codex CLI emits this shape, so a Codex session against any provider without a native Responses config lost its entire toolset with no error and no log line. Lifting makes those tools live, so the /v1/responses allowlist and guardrail extractor has to see them too; otherwise a nested tool reaches the model without passing tool authorization. The bridge and the extractor now share one parser so they cannot disagree about what the effective tool list is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5d3dd54 to
29bb4e3
Compare
| merge_guardrailed_tools( | ||
| original_tools, | ||
| flattened_tool_groups, | ||
| tuple(t for t in guardrailed_tools if _guardrail_tool_identity(t) not in excluded), |
There was a problem hiding this comment.
Medium: Nested tool filtering is discarded
If a guardrail removes a nested tool from guardrailed_tools, this filter changes nothing in input; the original additional_tools item remains and the chat bridge later lifts that rejected tool. An attacker can therefore retain a tool that a filtering guardrail removed. Rebuild the nested items from the guardrail result, or remove them from input and hoist the guardrailed versions exactly once.
|
Closing as superseded by #40989, which carries the same additional_tools hoist on main plus the streaming and guardrail merge fixes. Thanks for the original fix |
TLDR
Problem this solves:
additional_toolsinput items never reach the providerHow it solves it:
User Flow
Before: a developer running Codex CLI through a LiteLLM gateway on a Bedrock-backed model gets an assistant that cannot use a single tool, and nothing says why.
POST https://litellm-domain/v1/responseswith a Bedrock GPT-5.6 deploymenthttps://litellm-domain/ui/?page=logsshows an ordinary successful callAfter: the same session uses its tools normally.
POST https://litellm-domain/v1/responseswith the same deploymenthttps://litellm-domain/ui/?page=logsshows the same request, now with the tool call recordedFor a key restricted by
metadata.allowed_tools, a tool nested ininputis now refused withtool_access_deniedinstead of silently reaching the model, and configured guardrails now see those tools instead of receiving none.Relevant issues
Related: #29818, #36182
Adjacent but deliberately out of scope: #27276 (tool-name and tool-type handling inside the same conversion).
Linear ticket
Pre-Submission checklist
uv run pytest tests/test_litellm/llms/openai/responses/test_openai_responses_guardrail_handler.py tests/test_litellm/proxy/test_tools_allowlist_enforcement.py tests/test_litellm/responses/litellm_completion_transformation/test_additional_tools_lifting.py -v(130 pass)make lintsub-checks verified locally: format-check-changed, ruff (litellm + tests config), ruff-strict ratchet, type-discipline ratchet, test-quality ratchet, circular-imports, import-safetyScreenshots / Proof of Fix
End-to-end against real Amazon Bedrock through a local proxy. No mocks.
config.yaml— resolves to theconverseroute via the bundled price map (litellm_provider: bedrock_converse):Request body: a Codex CLI 0.149 "responses lite" payload — top-level
tools: [], with 9 tool definitions nested in anadditional_toolsinput item across twonamespacecontainers (functions,collaboration),parallel_tool_calls: false,reasoning.context: all_turns.Two measurements are reported. Whether the model chooses to call a tool is not deterministic, so the authoritative signal is the tool count that reaches the outbound chat request.
Before (2e73400)
uv run litellm --config config.yaml --port 4000POST http://127.0.0.1:4000/v1/responseswith the body above →HTTP 200['message']— nofunction_call"I can't access terminal or file-reading tools in this session, so I'm unable to read ./README.md."After (29bb4e3)
uv run litellm --config config.yaml --port 4000POST http://127.0.0.1:4000/v1/responseswith the same body →HTTP 200functions__wait,functions__request_user_input,collaboration__followup_task,collaboration__interrupt_agent,collaboration__list_agents,collaboration__send_message,collaboration__spawn_agent,collaboration__wait_agent['function_call', 'message']collaboration__spawn_agent, and 3function_callevents across the turnType
🐛 Bug Fix
Caveats (if any)
Medium
additional_tools, but not redact one. Nested tools are sent for inspection and excluded from the write-back by identity, so a guardrail's edits to them are discarded. Strictly better than the previous behaviour, where tool extraction was skipped entirely whenever top-leveltoolswas empty, but not complete.spawn_agent, guardrails seecollaboration__spawn_agent(the namespace prefix). The bare inner name was chosen for the allowlist because that is what an operator writes inallowed_tools. Worth a maintainer's opinion.bedrock_mantlealready lifts it in its own transformation, others (xai, perplexity, openrouter) would need a separate change.bedrock_mantlestill carries its own copy of the parse. Migrating it onto the shared helper is a follow-up: its tests couple to internals including a debug-log assertion, and it is not needed here.Low
TestOpenAIResponsesHandlerToolInjectionandTestOpenAIResponsesHandlerNamespaceToolscover that.exec, typecustom) does not survive conversion to a chat tool; that is existingcustomtool handling, unchanged by this PR. See [Bug]: Responses API → Chat Completions bridge drops tool names and forwards unsupported tool types (custom, shell) #27276.toolsarray is introduced (cf. [Bug]: Responses→Chat bridge forwardstools: [](vLLM 422s) when request has no tools #30539)liston purpose: the input-to-messages conversion narrows onisinstance(input, list), so a tuple silently yields zero messages. Covered by a regression test.Final Attestation
🤖 Generated with Claude Code