Conversation
Under SEP-2575, ClientSession.Subscribe opened a subscriptions/listen stream per resource URI but never observed its completion: the transport call was fire-and-forget, so when the stream ended for any reason other than a client Unsubscribe (server teardown, resource revocation, a dropped connection) the resourceSubs entry was left behind. A later Subscribe for the same URI then saw the stale entry and returned as a no-op, so the subscription could never be re-established. This diverged from the python and typescript SDKs, whose clients await the listen and re-open on a bare re-subscribe. Await the listen call on its own goroutine (Subscribe stays non-blocking) and, when it completes while its listen context has not been client-cancelled, clear the resourceSubs entry so a bare re-Subscribe re-opens the stream. The map value becomes a small *resourceSub carrying a per-session generation, so a listen goroutine only clears the entry it created and never one a racing Unsubscribe->Subscribe (or a re-subscribe from a completing listen) has since installed; context.CancelFunc values are not comparable, so the previous map type could not express this guard. The SDK does not auto-resubscribe: a permanently revoked URI would hot-loop, so reopening is left to the application. The legacy (pre-2575) resources/subscribe path and the wire protocol are unchanged.
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.
Problem
Under SEP-2575,
ClientSession.Subscribeopens asubscriptions/listenstream per resource URI but never observes that stream ending. When it ends for any reason other than a clientUnsubscribe— the server tearing it down, the resource being revoked, or the connection dropping — the internalresourceSubs[uri]entry is left behind. A laterSubscribefor the same URI then hits the "already subscribed" guard and returns as a no-op, so the subscription can never be re-established.This brings the Go SDK in line with the python and typescript SDKs, whose clients already await the listen and re-open on a bare re-subscribe (they surface a three-valued termination and impose no such no-op guard on a repeat subscribe).
Root cause
SubscriberegistersresourceSubs[uri]and opens the listen, but the SEP-2575 dispatch routes that call to the fire-and-forgetcallSubscriptionsListen(mcp/transport.go), which only reacts to client cancellation (ctx.Done) and never notices the call completing. Nothing clearsresourceSubs[uri]when the stream ends on its own, and theif _, existsguard inSubscribethen short-circuits every re-subscribe.Fix
Await the listen on its own goroutine (
Subscribestays non-blocking — it already was, since the transport call was fire-and-forget). When the listen completes while its listen context has not been client-cancelled — a graceful result, a synthetic transport "terminated" error, or any jsonrpc error — clear the entry so a bare re-Subscribere-opens the stream.The map value changes from
context.CancelFuncto a small*resourceSub{cancel, gen}carrying a per-session generation, so a completing goroutine only clears the entry it created, never one a racingUnsubscribe→Subscribe(or a re-subscribe from a completing listen) has since installed.context.CancelFuncvalues aren't comparable, so the previous map type couldn't express this guard.The SDK does not auto-resubscribe (a permanently revoked URI would hot-loop); reopening is left to the application calling
Subscribeagain.Tests
Added to the SEP-2575 client suite:
Subscribere-fires the serverSubscribeHandler;Unsubscribe→Subscribegeneration-guard invariant (deterministic).Green under
-race; fullgo test ./...passes;gofmt/go vetclean;go generate ./...produces no doc diff.Backwards compatibility
No new exported API and no wire-protocol change — this restores the documented contract that a
Subscribeafter a subscription has ended re-opens it. The legacy (pre-2575)resources/subscribe/resources/unsubscribepath is untouched.A follow-up proposal covers an optional exported handler (
ClientOptions.ResourceSubscriptionEndedHandler) for applications that want to observe termination and drive their own retry/backoff, per the repo's exported-API proposal process. This PR intentionally keeps to the behavior fix so it needs no proposal.Follow-up proposal for the optional ended-handler: #1284.