mcp: retire only the subscriptions a listen registered

[cgair]

Narrowed after review — see the discussion below. The earlier revision changed the
registry keying; this one changes only the teardown.

The problem

subscriptionsListen registers a session according to what the listen asked for:

if allowed.ToolsListChanged {
    s.toolChangeSubscriptions[req.Session] = requestID
}
if allowed.PromptsListChanged {
    s.promptChangeSubscriptions[req.Session] = requestID
}
if allowed.ResourcesListChanged {
    s.resourceChangeSubscriptions[req.Session] = requestID
}

Its deferred cleanup does not:

delete(s.toolChangeSubscriptions, req.Session)
delete(s.promptChangeSubscriptions, req.Session)
delete(s.resourceChangeSubscriptions, req.Session)

It runs whether or not this listen ever wrote those registries. A session can
hold several listens at once, so the first one to unwind takes the others'
registrations with it, and those streams stay open while receiving nothing.

The scope is the persistent-session transports, stdio and in-memory. Over
streamable HTTP the registries cannot collide: a stateful handler refuses
SEP-2575 requests outright with CodeUnsupportedProtocolVersion, and a
stateless one builds a temporary session per POST and closes it when the
request ends.

Reachable through this SDK's own client, on stdio:

1. Client.Connect opens the auto-listen for list-changed notifications.
2. ClientSession.Subscribe(uri) opens a second listen, carrying only a
   resource subscription.
3. ClientSession.Unsubscribe(uri) retires that second listen.
4. Its cleanup deletes toolChangeSubscriptions[session], which belongs to the
   auto-listen.
5. The auto-listen is still open and never receives tools/list_changed again.

How I pinned it down

A diagnostic run over the three states, reading the registries and
ServerSession.listenIDs directly:

after connect      toolSub=2       promptSub=2       resSub[r1]=false  listenIDs=[{2}]
after Subscribe    toolSub=2       promptSub=2       resSub[r1]=true   listenIDs=[{2} {3}]
after Unsubscribe  toolSub=<nil>   promptSub=<nil>   resSub[r1]=false  listenIDs=[{2}]

Line 2 rules out the second listen having overwritten the entry: toolSub is
still 2, listen 3 never wrote that registry. Line 3 rules out the client
cancelling the wrong stream: listenIDs still holds listen 2, so the
auto-listen is alive on the server, parked on <-ctx.Done(). What changed is
only that its registration is gone, deleted by listen 3's teardown.

The change

Guard each delete with the condition that guarded the registration, so a listen
retires what it registered and nothing else.

Validation

Two tests, next to the existing TestSubscriptionsListen_* ones and reusing
their fixtures. Against 826e653 without the change:

--- FAIL: TestSubscriptionsListen_TeardownKeepsOtherListens (5.01s)
    mcp_test.go:3442: timed out waiting for "tool"
--- PASS: TestSubscriptionsListen_TeardownRetiresOwnRegistration (0.01s)

The second passes before and after on purpose: it asserts that the only listen
on a session does still get retired, so that the guards cannot be tightened
into a registry leak. The eight pre-existing TestSubscriptionsListen_* tests
pass in both runs, and all ten pass with the change.

Run on the same commit, before and after: go test ./... and
go test -race ./... ok with no races, gofmt -l ., go vet ./... and
staticcheck v0.6.1 clean. Also go test ./... on Go 1.25.0 and 1.26.8 to
match the CI matrix, and go generate ./internal/readme ./internal/docs
produces no diff. Environment: Intel Core i5-1038NG7, macOS 14.

Known limitations

Two cases are outside this change, both needing two listens on one session that
overlap:

• Two listens opting in to the same notification type. allowed.ToolsListChanged
  is true for both, so the registration overwrites and this teardown still
  retires the survivor's entry.
• Two listens subscribed to the same URI. resourceSubscriptions[uri][session]
  overwrites the same way, and the deferred unsubscribe deletes it.

Neither is reachable from this SDK's client — Subscribe dedups per URI and
there is no exported way to open a second list-changed listen — so both require
a non-Go client on stdio. I have tests for them and am happy to open a separate
issue if you want them tracked.

---

AI assistance was used for reading through the code, the diagnostic and
regression tests, and running the verification above. The root cause analysis
and the design decisions are mine, and I can explain every line without it.

[guglielmo-san]

Hi @cgair, thanks for the diagnostics.
I'd like to push back on the scope of the fix, because I think the map[*ServerSession] keying is actually fine and only the teardown is wrong.
subscriptions/listen is new-protocol only, and on the streamable HTTP transport the new protocol is served only in stateless mode, this means a new ServerSession is created on every call and the registries can't collide there.
The collision only exists on the persistent-session transports (stdio / in-memory). But a second listen for the same type delivers nothing new.
The resource path is already correct. subscriptionsListen defers unsubscribe per URI, and unsubscribe deletes only resourceSubscriptions[uri][session]. That teardown is already scoped.

So the only bug is the change-registry defer — it deletes all three maps unconditionally, including ones this listen never wrote:

defer func() {
    s.mu.Lock()
    delete(s.toolChangeSubscriptions, req.Session)
    delete(s.promptChangeSubscriptions, req.Session)
    delete(s.resourceChangeSubscriptions, req.Session)
    s.mu.Unlock()
}()

Scoping it to what this listen actually registered, mirroring the registration block right above it:

defer func() {
    s.mu.Lock()
    if allowed.ToolsListChanged {
        delete(s.toolChangeSubscriptions, req.Session)
    }
    if allowed.PromptsListChanged {
        delete(s.promptChangeSubscriptions, req.Session)
    }
    if allowed.ResourcesListChanged {
        delete(s.resourceChangeSubscriptions, req.Session)
    }
    s.mu.Unlock()
}()

[cgair]

Thanks — the transport point is right and I had not established it before claiming a general impact. I checked it: a stateful StreamableHTTPHandler refuses SEP-2575 requests outright with CodeUnsupportedProtocolVersion ("this server is stateful; set StreamableHTTPOptions.Stateless = true to accept it"), and serveStateless builds a temporary session per POST and closes it when the request ends. So the registries cannot collide over streamable HTTP either way, and the surface is stdio and in-memory only. The PR says that now instead of implying it is general.

On the size of the change I'll follow you. Pushed, in the form you wrote — the three guards mirroring the registration block, nothing else. mcp/server.go is +12/-3.

One correction first, because it is why I did not stop at the teardown the first time. I applied exactly your patch and ran it against the three tests the earlier revision carried:

    mcp_test.go:3447: tool notification IDs = [3], want one per listen [2 3]
--- FAIL: TestSubscriptionsListen_PerListenRegistration (2.01s)
--- PASS: TestSubscriptionsListen_TeardownKeepsOtherListens (0.02s)
    mcp_test.go:3554: timed out waiting for "updated"
--- FAIL: TestSubscriptionsListen_ResourceSubscriptionPerListen (5.01s)
--- PASS: TestSubscriptionsListen_TeardownRetiresOwnRegistration (0.01s)

The resource teardown is scoped per URI but not per listen. Two listens on one session subscribed to the same URI overwrite resourceSubscriptions[uri][session], so when the second unwinds its deferred unsubscribe deletes the entry and the first goes deaf while still open. The change registries behave the same way once two listens want the same type: allowed.ToolsListChanged is true for both, so the guarded delete still takes the survivor's registration.

Neither is reachable from this SDK's client — Subscribe dedups per URI and there is no exported way to open a second list-changed listen — so your read that a second same-type listen delivers nothing new holds for a Go client. Both are reachable from a non-Go client on stdio, which is the deployment I had in mind, but I take the point that it is a separate question from the defect this PR set out to fix. It is in Known limitations now; say the word and I'll open an issue with those two tests rather than carrying them here.

What's left in the PR: your three guards, TeardownKeepsOtherListens as the regression (red on 826e653, green with the change), and TeardownRetiresOwnRegistration as a guard that the guards themselves cannot be tightened into a leak — it passes before and after. Rebased onto 826e653. go test ./... and go test -race ./... green, gofmt/go vet/staticcheck v0.6.1 clean, both Go 1.25.0 and 1.26.8, and go generate ./internal/readme ./internal/docs produces no diff.

[guglielmo-san]

Thank you for the contribution!