Skip to content

Commit 3b79c8e

Browse files
authored
Merge pull request #533 from koic/make_subscriptions_listen_refusable
Make `subscriptions/listen` refusable for hosts that cannot stream
2 parents 9cd1041 + 3da0c10 commit 3b79c8e

4 files changed

Lines changed: 115 additions & 9 deletions

File tree

docs/_server/subscriptions.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ The first SSE event on the stream is the acknowledgement:
5252
## Transport Support
5353

5454
The stream is served on the Streamable HTTP modern path; stdio answers `-32601`.
55+
A host that cannot hold an SSE response open (such as the [Rails controller pattern](/server/transports/#rails-controller),
56+
which buffers the body) declines the method with `serve_subscriptions_listen: false`, answering `-32601` and dropping
57+
the `listChanged`/`subscribe` flags from discovery.
5558

5659
## Limits and Keepalives
5760

docs/_server/transports.md

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,29 @@ class McpController < ActionController::API
153153
prompts: [MyPrompt],
154154
server_context: { user_id: current_user.id },
155155
)
156-
# Since the `MCP-Session-Id` is not shared across requests, `stateless: true` is set.
157-
transport = MCP::Server::Transports::StreamableHTTPTransport.new(server, stateless: true)
156+
# Since the `MCP-Session-Id` is not shared across requests, `stateless: true` is set,
157+
# and since `render` buffers the whole body, `subscriptions/listen` streams are declined.
158+
transport = MCP::Server::Transports::StreamableHTTPTransport.new(
159+
server,
160+
stateless: true,
161+
serve_subscriptions_listen: false,
162+
)
158163
status, headers, body = transport.handle_request(request)
159164

160165
render(json: body.first, status: status, headers: headers)
161166
end
162167
end
163168
```
164169

170+
{: .important }
171+
> The controller pattern builds a fresh transport per request and `render` buffers the whole body,
172+
> so it cannot serve the open SSE stream that [`subscriptions/listen`](/server/subscriptions/) needs:
173+
> without `serve_subscriptions_listen: false`, that request returns a streaming `Proc` body
174+
> that `body.first` cannot render, and a modern client's notification stream fails with a 500.
175+
> With the flag, the method answers 404 with JSON-RPC `-32601`, and `server/discover` stops
176+
> advertising the `listChanged`/`subscribe` capability flags, so well-behaved clients do not ask.
177+
> To actually serve listen streams, use the [mount approach](#rails-mount).
178+
165179
### Stateless Mode
166180

167181
You can use Stateless Streamable HTTP, where notifications are not supported and all calls are request/response interactions.

lib/mcp/server/transports/streamable_http_transport.rb

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,12 @@ class InvalidJsonError < StandardError; end
129129
# on a `subscriptions/listen` stream; the periodic write frees the stream's slot when the peer
130130
# has gone away. Defaults to `DEFAULT_LISTEN_KEEPALIVE_INTERVAL` (15); pass `nil` to disable
131131
# when an upstream proxy already keeps the stream alive.
132+
# @param serve_subscriptions_listen [Boolean] whether `subscriptions/listen` opens a stream.
133+
# A host that buffers responses and cannot serve an open SSE stream (e.g. the Rails controller pattern,
134+
# which builds a fresh transport per request and renders the body) passes `false`:
135+
# the method then answers 404 with JSON-RPC `-32601` like any unimplemented method,
136+
# and `Server#discover` stops advertising the `listChanged`/`subscribe` capability flags,
137+
# keeping the advertisement and the actual behavior in agreement. Defaults to `true`.
132138
# @param server_to_client_request_timeout [Numeric] seconds a server-to-client request waits for its
133139
# response before the transport stops waiting and raises `MCP::Server::RequestTimeoutError`.
134140
# Defaults to `DEFAULT_SERVER_TO_CLIENT_REQUEST_TIMEOUT` (600); individual calls override it with `timeout:`.
@@ -145,6 +151,7 @@ def initialize(
145151
max_request_bytes: DEFAULT_MAX_REQUEST_BYTES,
146152
max_listen_subscriptions: DEFAULT_MAX_LISTEN_SUBSCRIPTIONS,
147153
listen_keepalive_interval: DEFAULT_LISTEN_KEEPALIVE_INTERVAL,
154+
serve_subscriptions_listen: true,
148155
server_to_client_request_timeout: DEFAULT_SERVER_TO_CLIENT_REQUEST_TIMEOUT
149156
)
150157
super(server)
@@ -211,6 +218,7 @@ def initialize(
211218
end
212219

213220
@listen_keepalive_interval = listen_keepalive_interval
221+
@serve_subscriptions_listen = serve_subscriptions_listen
214222

215223
unless server_to_client_request_timeout.is_a?(Numeric) && server_to_client_request_timeout.positive?
216224
raise ArgumentError, "server_to_client_request_timeout must be a positive number"
@@ -260,10 +268,12 @@ def call(env)
260268
handle_request(Rack::Request.new(env))
261269
end
262270

263-
# The `subscriptions/listen` notification stream (SEP-2575) is served on the modern path,
264-
# so `Server#discover` may advertise `listChanged`/`subscribe` capability flags.
271+
# Whether this transport serves the `subscriptions/listen` notification stream (SEP-2575).
272+
# Gates both the route (a refusing transport answers the method as unimplemented) and
273+
# the `listChanged`/`subscribe` capability flags `Server#discover` advertises,
274+
# so the two always agree. Set via the `serve_subscriptions_listen:` constructor keyword.
265275
def serves_subscriptions_listen?
266-
true
276+
@serve_subscriptions_listen
267277
end
268278

269279
def handle_request(request)
@@ -723,8 +733,13 @@ def handle_modern(request, header_version, body_string: nil)
723733
return mismatch_error if mismatch_error
724734

725735
# `subscriptions/listen` is a long-lived notification stream served at the transport layer;
726-
# it never dispatches through `Server#handle`.
727-
return handle_subscriptions_listen(body) if body[:method] == Methods::SUBSCRIPTIONS_LISTEN
736+
# it never dispatches through `Server#handle`. A transport constructed with
737+
# `serve_subscriptions_listen: false` skips the interception, so the method falls through
738+
# to the dispatcher as unimplemented (404 with `-32601`) - the refusal a host that cannot
739+
# serve an open SSE stream needs, instead of a `Proc` body it can never call.
740+
if body[:method] == Methods::SUBSCRIPTIONS_LISTEN && serves_subscriptions_listen?
741+
return handle_subscriptions_listen(body)
742+
end
728743

729744
session = modern_session
730745
notifications = @mutex.synchronize { @modern_request_sinks[session.session_id] = [] }
@@ -881,10 +896,34 @@ def too_many_listen_subscriptions_response(request_id)
881896
)
882897
end
883898

884-
# The proc registers the stream and returns, leaving the response open like
899+
# The Rack streaming body of a `subscriptions/listen` response. It responds to `call`
900+
# and deliberately not to `each`, so Rack keeps classifying it as a streaming body;
901+
# `first` exists only to turn the buffered-host mistake (e.g. `render(json: body.first)`
902+
# in the Rails controller pattern) from a bare `NoMethodError` into guidance naming the fix.
903+
class ListenStreamBody
904+
def initialize(&block)
905+
@block = block
906+
end
907+
908+
def call(stream)
909+
@block.call(stream)
910+
end
911+
912+
def first
913+
raise <<~MESSAGE
914+
subscriptions/listen returned a streaming SSE body, which cannot be buffered into a JSON response. \
915+
A host that cannot hold an SSE response open should construct the transport with `serve_subscriptions_listen: false`, \
916+
so the method is answered as unimplemented instead.
917+
See the Rails (controller) section at https://ruby.sdk.modelcontextprotocol.io/server/transports/ for the hosting patterns.
918+
MESSAGE
919+
end
920+
end
921+
private_constant :ListenStreamBody
922+
923+
# The body registers the stream and returns, leaving the response open like
885924
# the legacy GET stream (`create_sse_body`).
886925
def listen_sse_body(request_id, honored)
887-
proc do |stream|
926+
ListenStreamBody.new do |stream|
888927
rejected = false
889928
@mutex.synchronize do
890929
if @listen_subscriptions.key?(request_id) ||

test/mcp/server/transports/streamable_http_transport_test.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6156,6 +6156,56 @@ def string
61566156
transport.close
61576157
end
61586158

6159+
test "a buffered read of the listen stream body raises guidance instead of NoMethodError" do
6160+
response = @transport.handle_request(modern_rack_request(
6161+
modern_listen_body(id: "listen-1", params: { notifications: { toolsListChanged: true } }),
6162+
))
6163+
6164+
error = assert_raises(RuntimeError) { response[2].first }
6165+
assert_includes error.message, "serve_subscriptions_listen: false"
6166+
6167+
# The cited page is a published-URL contract, like the kwarg name above.
6168+
assert_includes error.message, "https://ruby.sdk.modelcontextprotocol.io/server/transports/"
6169+
6170+
# Responding to `each` would make Rack classify the body as enumerable and break streaming.
6171+
refute_respond_to response[2], :each
6172+
end
6173+
6174+
test "subscriptions/listen is refused as unimplemented when the transport does not serve it" do
6175+
transport = StreamableHTTPTransport.new(@server, serve_subscriptions_listen: false)
6176+
6177+
status, headers, body = transport.handle_request(modern_rack_request(
6178+
modern_listen_body(id: "listen-1", params: { notifications: { toolsListChanged: true } }),
6179+
))
6180+
6181+
assert_equal 404, status
6182+
assert_equal "application/json", headers["content-type"]
6183+
6184+
# The documented Rails controller pattern renders `body.first`, so the refusal must be an Array body,
6185+
# never the streaming Proc.
6186+
assert_kind_of Array, body
6187+
assert_equal(-32601, JSON.parse(body.first).dig("error", "code"))
6188+
ensure
6189+
transport.close
6190+
end
6191+
6192+
test "discover stops advertising subscription capabilities when listen is not served" do
6193+
server = Server.new(
6194+
name: "listen_test",
6195+
capabilities: { tools: { listChanged: true }, resources: { listChanged: true, subscribe: true } },
6196+
)
6197+
transport = StreamableHTTPTransport.new(server, serve_subscriptions_listen: false)
6198+
6199+
response = transport.handle_request(modern_rack_request(modern_body("server/discover", {})))
6200+
capabilities = JSON.parse(response[2].first).dig("result", "capabilities")
6201+
6202+
assert_nil capabilities.dig("tools", "listChanged")
6203+
assert_nil capabilities.dig("resources", "listChanged")
6204+
assert_nil capabilities.dig("resources", "subscribe")
6205+
ensure
6206+
transport.close
6207+
end
6208+
61596209
test "subscriptions/listen past the concurrent stream cap is rejected with 503" do
61606210
transport = StreamableHTTPTransport.new(@server, max_listen_subscriptions: 1)
61616211
open_listen_stream(id: "listen-1", notifications: { toolsListChanged: true }, transport: transport)

0 commit comments

Comments
 (0)