diff --git a/Directory.Packages.props b/Directory.Packages.props index 67721b5..fc2f2bc 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -17,7 +17,7 @@ - + diff --git a/src/ProjGraph.Mcp/ProjGraphTools.cs b/src/ProjGraph.Mcp/ProjGraphTools.cs index 7f0249f..5a412ef 100644 --- a/src/ProjGraph.Mcp/ProjGraphTools.cs +++ b/src/ProjGraph.Mcp/ProjGraphTools.cs @@ -28,7 +28,6 @@ internal sealed class ProjGraphTools( DiagramRenderers renderers, IFileSystem fileSystem, DiagramResourceCache cache, - McpServer server, WorkspaceRootService rootService, CollectingOutputConsole outputConsole) { @@ -48,6 +47,9 @@ public async Task GetClassDiagramAsync( [Description("Whether to include the title in the diagram (default: true).")] bool showTitle = true, IProgress? progress = null, + // Request-scoped, never the singleton's root instance — see PreparePathAsync. Defaulted so + // tests can call the method directly with an absolute path, which never consults the server. + McpServer server = null!, CancellationToken cancellationToken = default) { if (options is { MaxDepth: < 0 }) @@ -56,7 +58,7 @@ public async Task GetClassDiagramAsync( throw new McpException($"maxDepth must not be negative; got {options.MaxDepth}."); } - path = await PreparePathAsync(path, cancellationToken); + path = await PreparePathAsync(path, server, cancellationToken); if (!fileSystem.FileExists(path) && !fileSystem.DirectoryExists(path)) throw new McpException($"Path not found: {path}"); @@ -133,9 +135,12 @@ public async Task GetProjectGraphAsync( [Description("Whether to include NuGet package dependencies in the graph (default: false).")] bool includePackages = false, IProgress? progress = null, + // Request-scoped, never the singleton's root instance — see PreparePathAsync. Defaulted so + // tests can call the method directly with an absolute path, which never consults the server. + McpServer server = null!, CancellationToken cancellationToken = default) { - path = await PreparePathAsync(path, cancellationToken); + path = await PreparePathAsync(path, server, cancellationToken); progress?.Report(new ProgressNotificationValue { @@ -187,6 +192,9 @@ public async Task GetProjectStatsAsync( [Description("Number of top most-referenced projects to include. Defaults to 5.")] int topN = 5, IProgress? progress = null, + // Request-scoped, never the singleton's root instance — see PreparePathAsync. Defaulted so + // tests can call the method directly with an absolute path, which never consults the server. + McpServer server = null!, CancellationToken cancellationToken = default) { if (topN < 1) @@ -194,7 +202,7 @@ public async Task GetProjectStatsAsync( throw new McpException($"topN must be at least 1; got {topN}."); } - path = await PreparePathAsync(path, cancellationToken); + path = await PreparePathAsync(path, server, cancellationToken); progress?.Report(new ProgressNotificationValue { @@ -248,6 +256,9 @@ public async Task GetErdAsync( [Description("How EF Core owned types are shown: 'mirror' (default) inlines table-split owned types onto the owner as EF names them; 'classic' gives every owned type its own entity")] string ownedMode = "mirror", IProgress? progress = null, + // Request-scoped, never the singleton's root instance — see PreparePathAsync. Defaulted so + // tests can call the method directly with an absolute path, which never consults the server. + McpServer server = null!, CancellationToken cancellationToken = default) { if (!ErdOwnedModeParser.TryParse(ownedMode, out var mode)) @@ -260,7 +271,7 @@ public async Task GetErdAsync( throw new McpException($"Invalid ownedMode '{ownedMode}'. Expected 'mirror' or 'classic'."); } - path = await PreparePathAsync(path, cancellationToken); + path = await PreparePathAsync(path, server, cancellationToken); RequireFileExists(path); RequireCsFile(path); @@ -437,7 +448,21 @@ private static async Task RunAnalysisAsync(Func> analysis) } } - private async Task PreparePathAsync(string path, CancellationToken cancellationToken) + /// + /// Validates the requested path and resolves it to an absolute one, using the client's + /// workspace roots when it is relative. + /// + /// The path supplied by the client. + /// + /// The request-scoped bound to the tool-method parameter, never one + /// captured at construction time: from protocol revision 2026-07-28 the client declares its + /// capabilities per request in _meta rather than in an initialize handshake, so + /// is populated only on the request-scoped instance. + /// + /// A token to cancel the operation. + /// The resolved absolute path. + /// Thrown when the path is empty or cannot be resolved. + private async Task PreparePathAsync(string path, McpServer server, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); if (string.IsNullOrWhiteSpace(path)) diff --git a/src/ProjGraph.Mcp/README.md b/src/ProjGraph.Mcp/README.md index 5453fd4..04c1a76 100644 --- a/src/ProjGraph.Mcp/README.md +++ b/src/ProjGraph.Mcp/README.md @@ -75,7 +75,13 @@ Diagram resources are automatically created when tools generate output. Clients ### Roots The server resolves relative file paths against workspace roots declared by the client. When a client declares roots via -`roots/list`, passing `"MyApp.slnx"` instead of `"D:/Projects/MyApp/MyApp.slnx"` just works. +`roots/list`, passing `"MyApp.slnx"` instead of `"D:/Projects/MyApp/MyApp.slnx"` just works. A client that declares no +roots is asked to pass an absolute path instead. + +Roots is deprecated by specification version 2026-07-28 ([SEP-2577](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/seps/2577-deprecate-roots-sampling-and-logging.md)) +and stays supported for at least twelve months. Until it is retired the server keeps serving it on both that revision and +down-level ones; on 2026-07-28 the roots are re-read per request, since that revision drops the session that +`roots/list_changed` would otherwise invalidate. ### Progress Notifications diff --git a/src/ProjGraph.Mcp/WorkspaceRootService.cs b/src/ProjGraph.Mcp/WorkspaceRootService.cs index 696c1ad..47c835c 100644 --- a/src/ProjGraph.Mcp/WorkspaceRootService.cs +++ b/src/ProjGraph.Mcp/WorkspaceRootService.cs @@ -6,6 +6,25 @@ namespace ProjGraph.Mcp; +/// +/// Resolves relative paths supplied to the MCP tools against the client's workspace roots. +/// +/// +/// The Roots feature is deprecated by specification version 2026-07-28 (SEP-2577), which is why the +/// SDK calls below are wrapped in MCP9005 suppressions. It stays wire-supported for at least +/// twelve months — removing it needs a separate SEP — so it is still served here, on both the new +/// revision and down-level ones. A client that does not advertise the capability degrades to +/// and is told to pass an absolute path. Retiring it means +/// taking the workspace root as a tool parameter or as server configuration, which is a behavioural +/// change tracked separately from this SDK upgrade. +/// +/// +/// Every method takes the request-scoped . From 2026-07-28 there is no +/// initialize handshake: the client restates its capabilities per request in _meta, so +/// is null on the root server and populated only on the +/// instance the SDK binds to a tool-method parameter. +/// +/// The file system used to probe candidate paths under each root. internal sealed class WorkspaceRootService(IFileSystem fileSystem) : IAsyncDisposable { private readonly SemaphoreSlim _initLock = new(1, 1); @@ -28,18 +47,20 @@ public async Task TryResolveAsync(string path, McpServer server, Cancell return path; } - await EnsureInitializedAsync(server, ct); + // Resolved into a local: on the per-request revision the roots belong to this request only, + // so an overlapping request must not be able to swap them out from under this one. + var roots = await ResolveRootsAsync(server, ct); // Every failure below throws McpException: the SDK replaces the message of any other // exception type with a generic "An error occurred invoking '…'", so the guidance // (most importantly "provide an absolute path") would never reach the client. - if (_status == RootsStatusKind.Unsupported) + if (roots is null) { throw new McpException( "Client does not support workspace roots. Please provide an absolute path."); } - var matches = ResolveMatches(_rootPaths, path); + var matches = ResolveMatches(roots, path); return matches.Count switch { @@ -143,6 +164,7 @@ private void EnsureRootsChangedHandler(McpServer server) // Only mark as registered after a successful call, so a failed registration can be retried // on the next initialization instead of permanently disabling roots invalidation. +#pragma warning disable MCP9005 // Roots is deprecated (SEP-2577); still served for down-level clients. See the file header. _rootsChangedRegistration = server.RegisterNotificationHandler( NotificationMethods.RootsListChangedNotification, (_, _) => @@ -150,27 +172,93 @@ private void EnsureRootsChangedHandler(McpServer server) InvalidateRoots(); return default; }); +#pragma warning restore MCP9005 _notificationHandlerRegistered = true; } - internal async Task RefreshRootsAsync(McpServer server, CancellationToken ct) + /// + /// Fetches the client's workspace roots over roots/list. + /// + /// The request-scoped server handling the current request. + /// A token to cancel the request. + /// + /// The root directories, or when the client refuses the request. + /// + private static async Task?> TryFetchRootsAsync(McpServer server, CancellationToken ct) { - var result = await server.RequestRootsAsync(new ListRootsRequestParams(), ct); - var paths = new List(); - foreach (var root in result.Roots) + try { - paths.Add(new Uri(root.Uri).LocalPath); +#pragma warning disable MCP9005 // Roots is deprecated (SEP-2577); still served for down-level clients. See the file header. + var result = await server.RequestRootsAsync(new ListRootsRequestParams(), ct); +#pragma warning restore MCP9005 + var paths = new List(); + foreach (var root in result.Roots) + { + paths.Add(new Uri(root.Uri).LocalPath); + } + + return paths; } + catch (Exception ex) when (ex is not OperationCanceledException) + { + // A client can advertise the capability and still refuse the request — most likely once + // it drops the deprecated feature. Reporting it as unsupported gets the caller the + // actionable "provide an absolute path" guidance instead of a generic SDK error. + return null; + } + } - _rootPaths = paths; - _status = RootsStatusKind.Ready; + private static bool HasRootsCapability(McpServer server) + { +#pragma warning disable MCP9005 // Roots is deprecated (SEP-2577); still served for down-level clients. See the file header. + return server.ClientCapabilities?.Roots is not null; +#pragma warning restore MCP9005 } - private async Task EnsureInitializedAsync(McpServer server, CancellationToken ct) + /// + /// Indicates whether the connection established client state once, via the initialize + /// handshake (protocol revision 2025-11-25 and earlier). Only such a connection has a + /// session for roots to be cached against and a durable channel for the client's + /// roots/list_changed notification to invalidate that cache; from 2026-07-28 the + /// client restates its capabilities on every request instead, so the roots are re-fetched each + /// time rather than served stale for the rest of the process's life. + /// + /// The request-scoped server handling the current request. + /// when client state is session-scoped. + private static bool UsesSessionScopedCapabilities(McpServer server) { - if (_status != RootsStatusKind.Unknown) + // Date-based revisions order correctly under an ordinal comparison. + var version = server.NegotiatedProtocolVersion; + return version is not null && string.CompareOrdinal(version, "2026-07-28") < 0; + } + + /// + /// Produces the workspace roots the current request must resolve against. + /// + /// The request-scoped server handling the current request. + /// A token to cancel the request. + /// + /// The root directories, or when the client offers none. + /// + private async Task?> ResolveRootsAsync(McpServer server, CancellationToken ct) + { + if (!UsesSessionScopedCapabilities(server)) { - return; + // The per-request revision keeps nothing: the roots are scoped to this request, so + // publishing them to the shared cache would let an overlapping request resolve against + // the wrong workspace, and there is no session for roots/list_changed to invalidate. + return HasRootsCapability(server) ? await TryFetchRootsAsync(server, ct) : null; + } + + if (_status == RootsStatusKind.Ready) + { + return _rootPaths; + } + + if (!HasRootsCapability(server)) + { + _status = RootsStatusKind.Unsupported; + return null; } await _initLock.WaitAsync(ct); @@ -178,20 +266,24 @@ private async Task EnsureInitializedAsync(McpServer server, CancellationToken ct { // Double-checked locking: re-check after acquiring lock #pragma warning disable CA1508 // Avoid dead conditional code — volatile field may change between outer check and lock acquisition - if (_status != RootsStatusKind.Unknown) + if (_status == RootsStatusKind.Ready) { - return; + return _rootPaths; } #pragma warning restore CA1508 - if (server.ClientCapabilities?.Roots is null) + EnsureRootsChangedHandler(server); + + var paths = await TryFetchRootsAsync(server, ct); + if (paths is null) { _status = RootsStatusKind.Unsupported; - return; + return null; } - EnsureRootsChangedHandler(server); - await RefreshRootsAsync(server, ct); + _rootPaths = paths; + _status = RootsStatusKind.Ready; + return paths; } finally { diff --git a/tests/ProjGraph.Tests.Contract/McpErdContractTests.cs b/tests/ProjGraph.Tests.Contract/McpErdContractTests.cs index 1422826..203e2ca 100644 --- a/tests/ProjGraph.Tests.Contract/McpErdContractTests.cs +++ b/tests/ProjGraph.Tests.Contract/McpErdContractTests.cs @@ -83,9 +83,10 @@ public void GetErd_ShouldHave_Parameters() var method = type.GetMethod("GetErdAsync"); var parameters = method!.GetParameters(); - // Assert parameters exist (path, contextName, showTitle, ownedMode, progress, cancellationToken) - parameters.Should().HaveCount(6, - "GetErd should have 6 parameters: path, contextName, showTitle, ownedMode, progress, and cancellationToken"); + // Assert parameters exist. 'server' is bound by the SDK to the request-scoped McpServer and + // is excluded from the tool's JSON schema, so it does not widen the client-facing contract. + parameters.Should().HaveCount(7, + "GetErd should have 7 parameters: path, contextName, showTitle, ownedMode, progress, server, and cancellationToken"); var pathParam = parameters.Should().ContainSingle(p => p.Name == "path").Which; pathParam.ParameterType.Should().Be(); diff --git a/tests/ProjGraph.Tests.Contract/McpProjectGraphContractTests.cs b/tests/ProjGraph.Tests.Contract/McpProjectGraphContractTests.cs index 692c471..e4d6c90 100644 --- a/tests/ProjGraph.Tests.Contract/McpProjectGraphContractTests.cs +++ b/tests/ProjGraph.Tests.Contract/McpProjectGraphContractTests.cs @@ -59,9 +59,10 @@ public void GetProjectGraph_ShouldHave_Parameters() var method = type.GetMethod("GetProjectGraphAsync"); var parameters = method!.GetParameters(); - // Assert parameters exist (path, showTitle, includePackages, progress, cancellationToken) - parameters.Should().HaveCount(5, - "GetProjectGraph should have 'path', 'showTitle', 'includePackages', 'progress', and 'cancellationToken' parameters"); + // Assert parameters exist. 'server' is bound by the SDK to the request-scoped McpServer and + // is excluded from the tool's JSON schema, so it does not widen the client-facing contract. + parameters.Should().HaveCount(6, + "GetProjectGraph should have 'path', 'showTitle', 'includePackages', 'progress', 'server', and 'cancellationToken' parameters"); var pathParam = parameters.Should().ContainSingle(p => p.Name == "path").Which; pathParam.ParameterType.Should().Be(); diff --git a/tests/ProjGraph.Tests.Contract/McpProjectStatsContractTests.cs b/tests/ProjGraph.Tests.Contract/McpProjectStatsContractTests.cs index 9f9ff8f..6efac2d 100644 --- a/tests/ProjGraph.Tests.Contract/McpProjectStatsContractTests.cs +++ b/tests/ProjGraph.Tests.Contract/McpProjectStatsContractTests.cs @@ -86,8 +86,10 @@ public void GetProjectStats_ShouldHave_CorrectParameterCount() var method = typeof(ProjGraphTools).GetMethod("GetProjectStatsAsync"); var parameters = method!.GetParameters(); - // path, topN, progress, cancellationToken = 4 parameters - parameters.Should().HaveCount(4, - "GetProjectStats should have 'path', 'topN', 'progress', and 'cancellationToken' parameters"); + // path, topN, progress, server, cancellationToken = 5 parameters. 'server' is bound by the + // SDK to the request-scoped McpServer and is excluded from the tool's JSON schema, so it + // does not widen the client-facing contract. + parameters.Should().HaveCount(5, + "GetProjectStats should have 'path', 'topN', 'progress', 'server', and 'cancellationToken' parameters"); } } diff --git a/tests/ProjGraph.Tests.Integration.Mcp/Helpers/InProcessMcpSession.cs b/tests/ProjGraph.Tests.Integration.Mcp/Helpers/InProcessMcpSession.cs index 82f6472..039d611 100644 --- a/tests/ProjGraph.Tests.Integration.Mcp/Helpers/InProcessMcpSession.cs +++ b/tests/ProjGraph.Tests.Integration.Mcp/Helpers/InProcessMcpSession.cs @@ -82,10 +82,23 @@ public static McpServerOptions CreateServerOptions() /// /// Supplies the workspace root directories for each request. /// Client options that answer roots/list from . - public static McpClientOptions CreateClientOptionsWithRoots(Func> rootProvider) + /// + /// When (the default), pins the client to the 2025-11-25 + /// handshake so client capabilities are established session-scoped on the root + /// — required by tests that drive that root instance directly. When + /// , the client negotiates the latest revision, where capabilities + /// arrive per request in _meta and are visible only on the request-scoped server. + /// + public static McpClientOptions CreateClientOptionsWithRoots( + Func> rootProvider, + bool pinDownLevel = true) { + // Roots is deprecated (SEP-2577) but still served by WorkspaceRootService for down-level + // clients, so this harness keeps exercising it. +#pragma warning disable MCP9005 return new McpClientOptions { + ProtocolVersion = pinDownLevel ? "2025-11-25" : null, ClientInfo = new Implementation { Name = "ProjGraph.Tests", @@ -109,6 +122,7 @@ public static McpClientOptions CreateClientOptionsWithRoots(Func diff --git a/tests/ProjGraph.Tests.Integration.Mcp/Helpers/McpTestHelper.cs b/tests/ProjGraph.Tests.Integration.Mcp/Helpers/McpTestHelper.cs index 0739695..4d313e0 100644 --- a/tests/ProjGraph.Tests.Integration.Mcp/Helpers/McpTestHelper.cs +++ b/tests/ProjGraph.Tests.Integration.Mcp/Helpers/McpTestHelper.cs @@ -24,7 +24,10 @@ public static ProjGraphTools CreateTools() return CreateTools(new CollectingOutputConsole()); } - public static ProjGraphTools CreateTools(CollectingOutputConsole console, DiagramResourceCache? cache = null) + public static ProjGraphTools CreateTools( + CollectingOutputConsole console, + DiagramResourceCache? cache = null, + WorkspaceRootService? rootService = null) { var fs = new PhysicalFileSystem(); var slnParser = new SlnParser(fs); @@ -59,8 +62,7 @@ public static ProjGraphTools CreateTools(CollectingOutputConsole console, Diagra new MermaidErdRenderer()), fs, cache ?? new DiagramResourceCache(), - null!, - new WorkspaceRootService(fs), + rootService ?? new WorkspaceRootService(fs), console); } } diff --git a/tests/ProjGraph.Tests.Integration.Mcp/McpRootsTests.cs b/tests/ProjGraph.Tests.Integration.Mcp/McpRootsTests.cs index 1110d26..6ad98dd 100644 --- a/tests/ProjGraph.Tests.Integration.Mcp/McpRootsTests.cs +++ b/tests/ProjGraph.Tests.Integration.Mcp/McpRootsTests.cs @@ -1,6 +1,7 @@ using ModelContextProtocol; using ProjGraph.Lib.Core.Infrastructure; using ProjGraph.Mcp; +using ProjGraph.Tests.Integration.Mcp.Helpers; using ProjGraph.Tests.Shared.Helpers; using System.Reflection; @@ -24,15 +25,16 @@ public async Task TryResolve_AbsolutePath_ShouldPassThrough() [Fact] public async Task TryResolve_RelativePath_NoRootsCapability_ShouldThrow() { - var service = new WorkspaceRootService(new PhysicalFileSystem()); + await using var session = await InProcessMcpSession.StartAsync( + clientOptions: InProcessMcpSession.CreateClientOptionsWithoutRoots()); + await using var service = new WorkspaceRootService(new PhysicalFileSystem()); - // Using a McpServer with null ClientCapabilities fails, so pass null - // which exercises the Unsupported path when server capabilities are unavailable - var act = async () => await service.TryResolveAsync("MySolution.slnx", null!, CancellationToken.None); + var act = async () => await service.TryResolveAsync("MySolution.slnx", session.Server, CancellationToken.None); - // Without a server, we expect a NullReferenceException trying to access ClientCapabilities - // In production, this is handled by the MCP server providing capabilities - await act.Should().ThrowAsync(); + // McpException so the guidance reaches the client; the SDK strips the message from any + // other exception type. + await act.Should().ThrowAsync() + .WithMessage("*does not support workspace roots*absolute path*"); } [Fact] @@ -150,10 +152,11 @@ public async Task TryResolve_RelativePath_FileFoundInRoot_ShouldReturnFullPath() const string fileName = "MySolution.slnx"; var filePath = _temp.CreateFile(fileName, ""); - var service = new WorkspaceRootService(new PhysicalFileSystem()); - SetRoots(service, [_temp.DirectoryPath]); + await using var session = await InProcessMcpSession.StartAsync( + clientOptions: InProcessMcpSession.CreateClientOptionsWithRoots(() => [_temp.DirectoryPath])); + await using var service = new WorkspaceRootService(new PhysicalFileSystem()); - var result = await service.TryResolveAsync(fileName, null!, CancellationToken.None); + var result = await service.TryResolveAsync(fileName, session.Server, CancellationToken.None); result.Should().Be(filePath); } @@ -164,10 +167,11 @@ public async Task TryResolve_RelativePath_FileFoundInSubdirectory_ShouldReturnFu const string fileName = "Deep.slnx"; var filePath = _temp.CreateFile(Path.Combine("src", "nested", fileName), ""); - var service = new WorkspaceRootService(new PhysicalFileSystem()); - SetRoots(service, [_temp.DirectoryPath]); + await using var session = await InProcessMcpSession.StartAsync( + clientOptions: InProcessMcpSession.CreateClientOptionsWithRoots(() => [_temp.DirectoryPath])); + await using var service = new WorkspaceRootService(new PhysicalFileSystem()); - var result = await service.TryResolveAsync(fileName, null!, CancellationToken.None); + var result = await service.TryResolveAsync(fileName, session.Server, CancellationToken.None); result.Should().Be(filePath); } @@ -175,10 +179,11 @@ public async Task TryResolve_RelativePath_FileFoundInSubdirectory_ShouldReturnFu [Fact] public async Task TryResolve_RelativePath_FileNotFound_ShouldThrowMcpException() { - var service = new WorkspaceRootService(new PhysicalFileSystem()); - SetRoots(service, [_temp.DirectoryPath]); + await using var session = await InProcessMcpSession.StartAsync( + clientOptions: InProcessMcpSession.CreateClientOptionsWithRoots(() => [_temp.DirectoryPath])); + await using var service = new WorkspaceRootService(new PhysicalFileSystem()); - var act = async () => await service.TryResolveAsync("missing.slnx", null!, CancellationToken.None); + var act = async () => await service.TryResolveAsync("missing.slnx", session.Server, CancellationToken.None); // McpException so the not-found guidance reaches the client instead of a stripped generic error. await act.Should().ThrowAsync() @@ -194,10 +199,12 @@ public async Task TryResolve_RelativePath_AmbiguousMatch_ShouldThrowMcpException using var temp2 = new TestDirectory(); temp2.CreateFile(fileName, ""); - var service = new WorkspaceRootService(new PhysicalFileSystem()); - SetRoots(service, [_temp.DirectoryPath, temp2.DirectoryPath]); + await using var session = await InProcessMcpSession.StartAsync( + clientOptions: InProcessMcpSession.CreateClientOptionsWithRoots( + () => [_temp.DirectoryPath, temp2.DirectoryPath])); + await using var service = new WorkspaceRootService(new PhysicalFileSystem()); - var act = async () => await service.TryResolveAsync(fileName, null!, CancellationToken.None); + var act = async () => await service.TryResolveAsync(fileName, session.Server, CancellationToken.None); // McpException so the ambiguity guidance reaches the client instead of a stripped generic error. await act.Should().ThrowAsync() @@ -210,10 +217,11 @@ public async Task TryResolve_RelativePath_FileInsideBinDirectory_ShouldNotBeFoun const string fileName = "Hidden.slnx"; _temp.CreateFile(Path.Combine("bin", fileName), ""); - var service = new WorkspaceRootService(new PhysicalFileSystem()); - SetRoots(service, [_temp.DirectoryPath]); + await using var session = await InProcessMcpSession.StartAsync( + clientOptions: InProcessMcpSession.CreateClientOptionsWithRoots(() => [_temp.DirectoryPath])); + await using var service = new WorkspaceRootService(new PhysicalFileSystem()); - var act = async () => await service.TryResolveAsync(fileName, null!, CancellationToken.None); + var act = async () => await service.TryResolveAsync(fileName, session.Server, CancellationToken.None); await act.Should().ThrowAsync(); } @@ -224,10 +232,11 @@ public async Task TryResolve_RelativePath_FileInsideObjDirectory_ShouldNotBeFoun const string fileName = "Artifact.slnx"; _temp.CreateFile(Path.Combine("obj", fileName), ""); - var service = new WorkspaceRootService(new PhysicalFileSystem()); - SetRoots(service, [_temp.DirectoryPath]); + await using var session = await InProcessMcpSession.StartAsync( + clientOptions: InProcessMcpSession.CreateClientOptionsWithRoots(() => [_temp.DirectoryPath])); + await using var service = new WorkspaceRootService(new PhysicalFileSystem()); - var act = async () => await service.TryResolveAsync(fileName, null!, CancellationToken.None); + var act = async () => await service.TryResolveAsync(fileName, session.Server, CancellationToken.None); await act.Should().ThrowAsync(); } @@ -240,16 +249,6 @@ public async Task DisposeAsync_ShouldReleaseSemaphore_WithoutThrowing() await act.Should().NotThrowAsync(); } - private static void SetRoots(WorkspaceRootService service, IEnumerable roots) - { - var type = typeof(WorkspaceRootService); - var rootPathsField = type.GetField("_rootPaths", BindingFlags.NonPublic | BindingFlags.Instance)!; - var statusField = type.GetField("_status", BindingFlags.NonPublic | BindingFlags.Instance)!; - rootPathsField.SetValue(service, roots.ToList()); - // RootsStatusKind.Ready = 2 (private enum inside WorkspaceRootService) - statusField.SetValue(service, Enum.ToObject(statusField.FieldType, 2)); - } - public void Dispose() { _temp.Dispose(); diff --git a/tests/ProjGraph.Tests.Integration.Mcp/McpToolRootsTests.cs b/tests/ProjGraph.Tests.Integration.Mcp/McpToolRootsTests.cs new file mode 100644 index 0000000..6a9cd1d --- /dev/null +++ b/tests/ProjGraph.Tests.Integration.Mcp/McpToolRootsTests.cs @@ -0,0 +1,145 @@ +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using ProjGraph.Lib.Core.Infrastructure; +using ProjGraph.Mcp; +using ProjGraph.Tests.Integration.Mcp.Helpers; +using ProjGraph.Tests.Shared.Helpers; +using System.Reflection; + +namespace ProjGraph.Tests.Integration.Mcp; + +/// +/// Drives the tools the way a real client does — an actual tools/call over the wire — so the +/// request-scoped is the one the SDK binds, not a root instance a test +/// handed in. This is what distinguishes protocol revision 2026-07-28 (client capabilities declared +/// per request in _meta, visible only on the request-scoped server) from the earlier +/// initialize handshake that WorkspaceRootServiceTests pins itself to. +/// +public sealed class McpToolRootsTests : IDisposable +{ + private readonly TestDirectory _temp = new(); + + /// + /// Builds server options exposing the real ProjGraph tools, so a tools/call reaches the + /// production code path including workspace-root resolution. + /// + /// + /// The roots service to wire in, when a test needs to inspect it afterwards. + /// + /// Server options whose tool collection is backed by the real tools. + private static McpServerOptions CreateServerOptionsWithTools(WorkspaceRootService? rootService = null) + { + var tools = McpTestHelper.CreateTools(new CollectingOutputConsole(), rootService: rootService); + var options = InProcessMcpSession.CreateServerOptions(); + + options.ToolCollection = + [ + McpServerTool.Create(tools.GetProjectGraphAsync), + McpServerTool.Create(tools.GetProjectStatsAsync) + ]; + + return options; + } + + [Fact] + public async Task CallTool_RelativePath_OnCurrentProtocol_ShouldResolveAgainstTheClientRoots() + { + // A minimal but real solution file, so resolution is the only thing under test. + _temp.CreateFile("App.slnx", ""); + + await using var session = await InProcessMcpSession.StartAsync( + CreateServerOptionsWithTools(), + // Not pinned: the client negotiates the latest revision (2026-07-28), which drops the + // initialize handshake. This is the case that silently regressed on the v2 upgrade. + InProcessMcpSession.CreateClientOptionsWithRoots(() => [_temp.DirectoryPath], pinDownLevel: false)); + + var result = await session.Client.CallToolAsync( + "get_project_graph", + new Dictionary { ["path"] = "App.slnx" }); + + result.IsError.Should().NotBeTrue( + "a relative path must resolve against the client's workspace roots on the current protocol revision"); + } + + [Fact] + public async Task CallTool_RelativePath_WithoutRootsCapability_ShouldAskForAnAbsolutePath() + { + await using var session = await InProcessMcpSession.StartAsync( + CreateServerOptionsWithTools(), + InProcessMcpSession.CreateClientOptionsWithoutRoots()); + + var result = await session.Client.CallToolAsync( + "get_project_graph", + new Dictionary { ["path"] = "App.slnx" }); + + result.IsError.Should().BeTrue(); + var text = string.Concat(result.Content.OfType().Select(block => block.Text)); + text.Should().Contain("absolute path", + "the guidance must survive the SDK's tool boundary instead of being replaced by a generic error"); + } + + [Fact] + public async Task CallTool_OnCurrentProtocol_ShouldNotServeStaleRootsAcrossRequests() + { + using var secondRoot = new TestDirectory(); + secondRoot.CreateFile("Moved.slnx", ""); + _temp.CreateFile("App.slnx", ""); + + var currentRoots = new List { _temp.DirectoryPath }; + await using var session = await InProcessMcpSession.StartAsync( + CreateServerOptionsWithTools(), + InProcessMcpSession.CreateClientOptionsWithRoots( + () => [.. Volatile.Read(ref currentRoots)], pinDownLevel: false)); + + var first = await session.Client.CallToolAsync( + "get_project_graph", + new Dictionary { ["path"] = "App.slnx" }); + first.IsError.Should().NotBeTrue(); + + // The workspace moves. On 2026-07-28 there is no session for roots/list_changed to + // invalidate, so the roots must be re-fetched per request rather than cached. + Volatile.Write(ref currentRoots, [secondRoot.DirectoryPath]); + + var second = await session.Client.CallToolAsync( + "get_project_graph", + new Dictionary { ["path"] = "Moved.slnx" }); + + second.IsError.Should().NotBeTrue( + "the roots of the current request must be used, not those cached from an earlier one"); + } + + [Fact] + public async Task CallTool_OnCurrentProtocol_ShouldNotPublishTheRootsToTheSharedCache() + { + _temp.CreateFile("App.slnx", ""); + + await using var rootService = new WorkspaceRootService(new PhysicalFileSystem()); + await using var session = await InProcessMcpSession.StartAsync( + CreateServerOptionsWithTools(rootService), + InProcessMcpSession.CreateClientOptionsWithRoots(() => [_temp.DirectoryPath], pinDownLevel: false)); + + var result = await session.Client.CallToolAsync( + "get_project_graph", + new Dictionary { ["path"] = "App.slnx" }); + result.IsError.Should().NotBeTrue(); + + // The roots of a per-request revision belong to the request that fetched them. Leaving them + // in the singleton's fields is what would let an overlapping request resolve its own path + // against them, so the shared cache must still be untouched. + var type = typeof(WorkspaceRootService); + var rootPaths = type.GetField("_rootPaths", BindingFlags.NonPublic | BindingFlags.Instance)! + .GetValue(rootService); + var status = type.GetField("_status", BindingFlags.NonPublic | BindingFlags.Instance)! + .GetValue(rootService); + + rootPaths.Should().BeAssignableTo>() + .Which.Should().BeEmpty("the request's roots must not be published to the shared cache"); + // RootsStatusKind.Unknown = 0 (private enum inside WorkspaceRootService) + ((int)status!).Should().Be(0, "the shared status must stay untouched on the per-request revision"); + } + + public void Dispose() + { + _temp.Dispose(); + } +} diff --git a/tests/ProjGraph.Tests.Integration.Mcp/WorkspaceRootServiceTests.cs b/tests/ProjGraph.Tests.Integration.Mcp/WorkspaceRootServiceTests.cs index f90d8ea..321ade6 100644 --- a/tests/ProjGraph.Tests.Integration.Mcp/WorkspaceRootServiceTests.cs +++ b/tests/ProjGraph.Tests.Integration.Mcp/WorkspaceRootServiceTests.cs @@ -129,7 +129,9 @@ public async Task RootsListChangedNotification_ShouldInvalidateTheCachedRoots() // The workspace switches to a different folder and the client announces it. Volatile.Write(ref currentRoots, [secondRoot.DirectoryPath]); +#pragma warning disable MCP9005 // Roots is deprecated (SEP-2577); still served for down-level clients. await session.Client.SendNotificationAsync(NotificationMethods.RootsListChangedNotification); +#pragma warning restore MCP9005 await WaitForRootsInvalidationAsync(service); var resolved = await service.TryResolveAsync("Moved.slnx", session.Server, CancellationToken.None);