-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Changes from 6 commits
a86f8f4
4ae633c
3b076be
f1f17f7
567875e
81123c6
5413bca
9268cab
149d8cb
0612dcb
a5e7efe
4da517e
991d855
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,8 @@ async def handle_sse(request): | |
from starlette.types import Receive, Scope, Send | ||
|
||
import mcp.types as types | ||
from mcp.shared.message import SessionMessage | ||
from mcp.shared.message import ServerMessageMetadata, SessionMessage | ||
from mcp.types import RequestData | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
@@ -203,7 +204,19 @@ async def handle_post_message( | |
await writer.send(err) | ||
return | ||
|
||
session_message = SessionMessage(message) | ||
# Extract request headers and other context | ||
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), | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should pass the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The latest changes use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 So for now, i'd suggest to go with request.scope as a dict. It's not perfect but:
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The way to do for this is to set a default value on the from typing_extensions import TypeVar
T = TypeVar("T", default=...)` I don't consider type changes as breaking changes btw. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, cool, thanks, didn't realise typing_extensions had TypeVar with default |
||
|
||
# Create session message with request context | ||
metadata = ServerMessageMetadata(request_context=request_context) | ||
session_message = SessionMessage(message, metadata=metadata) | ||
logger.debug(f"Sending session message to writer: {session_message}") | ||
response = Response("Accepted", status_code=202) | ||
await response(scope, receive, send) | ||
|
Uh oh!
There was an error while loading. Please reload this page.