Skip to content

mcp: clear a resource subscription when its listen stream ends - #1283

Merged
guglielmo-san merged 4 commits into
modelcontextprotocol:mainfrom
jeremy:resource-subscription-listen-cleanup-behavioronly
Oct 6, 2026
Merged

guglielmo-san merged 4 commits into
modelcontextprotocol:mainfrom
jeremy:resource-subscription-listen-cleanup-behavioronly

Conversation

@jeremy

@jeremy jeremy commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Under SEP-2575 the client treated subscriptions/listen as fire-and-forget. The default sending handler issued the call, returned an empty result immediately, and only watched the listen context so it could send notifications/cancelled. Nothing observed the stream ending, which caused three problems:

  • Resource subscriptions could not be re-established. Subscribe registers resourceSubs[uri] and opens a listen. When that listen ended for any reason other than Unsubscribe (the server tearing it down, the resource being revoked, the connection dropping), the entry stayed. Every later Subscribe for the URI then hit the "already subscribed" guard and did nothing. An entry added by Subscribe on a closing session was never removed either.
  • Middleware saw a placeholder instead of the stream. Sending middleware got the empty result as soon as the request was written, never the stream's real outcome. A middleware that derived a cancellable context (for example a timeout with defer cancel()) cancelled the stream as soon as next returned.
  • Goroutines outlived their streams. Each listen kept a goroutine waiting on its context. That goroutine outlived the stream, and on Unsubscribe or Close it sent notifications/cancelled even for a stream the server had already closed.

This brings the Go SDK in line with the Python and TypeScript SDKs, whose clients await the listen and re-open it on a plain re-subscribe.

Fix

  • defaultSendingMethodHandler now awaits subscriptions/listen like any other call, and callSubscriptionsListen is removed. Cancelling the listen context still sends notifications/cancelled, through call.
  • Connect (for list-changed notifications) and Subscribe (one listen per resource URI) run the listen on a goroutine through subscriptionsListen. The listen therefore goes through the sending middleware chain, and neither method blocks on it.
  • When a resource listen ends while its context has not been cancelled (a normal result, a transport "terminated" error, or any JSON-RPC error), awaitListen clears the resourceSubs entry, so a plain Subscribe re-opens the stream.
  • The map value is now *resourceSub{cancel, gen}, carrying a per-session generation number. A finishing listen therefore only clears the entry it created, never one installed by a racing Unsubscribe→Subscribe.

The SDK does not resubscribe on its own, because a revoked URI would loop forever. Reopening is left to the application calling Subscribe again. The legacy resources/subscribe / resources/unsubscribe path is unchanged.

Behavior changes

There are no exported API changes. The visible differences on 2026-07-28 sessions are:

  • Sending middleware: next for subscriptions/listen now returns when the stream ends or is cancelled, with the stream's actual result or error, instead of an empty result right away. Middleware that applies a per-request timeout, or holds a resource while calling next, now does so for the life of the stream.
  • Middleware errors: Connect no longer fails with opening subscriptions/listen when a sending middleware rejects the list-changed listen, and Subscribe no longer returns such an error. A listen rejected by the server was not reported before either. proposal: ClientOptions.ResourceSubscriptionEndedHandler for observing resource-subscription termination #1284 proposes a way to observe that.
  • Cancellation: notifications/cancelled is no longer sent for a listen that has already ended.

Follow-up proposal for the optional ended-handler: #1284.

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.
Comment thread mcp/client.go Outdated
Copilot AI balanced review requested due to automatic review settings October 5, 2026 17:27
…ubscriptionsListen method to unify error handling and cleanup.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@guglielmo-san

Copy link
Copy Markdown
Contributor

@jeremy I added some code on top of your PR to simplify the overall implementation of subscriptions/listen

@guglielmo-san
guglielmo-san merged commit 6737e73 into modelcontextprotocol:main Oct 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants