Skip to content

Fix Streamable HTTP WebClient GET SSE handling #318

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

Merged
merged 1 commit into from
Jun 16, 2025

Conversation

chemicL
Copy link
Member

@chemicL chemicL commented Jun 16, 2025

The WebClient Streamable HTTP implementation was missing the incoming message handling on the SSE channel opened with GET HTTP method. This caused sampling to not work when used against the typescript-sdk based everything server

The specification uses the word SHOULD when talking about requests from the server to the client that they should use the same stream which is used for the related client request (tool call in this case) but apparently the typescript-sdk uses a different SSE connection to send the createMessage request. We should handle that properly.


Also, this PR fixes a problematic DELETE method handling in which the current session was replaced with an uninitialized one first before closing the existing session. That call would then extract the session id to send DELETE for, but it used the uninitialized one instaed of the one being closed. This PR fixes it too.

@chemicL chemicL added this to the 0.11.0 milestone Jun 16, 2025
@chemicL chemicL added the bug Something isn't working label Jun 16, 2025
@@ -424,6 +444,20 @@ void testInitializeWithSamplingCapability() {
});
}

@Test
void testInitializeWithElicitationCapability() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this when synchronizing between mcp/src/test and mcp-test/src/main (the files should have the same contents) but the elicitation was not synchronized properly.

Copy link
Contributor

@tzolov tzolov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tzolov tzolov merged commit 8e9857f into main Jun 16, 2025
1 check passed
@tzolov tzolov deleted the streamable-http-sampling branch June 16, 2025 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants