Conversation
…rver registerSessionResource kept its re-registration bookkeeping in a module-level Map keyed by URI only. Every session gets its own McpServer from createServer(), so that map is shared: when one session registered a URI another session had already registered, existingResource.remove() unregistered the first session's resource from its own server, and that session's resources/read then failed with -32602 while the resource also disappeared from its resources/list. Session resources are documented as per-session (docs/features.md, "available only for the lifetime of the session"; docs/how-it-works.md, "served from memory for the life of the session"), and the sibling subscriptions.ts already keys its state by sessionId. A module-level map is the odd one out. Scope the registry to the McpServer that owns the resource with a WeakMap<McpServer, Map<string, RegisteredResource>>, so remove()/set() only ever touch resources registered on that same server instance. A WeakMap also lets a closed session's registry be collected. Re-registration within one session still replaces that session's own previous registration, which is what 3e1be88 added the map for.
The previous commit pushed working-tree bytes, which carried CRLF because of a local core.autocrlf setting. GitHub therefore rendered both files as fully rewritten instead of the actual change. Content is identical apart from line endings.
Author
|
Note: the first commit pushed working-tree bytes that carried CRLF line endings (a local |
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.
Description
registerSessionResourcekeeps its "already registered" bookkeeping in a module-levelMapkeyed by URI only. Every session gets its ownMcpServerfromcreateServer(), so that map is shared across sessions: when session B registers a URI session A already registered, the existing entry'sremove()is called and A's resource is unregistered from A's server. A's laterresources/readthen fails with-32602and the resource also disappears from A'sresources/list.The registry is now scoped to the
McpServerthat owns the resource, via aWeakMap<McpServer, Map<string, RegisteredResource>>, so removal only ever touches resources registered on that same server instance. AWeakMapalso lets a closed session's registry be collected.Server Details
Motivation and Context
Session resources are documented as per-session, and the server is built per session:
src/everything/docs/features.md:44— "Session Scoped:demo://resource/session/<name>(per-session resources registered dynamically; available only for the lifetime of the session)"src/everything/docs/how-it-works.md:36— "The content is served from memory for the life of the session only."src/everything/server/index.ts:35—createServer()returns a fresh server per sessionsrc/everything/resources/subscriptions.ts— the sibling module already keys its state bysessionIdThe module-level map came from
3e1be88("fix(everything): allow re-registration of session resources", 2026-01-26), whose purpose was re-registration within a session so that a retriedgzip-file-as-resourcecall does not fail with "Resource already registered". Cross-session eviction is an unintended side effect of that fix, not a product decision — no doc scopes session resource names globally.How Has This Been Tested?
Two real
Clients against two realMcpServers overInMemoryTransport, calling the realgzip-file-as-resourcetool. No network, no API keys. Session C registers a different name as a control.Before:
After:
Unit tests added to
__tests__/resources.test.ts, run with the repo's vitest setup:One test covers the cross-session eviction; the other asserts that re-registration within one session still replaces that session's own previous registration, so
3e1be88's behaviour is pinned rather than regressed. That second test passes before and after.Breaking Changes
No. The change only narrows which previously-registered resources a re-registration may remove; nothing observable changes for a single session.
Types of changes
Checklist
Four boxes above are deliberately left unticked rather than ticked by default:
InMemoryTransportdriving the real tool, plus the unit tests above. If an LLM-client pass is required before review, tell me the client/preferred way and I will run it.Additional context
The registry is a
WeakMapkeyed by server instance, so no explicit cleanup is needed when a session ends and its server is collected.Scope of this PR is the collision between sessions only. A separate, pre-existing observation, not addressed here: the sync/streamable-HTTP path is unchanged, and nothing scopes session resource names within one server if the same URI is registered through different tools.