Answer subscriptions/listen, agreeing to nothing, and close it cleanly (#111) - #127
Merged
Conversation
#111) SEP-2575 replaces the HTTP GET endpoint and resources/subscribe with subscriptions/listen: one long-lived stream carrying the change notifications a client opted into. We had it falling through to MethodNotFound. There is genuinely nothing here to subscribe to. The tool list is fixed at start-up - which is what the listChanged:false we advertise says - and the server exposes no resources or prompts. So the honest answer is not an error and not an open stream: acknowledge with an empty `notifications` filter, which is precisely how the spec says a server reports the subset it agreed to honour, and then send the empty result that means graceful closure. A client learns what is supported (nothing) and that the subscription ended cleanly rather than dropping. Holding the stream open instead would promise a message that can never arrive. The acknowledgement MUST be the first message on the subscription, so it goes out on the outbound channel before the response is returned - there is a test on that ordering, since the wrong way round would have a client see a closed subscription before it knew one existed. The subscription id is the request's own JSON-RPC id, whatever its type, carried in _meta on both messages. The HTTP transport has no outbound channel - a POST gets one JSON response - so a caller there receives the closure alone. With no notification types agreed the two messages carry the same information, so nothing is lost. 7 tests, including one asserting no notifications/* is ever sent unasked.
12 tasks
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.
Closes #111. Last of the conformance block.
SEP-2575 replaces the HTTP GET endpoint and
resources/subscribewithsubscriptions/listen: one long-lived stream carrying the change notifications a client opted into. We had it falling through toMethodNotFound.The answer
There is genuinely nothing here to subscribe to — the tool list is fixed at start-up (which is what the
listChanged: falsewe advertise says) and the server exposes no resources or prompts. So the honest answer is neither an error nor an open stream:notificationsfilter. That field is precisely how the spec says a server reports the subset it agreed to honour, so an empty one tells a client "none of what you asked for" in the protocol's own terms.Holding the stream open instead would promise a message that can never arrive.
Details worth noting
_metaon both messages.Verification
notifications/*is ever sent unasked, which is the rule that matters as the server grows.README: "Protocol revisions" covers it, and points at
notifications/progressas the stream that actually matters here — request-scoped, so it rides its own request.