fix(docker): restrict MCP SSE route to GET, fixing client connect hangs (#2120) - #2153
Open
chelsealong wants to merge 1 commit into
Open
fix(docker): restrict MCP SSE route to GET, fixing client connect hangs (#2120)#2153chelsealong wants to merge 1 commit into
chelsealong wants to merge 1 commit into
Conversation
Route(f"{base}/sse", endpoint=_MCPSseApp()) left `methods=None` because
Starlette only defaults to ["GET"] for function/method endpoints, not
class-based ASGI ones. That let *any* verb match the route: a client
that POSTs to probe for the newer Streamable HTTP transport before
falling back to legacy SSE (as several MCP clients do) got silently
routed into the SSE handshake instead of a fast 405, and the request
just sat open until the client's own timeout fired.
Fixes unclecode#2120
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2120 — "A timeout occurs when attempting to connect via MCP from LM Studio."
deploy/docker/mcp_bridge.pymounts the MCP SSE endpoint like this:_MCPSseAppis a callable class (used deliberately, per the comment above it,to get a raw ASGI app instead of Starlette's
request_response()wrapper —see #1594/#1850). But Starlette's
Route.__init__only defaultsmethodsto["GET"]when the endpoint is a plain function/method:With
self.methods = None,Route.matches()accepts every HTTP verb on/mcp/sse, not just GET. A client that POSTs to that URL — which is exactlywhat happens when an MCP client probes for the newer Streamable HTTP
transport before falling back to the legacy two-endpoint SSE transport the
URL name implies — gets routed straight into
sse.connect_sse(). That opensan SSE stream and waits for a POST to the (different)
/mcp/messages/endpoint that will never arrive from a client that already gave up on this
URL, so the original POST just hangs. There is no error, no fast 404/405 —
the connection sits open until the client's own timeout fires, which matches
the reported symptom exactly ("no response ... times out after several tens
of seconds").
The fix is a one-line addition of the
methods=["GET"]argument, so theroute only ever matches GET/HEAD, and any other verb gets a normal fast 405
Allow: GET, HEADresponse instead of being silently absorbed.I ruled out the currently-open lookalike PRs before writing this — none of
them touch
mcp_bridge.py, the MCP transport, or Docker auth (#2113 boundsPlaywright
evaluate()/page.content()calls, #2142 is an egress-proxyfeature, #1923 is a dispatcher-level
arun_manywatchdog, #2005 is a docs-onlyexample) — and confirmed no commit on
developsince the issue was filedtouches this file either.
Root cause verification
Reproduced against the actual
mcp_bridge.attach_mcp()+AuthGateMiddlewaremcp,fastapi,sse-starletteversions fromdeploy/docker/requirements.txtand booted aminimal app):
List of files changed and why
deploy/docker/mcp_bridge.py— passmethods=["GET"]explicitly to the/mcp/sseRoute()call.deploy/docker/tests/test_security_mcp_sse_methods.py— new regressiontest (named
test_security_*to match the existingpytest deploy/docker/tests/test_security_*.pyCI glob in.github/workflows/security.yml). Boots the real app via thestock_clientfixture from
conftest.py, authenticates pastAuthGateMiddlewarewith areal JWT (an unauthenticated request would get a fast 401 regardless of
methods, which wouldn't exercise this bug), and POSTs to/mcp/ssefrom adaemon thread with a 5s wall-clock bound — so on the buggy code the test
fails after 5s instead of hanging the whole suite forever.
How Has This Been Tested?
Confirmed the test fails without the fix and passes with it (reverted only
the source change locally, restored afterward):
Full existing security suite, unaffected:
(Installed via
pip install -e .+pip install -r deploy/docker/requirements.txt -r deploy/docker/tests/requirements.txt pytest pytest-asyncio, matching.github/workflows/security.yml.)ruff checkandblack --checkare clean on the new test file. (Pre-existingmcp_bridge.pyhas long-standing, unrelated ruff findings — noted in #2113 aswell — that this PR does not touch beyond the one added line.)
AI assistance disclosure
This PR was prepared with AI assistance (root-cause investigation, fix, and
test authored by an AI coding agent), then verified by running the actual
pinned dependency versions locally as shown above.
Checklist
only fixes a routing gap that made bad requests hang instead of erroring)