fix(mcp): apply outbound concurrency limit to OBO tool calls - #32071
Conversation
Greptile SummaryThe OBO (OAuth2 token-exchange) tool-call path in
Confidence Score: 5/5Safe to merge — the change is a narrow, well-tested fix to a single missing context-manager wrapping, with no behavioural impact on the non-OBO path. The fix is a small, surgical closure of a missing semaphore acquisition. The logic is structurally identical to the non-OBO branch it mirrors. The new test uses only mocks, reproduces the failure mode, and directly asserts the corrected peak concurrency. No pre-existing tests are modified, and no production-code paths other than the OBO branch are touched. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Wraps the OBO call coroutine in _limit_outbound_concurrency, fixing the missing semaphore acquisition that let OBO tool calls bypass the per-server max_concurrent_requests cap |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py | Adds TestOBOConcurrencyLimit with a mock-only asyncio concurrency test that blocks on an event until the peak inflight count has stabilised, then asserts the peak equals the configured cap — correctly fails on the unfixed code |
Reviews (3): Last reviewed commit: "fix(mcp): apply outbound concurrency lim..." | Re-trigger Greptile
Greptile SummaryThis PR fixes a concurrency-limit bypass in the MCP OBO (OAuth2 token-exchange) tool-call path. The OBO branch in
Confidence Score: 5/5Safe to merge — the change is minimal and targeted, the fix exactly mirrors the already-trusted non-OBO code path, and the new test demonstrates the corrected behavior. The OBO wrapper is a straightforward one-liner delta: a local No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Wraps the OBO tool-call coroutine in _limit_outbound_concurrency, mirroring the existing non-OBO _call_tool_via_client pattern; the semaphore is held across the initial call and any 401-triggered retry, fixing the bypass. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py | Adds TestOBOConcurrencyLimit.test_obo_dispatch_respects_max_concurrent_requests: drives 5 concurrent OBO dispatches through a blocking mock client with max_concurrent_requests=2, observes peak inflight, and asserts it stays at 2; all mocked, no real network calls. |
Reviews (2): Last reviewed commit: "fix(mcp): apply outbound concurrency lim..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
329c4b6 to
9c72739
Compare
The token_exchange (OBO) branch of _call_regular_mcp_tool built its coroutine by calling _obo_call_tool_with_retry directly, outside the _limit_outbound_concurrency context manager that the regular branch uses. OBO tool calls (and the internal re-mint retry, which issues a second upstream call_tool) therefore bypassed the per-server max_concurrent_requests semaphore, so an authenticated caller could run unlimited concurrent tool calls against an OBO MCP server despite an admin-configured limit. Wrap the OBO coroutine in _limit_outbound_concurrency the same way the regular path does, holding one permit across the initial call, the on-401 re-mint, and the retry, so OBO calls honor the configured cap.
40c7809 to
0534fde
Compare
|
bugbot run |
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 0534fde. Configure here.
Merging this PR will improve performance by 17.11%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_completion_simple_message |
4.8 ms | 4.1 ms | +17.11% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing litellm_mcp_v2_obo_concurrency_limit (0534fde) with litellm_internal_staging (ff6dc33)
Relevant issues
Stacked on #31762 (
litellm_mcp_v2_obo_endpoint_discovery), which is where the OBO token-exchange tool-call path and the per-server_limit_outbound_concurrencylimiter were introduced. #31983 stacks on the same base, so this fix flows up to it once it syncs with #31762Linear ticket
N/A (found during review of the OBO stack)
Pre-Submission checklist
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Root cause: in
_call_regular_mcp_tooltheoauth2_token_exchange(OBO) branch built its coroutine by calling_obo_call_tool_with_retrydirectly, outside the_limit_outbound_concurrency(mcp_server)context manager that the regular branch wraps its call in. OBO tool calls, and the on-401 re-mint retry (a second upstreamcall_tool), therefore never acquired the per-servermax_concurrent_requestssemaphore, so a caller with access to an OBO MCP server could run unlimited concurrent tool calls against it regardless of the admin-configured capReproduced against a live proxy on
localhost:4011backed by real Postgres, a real RFC 8693 token-exchange round-trip to a local IdP, and a real upstream MCP server (streamable-http). The upstream records how manycall_toolinvocations overlap. The MCP server is configured withauth_type: oauth2_token_exchangeandmax_concurrent_requests: 2; the proxy key travels inx-litellm-api-keysoAuthorizationcarries the OBO subject token. The same subject token is reused across the batch, so the exchanger single-flights to one exchange and all six calls pile onto the upstream semaphoreCommand (identical before and after):
Before the fix, all six calls run at once (peak upstream concurrency 6, every call returns in ~1.6s) despite the limit of 2:
After the fix, the six calls serialize into three waves of two (peak upstream concurrency 2, completions bucket at ~1.6s / ~3.2s / ~4.7s):
Type
🐛 Bug Fix
Changes
_call_regular_mcp_toolnow wraps the OBO coroutine in_limit_outbound_concurrency(mcp_server), mirroring the regular branch's_call_tool_via_client. One permit is held across the whole logical call: the initialclient.call_tool, the on-401 credential invalidation and client re-mint, and the retry. The initial client build stays outside the semaphore, matching the regular path, so OBO and non-OBO tool calls now enforcemax_concurrent_requestsidenticallyAdded
TestOBOConcurrencyLimit.test_obo_dispatch_respects_max_concurrent_requests, which drives concurrent OBO dispatches through_call_regular_mcp_toolagainst a blocking client and asserts the observed peak equals the configured cap. It fails on the unfixed code (peak equals the number of callers) and passes with the fixNote
Low Risk
Targeted concurrency fix aligned with existing non-OBO behavior; regression test covers the OBO dispatch path with no auth or data-model changes.
Overview
OBO (
oauth2_token_exchange) MCP tool calls now respect each server’smax_concurrent_requestscap, matching the non-OBO dispatch path.In
_call_regular_mcp_tool, the token-exchange branch previously scheduled_obo_call_tool_with_retrywithout entering_limit_outbound_concurrency, so concurrent OBO calls could bypass the per-server semaphore. The fix wraps that work in_obo_call_tool_limited(), holding one permit for the full logical call (initialcall_tool, optional 401 re-mint, and retry).Adds
TestOBOConcurrencyLimit.test_obo_dispatch_respects_max_concurrent_requeststo assert peak in-flight calls equals the configured limit.Reviewed by Cursor Bugbot for commit 0534fde. Bugbot is set up for automated code reviews on this repo. Configure here.