Handle an initialize request sent without an ID#456
Open
koic wants to merge 1 commit into
Open
Conversation
## Motivation and Context `StreamableHTTPTransport#handle_post` routes to `handle_initialization` via `initialize_request?`, which matches on the method name alone. An `initialize` message sent without an id (framed as a notification) therefore reaches `handle_initialization`. `Server#init` runs and marks the session initialized, but JSON-RPC emits no response for an id-less request, so `handle_json` returns nil. Two things then went wrong: - `handle_initialization` returned `[200, headers, [nil]]`. A `nil` body element is not a valid Rack response, so the web server raises while serializing it (`NoMethodError` on `nil.bytesize`) and the client gets a malformed response or a 500. - The `if session_id && !server_session.initialized?` cleanup guard was skipped, because the session is marked initialized. The session was retained with a `Mcp-Session-Id` header, so a stream of id-less `initialize` messages accumulates sessions toward the `max_sessions` cap until the reaper reclaims them. `handle_initialization` now treats a nil response as an id-less initialize: it discards the orphaned session and acks with 202, mirroring how `handle_regular_request` already handles a nil response. A well-formed `initialize` request (with an id) is unaffected. ## How Has This Been Tested? A new test in `test/mcp/server/transports/streamable_http_transport_test.rb` posts an `initialize` with no id and asserts a 202 with an empty body, no `Mcp-Session-Id` header, and an empty session map. It fails against the previous code (a 200 with a nil body element and a retained session) and passes now. ## Breaking Changes None. A conforming client sends `initialize` as a request with an id and is unaffected; only the malformed id-less form changes, from a broken 200 to a 202 with no retained session.
62eb2fa to
fc0c6b8
Compare
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
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.
Motivation and Context
StreamableHTTPTransport#handle_postroutes tohandle_initializationviainitialize_request?, which matches on the method name alone. Aninitializemessage sent without an id (framed as a notification) therefore reacheshandle_initialization.Server#initruns and marks the session initialized, but JSON-RPC emits no response for an id-less request, sohandle_jsonreturns nil.Two things then went wrong:
handle_initializationreturned[200, headers, [nil]]. Anilbody element is not a valid Rack response, so the web server raises while serializing it (NoMethodErroronnil.bytesize) and the client gets a malformed response or a 500.if session_id && !server_session.initialized?cleanup guard was skipped, because the session is marked initialized. The session was retained with aMcp-Session-Idheader, so a stream of id-lessinitializemessages accumulates sessions toward themax_sessionscap until the reaper reclaims them.handle_initializationnow treats a nil response as an id-less initialize: it discards the orphaned session and acks with 202, mirroring howhandle_regular_requestalready handles a nil response. A well-formedinitializerequest (with an id) is unaffected.How Has This Been Tested?
A new test in
test/mcp/server/transports/streamable_http_transport_test.rbposts aninitializewith no id and asserts a 202 with an empty body, noMcp-Session-Idheader, and an empty session map. It fails against the previous code (a 200 with a nil body element and a retained session) and passes now.Breaking Changes
None. A conforming client sends
initializeas a request with an id and is unaffected; only the malformed id-less form changes, from a broken 200 to a 202 with no retained session.Types of changes
Checklist