-
Notifications
You must be signed in to change notification settings - Fork 186
feat: add streamable http client #136
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
feat: add streamable http client #136
Conversation
ebbd287 to
8fd0c41
Compare
| } | ||
|
|
||
| ContentType.Text.EventStream.contentType -> { | ||
| logger.trace { "Client received SSE stream in POST response. Messages will be handled by the main SSE session." } |
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.
I hope you don't mind me commenting, and correct me if I'm wrong, but I don't think this is correct.
Reading the relevant part of the spec, it seems to me that when the server returns Content-Type: text/event-stream, it's opening a new SSE connection for this request only, separate to the possibly established 'main' one that's kept open for notifications. In fact, the server MUST NOT send responses on this main SSE session unless resuming after a disconnect.
This in turn means you'll need to process the SSE events right here.
| error("StreamableHttpClientTransport already started!") | ||
| } | ||
| logger.debug { "Client transport starting..." } | ||
| startSseSession() |
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.
I also don't think you should start an SSE session here, as this will be called by the Client/Protocol during connect — before initialization is complete. Servers may not care, but this still violates the spec, I think. This wasn't an issue in the SSE transport, as the SSE session is the medium by which the initialization is done, but that's not the case with HTTP transport.
The typescript-sdk handles this in the send method, by checking whether the response is to the initialization request, though I don't find that particularly elegant either.
|
Hello @zdtango , do you plan on adding unit tests? |
|
Based on this pr, I created a new one, with fixes and tests #147 |
|
changes have been added in #147 |
Summary
Added streamable HTTP client support for MCP connections due to SSE deprecation
Motivation and Context
SSE transport is deprecated and many remote MCP servers only support streamable HTTP transport. Currently, the SDK only supports MCP over SSE, making it incompatible with remote MCP servers that have moved to streamable HTTP. This change adds the necessary streamable HTTP client support to maintain compatibility with current MCP server implementations.
How Has This Been Tested?
Tested with multiple remote MCP servers including GitHub, Atlassian, and other MCP implementations that use streamable HTTP transport. Verified successful connection establishment, tool listing, and tool calling.
Breaking Changes
None. This change only adds a new interface alongside the existing SSE transport without modifying existing functionality.
Types of changes
Checklist
Additional context
This addition ensures the SDK can connect to modern MCP servers as the ecosystem transitions away from deprecated SSE transport to streamable HTTP. The new transport method is essential for compatibility with remote MCP servers that no longer support SSE.