Propagate backend list_changed through vMCP - #5968
Closed
JAORMX wants to merge 1 commit into
Closed
Conversation
vMCP consumed backend progress/message notifications but dropped
*/list_changed, and advertised listChanged:false downstream. Aggregated
tool/resource/prompt views therefore went stale until the short-TTL cache
happened to expire, and no notification was ever pushed to clients.
Consume backend notifications/{tools,resources,prompts}/list_changed on
both the per-call client and a new persistent (idle) listener, then run a
coordinator that invalidates only the affected backend's aggregation-cache
entries and re-projects each affected session's capabilities. The
per-session go-sdk server auto-emits list_changed downstream, and the
advertised listChanged capability is flipped on for tools, resources, and
prompts. Backend changes now converge to clients promptly instead of on
cache expiry.
Notification storms are coalesced with a debounce keyed by backend; the
coordinator does heavy work off the receive loop under a shutdown-tied
context and is untracked on graceful session termination.
Known limitations (tracked as follow-ups, see PR):
- Resources/prompts REMOVAL does not propagate (mcpcompat sync is
add-only); additions do. Needs a toolhive-core change.
- In-process Stop does not eagerly close persistent backend streams, and
the session-tracking map is unbounded for ungracefully-dropped sessions
never revisited by a sweep (no upstream unregister hook exists).
Closes #5748
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MRh394VZgnqbYjJwjrcJWm
JAORMX
requested review from
ChrisJBurns,
amirejaz,
jerm-dro,
jhrozek and
tgrunnagle
as code owners
July 24, 2026 13:09
Collaborator
Author
|
Closing as a duplicate of #5965, which was merged first and already implements the tools This PR overlapped because it was developed in parallel; it is a superset — it additionally propagates resources/prompts No functional loss from closing this — #5965 covers the tools path that closed the issue. |
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.
Summary
vMCP already consumed backend
notifications/progressandnotifications/message, but dropped*/list_changedand advertisedlistChanged: falsetoward clients. As a result, aggregated tool/resource/prompt views went stale until the short-TTL cache happened to expire, and nothing was ever pushed to clients — the core complaint in #5748.This wires the
list_changedpath end-to-end so backend capability changes converge to clients promptly:notifications/{tools,resources,prompts}/list_changedon both the per-call client (pkg/vmcp/client) and a new persistent (idle) listener on the session-backed connector (pkg/vmcp/session/internal/backend), so changes are caught whether or not a call is in flight.pkg/vmcp/server/list_changed.go): on a backend change, invalidate only that backend's aggregation-cache entries (scoped, not whole-cache) and re-project each affected session's capabilities. The per-session go-sdk server then auto-emitslist_changeddownstream, so a subsequenttools/listreflects the new set.listChangedcapability totruefor tools, resources, and prompts (pkg/vmcp/server/serve.go).Closes #5748
Type of change
Test plan
task test)task lint-fix)task lint-fixreports 0 issues;go build/go vetclean.go test -race(Taskfile ldflags) across./pkg/vmcp/...+./pkg/cache/...: 44 packages pass, 0 fail. Coverage added:ListChangedKindForMethod,LateBoundListChangedNotifier, per-call + persistent handler routing (method→sink with backendID), scopedCacheInvalidator.InvalidateBackend(incl. multi-backend entries), coordinator coalescing/kind-routing/terminated-prune/ListTools-error-retains-old-set/start-stop/untrack-on-terminate,setSessionToolsReplaceremoval.list_changed_realbackend_integration_test.go): mid-call mutation, idle mutation (proves the persistent listener), two concurrent sessions (fan-out), resources/prompts additions, capability-regression assertions, and a regression guard pinning the documented resources/prompts removal gap.Does this introduce a user-facing change?
Yes (observability/correctness): with vMCP, backend tool/resource/prompt changes now propagate to connected clients as
list_changednotifications (and updated*/listresults) promptly, instead of only after the aggregation cache's TTL expires. vMCP now advertiseslistChanged: true. Clients that ignore the notifications continue to work via polling/TTL.Special notes for reviewers
Reviewed by a 5-axis Opus panel (security, MCP-correctness, code-quality, observability/concurrency, test-coverage). The concurrency axis initially flagged three MAJOR lifecycle bugs (a 30s client timeout that capped the idle stream, a session-registry leak on normal termination, and a shutdown hang); all were fixed and independently re-verified.
Known limitations — tracked follow-ups (not blockers):
syncSessionResources/syncSessionPromptsare add-only (unlikesyncSessionTools, which fully reconciles). AdvertisinglistChanged: trueis still correct for additions; removal propagation needs a reconciling change instacklok/toolhive-core(mcpcompat) + a version bump. Documented at the coordinator's resource/prompt apply path and pinned by a regression-guard test. Read-time access is not affected —ReadResourcere-derives admission per call, so a removed/deadmitted resource is denied at read time even while it lingers in the advertised list.Stop()does not eagerly close the new persistent backend streams, and the coordinator's session-tracking map is unbounded for ungracefully-dropped sessions never revisited by a sweep. This is bounded to the embed-and-Stop-without-process-exit and client-crash scenarios (the normalservepath reclaims everything at process exit), and closing it eagerly is blocked today by the absence of a public unregister-session hook in mcpcompat/go-sdk (onlyAddOnRegisterSessionexists). A shimAddOnUnregisterSessionin toolhive-core is the clean fix — follow-up.Both follow-ups are in
stacklok/toolhive-core(which we own); I can file them.Size: larger than the usual limit (~33 files; the diff includes substantial docs/tests and generated mocks) — the maintainer approved a single comprehensive PR for this feature.
Implementation plan
Approved implementation plan
Architected in two passes (Opus gap-analysis, then a Fable architect finalized the authoritative single-PR spec). Key design points, all grounded in the current tree:
OnNotificationhandler and progress/message forwarding already existed — this adds thelist_changedarm + a persistent idle listener.BackendListChangedNotifier,ListChangedKind, late-bound holder); W2 per-call handler; W3 persistent listener (WithContinuousListening, no client timeout so the standalone GET stream isn't capped); W4 factory threading; W5 per-backend scoped cache invalidation via a bounded scan (a reverse index would deadlock against the LRU evict path); W6 coordinator (debounce, sessionID→ClientSession registry, fan-out) — enumerating the session cache was rejected (wrong object type; would require forbidden new cache API), so the coordinator keeps its own registry populated at registration and pruned viaSetOnTerminate+ sweep-timeValidate; W7 REPLACE tool setter (so removals propagate); W8 wiring.toolhive-corechange in this PR (tools propagate fully; resources/prompts removal is the documented follow-up).🤖 Generated with Claude Code
https://claude.ai/code/session_01MRh394VZgnqbYjJwjrcJWm