-
Notifications
You must be signed in to change notification settings - Fork 535
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
Add sessionID to RequestHandlerExtra type for tool invocations so multi-user services can distinguish users #158
Add sessionID to RequestHandlerExtra type for tool invocations so multi-user services can distinguish users #158
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good idea! Just the one comment. If you're able to add a test for this as well, that would be stellar—but not a blocker.
@@ -41,4 +41,6 @@ export interface Transport { | |||
* Callback for when a message (request or response) is received over the connection. | |||
*/ | |||
onmessage?: (message: JSONRPCMessage) => void; | |||
|
|||
sessionId?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a docstring here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You bet, I'll take a look this afternoon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey sir - added a test and a doc string, let me know if you need anything else!
👍 for this, we also got into the same problem with the same fix |
Very sorry, we've been underwater recently. Looks good—thank you! |
…id-to-context Add sessionID to RequestHandlerExtra type for tool invocations so multi-user services can distinguish users
…id-to-context Add sessionID to RequestHandlerExtra type for tool invocations so multi-user services can distinguish users
Added a sessionID field to the RequestHandlerExtra data that gets passed to tool callbacks, and wired it through from the transport session ID. Exposed the sessionID field on the abstract Transport interface.
Motivation and Context
This allows multi-user SSE-based MCP servers distinguish which user is invoking the tool based on their session ID (which in turn can be mapped to any data derived from the authentication token passed in HTTP headers).
Without this change, tool callbacks being served over SSE can't tell which user is making the request.
How Has This Been Tested?
Made a test service and tested it with Inspector. The tool callback was able to resolve the user ID associated with the requester's API key by using a map from session ID to user ID.
Breaking Changes
No.
Types of changes
Checklist
Additional context
N/A