Skip to content

test(e2e): poll MCP tools across multi-worker lag - #35042

Closed
mubashir1osmani wants to merge 4 commits into
BerriAI:litellm_internal_stagingfrom
mubashir1osmani:litellm_mcp_tool_resolve_multiworker
Closed

test(e2e): poll MCP tools across multi-worker lag#35042
mubashir1osmani wants to merge 4 commits into
BerriAI:litellm_internal_stagingfrom
mubashir1osmani:litellm_mcp_tool_resolve_multiworker

Conversation

@mubashir1osmani

@mubashir1osmani mubashir1osmani commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Stage multi-worker MCP e2e raced cold workers that 500 Tool not found after tools/list already worked on another process. The harness now polls until the server is listed, tools/list shows the tool, and tools/call is not a cold-worker not-found miss (and until 403 access_denied for the ungranted key path)

No product changes. Diff is tests/e2e/mcp only

Linear ticket

Test plan

  • Stage: mcp/test_mcp_key_access_e2e.py, test_mcp_datadog_e2e.py after e2e image includes this SHA

Multi-worker reloads put MCP servers in the registry from the DB but do
not re-run tools/list on every process. Gating call_tool on
tool_name_to_mcp_server_name_mapping made cold workers 500 with Tool not
found after another worker had already listed the tool. Treat a registry
match on server id/name/alias as enough; upstream rejects unknown tools
Comment thread litellm/proxy/_experimental/mcp_server/mcp_server_manager.py Outdated
@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates the MCP end-to-end harness to tolerate transient multi-worker registry misses

  • Adds polling helpers for successful and access-denied tool calls
  • Waits for MCP registration before selected access-group and Datadog scenarios
  • Migrates existing MCP end-to-end calls to the polling helpers

Confidence Score: 3/5

The PR is not yet safe to merge because the previously reported normalized-identifier collision can still route calls to the wrong MCP server

Distinct servers can coexist under raw stable IDs while the call resolver collapses space and underscore variants into the same normalized lookup key, selecting the first matching registry entry

Files Needing Attention: litellm/proxy/_experimental/mcp_server/mcp_server_manager.py; litellm/proxy/_experimental/mcp_server/utils.py

Important Files Changed

Filename Overview
tests/e2e/mcp/mcp_client.py Adds bounded polling for transient MCP tool or server not-found responses
tests/e2e/mcp/test_mcp_access_group_e2e.py Waits for server registration before exercising access-group selection
tests/e2e/mcp/test_mcp_datadog_e2e.py Uses registration and tool-call polling in the Datadog round-trip test
tests/e2e/mcp/test_mcp_key_access_e2e.py Uses polling helpers for permitted and denied MCP tool calls

Reviews (2): Last reviewed commit: "merge litellm_internal_staging into lite..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

…-worker lag

Stage multi-worker gateways only load MCP servers and tool maps on the
process that handled the request. Poll until the server is listed, the
tool appears on tools/list, and tools/call is not a cold-worker 500 so
key-access and Datadog MCP e2e stop racing the LB
Resolve e2e MCP conflicts by keeping staging await_registered/await_tool
and layering await_call_tool / await_call_tool_denied for multi-worker lag
@mubashir1osmani

Copy link
Copy Markdown
Contributor Author

@greptile-apps

@mubashir1osmani mubashir1osmani changed the title fix(mcp): resolve call_tool by registry without requiring tool map test(e2e): poll MCP tools across multi-worker lag Jul 29, 2026
@mubashir1osmani

Copy link
Copy Markdown
Contributor Author

Greptile's 3/5 note about normalized ID collision in mcp_server_manager.py is stale. That product change was reverted. Current net diff is tests/e2e/mcp only (poll helpers). No registry-normalize path is modified in this PR

@mubashir1osmani

Copy link
Copy Markdown
Contributor Author

@greptile-apps the pr has e2e tests only. it brings a timeout to make the mcp tools available across pods before they are called in the tests

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

The implementation looks correct. The polling logic in all three helpers is sound:

  • await_call_tool — retries only on 500 "not found" (cold-worker miss), propagates everything else immediately via unwrap.
  • await_call_tool_denied — retries on cold-worker misses, returns on 403, raises on any other outcome (including an unexpected 200, which would correctly surface a permissions bug).
  • _is_mcp_not_synced — conservative match: requires both a 500 status and "not found" plus either "tool" or "server" in the body, reducing false positives from unrelated 500s.

The test-side cleanup is also clean: UnknownApiError moves out of the test file import (it's now encapsulated inside await_call_tool_denied), and the match/case block in test_mcp_key_access_e2e.py is replaced with the simpler polling helper.

One minor thing worth confirming: await_call_tool_denied has no sleep before the deadline check when the last result is a non-cold-worker-miss non-403. But since it raises immediately in that branch, there's no infinite loop risk — correct by construction.

This is a clean, targeted fix for multi-worker test flakiness with no production code changes. LGTM.

@mubashir1osmani

Copy link
Copy Markdown
Contributor Author

Closing: reopened as a same-repo PR from BerriAI/litellm (not the fork). See the replacement PR linked above.

@codspeed-hq

codspeed-hq Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing mubashir1osmani:litellm_mcp_tool_resolve_multiworker (8b56e51) with litellm_internal_staging (1a6642e)

Open in CodSpeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant