-
Notifications
You must be signed in to change notification settings - Fork 2
feat(mcp): migrate to ModelContextProtocol 2.2.0 #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
207e390
aeeb8df
b7bf5c3
d4ecc96
69898e5
48bf7fd
9c1b8ce
ddce4d4
01b2403
3f98670
84d73d7
3190574
c6b4cae
4c9fd42
31953dc
f561022
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,35 @@ | ||
| using ModelContextProtocol.Protocol; | ||
| using ModelContextProtocol.Server; | ||
|
|
||
| // Deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005); kept for existing hosts. | ||
| // Rationale and successor: docs/mcp-reference.md#sdk-and-protocol-versions (#51). | ||
| #pragma warning disable MCP9005 | ||
|
|
||
| namespace Repl.Mcp; | ||
|
|
||
| // One instance PER SESSION (owned by McpSessionContext): hard roots, soft roots, and | ||
| // their cache/version state are session state — one handler can serve several sessions, | ||
| // and serving session A's roots to session B would expose A's workspace URIs and build | ||
| // B's root-dependent snapshot from the wrong workspace. Outbound transport still goes | ||
| // through the request-bound accessor (the destination is per request, finer than the | ||
| // session). | ||
| internal sealed class McpClientRootsService : IMcpClientRoots | ||
| { | ||
| private readonly ICoreReplApp _app; | ||
| private readonly McpRequestServerAccessor _servers; | ||
| private readonly Lock _syncRoot = new(); | ||
| private McpServer? _server; | ||
| private McpClientRoot[] _hardRoots = []; | ||
| private McpClientRoot[] _softRoots = []; | ||
| private bool _hardRootsLoaded; | ||
| private long _hardRootsVersion; | ||
|
|
||
| public McpClientRootsService(ICoreReplApp app) | ||
| public McpClientRootsService(ICoreReplApp app, McpRequestServerAccessor servers) | ||
| { | ||
| _app = app; | ||
| _servers = servers; | ||
| } | ||
|
|
||
| public bool IsSupported => _server?.ClientCapabilities?.Roots is not null; | ||
| public bool IsSupported => _servers.Effective?.ClientCapabilities?.Roots is not null; | ||
|
|
||
| public bool HasSoftRoots | ||
| { | ||
|
|
@@ -42,15 +53,11 @@ public IReadOnlyList<McpClientRoot> Current | |
| } | ||
| } | ||
|
|
||
| public void AttachServer(McpServer server) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(server); | ||
| _server = server; | ||
| } | ||
|
|
||
| public async ValueTask<IReadOnlyList<McpClientRoot>> GetAsync(CancellationToken cancellationToken = default) | ||
| { | ||
| var server = _server; | ||
| // Single read: the effective server must not change between the support check and | ||
| // the roots request (a concurrent request re-binding the accessor must not be observed). | ||
| var server = _servers.Effective; | ||
| if (server?.ClientCapabilities?.Roots is null) | ||
| { | ||
| return Current; | ||
|
Comment on lines
+60
to
63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When one Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed independently on
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in the follow-up commit. Hard roots are now session state: cache entries are keyed by the destination server in a ConditionalWeakTable (weak keys — entries die with their session), with a global version stamp handling roots-list-changed invalidation (coarse but correct; the event is rare). Client B now performs its own roots/list round-trip and can no longer observe client A's workspace roots, and the root-dependent snapshot builds from the right workspace. Regression:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Revalidated on exact head |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When one handler serves two clients that lack native Roots support, this single
_softRootsarray is shared by both sessions: after client A callsSetSoftRoots, client B sees A's URI throughCurrentandHasSoftRoots, and its soft-root-dependent modules can be enabled. This exposes one client's workspace context to another and contradicts the documented current-session semantics; store fallback roots per effective server/session just as hard roots are scoped.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed independently on
b094d055. Two sessions without native Roots shared one handler; A calledSetSoftRoots(file:///root-a), then B called a separatesoft_showtool and received"file:///root-a". This directly violates the current-session contract and leaks workspace context. Please key soft roots by the effective destination/session just like hard roots, includingHasSoftRoots,Current,SetSoftRoots, andClearSoftRoots.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revalidated on exact head
ddce4d408466630cacc59d5c67b53cb92f6d8653with the same deterministic two-session probe onRunAsync. After A setsfile:///root-a, B returnsnoneand never observes A's root. The original soft-root leak is fixed on that path; the public static-options path is covered by a separate verified blocker.