-
Notifications
You must be signed in to change notification settings - Fork 279
Add options to keep the state of the existing system #347
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
Comments
At first, I thought you might be discussing a way to serialize the MCP server's session state so it could then be encrypted and later restored for use in a "stateless" Streamable HTTP server implementation as I described in the comments of my PR to add stateful server-side Streamable HTTP support
I'm not sure what #334 has to do with state though.
I provided a WithOrder()-based workaround if you really want to map the SSE endpoint to "/sse" while using "/" for something else. app.MapMcp("/mcp");
app.MapMcp("/").WithOrder(1); // The higher the order, the lower the priority.
app.MapGet("/", () => "Hello World!"); // Preferred with default order 0. This could be app.MapRazorPages(); instead.
I agree WithOrder() isn't the cleanest thing, but I don't want to add extra public API that might soon be deleted when we drop SSE support for Streamable HTTP only. Contrary to your assertion, I think adding these parameters or options now will cause more compatibility issues down the road. If we drop SSE, MapMcp would then only bind to a single route, since the Streamable HTTP transport has you bind your POST, GET and DELETE handlers all to the same route. // This would work in a non-SSE future.
// We just delete the line with WithOrder(1) above, but we don't even have to do that.
app.MapMcp("/mcp");
app.MapGet("/", () => "Hello World!"); // This could be app.MapRazorPages(); instead. |
What I mean the compatibility problem is not for the code, it should be discovered during compilation while missing such as WithOrder() in the future. The compatibility problem is for the default behavior, as an SDK, it is best not to change any system-level configuration easily; otherwise, it is not a good thing for developers and system reliability. Of course, in my current practice, the |
@halter73 @JeffreySu Will the PR #392 basically allow one to run the MCP server instance with replicas. For example, if I have 3 replicas for my MCP docker container and my MCP Client (https://cline.bot) starts an SSE session, the SSE session will be able to be maintained across the replicas. I ran into this yesterday using 0.1.0-preview.7 and it was erroring out...eliminating the replicas solved the session issue. |
Yes, but the client will have to support the new "Streamable HTTP" transport to connect to a server in stateless mode. Enabling the stateless mode disables the long-running SSE endpoint, since by enabling stateless mode, you're indicating it's impossible to guarantee that the /message requests will arrive at the same endpoint /sse requst. Streamable HTTP resolves this issue because JSON-RPC responses (and notifications like progress notifications) are sent directly in the response body of what used to be the /message request, but it does require the client supports it. As you can see, the MCP SDKs are still early in the process of supporting Streamable HTTP, and clients like Cline are generally even slower to get support since they often (but not always) rely on the SDKs. From the looks of it, Cline doesn't support Streamable HTTP yet](cline/cline#2917), so I doubt this will help you yet, but I think it's likely they'll add support sooner or later. |
Amazing response. Very informative. I am very happy with the current state of things but this is great background to know when I might need to scale As always. You folks and the community rock 🤘✨ |
As we discussed in #334, I think as an SDK, during the process of minimal integration, it should not disrupt the default behavior of the original system (unless this SDK is so special that it must do so), for example, the current setting will overwrite the home page (/) of the existing system. I think that more flexible options should be provided in MapMcp() or related methods, whether it is one parameter or an option object, the final effect will be the same.
If the SDK does not handle these issues in the early stage, more developers and projects may face more compatibility adjustments in the later stage.
The text was updated successfully, but these errors were encountered: