Fix stale request context in StatefulProxyClient handlers#3172
Conversation
StatefulProxyClient reuses sessions across requests, so its receive-loop task inherits a stale request_ctx ContextVar from the first request. Server-initiated messages (elicitation, sampling, etc.) that depend on related_request_id routing get sent to a closed stream and hang forever. Closes #3169
WalkthroughThe PR adds request context restoration mechanisms to the proxy module to fix context propagation issues in stateful proxy flows. It introduces internal utility functions Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/fastmcp/server/providers/proxy.py (2)
814-826: Consider applyingfunctools.wrapsto preserve handler metadata.The wrapper replaces the original handler's
__name__,__qualname__, and__doc__. If any code path logs or introspects handler identity, this will showwrapperinstead of the original name.functools.wrapsis a low-cost improvement.Suggested fix
+import functools + def _make_restoring_handler(handler: Callable, rc_ref: list[Any]) -> Callable: ... + `@functools.wraps`(handler) async def wrapper(*args: Any, **kwargs: Any) -> Any: _restore_request_context(rc_ref) return await handler(*args, **kwargs) return wrapper
879-887:_proxy_rc_reftype annotation could be more descriptive.
list[Any]is used throughout for the ref container. A type alias or a narrower annotation likelist[RequestContext[ServerSession, Any, Any] | None]would improve readability and catch misuse, thoughAnyis pragmatic given the import landscape.
| import mcp.types | ||
| from mcp import ServerSession | ||
| from mcp.client.session import ClientSession | ||
| from mcp.server.lowlevel.server import request_ctx |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
mcp python SDK request_ctx public API documentation
💡 Result:
There doesn’t appear to be a documented public Python SDK API named request_ctx in the official MCP docs; the public, documented way to access per-request data is via Context (FastMCP) and its ctx.request_context property (and, for low-level usage, the server’s request context object). (github.com)
Where the official docs cover this:
- MCP Python SDK docs home → links to the API Reference. (modelcontextprotocol.github.io)
- API Reference (generated docs) for the SDK types and classes. (modelcontextprotocol.github.io)
- “Request Context Properties” section (in the SDK README/docs content) describes what you can access via
ctx.request_context:ctx.request_context.lifespan_contextctx.request_context.meta(includingprogressToken)ctx.request_context.requestctx.request_context.request_id(github.com)
If you meant “how do I get request context?”, the documented answer is: add a Context parameter to your tool/resource handler and then use ctx.request_context (FastMCP). (github.com)
Citations:
- 1: https://github.com/modelcontextprotocol/python-sdk
- 2: https://modelcontextprotocol.github.io/python-sdk/
- 3: https://modelcontextprotocol.github.io/python-sdk/api/
- 4: https://github.com/modelcontextprotocol/python-sdk
- 5: https://github.com/modelcontextprotocol/python-sdk
🌐 Web query:
mcp server framework request_ctx ContextVar anthropic
💡 Result:
In Anthropic’s Model Context Protocol (MCP) ecosystem, the server-side frameworks/SDKs typically use a contextvars.ContextVar (often named request_ctx) to hold “the current request” context for the lifetime of handling one MCP message. This makes the context available anywhere in the same async task without threading it through every function call. [1][2]
Where request_ctx shows up (Python MCP servers)
- In MCP Python server implementations, you’ll commonly see something like:
request_ctx = contextvars.ContextVar("request_ctx")- a server property like
request_contextthat doesrequest_ctx.get()and raisesLookupErrorif you call it outside an active request. [2]
- The MCP Python SDK documents the public way you’re expected to use this: take a
Contextparameter in your tool/resource, then read request-scoped info viactx.request_context(lifespan context, request metadata, request id, etc.). [3]
How to use it (recommended pattern)
- Preferred (framework-supported): accept an injected
Contextand usectx.request_context:ctx.request_context.lifespan_contextctx.request_context.metactx.request_context.requestctx.request_context.request_id[3]
- Low-level/internal: reach for
request_ctx.get()/server.request_contextonly when you’re building middleware-like plumbing or helpers that can’t easily receivectxas a parameter—while handling theLookupErrorcase (no active request). [2]
“Anthropic” connection
MCP is the protocol Anthropic documents and supports across Claude products (e.g., connecting to MCP servers and calling tools). [1]
Sources:
[1] Anthropic MCP docs (protocol overview) (docs.anthropic.com)
[2] Example MCP server code showing request_ctx: ContextVar("request_ctx") and .request_context using request_ctx.get() (codebase.md)
[3] MCP Python SDK docs/README describing ctx.request_context and its properties (github.com)
Citations:
- 1: https://docs.anthropic.com/en/docs/mcp?utm_source=openai
- 2: https://codebase.md/ryaneggz/mcpserve?utm_source=openai
- 3: https://github.com/modelcontextprotocol/python-sdk?utm_source=openai
🌐 Web query:
mcp.server.lowlevel.server module public API
💡 Result:
mcp.server.lowlevel.server public API (Python MCP SDK)
Re-exported “official” low-level API (what the package intends you to use)
From mcp.server.lowlevel.__init__, the low-level server package publicly exposes only: Server, NotificationOptions. [1]
Symbols defined by mcp.server.lowlevel.server
-
class NotificationOptions: controls whether the server advertises “listChanged” notifications for prompts/resources/tools. [2] -
asynccontextmanager lifespan(server): default no-op lifespan context manager used for startup/shutdown resource management (can be overridden viaServer(..., lifespan=...)). [2] -
class Server[...](core low-level server implementation). [2][3]- Constructor includes (at least)
name, optionalversion, optionalinstructions, and optionallifespancontext manager. [2][3] create_initialization_options(...) -> InitializationOptionsconvenience builder. [2]get_capabilities(notification_options, experimental_capabilities) -> types.ServerCapabilitiescapability derivation from registered handlers. [2]request_contextproperty to access the currentRequestContextduring a request. [2][3]- Decorator-based handler registration methods (register request handlers), including:
list_prompts(),get_prompt()[2]list_resources(),list_resource_templates(),read_resource()[2]list_tools(),call_tool()(with argument validation / tool cache behavior described in the SDK internals). [2][3]
- Request/transport loop: the
Serverhandles request dispatching and protocol compliance (implemented around an internal_handle_requestpipeline). [3]
- Constructor includes (at least)
-
Likely-internal / not intended as stable public API
request_ctx(contextvars.ContextVar[...]) used to implementServer.request_context. [2]
request_ctx is an internal ContextVar; consider documenting the version dependency or using the public API.
Direct imports from mcp.server.lowlevel.server couple to implementation details not part of the mcp SDK's public API. The request_ctx ContextVar is undocumented and subject to change. If using this import is necessary, add a comment pinning it to a minimum mcp version (e.g., mcp>=1.24.0), or consider wrapping it defensively to handle potential future restructuring. Alternatively, use the public API pattern (ctx.request_context via the Context parameter) if the code structure allows.
| # StatefulProxyClient reuses sessions across requests, so | ||
| # its receive-loop task has stale ContextVars from the first | ||
| # request. Stash the current RequestContext in the shared | ||
| # ref so handlers can restore it before forwarding. | ||
| if isinstance(client, StatefulProxyClient): | ||
| cast(list[Any], client._proxy_rc_ref)[0] = ctx.request_context |
There was a problem hiding this comment.
Concurrent tool calls on the same StatefulProxyClient will race on _proxy_rc_ref[0].
If two requests invoke ProxyTool.run concurrently against the same cached StatefulProxyClient (from new_stateful), both write to _proxy_rc_ref[0] before their respective call_tool_mcp. The second write overwrites the first, and a handler triggered by the first request's backend call could restore the wrong RequestContext.
This may be acceptable if stateful backends are assumed to serialize calls, but worth documenting explicitly or guarding with a per-call ref.
Also, the cast on line 130 can be avoided by narrowing:
Suggested simplification
if isinstance(client, StatefulProxyClient):
- cast(list[Any], client._proxy_rc_ref)[0] = ctx.request_context
+ client._proxy_rc_ref[0] = ctx.request_context
StatefulProxyClientreuses MCP sessions across requests so that stateful backends (like Playwright) maintain their state. But the receive-loop task that handles server-initiated messages (elicitation, sampling, roots, etc.) inherits a frozenrequest_ctxContextVar snapshot from when the session was first created. On the second request, handlers likedefault_proxy_elicitation_handlercallget_context()and resolve a stalerequest_id— the streamable HTTP transport routes the response to a closed stream, and the call hangs forever.The fix wraps the default proxy handlers in
StatefulProxyClientonly (notProxyClient, which creates fresh sessions per request) with a shim that restores therequest_ctxContextVar before forwarding.ProxyTool.run()stashes the currentRequestContextbefore each backend call, and the shim detects staleness (same session, different request_id) to avoid corrupting concurrent sessions that share the ref viacopy.copy.Note: concurrent tool calls within the same stateful session can race on the shared context ref. This is a pre-existing limitation of the stateful session model (concurrent mutations to stateful backends like Playwright are already semantically broken), not something introduced by this fix.
Closes #3169