fix(mcp): bind the request-scoped server so roots survive the MCP v2 upgrade - #180
Merged
Conversation
…upgrade Bumps ModelContextProtocol 1.4.1 -> 2.0.0. The upgrade broke the build on four MCP9005 errors: the Roots feature is deprecated by specification version 2026-07-28 (SEP-2577). Roots stays wire-supported for at least twelve months and its replacement — taking the workspace root as a tool parameter or as server configuration — is a behavioural change, so it is kept and the diagnostic is suppressed narrowly at each call site rather than project-wide. Silencing the diagnostic then surfaced a real regression. From 2026-07-28 there is no initialize handshake; the client restates its capabilities per request in _meta, so ClientCapabilities is null on the root McpServer and is populated only on the request-scoped instance the SDK binds to a tool-method parameter. ProjGraphTools is a singleton and took McpServer as a constructor parameter, so it always held the root instance: every relative path would have failed with "client does not support workspace roots" for any client on the current revision. McpServer moves to the tool methods, where the SDK binds it per request and excludes it from the tool's JSON schema. That revision also drops the session that roots/list_changed invalidates, so caching the roots across requests would serve them stale for the life of the process. The roots are now re-read per request there, and cached only on the initialize-handshake revisions that can invalidate them. "Unsupported" is never cached either, since a request without roots says nothing about the next one. A client that advertises roots but refuses the request degrades to the actionable "provide an absolute path" guidance instead of a generic error. McpToolRootsTests drives real tools/call requests, which is the only way to exercise the request-scoped server; the pre-existing roots tests drive the root McpServer directly and are pinned to 2025-11-25 accordingly. The six tests that reached the resolver through a reflection-seeded cache and a null server now use a live session instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the MCP server/tool integration for the ModelContextProtocol v2 upgrade by ensuring workspace-root resolution uses the request-scoped McpServer (required on protocol revision 2026-07-28, where capabilities are restated per request), while keeping deprecated Roots wire support for down-level clients.
Changes:
- Move
McpServerbinding from a singleton constructor dependency to tool-method parameters so request-scoped capabilities/roots are visible. - Adjust
WorkspaceRootServicecaching/invalidations to avoid stale roots on2026-07-28(re-read per request) while keeping cache +roots/list_changedinvalidation for2025-11-25. - Add/adjust integration + contract tests to exercise real
tools/callrequests and the updated method signatures.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/ProjGraph.Tests.Integration.Mcp/WorkspaceRootServiceTests.cs | Suppresses MCP9005 around roots/list_changed notification to keep down-level Roots behavior under warnings-as-errors. |
| tests/ProjGraph.Tests.Integration.Mcp/McpToolRootsTests.cs | New wire-level tools/call tests to validate request-scoped server roots behavior on 2026-07-28. |
| tests/ProjGraph.Tests.Integration.Mcp/McpRootsTests.cs | Reworks roots tests to use a live in-process MCP session instead of reflection-seeded caches/null server. |
| tests/ProjGraph.Tests.Integration.Mcp/Helpers/McpTestHelper.cs | Updates tool construction to match ProjGraphTools signature (no constructor McpServer). |
| tests/ProjGraph.Tests.Integration.Mcp/Helpers/InProcessMcpSession.cs | Adds client pinning option for down-level handshake vs latest protocol; wraps Roots usage with MCP9005 suppression. |
| tests/ProjGraph.Tests.Contract/McpProjectStatsContractTests.cs | Updates parameter-count expectations for added schema-excluded server parameter. |
| tests/ProjGraph.Tests.Contract/McpProjectGraphContractTests.cs | Updates parameter-count expectations for added schema-excluded server parameter. |
| tests/ProjGraph.Tests.Contract/McpErdContractTests.cs | Updates parameter-count expectations for added schema-excluded server parameter. |
| src/ProjGraph.Mcp/WorkspaceRootService.cs | Implements per-protocol roots caching semantics and narrow MCP9005 suppressions at call sites. |
| src/ProjGraph.Mcp/README.md | Documents Roots deprecation, behavior when no roots are declared, and per-request re-reading on 2026-07-28. |
| src/ProjGraph.Mcp/ProjGraphTools.cs | Accepts request-scoped McpServer on tool methods and routes it through path preparation and caching. |
| Directory.Packages.props | Bumps ModelContextProtocol to 2.0.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+246
to
+250
| try | ||
| { | ||
| await RefreshRootsAsync(server, ct); | ||
| } | ||
| catch (Exception ex) when (ex is not OperationCanceledException) |
Owner
Author
There was a problem hiding this comment.
Good catch — confirmed and fixed in d784fad.
The write was real: _rootPaths was set under _initLock but read back after the lock was released, so two overlapping tool calls could interleave and one would resolve against the other's roots.
Rather than only moving the fetch into a local, roots resolution now returns the list for the current request instead of publishing it. ResolveRootsAsync returns IReadOnlyList<string>? and TryResolveAsync resolves against that value, so the per-request revision touches no shared field at all (not _rootPaths, not _status). The <=2025-11-25 handshake path keeps the cache and its roots/list_changed invalidation, as you suggested. RefreshRootsAsync split into a pure TryFetchRootsAsync, leaving the caller to decide whether the result is cached.
Added CallTool_OnCurrentProtocol_ShouldNotPublishTheRootsToTheSharedCache, which asserts the shared fields are still untouched after a real tools/call. Verified it fails if the fetched roots are published back to the cache.
Addresses Copilot's review on #180. On the per-request revision the roots belong to the request that fetched them, but they were still written to the singleton's _rootPaths under the init lock and read back after the lock was released. Two overlapping tool calls could therefore interleave so that one resolved its path against the other's workspace roots. Roots resolution now returns the list for the current request instead of publishing it: the per-request revision fetches into a local and touches no shared field, while the initialize-handshake revisions keep the cache and its roots/list_changed invalidation. RefreshRootsAsync splits into a pure TryFetchRootsAsync, so the caller decides whether the result is cached. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Supersedes #178 (ModelContextProtocol 1.4.1 → 2.0.0).
Why #178 was red
Four
MCP9005errors — the Roots feature is deprecated by specification version 2026-07-28 (SEP-2577), andTreatWarningsAsErrorspromotes the obsoletion warning. Roots stays wire-supported for at least twelve months (removal needs a separate SEP) and its replacement — taking the workspace root as a tool parameter or as server configuration — is a behavioural change. So it is kept, and the diagnostic is suppressed narrowly at each call site rather than project-wide.The regression that suppression exposed
Silencing the diagnostic turned a build failure into six failing tests, which turned out to be a real defect rather than test noise.
From 2026-07-28 there is no
initializehandshake. The client restates its capabilities per request in_meta, soMcpServer.ClientCapabilitiesis null on the root server and populated only on the request-scoped instance the SDK binds to a tool-method parameter.ProjGraphToolsis a singleton and tookMcpServeras a constructor parameter, so it always held the root instance. Every relative path would have failed with"Client does not support workspace roots"for any client negotiating the current revision — silently, since the error reads like a client limitation.McpServermoves to the tool methods, where the SDK binds it per request and excludes it from the tool's JSON schema (the client-facing contract is unchanged).Caching
That revision also drops the session
roots/list_changedinvalidates, so caching roots across requests would serve them stale for the life of the process. Roots are now re-read per request there, and cached only on the initialize-handshake revisions that can invalidate them.Unsupportedis never cached either — a request without roots says nothing about the next one. A client that advertises roots but refuses the request degrades to the actionable "provide an absolute path" guidance instead of a generic SDK error.Tests
McpToolRootsTestsdrives realtools/callrequests — the only way to exercise the request-scoped server. Verified to fail against the pre-fix behaviour, and the staleness test fails if caching is re-enabled for 2026-07-28.McpServerdirectly, so they are pinned to2025-11-25with the reason recorded.null!server now use a live session, dropping that reflection helper.serverparameter.dotnet buildclean,dotnet formatclean, 1121 tests passing.Also note: #178 branched before #179 and would revert SonarAnalyzer 10.31 → 10.30; this branch is off current
developand keeps 10.31.🤖 Generated with Claude Code