Skip to content

Support for http request injection propagation to tools #816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
May 28, 2025

Conversation

ihrpr
Copy link
Contributor

@ihrpr ihrpr commented May 27, 2025

This PR implements request context propagation in the MCP Python SDK for SSE transport, build on top of PR #380 and addressing concerns in the comments. The implementation captures HTTP request metadata (headers, method, URL, etc.) for each individual request and makes it available to request handlers for authentication and observability purposes.

Commends addressed based on PR #380

  1. Per-Request Context: implementation captures context for each individual POST request
  2. Non-Breaking: No changes to public API signatures - adding default to type definition
  3. Added tests verifying header propagation and context isolation

Implementation Details

  • Modified ServerMessageMetadata to carry request context through the message pipeline
  • Updated SSE transport to extract request metadata for each POST message
  • Server class extracts context from message metadata and makes it available via server.request_context
  • Request handlers can access HTTP headers and other metadata for authentication/authorization decisions

Follow ups

  • Implement request context propagation for Streamable Http
  • Refactor SSE tests + Streamable Http

@ihrpr ihrpr marked this pull request as ready for review May 27, 2025 15:48
Comment on lines 208 to 215
request_context: RequestData = {
"headers": dict(request.headers),
"method": request.method,
"url": str(request.url),
"client": request.client,
"path_params": request.path_params,
"query_params": dict(request.query_params),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we instead of passing an open dict, just pass the request object? This gives the full power to the consumer. They can still isinstance check to see what specific object they are getting. This way we are a bit more generic than specifiying an open dict.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern with that is that we are putting starlette.requests into the core and types. Not sure I'm super happy about that, but equally we use starlette everywhere in the SDK

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should pass the request.scope, which is a dictionary, and contains all the connection information the user will ever need.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest changes use request, not the request.scope I mentioned tho 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw request.scope only after pushed the update... will update in a few hours

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kludex would that work for any transport? the problem with the general RequestData is that it's:

  1. Has no clear structure.
  2. Might de-facto depend on HTTP specifics

I was wondering if we can make this a generic and actually pass the origianl request objects. This could be in gRPC context something different than Websocket or HTTP.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly none of the options are great here. I'd love to use generics to make this properly typed, but that would break backwards compatibility for everyone using RequestContext[SessionT, LifespanContextT] - they'd get errors about missing type arguments.

So for now, i'd suggest to go with request.scope as a dict. It's not perfect but:

  • No breaking changes
  • No framework dependencies in core types
  • Gets us all the request data we need

For WebSocket, it'll work the same way - we can pass the WebSocket's ASGI scope which contains initial handshake headers etc

The scope structure is consistent across HTTP and WebSocket per the ASGI spec, so tools can handle both transports uniformly.

Not ideal, but it gets the job done without breaking existing code. I'd love to revisit this in sdk v2 though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly none of the options are great here. I'd love to use generics to make this properly typed, but that would break backwards compatibility for everyone using RequestContext[SessionT, LifespanContextT] - they'd get errors about missing type arguments.

The way to do for this is to set a default value on the TypeVar - so people don't get errors.

from typing_extensions import TypeVar

T = TypeVar("T", default=...)`

I don't consider type changes as breaking changes btw.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, cool, thanks, didn't realise typing_extensions had TypeVar with default

Comment on lines 208 to 215
request_context: RequestData = {
"headers": dict(request.headers),
"method": request.method,
"url": str(request.url),
"client": request.client,
"path_params": request.path_params,
"query_params": dict(request.query_params),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should pass the request.scope, which is a dictionary, and contains all the connection information the user will ever need.

]


def run_context_server(server_port: int) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid running uvicorn in the test suite and just use the httpx.AsyncClient with the ASGITransport?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do as a follow up, as sse tests are using it everywhere

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably why I found some flaky issues the other day. We shouldn't have those sleeps around.

@ihrpr ihrpr marked this pull request as draft May 27, 2025 17:42
@ihrpr ihrpr marked this pull request as ready for review May 27, 2025 19:09
@dsp-ant dsp-ant self-requested a review May 28, 2025 10:23
@ihrpr ihrpr marked this pull request as draft May 28, 2025 10:31
dsp-ant
dsp-ant previously approved these changes May 28, 2025
@ihrpr ihrpr marked this pull request as ready for review May 28, 2025 11:08
@ihrpr ihrpr requested review from Kludex and dsp-ant May 28, 2025 12:39
@ihrpr ihrpr merged commit 70014a2 into main May 28, 2025
13 checks passed
@ihrpr ihrpr deleted the ihrpr/raw-request-propagation branch May 28, 2025 14:59
@bahattincinic
Copy link

When will you plan to release this PR @ihrpr ?

@RRRajput
Copy link

RRRajput commented Jun 2, 2025

Is there a guide on how this is supposed to be used?

@RRRajput
Copy link

RRRajput commented Jun 2, 2025

Found out how to use it:

from mcp.server.fastmcp import Context

def get_authentication_headers(context: Context) -> dict[str, str]:
    """A function that gives the headers for the request.
    """
    request_object = context.request_context.request
    assert request_object is not None
    assert hasattr(request_object, "headers")
    headers: dict[str, str] = request_object.headers
    return headers

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.

5 participants