Skip to content

notify every listen held open on a session - #1490

Merged
guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
jmrplens:jmrp-listen-keeps-every-subscription
Nov 3, 2026
Merged

guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
jmrplens:jmrp-listen-keeps-every-subscription

Conversation

@jmrplens

@jmrplens jmrplens commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

resourceSubscriptions holds one request ID per URI and session, so on a 2026-07-28 session a second subscriptions/listen for a URI the session already holds replaced the first listen's ID with its own. Both listens are acknowledged with the URI, but ResourceUpdated delivered only on the second, and once the second ended the first, still open, received nothing. The three list-changed registries did the same to two listens asking for one list-changed notification.

This is the second shape from #1359, keeping the keying by session that #1275 kept:

  • Each of the four registries holds the ID of every open listen of the session (per URI for resourceSubscriptions), through addListen and removeListen, and notifySubscribedSessions delivers on each with its own subscriptionId.
  • A listen holds its ID once even if it names the URI twice; its teardown removes only that ID, and the session's entry with the last one.
  • resources/subscribe and resources/unsubscribe keep one ID per URI and session, as before.
  • SubscribeHandler and UnsubscribeHandler are still called once for each listen, as mcp: retire a listen's subscriptions only while they are its own #1360 left them, so a handler that counts them still holds the session subscribed while one listen remains.

There are no exported API changes.

TestSubscriptionsListen_EveryListenOnAURIIsNotified holds two listens open on one URI and checks, as it ends them one at a time (later first in one subtest, earlier in the other), which IDs each update reaches, the handler counts, and that the session ends unsubscribed. On main at ae26252 both subtests fail with both listens open: update delivered under subscription IDs [3], want [2 3]. TestSubscriptionsListen_EveryListenIsNotifiedOfListChanges does the same for a tools, a prompts and a resources list change, and fails the same way for each on main. TestSubscriptionsListen_URINamedTwiceIsNotifiedOnce and TestServerSubscribe_LegacyResubscribeReplacesID keep what main did for a listen naming its URI twice and for a second resources/subscribe, and each fails if its path always appends.

gofmt -l, go vet ./... and staticcheck v0.6.1 are clean, go test -race ./... passes on Go 1.26.10 and go test ./... on Go 1.25.14 and 1.26.10, and the listen and subscribe tests pass 20 runs in a row under -race.

Three existing tests are flaky under load, on main as well as with this change: in batches of 50 on Go 1.25.14 TestStreamableStateful_DiscoverDoesNotLeakSession failed 0 to 13 times on main and 0 to 8 with the change, and in batches of 160 under -race TestResourceSubscriptions_InMemory and TestResourceSubscriptions_MultipleURIs failed 1 to 25 times on main and 1 to 29 with the change. Those two call ResourceUpdated as soon as the SubscribeHandler runs, before the listen is registered.

Fixes #1489

Comment thread mcp/server.go Outdated
Comment on lines +80 to +83
toolChangeSubscriptions map[*ServerSession]jsonrpc.ID // session -> requestID for "tools/changed"
promptChangeSubscriptions map[*ServerSession]jsonrpc.ID // session -> requestID for "prompts/changed"
resourceChangeSubscriptions map[*ServerSession]jsonrpc.ID // session -> requestID for "resources/changed"
resourceSubscriptions map[string]map[*ServerSession][]jsonrpc.ID // uri -> session -> requestID of each subscription

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what do you think about adding the same change to toolChangeSubscriptions, promptChangeSubscriptions and resourceChangeSubscriptions ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 655fc9f: the three list-changed registries now hold the ID of every open listen per session, as resourceSubscriptions does, through the same addListen and removeListen, and notifySubscribedSessions delivers on each of them. TestSubscriptionsListen_EveryListenIsNotifiedOfListChanges fails for each of the three on main and on the first commit, and passes with this one. I updated the description to match.

resourceSubscriptions holds one request ID per URI and session, so a
second subscriptions/listen for a URI the session already holds
replaced the first listen's ID with its own. Both listens were
acknowledged with the URI, but ResourceUpdated delivered only on the
second, and once the second ended its teardown removed the entry and
the first, still open, received nothing.

The session's entry now holds the ID of every open listen for the URI,
and ResourceUpdated delivers the update on each of them under its own
subscription ID. A listen's teardown removes only its own ID, and the
entry with the last one. resources/subscribe and resources/unsubscribe
keep one ID per URI and session, as before, and the SubscribeHandler
and UnsubscribeHandler are still called once for each listen.

Fixes modelcontextprotocol#1489
toolChangeSubscriptions, promptChangeSubscriptions and
resourceChangeSubscriptions held one request ID per session, so a
second subscriptions/listen asking for the same list-changed
notification replaced the first listen's ID with its own. Only the
second was notified, and once it ended its teardown removed the entry
and the first, still open, received nothing.

Each of them now holds the ID of every open listen, as
resourceSubscriptions does, and notifySubscribedSessions takes the
same map of session to IDs for both kinds of notification, delivering
on each listen under its own subscription ID. addListen, removeListen
and cloneListens keep the four registries in step.

Updates modelcontextprotocol#1489
@jmrplens
jmrplens force-pushed the jmrp-listen-keeps-every-subscription branch from 655fc9f to 5bf666d Compare November 3, 2026 11:41
@guglielmo-san guglielmo-san changed the title mcp: notify every listen held open on a resource URI notify every listen held open on a session Nov 3, 2026
@guglielmo-san
guglielmo-san merged commit 5f942ea into modelcontextprotocol:main Nov 3, 2026
10 checks passed
Tangerg added a commit to Tangerg/go-sdk that referenced this pull request Nov 5, 2026
Sync 109 upstream commits through f91620f, including the same-origin
redirect restriction (modelcontextprotocol#1440), linear-time WWW-Authenticate parsing
(modelcontextprotocol#1456), OIDC issuer validation (modelcontextprotocol#1442), per-listen notification (modelcontextprotocol#1490)
and caller-params preservation (modelcontextprotocol#1473).

The fork keeps its own module path, exact encoded-JSON Meta, strict
ResourceContents encoding and the transport cancellation and frame budget
fixes. Upstream tests that used map-based Meta read it through
decodeMetaValue. The protocol number precision suite from the retired
preserve-protocol-json-numbers branch now guards the exact-number
contract against future syncs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

mcp: a second subscriptions/listen for the same URI silences the first

2 participants