Add stateless feature to Streamable HTTP protocol
#101
+169
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Does |
My understanding is that it does not. I've tested this against my own service and it works properly. What would it take to get this merged? |
|
Ah, I see. Thank you for the explanation. Can you squash your commits into one? |
|
Happy to. |
a2c47d2 to
ca3537e
Compare
|
Rebased on to latest main and squashed my commits. |
koic
previously approved these changes
Aug 6, 2025
- Create failing tests for `stateless` mode - Implement `stateless` mode by turning of SSE in Streamable HTTP, making the interaction a standard HTTP Req/Resp
|
Bump @koic. I think this is good to review/merge. |
koic
reviewed
Aug 9, 2025
Comment on lines
+671
to
+677
| assert_raises(RuntimeError, "Stateless mode does not support notifications") do | ||
| stateless_transport.send_notification( | ||
| "test_notification", | ||
| { message: "Hello" }, | ||
| session_id: "some_session_id", | ||
| ) | ||
| end |
There was a problem hiding this comment.
If my understanding is correct, it would be as follows.
Suggested change
| assert_raises(RuntimeError, "Stateless mode does not support notifications") do | |
| stateless_transport.send_notification( | |
| "test_notification", | |
| { message: "Hello" }, | |
| session_id: "some_session_id", | |
| ) | |
| end | |
| e = assert_raises(RuntimeError) do | |
| stateless_transport.send_notification( | |
| "test_notification", | |
| { message: "Hello" }, | |
| session_id: "some_session_id", | |
| ) | |
| end | |
| assert_equal("Stateless mode does not support notifications", e.message) |
koic
reviewed
Aug 9, 2025
| assert_equal({ "Content-Type" => "application/json" }, response[1]) | ||
|
|
||
| body = response[2][0] | ||
| assert body.blank? |
There was a problem hiding this comment.
That's just my two cents.
Suggested change
| assert body.blank? | |
| assert_nil(body) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See: #99
Currently, this implementation of MCP requires that sessions are held in memory for Server-Sent Events (SSEs). The session data maps an ID to a live IO stream. This doesn't work for Chime for a few reasons:
So, here we add tests and validate that
statelessconfiguration behaves according to spec. In situations where the spec is unclear about something, we defer to the Python SDK.