-
Notifications
You must be signed in to change notification settings - Fork 272
Add stateless Streamable HTTP support #392
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
base: main
Are you sure you want to change the base?
Conversation
- This allows a single MCP session spanning multiple requests to be handled by different servers without sharing state - This does require the servers share data protection keys, but this is standard for ASP.NET Core cookies and antiforgery as well
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.
No changes are necessary on the client?
/// <summary> | ||
/// Gets or sets whether the server should run in a stateless mode that does not require all requests for a given session | ||
/// to arrive to the same ASP.NET Core application process. If true, the /sse endpoint will be disabled, and | ||
/// client capabilities will be round-tripped as part of the mcp-session-id header instead of stored in memory. Defaults to false. | ||
/// </summary> |
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.
/// <summary> | |
/// Gets or sets whether the server should run in a stateless mode that does not require all requests for a given session | |
/// to arrive to the same ASP.NET Core application process. If true, the /sse endpoint will be disabled, and | |
/// client capabilities will be round-tripped as part of the mcp-session-id header instead of stored in memory. Defaults to false. | |
/// </summary> | |
/// <summary> | |
/// Gets or sets whether the server should run in a stateless mode that does not require all requests for a given session | |
/// to arrive to the same ASP.NET Core application process. | |
/// </summary> | |
/// <remarks> | |
/// If <see langword="true"/>, the "/sse" endpoint will be disabled, and | |
/// client capabilities will be round-tripped as part of the "mcp-session-id" header instead of stored in memory. | |
/// Defaults to <see langword="false"/>. | |
/// </remarks> |
// One of the few other usages I found was from some Ethereum JSON-RPC documentation and this | ||
// JSON-RPC library from Microsoft called StreamJsonRpc where it's called JsonRpcErrorCode.NoMarshaledObjectFound | ||
// https://learn.microsoft.com/dotnet/api/streamjsonrpc.protocol.jsonrpcerrorcode?view=streamjsonrpc-2.9#fields | ||
await WriteJsonRpcErrorAsync(context, "Session not found", StatusCodes.Status404NotFound, 32001); |
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.
Did you mean for this to be -32001?
} | ||
else if (!Sessions.TryGetValue(sessionId, out session)) | ||
{ | ||
// -32001 isn't part of the MCP standard, but this is what the typescript-sdk currently does. |
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.
Should we add it to McpErrorCode anyway? Should we work with the MCP spec folks to try to make it official, with a set of code beyond the core JSON RPC ones that are standardized for MCP?
Any ETA on this getting merged and a new package released? Happy to help test, I can replicate the issue this resolves pretty easily. |
We're looking at getting out on NuGet early next week. Hopefully, Monday. I want to resolve some issues like sampling requests currently hanging on the server rather than producing a clear error that sampling is not supported in stateless mode, since there is no guarantee that the sampling response will come back to the same server process like we expect in the default stateful scenario. If you want to test this before then, I welcome you cloning https://github.com/halter73/csharp-sdk/tree/stateless and trying it out yourself. Maybe you can provide feedback to improve it before we ship it on NuGet.
What issue are you referring to specifically? I'm just curious. This resolves a couple issues like running behind a non-sticky load balancer and supporting indefinite sessions that outlive even a server restart. It's a tradeoff since it doesn't support some features like sampling and roots, but that's a tradeoff I think many MCP server developers will be willing to take. |
Sorry, should have specified that in my original response. It's the scenario when running multiple replicas without affinity. Since these MCP servers live a number of layers down in our service stack it's difficult to affinitize requests all the way through. Until I saw your changes, we were looking at hooking in either some sort of intelligent router or a distributed session store (if we could plumb it in somehow). Your changes are a much simpler way to solve it. |
I don't think we have an issue tracking this precisely, but this has been previously discussed in #347 and #330 (comment)