-
Notifications
You must be signed in to change notification settings - Fork 57
Add stateless
feature to Streamable HTTP protocol
#101
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
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. |
- 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. |
Co-authored-by: Koichi ITO <koic.ito@gmail.com>
Co-authored-by: Koichi ITO <koic.ito@gmail.com>
|
||
response = stateless_transport.handle_request(request) | ||
assert_equal 202, response[0] | ||
assert_equal({ "Content-Type" => "application/json" }, response[1]) |
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.
It seems that this assertion is failing due to the changes in #105. Can you rebase with the latest main branch and update this assertion as follows?
assert_equal({ "Content-Type" => "application/json" }, response[1]) | |
assert_empty(response[1]) |
@bigH This looks good overall to me. Can you rebase on the latest main branch, make sure the tests pass, and squash the commits into a single one? |
I tested this change on an internal MCP server and it looks good 👍 |
On further testing I may have found a bug with this feature, but only when specifically using socketry/falcon (vs Puma).
Digging into it, it looks like
|
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
stateless
configuration behaves according to spec. In situations where the spec is unclear about something, we defer to the Python SDK.