fix: handle /mcp without trailing slash by adding explicit route - #27167
fix: handle /mcp without trailing slash by adding explicit route#27167krrish-berri-2 wants to merge 1 commit into
Conversation
Starlette's Mount('/mcp', ...) issues a 307 redirect from /mcp to /mcp/,
which drops the request body and breaks MCP clients. Add an explicit
@app.api_route('/mcp') on the parent app that forwards directly to the
MCP streamable-HTTP handler, so both /mcp and /mcp/ work identically.
Co-authored-by: Krrish Dholakia <krrish-berri-2@users.noreply.github.com>
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes the 307-redirect loop that occurs when Starlette's
Confidence Score: 4/5Safe to merge; the change adds one small forwarding route with no effect on existing /mcp/ traffic or any other endpoint. The implementation is correct and consistent with the existing toolset/dynamic MCP route pattern. The only findings are a redundant scope path assignment and minor test coverage gaps. No files require special attention; both changed files are straightforward.
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | Adds bare_mcp_route — an explicit @app.api_route("/mcp", ...) that forwards directly to handle_streamable_http_mcp, preventing Starlette's Mount 307 redirect; consistent with existing toolset/dynamic MCP route patterns; a redundant scope["path"] = "/mcp" assignment is present but harmless. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_bare_route.py | Structural unit test verifying the /mcp route exists on the parent app; no network calls made; has an unused pytest import and only spot-checks GET/POST rather than asserting the full set of seven registered HTTP methods. |
Reviews (1): Last reviewed commit: "fix: handle /mcp without trailing slash ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import pytest | ||
| from starlette.routing import Route |
There was a problem hiding this comment.
pytest import is unused — no @pytest.mark decorators or fixtures are referenced anywhere in this test file.
| import pytest | |
| from starlette.routing import Route | |
| from starlette.routing import Route |
| route = mcp_routes[0] | ||
| assert "GET" in route.methods | ||
| assert "POST" in route.methods |
There was a problem hiding this comment.
| route = mcp_routes[0] | |
| assert "GET" in route.methods | |
| assert "POST" in route.methods | |
| route = mcp_routes[0] | |
| assert route.methods == {"GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS", "HEAD"} |
| scope = dict(request.scope) | ||
| scope["path"] = "/mcp" | ||
| return await _stream_mcp_asgi_response( |
There was a problem hiding this comment.
scope["path"] = "/mcp" assignment is redundant: any request routed to bare_mcp_route already has a scope path of /mcp. Removing it makes the intent of the copied scope clearer and aligns with the fact that no path transformation is happening here (unlike the toolset route, which translates /toolset/<name>/mcp → /mcp).
| scope = dict(request.scope) | |
| scope["path"] = "/mcp" | |
| return await _stream_mcp_asgi_response( | |
| scope = dict(request.scope) | |
| return await _stream_mcp_asgi_response( |
|
🤖 litellm-agent: Merged into staging branch |
Pull request was closed
3 participants
Relevant issues
Fixes the issue where
/mcp(without trailing slash) doesn't work but/mcp/does.Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitType
🐛 Bug Fix
Changes
When the MCP sub-app is mounted via
app.mount("/mcp", mcp_app), Starlette issues a 307 redirect from/mcpto/mcp/. This drops the request body and breaks MCP clients that don't follow redirects.Fix: Added an explicit
@app.api_route("/mcp", ...)on the parent FastAPI app that forwards requests directly to the MCP streamable-HTTP handler via the existing_stream_mcp_asgi_responsehelper. This ensures both/mcpand/mcp/work identically without any redirect.Files changed:
litellm/proxy/proxy_server.py— Addedbare_mcp_routehandler before the toolset/dynamic MCP routestests/test_litellm/proxy/_experimental/mcp_server/test_mcp_bare_route.py— Unit test verifying the explicit/mcproute exists on the parent app with correct HTTP methodsSlack Thread