- 
                Notifications
    You must be signed in to change notification settings 
- Fork 77
          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. | 
89dcd20    to
    0ecdd98      
    Compare
  
    | Bump @koic. I think this is good to review/merge. | 
|  | ||
| 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          def handle_regular_request(body_string, session_id)
          unless @stateless
            # If session ID is provided, but not in the sessions hash, return an error
            if session_id && !@sessions.key?(session_id)
              return [400, { "Content-Type" => "application/json" }, [{ error: "Invalid session ID" }.to_json]]
            end
          end
          # Ensure we always have a valid response body - convert nil to empty string
          response = @server.handle_json(body_string) || "" | 
| Can you add user-facing documentation to the README? | 
| @bigH any chance you have time to revisit this? | 
| @bigH ping. | 
| We would love to see this merged! | 
| I'm pretty busy, but will take a look at finalizing this during the weekend. | 
- Create failing tests for `stateless` mode - Implement `stateless` mode by turning of SSE in Streamable HTTP, making the interaction a standard HTTP Req/Resp
Co-authored-by: Koichi ITO <koic.ito@gmail.com>
Co-authored-by: Koichi ITO <koic.ito@gmail.com>
| @bigH Can you add documentation for this feature to the README.md and finally squash commits into one? | 
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.