fix(responses): fail loudly on MCP gateway failures (initial call, mid-stream, zero resolved tools) - #32579
Conversation
…emitting a broken stream When the initial LLM call inside MCPEnhancedStreamingIterator fails (e.g. an invalid previous_response_id -> provider 400 'No tool output found for function call ...'), the proxy returned HTTP 200 and the stream emitted the pre-generated mcp_list_tools discovery events with no response.created before them. That violates the Responses API streaming contract and crashes SDK stream accumulators (openai-node: "expected 'response.created' event, got response.mcp_list_tools.in_progress"). - aresponses_api_with_mcp now makes the initial call eagerly, before any SSE bytes are written, and re-raises the stashed failure so the client gets a real 4xx/5xx with the provider error body. - If a creation failure still surfaces during iteration, the stream emits a single terminal 'error' event instead of discovery events. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…low-up failures When tool execution failed as a batch, the stream proceeded to a follow-up call carrying function_call items with no outputs — rejected by the provider with 'No tool output found for function call ...' — and when the follow-up call itself failed, the stream simply ended with no terminal event. In both cases the client received HTTP 200 and a stream that looks like a truncated success: tool events, then silence. - Stash tool-execution and follow-up failures on the iterator. - Skip the doomed follow-up call entirely after a tool-execution failure. - Emit a single terminal OpenAI-style 'error' stream event carrying the mapped failure instead of ending silently. Builds on the initial-call failure handling from the previous commit (shares the _stream_error stash and _make_stream_error_event helper). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A request that explicitly asks for MCP tools via server_url litellm_proxy/... but resolves none of them (the API key/team has no access to the MCP server via allow_all_keys=false and no object-permission grant, the server name does not exist, or allowed_tools matches nothing) was silently sent to the model with no tools. The model then hallucinates, and the only trace is a list_mcp_tools spend log with status success and an empty response — the request looks healthy end to end while being completely broken. Raise a 400 BadRequestError naming the requested server URLs and the likely causes instead. Guard scope: - Mixed requests are exempt: with other (function) tools present, the request proceeds using those tools, matching the previous fallback behaviour. - Opt-out via litellm.reject_empty_mcp_resolved_tools = False (default True, per maintainer guidance). The auth-header pass-through test in tests/mcp_tests now resolves a dummy tool, since its purpose is header propagation, not zero-tool behaviour. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Greptile SummaryThis PR fixes three silent failure modes in the MCP gateway where client-visible errors were swallowed, producing HTTP 200 responses with broken or empty streams, or a no-tools model call that hallucinated. The fixes share a common stash-and-surface pattern: failures are stashed in
Confidence Score: 5/5Safe to merge — changes are well-scoped to the MCP streaming path, add an opt-out flag for the behavioural change, and are backed by 9 targeted mock tests covering every new code branch. All three failure modes are addressed through a coherent stash-and-surface pattern with no unsafe assumptions. The zero-tool guard is gated behind a module-level flag so existing users can opt out. The eager initial call correctly prevents double-invocation via the existing guard in No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/responses/mcp/mcp_streaming_iterator.py | Core fix: adds error stashing, monotonic sequence tracking, and terminal error event emission for initial-call, tool-execution, and follow-up failures; drops orphaned per-tool events on batch failure and skips the doomed follow-up call. |
| litellm/responses/main.py | Adds the zero-resolved-tools guard (raises BadRequestError before any model call) and the eager initial-call creation that re-raises pre-stream failures as real HTTP errors instead of broken HTTP-200 SSE streams. |
| litellm/init.py | Adds reject_empty_mcp_resolved_tools: bool = True module-level flag with a clear comment; follows the existing opt-out flag pattern used elsewhere in the module. |
| tests/test_litellm/responses/mcp/test_mcp_streaming_iterator.py | New test file with 6 well-focused tests covering the three failure scenarios (initial call, tool execution, follow-up) plus the happy path, with assertions on sequence-number monotonicity and absence of orphaned events. |
| tests/test_litellm/responses/mcp/test_mcp_empty_resolved_tools.py | New test file with 5 tests covering: zero-tool raises (stream and non-stream), fallback to function tools, flag-off restores old behaviour, happy path with resolved tools, and plain function-tool requests being unaffected. |
| tests/mcp_tests/test_aresponses_api_with_mcp.py | Legitimate test fix: mock_process now returns a dummy tool so the auth-header propagation test avoids the new zero-tool guard; the test's purpose (verifying mcp_server_auth_headers forwarding) is unchanged and coverage is not weakened. |
| litellm/proxy/dev_config.yaml | Adds the deepwiki MCP server entry used for manual e2e validation of the happy path; dev-only config, no production impact. |
Reviews (2): Last reviewed commit: "fix(responses): monotonic sequence numbe..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…rop orphaned tool events on batch failure Review feedback (Greptile on BerriAI#32579): - The terminal error event was numbered sequence_number=1, out of order after tool-execution events. __anext__ now tracks the highest sequence_number that passed through the stream and the error event is numbered after it. - A batch tool-execution failure queued mcp_call.in_progress events that never received a terminal per-item event. Those queued events are now dropped; the terminal error event carries the failure instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Both review points addressed in the latest commit:
|
- Drop test_initial_call_success_does_not_emit_error_event: the tool-call happy path (test_tool_call_happy_path_emits_no_error_event) already guards against false-positive error events and exercises more of the changed code (tool-exec + follow-up success paths). - Drop the stream=True parametrization on the zero-resolved-tools guard: the guard runs before the stream/non-stream branch in aresponses_api_with_mcp, so both cases hit identical code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removed MCP server configuration for deepwiki.
Relevant issues
Fixes #32561, fixes #32562, fixes #32563
Combines #32565, #32566 and #32567 into a single PR per review logistics (the three fixes share the
_stream_errorstash /_make_stream_error_eventhelper and their tests interact). All Greptile feedback from those PRs is already incorporated; see the closed PRs for the per-fix review threads.Pre-Submission checklist
Screenshots / Proof of Fix
Real proxy (
litellm --config, real OpenAI calls, deepwiki MCP server registered). Commit hashes are from the original per-fix branches; the combined branch is the same changes rebased ontolitellm_oss_staging.1. Initial LLM call fails (bogus
previous_response_id), streamingBefore (main @
999637883c) — HTTP 200 + discovery events with noresponse.created(crashes openai-node:expected 'response.created' event, got response.mcp_list_tools.in_progress):After (@
b6bbee9c92) — real 400 with the provider error body, before any SSE bytes:2. Tool execution / follow-up failure mid-stream (requires fault injection; captures drive the real iterator with only the failing dependency mocked)
Before — tool events, then the stream ends with no terminal event; a batch tool-execution failure additionally attempted the doomed follow-up (
follow-up model call attempted: True), rejected by the provider with"No tool output found for function call ...".After (@
bd9e13914e) — terminalerrorevent; doomed follow-up skipped:3. MCP tools requested, zero resolved (key lacks server access / unknown server)
Before (main @
999637883c) — HTTP 200, model called withtools=None, fabricated answer ("It seems I can't access external tools or links, including Linktree. You can check the number of links on your Linktree by logging into your account ..."); only trace is a "success"list_mcp_toolsspend log with[].After (@
60dc62fb84) — 400 with an actionable message:Happy path, fully e2e (real deepwiki
read_wiki_structurecall): unchanged on every fix — full two-phase flow, real answer ("The first topic name is "Overview.""), zeroerrorevents.Type
🐛 Bug Fix
Changes
Three swallow-points in the MCP gateway made failures invisible to clients (HTTP 200 + broken/silent streams, or silent no-tools model calls):
aresponses_api_with_mcp+MCPEnhancedStreamingIterator): the initial call is now made eagerly, before any SSE bytes are written, and a stashed creation failure is re-raised so the proxy returns a real 4xx/5xx. If a failure still surfaces during iteration, the stream emits a single terminalerrorevent instead ofmcp_list_toolsdiscovery events with noresponse.created.errorevent instead of silently ending the stream; after a batch tool-execution failure the doomed follow-up call (function_calls with no outputs → provider 400) is skipped entirely.litellm_proxyMCP tools but resolved none (and carry no other tools to fall back on) now raise a 400 naming the requested server URLs and likely causes. Mixed requests with function tools fall back to those tools (previous behaviour). Opt-out:litellm.reject_empty_mcp_resolved_tools = False(default True per maintainer guidance).Tests: 9 new tests across
tests/test_litellm/responses/mcp/test_mcp_streaming_iterator.py(new file) andtests/test_litellm/responses/mcp/test_mcp_empty_resolved_tools.py(new file); the auth-header pass-through test intests/mcp_tests/test_aresponses_api_with_mcp.pynow resolves a dummy tool (its purpose is header propagation, not zero-tool behaviour). Full local run: 50 passed.