From 207e390444449fe7172715215599cbcb1824e2a8 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 16 Jul 2026 16:17:17 -0400 Subject: [PATCH 01/16] feat(mcp): migrate to ModelContextProtocol 2.0 line (2.0.0-preview.3) - Bump ModelContextProtocol 1.4.1 -> 2.0.0-preview.3. - Remove the Tool.Execution mapping for .LongRunning() commands: SDK 2.0 dropped the experimental MCP Tasks tool augmentation (Tasks SEP deferred out of the 2.0 protocol release). The annotation stays in Repl's model; protocol-level task support returns with the SDK Tasks runtime. - Keep supporting Roots, Sampling, and Logging: deprecated by spec 2026-07-28 (SEP-2577, MCP9005) with no replacement, still relied on by current hosts. Scoped, documented pragmas at the feature touchpoints. - Document the SDK/protocol version posture in docs/mcp-reference.md. Full suite green against the new SDK (1312 passed, 1 known skip), including all MCP capability, tool-call, roots, sampling, and logging regressions. Note: re-pin to the stable 2.0.0 release before cutting stable 0.12. Refs #51 --- docs/mcp-reference.md | 6 ++++++ src/Directory.Packages.props | 2 +- src/Repl.Mcp/IMcpFeedback.cs | 5 +++++ src/Repl.Mcp/McpClientRootsService.cs | 5 +++++ src/Repl.Mcp/McpFeedbackService.cs | 5 +++++ src/Repl.Mcp/McpInteractionChannel.cs | 5 +++++ src/Repl.Mcp/McpSamplingService.cs | 5 +++++ src/Repl.Mcp/McpServerHandler.cs | 5 +++++ src/Repl.Mcp/ReplMcpServerTool.cs | 12 ++++-------- src/Repl.McpTests/Given_McpAgentCapabilities.cs | 5 +++++ src/Repl.McpTests/Given_McpIntegration.cs | 5 +++++ src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs | 5 +++++ src/Repl.McpTests/Given_McpUserFeedback.cs | 5 +++++ 13 files changed, 61 insertions(+), 9 deletions(-) diff --git a/docs/mcp-reference.md b/docs/mcp-reference.md index 962da647..76e4893e 100644 --- a/docs/mcp-reference.md +++ b/docs/mcp-reference.md @@ -512,6 +512,12 @@ side-channel command output and are not included in `resources/read` bodies. Feature support varies across agents. Check [mcp-availability.com](https://mcp-availability.com/) for current data. +### SDK and protocol versions + +- Repl.Mcp builds on the official C# SDK (`ModelContextProtocol`), currently on the **2.0 line** (`2.0.0-preview.3`). The SDK negotiates the protocol version with each client, including fallback to the legacy `initialize` handshake for older hosts. +- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577) but remain fully functional; Repl.Mcp keeps supporting them until the SDK removes them, since current hosts still rely on these features. +- **MCP Tasks**: SDK 2.0 removed the experimental tool-execution augmentation (`Tool.Execution`), so `.LongRunning()` commands no longer advertise task support at the protocol level. The annotation stays in Repl's own model (help/docs), and protocol-level task support will return once the SDK ships its Tasks runtime. + | Feature | Claude Desktop | Claude Code | Codex | VS Code Copilot | Cursor | Continue | |---|---|---|---|---|---|---| | Tools | Yes | Yes | Yes | Yes | Yes | Yes | diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 1aa0edeb..77d7b240 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -14,7 +14,7 @@ - + diff --git a/src/Repl.Mcp/IMcpFeedback.cs b/src/Repl.Mcp/IMcpFeedback.cs index 3fff6a16..5f38df4a 100644 --- a/src/Repl.Mcp/IMcpFeedback.cs +++ b/src/Repl.Mcp/IMcpFeedback.cs @@ -1,6 +1,11 @@ using ModelContextProtocol.Protocol; using Repl.Interaction; +// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK +// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps +// supporting the features until the SDK removes them. Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.Mcp; /// diff --git a/src/Repl.Mcp/McpClientRootsService.cs b/src/Repl.Mcp/McpClientRootsService.cs index fa53d4e3..fc5ac7e5 100644 --- a/src/Repl.Mcp/McpClientRootsService.cs +++ b/src/Repl.Mcp/McpClientRootsService.cs @@ -1,6 +1,11 @@ using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; +// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK +// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps +// supporting the features until the SDK removes them. Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.Mcp; internal sealed class McpClientRootsService : IMcpClientRoots diff --git a/src/Repl.Mcp/McpFeedbackService.cs b/src/Repl.Mcp/McpFeedbackService.cs index 92a3a2ac..66c7cb93 100644 --- a/src/Repl.Mcp/McpFeedbackService.cs +++ b/src/Repl.Mcp/McpFeedbackService.cs @@ -5,6 +5,11 @@ using ModelContextProtocol.Server; using Repl.Interaction; +// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK +// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps +// supporting the features until the SDK removes them. Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.Mcp; /// diff --git a/src/Repl.Mcp/McpInteractionChannel.cs b/src/Repl.Mcp/McpInteractionChannel.cs index ab6531ad..233da8cf 100644 --- a/src/Repl.Mcp/McpInteractionChannel.cs +++ b/src/Repl.Mcp/McpInteractionChannel.cs @@ -5,6 +5,11 @@ using ModelContextProtocol.Server; using Repl.Interaction; +// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK +// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps +// supporting the features until the SDK removes them. Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.Mcp; /// diff --git a/src/Repl.Mcp/McpSamplingService.cs b/src/Repl.Mcp/McpSamplingService.cs index 5e6a09e0..53926aa0 100644 --- a/src/Repl.Mcp/McpSamplingService.cs +++ b/src/Repl.Mcp/McpSamplingService.cs @@ -1,6 +1,11 @@ using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; +// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK +// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps +// supporting the features until the SDK removes them. Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.Mcp; /// diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index eaddea03..434dbc86 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -8,6 +8,11 @@ using Repl.Interaction; using Repl.Internal.Options; +// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK +// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps +// supporting the features until the SDK removes them. Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.Mcp; /// diff --git a/src/Repl.Mcp/ReplMcpServerTool.cs b/src/Repl.Mcp/ReplMcpServerTool.cs index cb4b1c0f..94845834 100644 --- a/src/Repl.Mcp/ReplMcpServerTool.cs +++ b/src/Repl.Mcp/ReplMcpServerTool.cs @@ -14,10 +14,10 @@ internal sealed class ReplMcpServerTool : McpServerTool private readonly McpToolAdapter _adapter; private readonly Tool _protocolTool; - // MCP Tasks are experimental in the SDK (MCPEXP001) but part of the MCP spec. - // LongRunning commands advertise optional task support so agents can use - // the call-now/poll-later pattern instead of blocking on slow operations. -#pragma warning disable MCPEXP001 + // SDK 2.0 removed the experimental MCP Tasks tool-augmentation surface + // (Tool.Execution / ToolTaskSupport, MCPEXP001 in 1.x): the Tasks SEP was deferred out + // of the 2.0 protocol release. Repl keeps .LongRunning() in its own model (help/docs); + // advertising task support returns with the SDK's Tasks runtime (tracked in issue #51). public ReplMcpServerTool( ReplDocCommand command, string toolName, @@ -31,15 +31,11 @@ public ReplMcpServerTool( InputSchema = McpSchemaGenerator.BuildInputSchema(command), OutputSchema = McpSchemaGenerator.BuildOutputSchema(command), Annotations = McpSchemaGenerator.MapAnnotations(command.Annotations), - Execution = command.Annotations?.LongRunning == true - ? new ToolExecution { TaskSupport = ToolTaskSupport.Optional } - : null, Meta = TryGetAppOptions(command, out var appOptions) ? McpAppMetadata.BuildToolMeta(appOptions) : null, }; } -#pragma warning restore MCPEXP001 /// public override Tool ProtocolTool => _protocolTool; diff --git a/src/Repl.McpTests/Given_McpAgentCapabilities.cs b/src/Repl.McpTests/Given_McpAgentCapabilities.cs index b87ef8d3..c1440bcb 100644 --- a/src/Repl.McpTests/Given_McpAgentCapabilities.cs +++ b/src/Repl.McpTests/Given_McpAgentCapabilities.cs @@ -4,6 +4,11 @@ using ModelContextProtocol.Protocol; using Repl.Mcp; +// These tests exercise Roots/Sampling/Logging, deprecated by MCP spec 2026-07-28 +// (SEP-2577, MCP9005) but still supported by Repl.Mcp until the SDK removes them. +// Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.McpTests; [TestClass] diff --git a/src/Repl.McpTests/Given_McpIntegration.cs b/src/Repl.McpTests/Given_McpIntegration.cs index 55de3709..5eba1b06 100644 --- a/src/Repl.McpTests/Given_McpIntegration.cs +++ b/src/Repl.McpTests/Given_McpIntegration.cs @@ -1,5 +1,10 @@ using Repl.Mcp; +// These tests exercise Roots/Sampling/Logging, deprecated by MCP spec 2026-07-28 +// (SEP-2577, MCP9005) but still supported by Repl.Mcp until the SDK removes them. +// Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.McpTests; [TestClass] diff --git a/src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs b/src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs index ed70c18b..5eb7b01b 100644 --- a/src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs +++ b/src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs @@ -4,6 +4,11 @@ using ModelContextProtocol.Protocol; using Repl.Mcp; +// These tests exercise Roots/Sampling/Logging, deprecated by MCP spec 2026-07-28 +// (SEP-2577, MCP9005) but still supported by Repl.Mcp until the SDK removes them. +// Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.McpTests; [TestClass] diff --git a/src/Repl.McpTests/Given_McpUserFeedback.cs b/src/Repl.McpTests/Given_McpUserFeedback.cs index 0a8bd859..28869839 100644 --- a/src/Repl.McpTests/Given_McpUserFeedback.cs +++ b/src/Repl.McpTests/Given_McpUserFeedback.cs @@ -5,6 +5,11 @@ using ModelContextProtocol.Protocol; using Repl.Interaction; +// These tests exercise Roots/Sampling/Logging, deprecated by MCP spec 2026-07-28 +// (SEP-2577, MCP9005) but still supported by Repl.Mcp until the SDK removes them. +// Tracked in issue #51. +#pragma warning disable MCP9005 + namespace Repl.McpTests; [TestClass] From aeeb8dfea9dfcc732a981b77dcd317bb57af20cf Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 16 Jul 2026 16:31:43 -0400 Subject: [PATCH 02/16] fix(mcp): correct Tasks narrative, scope pragmas, lock tools/list wire shape (review) - Correct the migration rationale: MCP Tasks was EXTRACTED to ModelContextProtocol.Extensions.Tasks (store, task results, client polling), not removed; the per-tool Tool.Execution augmentation is gone from the protocol surface. Comments and docs now say so, and Repl still deliberately does not advertise task support without the runtime. - Name the designated successor (SEP-2322 multi-round-trip requests) in the deprecation pragmas instead of claiming 'no replacement API'. - Narrow MCP9005 pragmas to their touchpoints in McpServerHandler and Given_McpIntegration (file-scoped kept only where usage is dense). - Lock the SDK-2.0 tools/list wire shape: a .LongRunning() tool serializes its annotations and emits no task/execution augmentation. - Align remaining .LongRunning() doc mentions (overview, coding-agents guide, package README) with the current no-advertisement posture. --- docs/for-coding-agents.md | 2 +- docs/mcp-overview.md | 2 +- src/Repl.Mcp/IMcpFeedback.cs | 5 ++-- src/Repl.Mcp/McpClientRootsService.cs | 5 ++-- src/Repl.Mcp/McpFeedbackService.cs | 5 ++-- src/Repl.Mcp/McpInteractionChannel.cs | 5 ++-- src/Repl.Mcp/McpSamplingService.cs | 5 ++-- src/Repl.Mcp/McpServerHandler.cs | 14 +++++++---- src/Repl.Mcp/README.md | 2 +- src/Repl.Mcp/ReplMcpServerTool.cs | 9 +++---- src/Repl.McpTests/Given_McpIntegration.cs | 29 +++++++++++++++++++---- 11 files changed, 56 insertions(+), 27 deletions(-) diff --git a/docs/for-coding-agents.md b/docs/for-coding-agents.md index 0e61fba8..085d1230 100644 --- a/docs/for-coding-agents.md +++ b/docs/for-coding-agents.md @@ -142,7 +142,7 @@ Use these annotations to help agents make safer decisions: | `.Destructive()` | May delete or mutate important state; ask for confirmation. | | `.Idempotent()` | Safe to retry. | | `.OpenWorld()` | Talks to external systems; expect latency and failures. | -| `.LongRunning()` | May take time; use call-now / poll-later patterns. | +| `.LongRunning()` | May take time. Documentation hint for now — no protocol-level task advertisement until the MCP Tasks runtime lands. | | `.AutomationHidden()` | Do not expose this command to MCP automation. | | `.WithOption(name, o => o.AutomationHidden())` | Keep this one option out of the tool schema; the command stays visible. | diff --git a/docs/mcp-overview.md b/docs/mcp-overview.md index 9f3f42a3..2abd8a1e 100644 --- a/docs/mcp-overview.md +++ b/docs/mcp-overview.md @@ -64,7 +64,7 @@ app.Map("deploy", handler).Destructive().LongRunning().OpenWorld(); | `.Destructive()` | Ask user for confirmation, sequential | | `.Idempotent()` | Safe to retry, can parallelize | | `.OpenWorld()` | Reaches external systems — expect latency and transient failures | -| `.LongRunning()` | Enables call-now/poll-later pattern | +| `.LongRunning()` | Slow-operation hint (protocol-level task advertisement returns with the SDK Tasks runtime — see [mcp-reference.md](mcp-reference.md#sdk-and-protocol-versions)) | | `.AutomationHidden()` | Not visible to agents | **Annotate every command exposed to agents.** Unannotated tools force agents to assume the worst: confirm everything, no parallelism, no retries. diff --git a/src/Repl.Mcp/IMcpFeedback.cs b/src/Repl.Mcp/IMcpFeedback.cs index 5f38df4a..9abbf943 100644 --- a/src/Repl.Mcp/IMcpFeedback.cs +++ b/src/Repl.Mcp/IMcpFeedback.cs @@ -2,8 +2,9 @@ using Repl.Interaction; // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps -// supporting the features until the SDK removes them. Tracked in issue #51. +// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, +// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on +// these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; diff --git a/src/Repl.Mcp/McpClientRootsService.cs b/src/Repl.Mcp/McpClientRootsService.cs index fc5ac7e5..16a4fd79 100644 --- a/src/Repl.Mcp/McpClientRootsService.cs +++ b/src/Repl.Mcp/McpClientRootsService.cs @@ -2,8 +2,9 @@ using ModelContextProtocol.Server; // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps -// supporting the features until the SDK removes them. Tracked in issue #51. +// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, +// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on +// these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; diff --git a/src/Repl.Mcp/McpFeedbackService.cs b/src/Repl.Mcp/McpFeedbackService.cs index 66c7cb93..efbfaaa2 100644 --- a/src/Repl.Mcp/McpFeedbackService.cs +++ b/src/Repl.Mcp/McpFeedbackService.cs @@ -6,8 +6,9 @@ using Repl.Interaction; // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps -// supporting the features until the SDK removes them. Tracked in issue #51. +// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, +// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on +// these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; diff --git a/src/Repl.Mcp/McpInteractionChannel.cs b/src/Repl.Mcp/McpInteractionChannel.cs index 233da8cf..f1e94a7e 100644 --- a/src/Repl.Mcp/McpInteractionChannel.cs +++ b/src/Repl.Mcp/McpInteractionChannel.cs @@ -6,8 +6,9 @@ using Repl.Interaction; // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps -// supporting the features until the SDK removes them. Tracked in issue #51. +// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, +// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on +// these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; diff --git a/src/Repl.Mcp/McpSamplingService.cs b/src/Repl.Mcp/McpSamplingService.cs index 53926aa0..1f8dcddb 100644 --- a/src/Repl.Mcp/McpSamplingService.cs +++ b/src/Repl.Mcp/McpSamplingService.cs @@ -2,8 +2,9 @@ using ModelContextProtocol.Server; // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps -// supporting the features until the SDK removes them. Tracked in issue #51. +// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, +// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on +// these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index 434dbc86..268e931a 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -8,11 +8,6 @@ using Repl.Interaction; using Repl.Internal.Options; -// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005) with no replacement API; hosts still rely on them, so Repl keeps -// supporting the features until the SDK removes them. Tracked in issue #51. -#pragma warning disable MCP9005 - namespace Repl.Mcp; /// @@ -535,6 +530,9 @@ private void EnsureRootsNotificationHandler(McpServer server) } var weakSelf = new WeakReference(this); + // Roots is deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005) but hosts still send + // this notification; Repl keeps supporting it until the SDK removes the surface (#51). +#pragma warning disable MCP9005 _ = server.RegisterNotificationHandler( NotificationMethods.RootsListChangedNotification, (_, _) => @@ -546,6 +544,7 @@ private void EnsureRootsNotificationHandler(McpServer server) return ValueTask.CompletedTask; }); +#pragma warning restore MCP9005 } internal static SnapshotVersionState PublishSnapshotInvalidation( @@ -640,6 +639,10 @@ private void UnsubscribeFromRoutingChanges() private ServerCapabilities BuildCapabilities() { + // Logging is deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005) but the feedback + // bridge still routes through logging notifications for current hosts; Repl keeps + // advertising it until the SDK removes the surface (#51). +#pragma warning disable MCP9005 var capabilities = new ServerCapabilities { Logging = new LoggingCapability(), @@ -647,6 +650,7 @@ private ServerCapabilities BuildCapabilities() Resources = new ResourcesCapability { ListChanged = true }, Prompts = new PromptsCapability { ListChanged = true }, }; +#pragma warning restore MCP9005 if (_options.EnableApps || HasMcpAppResources()) { diff --git a/src/Repl.Mcp/README.md b/src/Repl.Mcp/README.md index 06ee966b..8ba4862f 100644 --- a/src/Repl.Mcp/README.md +++ b/src/Repl.Mcp/README.md @@ -104,7 +104,7 @@ app.Map("debug reset", handler) .AutomationHidden(); ``` -Unannotated tools force agents to assume the worst. Use `.ReadOnly()` for safe queries, `.Destructive()` for important mutations, `.OpenWorld()` for external systems, `.LongRunning()` for operations that should use call-now / poll-later patterns, and `.AutomationHidden()` for commands that should stay available to humans but invisible to MCP automation. +Unannotated tools force agents to assume the worst. Use `.ReadOnly()` for safe queries, `.Destructive()` for important mutations, `.OpenWorld()` for external systems, `.LongRunning()` for slow operations (a documentation hint today — protocol-level MCP task advertisement returns with the SDK Tasks runtime), and `.AutomationHidden()` for commands that should stay available to humans but invisible to MCP automation. Prefer returning JSON-friendly objects instead of writing prose-only output. Structured results are easier for agents to inspect, retry, test, and summarize. diff --git a/src/Repl.Mcp/ReplMcpServerTool.cs b/src/Repl.Mcp/ReplMcpServerTool.cs index 94845834..c5e3bbf7 100644 --- a/src/Repl.Mcp/ReplMcpServerTool.cs +++ b/src/Repl.Mcp/ReplMcpServerTool.cs @@ -14,10 +14,11 @@ internal sealed class ReplMcpServerTool : McpServerTool private readonly McpToolAdapter _adapter; private readonly Tool _protocolTool; - // SDK 2.0 removed the experimental MCP Tasks tool-augmentation surface - // (Tool.Execution / ToolTaskSupport, MCPEXP001 in 1.x): the Tasks SEP was deferred out - // of the 2.0 protocol release. Repl keeps .LongRunning() in its own model (help/docs); - // advertising task support returns with the SDK's Tasks runtime (tracked in issue #51). + // SDK 2.0 extracted MCP Tasks into ModelContextProtocol.Extensions.Tasks (store, task + // results, client polling) and dropped the per-tool Tool.Execution / ToolTaskSupport + // augmentation from the protocol surface. Repl keeps .LongRunning() in its own model + // (help/docs) and deliberately does not advertise task support until the Tasks runtime + // (tasks/get|update|cancel) is implemented end-to-end — tracked in issue #51. public ReplMcpServerTool( ReplDocCommand command, string toolName, diff --git a/src/Repl.McpTests/Given_McpIntegration.cs b/src/Repl.McpTests/Given_McpIntegration.cs index 5eba1b06..ce00729d 100644 --- a/src/Repl.McpTests/Given_McpIntegration.cs +++ b/src/Repl.McpTests/Given_McpIntegration.cs @@ -1,10 +1,5 @@ using Repl.Mcp; -// These tests exercise Roots/Sampling/Logging, deprecated by MCP spec 2026-07-28 -// (SEP-2577, MCP9005) but still supported by Repl.Mcp until the SDK removes them. -// Tracked in issue #51. -#pragma warning disable MCP9005 - namespace Repl.McpTests; [TestClass] @@ -79,7 +74,10 @@ public void When_BuildingMcpOptions_Then_LoggingCapabilityIsAdvertised() var options = app.BuildMcpServerOptions(); + // Logging is deprecated (SEP-2577, MCP9005) but still supported by Repl.Mcp (#51). +#pragma warning disable MCP9005 options.Capabilities!.Logging.Should().NotBeNull(); +#pragma warning restore MCP9005 } [TestMethod] @@ -122,6 +120,27 @@ public void When_EnrichedCommands_Then_DocModelContainsAllFields() cmd.Arguments.Should().ContainSingle(a => string.Equals(a.Name, "env", StringComparison.Ordinal)); } + [TestMethod] + [Description("Locks the SDK-2.0 tools/list wire shape for .LongRunning() commands: annotations survive serialization, and no task/execution augmentation is emitted — Repl deliberately does not advertise MCP task support until the Tasks runtime is implemented end-to-end (issue #51).")] + public void When_SerializingLongRunningTool_Then_NoTaskAugmentationIsEmitted() + { + var app = ReplApp.Create(); + app.Map("deploy", () => "deployed") + .WithDescription("Deploy application") + .LongRunning() + .OpenWorld(); + + var options = app.BuildMcpServerOptions(); + var tool = options.ToolCollection!.Single(tool => + string.Equals(tool.ProtocolTool.Name, "deploy", StringComparison.Ordinal)); + var json = System.Text.Json.JsonSerializer.Serialize( + tool.ProtocolTool, ModelContextProtocol.McpJsonUtilities.DefaultOptions); + + json.Should().Contain("\"openWorldHint\""); + json.Should().NotContain("execution"); + json.Should().NotContain("taskSupport"); + } + [TestMethod] [Description("Modules excluded from Programmatic channel are not visible as MCP tools.")] public void When_ModuleExcludedFromProgrammatic_Then_NotInToolCandidates() From b7bf5c398d9a40c8acdeb247f3ac8e4f6f713fbb Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 16 Jul 2026 21:39:37 -0400 Subject: [PATCH 03/16] fix(mcp): bind capability services to the flowing request, not a shared server field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SDK 2.0's 2026-07-28 protocol path hands each request a destination-bound McpServer, and one handler can serve several sessions. The four capability services (roots, sampling, elicitation, feedback) stored the last-attached server in a shared mutable field, so a concurrent request from another session could cross-wire capabilities mid-call (IsSupported flipping while a handler was awaiting). - McpRequestServerAccessor: AsyncLocal request binding flowing with the invocation, session-level server as fallback for code outside a request (routing notifications, roots list-changed handler). - Services resolve the effective server through the accessor with a single read per operation (no torn check-then-use). - McpServerHandler splits session-level attach (RunAsync, once) from request-level binding (every handler); externally hosted servers adopt the first observed server for session concerns. - Deterministic regression: two sessions on one handler, sampling-capable client pauses mid-call while a sampling-less client is served — the paused call must keep observing ITS client's capabilities (RED observed: 'True|False' on the pre-fix code, exactly the reported repro). --- src/Repl.Mcp/McpClientRootsService.cs | 17 ++-- src/Repl.Mcp/McpElicitationService.cs | 14 ++- src/Repl.Mcp/McpFeedbackService.cs | 23 +++-- src/Repl.Mcp/McpRequestServerAccessor.cs | 31 ++++++ src/Repl.Mcp/McpSamplingService.cs | 13 ++- src/Repl.Mcp/McpServerHandler.cs | 53 +++++++--- .../Given_McpConcurrentSessions.cs | 97 +++++++++++++++++++ src/Repl.McpTests/McpTestFixture.cs | 7 ++ 8 files changed, 202 insertions(+), 53 deletions(-) create mode 100644 src/Repl.Mcp/McpRequestServerAccessor.cs diff --git a/src/Repl.Mcp/McpClientRootsService.cs b/src/Repl.Mcp/McpClientRootsService.cs index 16a4fd79..8db57690 100644 --- a/src/Repl.Mcp/McpClientRootsService.cs +++ b/src/Repl.Mcp/McpClientRootsService.cs @@ -12,19 +12,20 @@ namespace Repl.Mcp; 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 { @@ -48,15 +49,11 @@ public IReadOnlyList Current } } - public void AttachServer(McpServer server) - { - ArgumentNullException.ThrowIfNull(server); - _server = server; - } - public async ValueTask> 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; diff --git a/src/Repl.Mcp/McpElicitationService.cs b/src/Repl.Mcp/McpElicitationService.cs index 12de5219..40a57af9 100644 --- a/src/Repl.Mcp/McpElicitationService.cs +++ b/src/Repl.Mcp/McpElicitationService.cs @@ -15,13 +15,11 @@ namespace Repl.Mcp; /// a multi-field variant would build the with /// multiple properties instead of one. /// -internal sealed class McpElicitationService : IMcpElicitation +internal sealed class McpElicitationService(McpRequestServerAccessor servers) : IMcpElicitation { private const string FieldName = "value"; - private McpServer? _server; - - public bool IsSupported => _server?.ClientCapabilities?.Elicitation is not null; + public bool IsSupported => servers.Effective?.ClientCapabilities?.Elicitation is not null; public async ValueTask ElicitTextAsync( string message, @@ -99,19 +97,19 @@ internal sealed class McpElicitationService : IMcpElicitation : null; } - internal void AttachServer(McpServer server) => _server = server; - private async ValueTask ElicitSingleFieldAsync( string message, ElicitRequestParams.PrimitiveSchemaDefinition schema, CancellationToken cancellationToken) { - if (!IsSupported) + // Single read: the effective server must not change between the support check and + // the call (a concurrent request re-binding the accessor must not be observed). + if (servers.Effective is not { ClientCapabilities.Elicitation: not null } server) { return null; } - var result = await _server!.ElicitAsync( + var result = await server.ElicitAsync( new ElicitRequestParams { Message = message, diff --git a/src/Repl.Mcp/McpFeedbackService.cs b/src/Repl.Mcp/McpFeedbackService.cs index efbfaaa2..6ad0505d 100644 --- a/src/Repl.Mcp/McpFeedbackService.cs +++ b/src/Repl.Mcp/McpFeedbackService.cs @@ -16,19 +16,15 @@ namespace Repl.Mcp; /// /// Internal implementation of backed by a live session. /// -internal sealed class McpFeedbackService : IMcpFeedback +internal sealed class McpFeedbackService(McpRequestServerAccessor servers) : IMcpFeedback { private const string LoggerName = "repl.interaction"; private readonly AsyncLocal _progressToken = new(); - // This service is created per McpServerHandler/session and overlaid into the - // per-connection service provider, so the attached server reference is not shared - // across concurrent MCP connections. - private McpServer? _server; - public bool IsProgressSupported => _server is not null && _progressToken.Value is not null; + public bool IsProgressSupported => servers.Effective is not null && _progressToken.Value is not null; - public bool IsLoggingSupported => _server is not null; + public bool IsLoggingSupported => servers.Effective is not null; public async ValueTask ReportProgressAsync( ReplProgressEvent progress, @@ -36,13 +32,17 @@ public async ValueTask ReportProgressAsync( { ArgumentNullException.ThrowIfNull(progress); - if (!IsProgressSupported || progress.State == ReplProgressState.Clear || _progressToken.Value is not { } progressToken) + // Single read: the effective server must not change between the support check and + // the send (a concurrent request re-binding the accessor must not be observed). + if (servers.Effective is not { } server + || progress.State == ReplProgressState.Clear + || _progressToken.Value is not { } progressToken) { return; } var percent = progress.ResolvePercent(); - await _server!.NotifyProgressAsync( + await server.NotifyProgressAsync( progressToken, new ProgressNotificationValue { @@ -58,12 +58,12 @@ public async ValueTask SendMessageAsync( object? data, CancellationToken cancellationToken = default) { - if (!IsLoggingSupported) + if (servers.Effective is not { } server) { return; } - await _server!.SendNotificationAsync( + await server.SendNotificationAsync( NotificationMethods.LoggingMessageNotification, new LoggingMessageNotificationParams { @@ -74,7 +74,6 @@ public async ValueTask SendMessageAsync( cancellationToken: cancellationToken).ConfigureAwait(false); } - internal void AttachServer(McpServer server) => _server = server; internal IDisposable PushProgressToken(ProgressToken? progressToken) => new ProgressTokenScope(_progressToken, progressToken); diff --git a/src/Repl.Mcp/McpRequestServerAccessor.cs b/src/Repl.Mcp/McpRequestServerAccessor.cs new file mode 100644 index 00000000..91ad2429 --- /dev/null +++ b/src/Repl.Mcp/McpRequestServerAccessor.cs @@ -0,0 +1,31 @@ +using ModelContextProtocol.Server; + +namespace Repl.Mcp; + +/// +/// Resolves the a capability call must target. +/// +/// +/// SDK 2.0's 2026-07-28 protocol path hands each request a destination-bound +/// , and one handler can serve several sessions. The capability +/// services are singletons (exposed through DI to command handlers), so the effective +/// server must be the one bound to the FLOWING request — a shared mutable field would be +/// overwritten by whichever request attached last, cross-wiring capabilities between +/// concurrent calls. flows with the invocation and cannot +/// leak across requests; the session-level server remains the fallback for code running +/// outside a request (e.g. routing-change notifications). +/// +internal sealed class McpRequestServerAccessor +{ + private readonly AsyncLocal _current = new(); + private McpServer? _session; + + /// Server for the flowing request, falling back to the session server. + public McpServer? Effective => _current.Value ?? _session; + + /// Binds the flowing async context to the request's destination server. + public void BindRequest(McpServer server) => _current.Value = server; + + /// Records the session-level server used outside request flows. + public void AttachSession(McpServer server) => _session = server; +} diff --git a/src/Repl.Mcp/McpSamplingService.cs b/src/Repl.Mcp/McpSamplingService.cs index 1f8dcddb..c04d6b52 100644 --- a/src/Repl.Mcp/McpSamplingService.cs +++ b/src/Repl.Mcp/McpSamplingService.cs @@ -12,23 +12,23 @@ namespace Repl.Mcp; /// /// Internal implementation of backed by a live session. /// -internal sealed class McpSamplingService : IMcpSampling +internal sealed class McpSamplingService(McpRequestServerAccessor servers) : IMcpSampling { - private McpServer? _server; - - public bool IsSupported => _server?.ClientCapabilities?.Sampling is not null; + public bool IsSupported => servers.Effective?.ClientCapabilities?.Sampling is not null; public async ValueTask SampleAsync( string prompt, int maxTokens = 1024, CancellationToken cancellationToken = default) { - if (!IsSupported) + // Single read: the effective server must not change between the support check and + // the call (a concurrent request re-binding the accessor must not be observed). + if (servers.Effective is not { ClientCapabilities.Sampling: not null } server) { return null; } - var result = await _server!.SampleAsync( + var result = await server.SampleAsync( new CreateMessageRequestParams { Messages = @@ -46,5 +46,4 @@ internal sealed class McpSamplingService : IMcpSampling return result.Content?.OfType().FirstOrDefault()?.Text; } - internal void AttachServer(McpServer server) => _server = server; } diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index 268e931a..d27c9661 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -27,6 +27,7 @@ internal sealed class McpServerHandler private readonly IServiceProvider _services; private readonly TimeProvider _timeProvider; private readonly char _separator; + private readonly McpRequestServerAccessor _requestServers = new(); private readonly McpClientRootsService _roots; private readonly McpSamplingService _sampling; private readonly McpElicitationService _elicitation; @@ -61,10 +62,10 @@ public McpServerHandler( _services = services; _timeProvider = services.GetService(typeof(TimeProvider)) as TimeProvider ?? TimeProvider.System; _separator = McpToolNameFlattener.ResolveSeparator(options.ToolNamingSeparator); - _roots = new McpClientRootsService(app); - _sampling = new McpSamplingService(); - _elicitation = new McpElicitationService(); - _feedback = new McpFeedbackService(); + _roots = new McpClientRootsService(app, _requestServers); + _sampling = new McpSamplingService(_requestServers); + _elicitation = new McpElicitationService(_requestServers); + _feedback = new McpFeedbackService(_requestServers); _sessionServices = new McpServiceProviderOverlay( services, new Dictionary @@ -170,7 +171,7 @@ private async ValueTask ListToolsAsync( RequestContext request, CancellationToken cancellationToken) { - AttachServer(request.Server); + BindRequestServer(request.Server); var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); if (_options.DynamicToolCompatibility == DynamicToolCompatibilityMode.DiscoverAndCallShim @@ -197,7 +198,7 @@ private async ValueTask CallToolAsync( RequestContext request, CancellationToken cancellationToken) { - AttachServer(request.Server); + BindRequestServer(request.Server); var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); IDictionary arguments = request.Params.Arguments ?? EmptyArguments; var toolName = request.Params.Name ?? string.Empty; @@ -229,7 +230,7 @@ private async ValueTask ListResourcesAsync( RequestContext request, CancellationToken cancellationToken) { - AttachServer(request.Server); + BindRequestServer(request.Server); var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); return new ListResourcesResult { @@ -246,7 +247,7 @@ private async ValueTask ListResourceTemplatesAsync( RequestContext request, CancellationToken cancellationToken) { - AttachServer(request.Server); + BindRequestServer(request.Server); var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); return new ListResourceTemplatesResult { @@ -263,7 +264,7 @@ private async ValueTask ReadResourceAsync( RequestContext request, CancellationToken cancellationToken) { - AttachServer(request.Server); + BindRequestServer(request.Server); var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); var uri = request.Params.Uri ?? string.Empty; var resource = snapshot.Resources.FirstOrDefault(candidate => candidate.IsMatch(uri)); @@ -279,7 +280,7 @@ private async ValueTask ListPromptsAsync( RequestContext request, CancellationToken cancellationToken) { - AttachServer(request.Server); + BindRequestServer(request.Server); var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); return new ListPromptsResult { @@ -291,7 +292,7 @@ private async ValueTask GetPromptAsync( RequestContext request, CancellationToken cancellationToken) { - AttachServer(request.Server); + BindRequestServer(request.Server); var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); var promptName = request.Params.Name ?? string.Empty; var prompt = snapshot.Prompts.FirstOrDefault(candidate => @@ -308,7 +309,7 @@ private async ValueTask GetSnapshotAsync( McpServer? server, CancellationToken cancellationToken) { - AttachServer(server); + BindRequestServer(server); var snapshotVersion = Volatile.Read(ref _snapshotState).Version; if (Volatile.Read(ref _builtSnapshotVersion) == snapshotVersion @@ -470,6 +471,29 @@ private void ValidateCompatibilityToolNames(IReadOnlyList tools) } } + // Request-level binding: capability services resolve the flowing request's + // destination-bound server through the AsyncLocal accessor, so concurrent requests + // (SDK 2.0 creates one destination-bound McpServer per request) cannot cross-wire + // each other's client capabilities. Externally hosted servers (options built via + // BuildDynamicServerOptions and run by the host, without RunAsync's session attach) + // adopt the first observed server for session-level concerns. + private void BindRequestServer(McpServer? server) + { + if (server is null) + { + return; + } + + _requestServers.BindRequest(server); + if (_server is null) + { + AttachServer(server); + } + } + + // Session-level attach: routing-change notifications and the roots list-changed + // handler belong to the session server, registered once — never to the per-request + // destination wrappers. private void AttachServer(McpServer? server) { if (server is null) @@ -485,10 +509,7 @@ private void AttachServer(McpServer? server) } _server = server; - _roots.AttachServer(server); - _sampling.AttachServer(server); - _elicitation.AttachServer(server); - _feedback.AttachServer(server); + _requestServers.AttachSession(server); EnsureRoutingSubscription(); EnsureRootsNotificationHandler(server); } diff --git a/src/Repl.McpTests/Given_McpConcurrentSessions.cs b/src/Repl.McpTests/Given_McpConcurrentSessions.cs index 6bfadfec..1bffbc75 100644 --- a/src/Repl.McpTests/Given_McpConcurrentSessions.cs +++ b/src/Repl.McpTests/Given_McpConcurrentSessions.cs @@ -1,10 +1,107 @@ +using System.IO.Pipelines; +using ModelContextProtocol; +using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using Repl.Mcp; + +// One test exercises Sampling, deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005) +// but still supported by Repl.Mcp until the SDK removes the surface (#51). +#pragma warning disable MCP9005 namespace Repl.McpTests; [TestClass] public sealed class Given_McpConcurrentSessions { + [TestMethod] + [Description("Guards capability binding against cross-session interference: with one handler serving two sessions (SDK 2.0 binds a destination server per request), a paused call from a sampling-capable client must still observe ITS OWN client's capabilities after a request from a sampling-less client has been served — the capability services must bind to the flowing request, not to a shared last-attached server.")] + public async Task When_TwoClientsWithDifferentCapabilitiesShareHandler_Then_CapabilityBindingIsPerRequest() + { + using var entered = new SemaphoreSlim(0, 1); + using var gate = new SemaphoreSlim(0, 1); + + var app = ReplApp.Create(); + app.UseMcpServer(); + app.Map("probe", async (IMcpSampling sampling) => + { + var before = sampling.IsSupported; + entered.Release(); + await gate.WaitAsync().ConfigureAwait(false); + var after = sampling.IsSupported; + return $"{before}|{after}"; + }); + app.Map("poke", () => "ok"); + + var options = new ReplMcpServerOptions + { + TransportFactory = static (serverName, io) => new StreamServerTransport( + ((McpTestFixture.PipeIoContext)io).InputStream, + ((McpTestFixture.PipeIoContext)io).OutputStream, + serverName), + }; + var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + using var cts = new CancellationTokenSource(); + + var (clientA, serverTaskA) = await StartSessionAsync(handler, BuildSamplingClientOptions(), cts.Token).ConfigureAwait(false); + var (clientB, serverTaskB) = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + + // Session A enters "probe" (sampling supported) and pauses on the gate; session B is + // then served in full; A resumes and must STILL see its own sampling capability. + var probeTask = clientA.CallToolAsync( + "probe", new Dictionary(StringComparer.Ordinal), cancellationToken: cts.Token); + (await entered.WaitAsync(TimeSpan.FromSeconds(10)).ConfigureAwait(false)).Should().BeTrue(); + + await clientB.CallToolAsync( + "poke", new Dictionary(StringComparer.Ordinal), cancellationToken: cts.Token) + .ConfigureAwait(false); + + gate.Release(); + var probeResult = await probeTask.ConfigureAwait(false); + + var text = probeResult.Content.OfType().First().Text; + text.Should().Contain("True|True"); + + await clientA.DisposeAsync().ConfigureAwait(false); + await clientB.DisposeAsync().ConfigureAwait(false); + await cts.CancelAsync().ConfigureAwait(false); + _ = serverTaskA; + _ = serverTaskB; + } + + private static McpClientOptions BuildSamplingClientOptions() => new() + { + Capabilities = new ClientCapabilities { Sampling = new SamplingCapability() }, + Handlers = new McpClientHandlers + { + SamplingHandler = static (request, _, _) => ValueTask.FromResult(new CreateMessageResult + { + Content = [new TextContentBlock { Text = "ga" }], + Model = "test-model", + }), + }, + }; + + private static async Task<(McpClient Client, Task ServerTask)> StartSessionAsync( + McpServerHandler handler, + McpClientOptions? clientOptions, + CancellationToken cancellationToken) + { + var clientToServer = new Pipe(); + var serverToClient = new Pipe(); + var io = new McpTestFixture.PipeIoContext( + clientToServer.Reader.AsStream(), + serverToClient.Writer.AsStream()); + var serverTask = handler.RunAsync(io, cancellationToken); + + var clientTransport = new StreamClientTransport( + clientToServer.Writer.AsStream(), + serverToClient.Reader.AsStream()); + var client = await McpClient.CreateAsync(clientTransport, clientOptions, cancellationToken: cancellationToken) + .ConfigureAwait(false); + return (client, serverTask); + } + [TestMethod] [Description("Two independent MCP sessions can run concurrently without interference.")] public async Task When_TwoSessionsRunConcurrently_Then_EachSeesOwnTools() diff --git a/src/Repl.McpTests/McpTestFixture.cs b/src/Repl.McpTests/McpTestFixture.cs index 04279eee..ab8b3c90 100644 --- a/src/Repl.McpTests/McpTestFixture.cs +++ b/src/Repl.McpTests/McpTestFixture.cs @@ -151,6 +151,13 @@ public async ValueTask DisposeAsync() _cts.Dispose(); } + internal static IServiceProvider EmptyServices => EmptyServiceProvider.Instance; + + private sealed class EmptyServiceProvider : IServiceProvider + { + public static readonly EmptyServiceProvider Instance = new(); + public object? GetService(Type serviceType) => null; + } internal sealed class PipeIoContext(Stream inputStream, Stream outputStream) : IReplIoContext { From d4ecc969d8e71eba8cef44690bfbaf272aa2ea70 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 16 Jul 2026 21:48:52 -0400 Subject: [PATCH 04/16] fix(mcp): legacy handshake regression, legacy-compat warnings, honest Tasks wording (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Regression pinning the last initialize-era protocol revision (2025-11-25): asserts the negotiated version and a tool list + call — the default client negotiates 2026-07-28 and never exercised the fallback path. - Roots/Sampling/Logging documented as legacy-compatibility only: deprecation notices in mcp-agent-capabilities.md and mcp-advanced.md steer new applications toward IReplInteractionChannel / soft roots; mcp-reference.md no longer reads as an endorsement. - Tasks wording corrected everywhere: the SDK has shipped the Tasks extension (ModelContextProtocol.Extensions.Tasks); what is pending is Repl's integration (issue #72) — including the CommandAnnotations.LongRunning XML doc that still promised task-based execution. --- docs/for-coding-agents.md | 2 +- docs/mcp-advanced.md | 7 ++++++ docs/mcp-agent-capabilities.md | 8 ++++++ docs/mcp-overview.md | 2 +- docs/mcp-reference.md | 4 +-- src/Repl.Core/CommandAnnotations.cs | 5 ++-- src/Repl.Mcp/README.md | 2 +- src/Repl.Mcp/ReplMcpServerTool.cs | 4 +-- src/Repl.McpTests/Given_McpIntegration.cs | 30 +++++++++++++++++++++++ 9 files changed, 55 insertions(+), 9 deletions(-) diff --git a/docs/for-coding-agents.md b/docs/for-coding-agents.md index 085d1230..3887440a 100644 --- a/docs/for-coding-agents.md +++ b/docs/for-coding-agents.md @@ -142,7 +142,7 @@ Use these annotations to help agents make safer decisions: | `.Destructive()` | May delete or mutate important state; ask for confirmation. | | `.Idempotent()` | Safe to retry. | | `.OpenWorld()` | Talks to external systems; expect latency and failures. | -| `.LongRunning()` | May take time. Documentation hint for now — no protocol-level task advertisement until the MCP Tasks runtime lands. | +| `.LongRunning()` | May take time. Documentation hint for now — no protocol-level task advertisement until Repl integrates the SDK Tasks extension. | | `.AutomationHidden()` | Do not expose this command to MCP automation. | | `.WithOption(name, o => o.AutomationHidden())` | Keep this one option out of the tool schema; the command stays visible. | diff --git a/docs/mcp-advanced.md b/docs/mcp-advanced.md index f82ebdd9..90ea3f62 100644 --- a/docs/mcp-advanced.md +++ b/docs/mcp-advanced.md @@ -20,6 +20,13 @@ If your tool list is static, stay with the default setup from [mcp-overview.md]( ## Client roots +> **⚠️ Deprecation notice (SEP-2577):** the MCP specification (2026-07-28) deprecates the +> Roots feature, and the SDK may remove it in a future version. Repl keeps supporting it +> **for existing hosts and applications only** — new applications should not build on +> native MCP roots and can use [soft roots](#soft-roots-fallback) or explicit command +> parameters instead. See +> [mcp-reference.md](mcp-reference.md#sdk-and-protocol-versions) for the version posture. + A **root** is a URI the client declares as being in scope for the session — typically an opened project folder, a working directory, or a boundary for what the agent should inspect or modify. Roots give the server session-specific workspace context without inventing a custom protocol. When the client supports native MCP roots, `Repl.Mcp` exposes them through `IMcpClientRoots`. diff --git a/docs/mcp-agent-capabilities.md b/docs/mcp-agent-capabilities.md index cac9ce4e..f4a1e8c4 100644 --- a/docs/mcp-agent-capabilities.md +++ b/docs/mcp-agent-capabilities.md @@ -8,6 +8,14 @@ See also: [sample 08-mcp-server](../samples/08-mcp-server/) for a working example that uses all three in a CSV import and feedback workflow. +> **⚠️ Deprecation notice (SEP-2577):** the MCP specification (2026-07-28) deprecates the +> Sampling and Logging features that `IMcpSampling` and `IMcpFeedback` build on, and the +> SDK may remove them in a future version. Repl keeps supporting them **for existing hosts +> and applications only** — new applications should not adopt these interfaces directly and +> should prefer the portable `IReplInteractionChannel`, which degrades gracefully across +> CLI, REPL, hosted sessions, and MCP. See +> [mcp-reference.md](mcp-reference.md#sdk-and-protocol-versions) for the version posture. + ## Overview Repl provides three MCP-oriented injectable interfaces: diff --git a/docs/mcp-overview.md b/docs/mcp-overview.md index 2abd8a1e..ac42537d 100644 --- a/docs/mcp-overview.md +++ b/docs/mcp-overview.md @@ -64,7 +64,7 @@ app.Map("deploy", handler).Destructive().LongRunning().OpenWorld(); | `.Destructive()` | Ask user for confirmation, sequential | | `.Idempotent()` | Safe to retry, can parallelize | | `.OpenWorld()` | Reaches external systems — expect latency and transient failures | -| `.LongRunning()` | Slow-operation hint (protocol-level task advertisement returns with the SDK Tasks runtime — see [mcp-reference.md](mcp-reference.md#sdk-and-protocol-versions)) | +| `.LongRunning()` | Slow-operation hint (protocol-level task advertisement returns once Repl integrates the SDK Tasks extension — see [mcp-reference.md](mcp-reference.md#sdk-and-protocol-versions)) | | `.AutomationHidden()` | Not visible to agents | **Annotate every command exposed to agents.** Unannotated tools force agents to assume the worst: confirm everything, no parallelism, no retries. diff --git a/docs/mcp-reference.md b/docs/mcp-reference.md index 76e4893e..845575ad 100644 --- a/docs/mcp-reference.md +++ b/docs/mcp-reference.md @@ -515,8 +515,8 @@ Feature support varies across agents. Check [mcp-availability.com](https://mcp-a ### SDK and protocol versions - Repl.Mcp builds on the official C# SDK (`ModelContextProtocol`), currently on the **2.0 line** (`2.0.0-preview.3`). The SDK negotiates the protocol version with each client, including fallback to the legacy `initialize` handshake for older hosts. -- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577) but remain fully functional; Repl.Mcp keeps supporting them until the SDK removes them, since current hosts still rely on these features. -- **MCP Tasks**: SDK 2.0 removed the experimental tool-execution augmentation (`Tool.Execution`), so `.LongRunning()` commands no longer advertise task support at the protocol level. The annotation stays in Repl's own model (help/docs), and protocol-level task support will return once the SDK ships its Tasks runtime. +- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577). Repl.Mcp keeps supporting them **for existing hosts and applications only** — new applications should not adopt these features (the SDK may remove them) and should prefer Repl's portable abstractions such as `IReplInteractionChannel`. The designated successor for server-initiated flows (SEP-2322, multi-round-trip requests) is not yet consumable in the SDK. +- **MCP Tasks**: the SDK reorganized Tasks into `ModelContextProtocol.Extensions.Tasks` and dropped the per-tool execution augmentation (`Tool.Execution`) from the protocol surface, so `.LongRunning()` commands no longer advertise task support at the protocol level. The annotation stays in Repl's own model (help/docs); protocol-level task support can return once Repl integrates the Tasks extension, store, and get/update/cancel lifecycle (tracked in issue #72). | Feature | Claude Desktop | Claude Code | Codex | VS Code Copilot | Cursor | Continue | |---|---|---|---|---|---|---| diff --git a/src/Repl.Core/CommandAnnotations.cs b/src/Repl.Core/CommandAnnotations.cs index 9b7a5dbf..8c66dad0 100644 --- a/src/Repl.Core/CommandAnnotations.cs +++ b/src/Repl.Core/CommandAnnotations.cs @@ -31,8 +31,9 @@ public sealed record CommandAnnotations public bool OpenWorld { get; init; } /// - /// Indicates the command may take a long time to complete. - /// Enables task-based execution in programmatic clients. + /// Indicates the command may take a long time to complete, so programmatic clients + /// should expect a slow call. Protocol-level task-based execution (MCP Tasks) is not + /// advertised until Repl integrates the SDK's Tasks extension. /// public bool LongRunning { get; init; } diff --git a/src/Repl.Mcp/README.md b/src/Repl.Mcp/README.md index 8ba4862f..09dfb2eb 100644 --- a/src/Repl.Mcp/README.md +++ b/src/Repl.Mcp/README.md @@ -104,7 +104,7 @@ app.Map("debug reset", handler) .AutomationHidden(); ``` -Unannotated tools force agents to assume the worst. Use `.ReadOnly()` for safe queries, `.Destructive()` for important mutations, `.OpenWorld()` for external systems, `.LongRunning()` for slow operations (a documentation hint today — protocol-level MCP task advertisement returns with the SDK Tasks runtime), and `.AutomationHidden()` for commands that should stay available to humans but invisible to MCP automation. +Unannotated tools force agents to assume the worst. Use `.ReadOnly()` for safe queries, `.Destructive()` for important mutations, `.OpenWorld()` for external systems, `.LongRunning()` for slow operations (a documentation hint today — protocol-level MCP task advertisement returns once Repl integrates the SDK Tasks extension), and `.AutomationHidden()` for commands that should stay available to humans but invisible to MCP automation. Prefer returning JSON-friendly objects instead of writing prose-only output. Structured results are easier for agents to inspect, retry, test, and summarize. diff --git a/src/Repl.Mcp/ReplMcpServerTool.cs b/src/Repl.Mcp/ReplMcpServerTool.cs index c5e3bbf7..a6376770 100644 --- a/src/Repl.Mcp/ReplMcpServerTool.cs +++ b/src/Repl.Mcp/ReplMcpServerTool.cs @@ -17,8 +17,8 @@ internal sealed class ReplMcpServerTool : McpServerTool // SDK 2.0 extracted MCP Tasks into ModelContextProtocol.Extensions.Tasks (store, task // results, client polling) and dropped the per-tool Tool.Execution / ToolTaskSupport // augmentation from the protocol surface. Repl keeps .LongRunning() in its own model - // (help/docs) and deliberately does not advertise task support until the Tasks runtime - // (tasks/get|update|cancel) is implemented end-to-end — tracked in issue #51. + // (help/docs) and deliberately does not advertise task support until Repl integrates + // the Tasks extension end-to-end (tasks/get|update|cancel) — tracked in issue #72. public ReplMcpServerTool( ReplDocCommand command, string toolName, diff --git a/src/Repl.McpTests/Given_McpIntegration.cs b/src/Repl.McpTests/Given_McpIntegration.cs index ce00729d..22cd245d 100644 --- a/src/Repl.McpTests/Given_McpIntegration.cs +++ b/src/Repl.McpTests/Given_McpIntegration.cs @@ -1,3 +1,5 @@ +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; using Repl.Mcp; namespace Repl.McpTests; @@ -120,6 +122,34 @@ public void When_EnrichedCommands_Then_DocModelContainsAllFields() cmd.Arguments.Should().ContainSingle(a => string.Equals(a.Name, "env", StringComparison.Ordinal)); } + [TestMethod] + [Description("Locks the legacy initialize handshake under SDK 2.0: a client pinning an initialize-era protocol revision still negotiates that exact version and can list and call tools — the fixture's default client otherwise negotiates the 2026-07-28 path and never exercises the fallback.")] + public async Task When_ClientPinsLegacyProtocolVersion_Then_InitializeHandshakeAndToolsWork() + { + // 2025-11-25 is the last initialize-era protocol revision (the SDK's + // McpProtocolVersions constants are internal, so the literal is pinned here). + const string legacyProtocolVersion = "2025-11-25"; + var clientOptions = new McpClientOptions + { + ProtocolVersion = legacyProtocolVersion, + }; + + await using var fixture = await McpTestFixture.CreateAsync( + app => app.Map("ping", () => "pong"), + configureOptions: null, + clientOptions: clientOptions); + + fixture.Client.NegotiatedProtocolVersion.Should().Be(legacyProtocolVersion); + + var tools = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + tools.Should().ContainSingle(tool => string.Equals(tool.Name, "ping", StringComparison.Ordinal)); + + var result = await fixture.Client.CallToolAsync( + toolName: "ping", + arguments: new Dictionary(StringComparer.Ordinal)).ConfigureAwait(false); + result.Content.OfType().First().Text.Should().Contain("pong"); + } + [TestMethod] [Description("Locks the SDK-2.0 tools/list wire shape for .LongRunning() commands: annotations survive serialization, and no task/execution augmentation is emitted — Repl deliberately does not advertise MCP task support until the Tasks runtime is implemented end-to-end (issue #51).")] public void When_SerializingLongRunningTool_Then_NoTaskAugmentationIsEmitted() From 69898e59d0dc909b7c79e983e3836f6680ad5d45 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Fri, 17 Jul 2026 10:18:26 -0400 Subject: [PATCH 05/16] fix(mcp): per-session roots cache + reference-counted session lifecycle (review) - Hard roots are now SESSION state: entries keyed by destination server in a ConditionalWeakTable (weak keys die with the session), with a global version stamp for roots-list-changed invalidation. One session can no longer receive another session's cached workspace roots, and the root-dependent snapshot builds from the right workspace (RED observed: client B received client A's roots). - Session attachment is reference-counted: the handler tracks every active session, discovery notifications fan out to ALL of them, the accessor fallback moves to a surviving session on close, and the routing subscription is dropped only when the LAST session ends. A first-session close no longer silences the survivors (RED observed: surviving session timed out waiting for tools/list_changed). - Roots list-changed handler registered once per session (per-server registration replaces the single global flag). --- src/Repl.Mcp/McpClientRootsService.cs | 51 +++++--- src/Repl.Mcp/McpRequestServerAccessor.cs | 4 +- src/Repl.Mcp/McpServerHandler.cs | 75 +++++++----- .../Given_McpConcurrentSessions.cs | 109 ++++++++++++++++++ 4 files changed, 195 insertions(+), 44 deletions(-) diff --git a/src/Repl.Mcp/McpClientRootsService.cs b/src/Repl.Mcp/McpClientRootsService.cs index 8db57690..41f0e7d4 100644 --- a/src/Repl.Mcp/McpClientRootsService.cs +++ b/src/Repl.Mcp/McpClientRootsService.cs @@ -14,10 +14,15 @@ internal sealed class McpClientRootsService : IMcpClientRoots private readonly ICoreReplApp _app; private readonly McpRequestServerAccessor _servers; private readonly Lock _syncRoot = new(); - private McpClientRoot[] _hardRoots = []; + // Hard roots are SESSION state: one handler can serve several root-capable sessions, + // and serving session A's cached roots to session B would expose A's workspace URIs + // and build B's root-dependent snapshot from the wrong workspace. Entries are keyed by + // the destination server (weak keys — they die with the session); a single global + // version stamp invalidates every entry on any roots-list change (coarse, but the + // event is rare and correctness beats granularity here). + private readonly System.Runtime.CompilerServices.ConditionalWeakTable _sessionRoots = []; private McpClientRoot[] _softRoots = []; - private bool _hardRootsLoaded; - private long _hardRootsVersion; + private long _rootsVersion; public McpClientRootsService(ICoreReplApp app, McpRequestServerAccessor servers) { @@ -25,6 +30,13 @@ public McpClientRootsService(ICoreReplApp app, McpRequestServerAccessor servers) _servers = servers; } + private sealed class SessionRoots + { + public McpClientRoot[] Roots = []; + public bool Loaded; + public long LoadedVersion; + } + public bool IsSupported => _servers.Effective?.ClientCapabilities?.Roots is not null; public bool HasSoftRoots @@ -42,9 +54,16 @@ public IReadOnlyList Current { get { + var server = _servers.Effective; lock (_syncRoot) { - return IsSupported ? _hardRoots : _softRoots; + if (server?.ClientCapabilities?.Roots is null) + { + return _softRoots; + } + + var entry = _sessionRoots.GetOrCreateValue(server); + return entry.Loaded && entry.LoadedVersion == _rootsVersion ? entry.Roots : []; } } } @@ -59,18 +78,18 @@ public async ValueTask> GetAsync(CancellationToken return Current; } + var entry = _sessionRoots.GetOrCreateValue(server); long versionAtStart; lock (_syncRoot) { - if (_hardRootsLoaded) + versionAtStart = _rootsVersion; + if (entry.Loaded && entry.LoadedVersion == versionAtStart) { - return _hardRoots; + return entry.Roots; } - - versionAtStart = _hardRootsVersion; } - return await GetAndMaybeCacheRootsAsync(server, versionAtStart, cancellationToken).ConfigureAwait(false); + return await GetAndMaybeCacheRootsAsync(server, entry, versionAtStart, cancellationToken).ConfigureAwait(false); } public void SetSoftRoots(IEnumerable roots) @@ -116,9 +135,7 @@ public void HandleRootsListChanged() { lock (_syncRoot) { - _hardRoots = []; - _hardRootsLoaded = false; - _hardRootsVersion++; + _rootsVersion++; } _app.InvalidateRouting(); @@ -126,6 +143,7 @@ public void HandleRootsListChanged() private async ValueTask> GetAndMaybeCacheRootsAsync( McpServer server, + SessionRoots entry, long versionAtStart, CancellationToken cancellationToken) { @@ -135,11 +153,12 @@ private async ValueTask> GetAndMaybeCacheRootsAsync lock (_syncRoot) { - if (_hardRootsVersion == versionAtStart) + if (_rootsVersion == versionAtStart) { - _hardRoots = mappedRoots; - _hardRootsLoaded = true; - return _hardRoots; + entry.Roots = mappedRoots; + entry.Loaded = true; + entry.LoadedVersion = versionAtStart; + return entry.Roots; } return mappedRoots; diff --git a/src/Repl.Mcp/McpRequestServerAccessor.cs b/src/Repl.Mcp/McpRequestServerAccessor.cs index 91ad2429..be138772 100644 --- a/src/Repl.Mcp/McpRequestServerAccessor.cs +++ b/src/Repl.Mcp/McpRequestServerAccessor.cs @@ -26,6 +26,6 @@ internal sealed class McpRequestServerAccessor /// Binds the flowing async context to the request's destination server. public void BindRequest(McpServer server) => _current.Value = server; - /// Records the session-level server used outside request flows. - public void AttachSession(McpServer server) => _session = server; + /// Records the session-level server used outside request flows (null when the last session ends). + public void AttachSession(McpServer? server) => _session = server; } diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index d27c9661..571fcc93 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -40,10 +40,12 @@ internal sealed class McpServerHandler private McpGeneratedSnapshot? _snapshot; private SnapshotVersionState _snapshotState = new(Version: 1, LastVisibilityRetractionVersion: 0); private long _builtSnapshotVersion; - private McpServer? _server; + // One handler can serve several concurrent sessions; session-scoped concerns (routing + // notifications, roots list-changed handler) track EVERY active session, not a single + // last- or first-attached server. Guarded by _attachLock. + private readonly List _sessions = []; private EventHandler? _routingChangedHandler; private ITimer? _debounceTimer; - private int _rootsNotificationRegistered; private int _compatibilityIntroServed; private static readonly TimeSpan DebounceDelay = TimeSpan.FromMilliseconds(100); @@ -99,7 +101,7 @@ public async Task RunAsync(IReplIoContext io, CancellationToken ct) } finally { - UnsubscribeFromRoutingChanges(); + DetachSession(server); await server.DisposeAsync().ConfigureAwait(false); } } @@ -485,15 +487,21 @@ private void BindRequestServer(McpServer? server) } _requestServers.BindRequest(server); - if (_server is null) + bool needsSessionAttach; + lock (_attachLock) + { + needsSessionAttach = _sessions.Count == 0; + } + + if (needsSessionAttach) { AttachServer(server); } } // Session-level attach: routing-change notifications and the roots list-changed - // handler belong to the session server, registered once — never to the per-request - // destination wrappers. + // handler belong to the session servers, registered once per session — never to the + // per-request destination wrappers. private void AttachServer(McpServer? server) { if (server is null) @@ -503,12 +511,12 @@ private void AttachServer(McpServer? server) lock (_attachLock) { - if (ReferenceEquals(_server, server)) + if (_sessions.Contains(server)) { return; } - _server = server; + _sessions.Add(server); _requestServers.AttachSession(server); EnsureRoutingSubscription(); EnsureRootsNotificationHandler(server); @@ -543,13 +551,24 @@ private void EnsureRoutingSubscription() coreApp.RoutingInvalidatedDetailed += handler; } - private void EnsureRootsNotificationHandler(McpServer server) + // Removes a closing session and repairs the shared state: the accessor's session + // fallback moves to a surviving session, and the routing subscription is dropped only + // when the LAST session ends — a first-session close must not silence the others. + private void DetachSession(McpServer server) { - if (Interlocked.Exchange(ref _rootsNotificationRegistered, 1) != 0) + lock (_attachLock) { - return; + _sessions.Remove(server); + _requestServers.AttachSession(_sessions.Count > 0 ? _sessions[^1] : null); + if (_sessions.Count == 0) + { + UnsubscribeFromRoutingChanges(); + } } + } + private void EnsureRootsNotificationHandler(McpServer server) + { var weakSelf = new WeakReference(this); // Roots is deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005) but hosts still send // this notification; Repl keeps supporting it until the SDK removes the surface (#51). @@ -622,24 +641,28 @@ private async Task SendDiscoveryNotificationsSafeAsync() private async Task SendNotificationSafeAsync(string method) { - try - { - var server = _server; - if (server is null) - { - return; - } - - using var timeoutCts = new CancellationTokenSource(NotificationSendTimeout); - await server.SendNotificationAsync(method, timeoutCts.Token).ConfigureAwait(false); - } - catch (OperationCanceledException) + McpServer[] sessions; + lock (_attachLock) { - // Notifications are best-effort. Cancellation is not actionable here. + sessions = [.. _sessions]; } - catch (Exception) + + foreach (var server in sessions) { - // Notifications are best-effort. The next list/read request will rebuild on demand. + try + { + using var timeoutCts = new CancellationTokenSource(NotificationSendTimeout); + await server.SendNotificationAsync(method, timeoutCts.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Notifications are best-effort. Cancellation is not actionable here. + } + catch (Exception) + { + // Notifications are best-effort per session. The next list/read request + // will rebuild on demand. + } } } diff --git a/src/Repl.McpTests/Given_McpConcurrentSessions.cs b/src/Repl.McpTests/Given_McpConcurrentSessions.cs index 1bffbc75..1b45ddc4 100644 --- a/src/Repl.McpTests/Given_McpConcurrentSessions.cs +++ b/src/Repl.McpTests/Given_McpConcurrentSessions.cs @@ -69,6 +69,115 @@ await clientB.CallToolAsync( _ = serverTaskB; } + [TestMethod] + [Description("Guards root isolation across sessions sharing one handler: the hard-roots cache must be keyed by session, otherwise the second root-capable client silently receives the FIRST client's workspace roots — a cross-session data exposure — instead of its own roots/list round-trip.")] + public async Task When_TwoRootCapableClientsShareHandler_Then_EachSeesOwnRoots() + { + var app = ReplApp.Create(); + app.UseMcpServer(); + app.Map("roots", async (IMcpClientRoots roots, CancellationToken ct) => + string.Join(',', (await roots.GetAsync(ct).ConfigureAwait(false)).Select(root => root.Uri.ToString()))); + + var options = new ReplMcpServerOptions + { + TransportFactory = static (serverName, io) => new StreamServerTransport( + ((McpTestFixture.PipeIoContext)io).InputStream, + ((McpTestFixture.PipeIoContext)io).OutputStream, + serverName), + }; + var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + using var cts = new CancellationTokenSource(); + + var (clientA, _) = await StartSessionAsync(handler, BuildRootsClientOptions("file:///ga"), cts.Token).ConfigureAwait(false); + var (clientB, _) = await StartSessionAsync(handler, BuildRootsClientOptions("file:///bu"), cts.Token).ConfigureAwait(false); + + var resultA = await clientA.CallToolAsync( + toolName: "roots", + arguments: new Dictionary(StringComparer.Ordinal), + cancellationToken: cts.Token).ConfigureAwait(false); + var resultB = await clientB.CallToolAsync( + toolName: "roots", + arguments: new Dictionary(StringComparer.Ordinal), + cancellationToken: cts.Token).ConfigureAwait(false); + + resultA.Content.OfType().First().Text.Should().Contain("file:///ga"); + var textB = resultB.Content.OfType().First().Text; + textB.Should().Contain("file:///bu"); + textB.Should().NotContain("file:///ga"); + + await clientA.DisposeAsync().ConfigureAwait(false); + await clientB.DisposeAsync().ConfigureAwait(false); + await cts.CancelAsync().ConfigureAwait(false); + } + + [TestMethod] + [Description("Guards routing-notification lifetime across sessions: when the first-attached session closes, the surviving session must still receive tools/list_changed after a routing invalidation — session attachment must be reference-counted, not first-wins with a handler-wide unsubscribe on first close.")] + public async Task When_FirstSessionCloses_Then_SurvivingSessionStillReceivesRoutingNotifications() + { + var app = ReplApp.Create(); + app.UseMcpServer(); + app.Map("alpha", () => "a"); + + var options = new ReplMcpServerOptions + { + TransportFactory = static (serverName, io) => new StreamServerTransport( + ((McpTestFixture.PipeIoContext)io).InputStream, + ((McpTestFixture.PipeIoContext)io).OutputStream, + serverName), + }; + var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + using var ctsA = new CancellationTokenSource(); + using var ctsB = new CancellationTokenSource(); + + var (clientA, serverTaskA) = await StartSessionAsync(handler, clientOptions: null, ctsA.Token).ConfigureAwait(false); + var (clientB, _) = await StartSessionAsync(handler, clientOptions: null, ctsB.Token).ConfigureAwait(false); + + var listChanged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var registration = clientB.RegisterNotificationHandler( + NotificationMethods.ToolListChangedNotification, + (_, _) => + { + listChanged.TrySetResult(); + return ValueTask.CompletedTask; + }); + await using var _ = registration.ConfigureAwait(false); + + // Both sessions are live; close the FIRST one, then invalidate routing. + await clientA.DisposeAsync().ConfigureAwait(false); + await ctsA.CancelAsync().ConfigureAwait(false); + try + { + await serverTaskA.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Expected: session A's RunAsync ends on cancellation. + } + + app.Map("late", () => "l"); + app.Core.InvalidateRouting(); + + await listChanged.Task.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); + + await clientB.DisposeAsync().ConfigureAwait(false); + await ctsB.CancelAsync().ConfigureAwait(false); + } + + private static McpClientOptions BuildRootsClientOptions(string rootUri) => new() + { + Capabilities = new ClientCapabilities + { + Roots = new RootsCapability { ListChanged = true }, + }, + Handlers = new McpClientHandlers + { + RootsHandler = (_, _) => ValueTask.FromResult(new ListRootsResult + { + Roots = [new Root { Uri = rootUri, Name = rootUri }], + }), + }, + }; + private static McpClientOptions BuildSamplingClientOptions() => new() { Capabilities = new ClientCapabilities { Sampling = new SamplingCapability() }, From 48bf7fdd385c953658d4e095567f3e5adf8ea17e Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Fri, 17 Jul 2026 11:47:53 -0400 Subject: [PATCH 06/16] refactor(mcp): McpSessionContext owns all per-session state (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One handler serves several sessions; everything that varied per client was still handler-global after the earlier point fixes. McpSessionContext now owns it all, per the architecture review: - hard AND soft roots: McpClientRootsService is one instance per session (plain fields again — the ConditionalWeakTable keying is gone); a session's 'workspace init' no longer sets another session's workspace. - generated snapshot + version + gate: the tool graph can be gated on session capabilities, so each session caches its own build against the handler-global routing version (RED observed: the roots-less session saw the roots-gated tool of the other session). - compatibility-shim intro: per-session flag, reset for every active session on routing invalidation (RED observed: only the first session received the discover_tools/call_tool intro). - per-session service overlay handed to McpServer.Create; request handlers recover their session through request.Server.Services instead of using a destination-bound per-request server as a surrogate session key. - externally hosted servers (BuildDynamicServerOptions) share one explicit lazy fallback context instead of racing a last-attached field. Request-bound OUTBOUND capabilities (sampling/elicitation/feedback) keep flowing through the per-request AsyncLocal accessor — finer than the session, unchanged. Related to #70 (per-session DI scopes generalize the lifetime contract; this context will construct from the session-scoped provider once both merge). --- src/Repl.Mcp/McpClientRootsService.cs | 57 ++-- src/Repl.Mcp/McpServerHandler.cs | 243 +++++++++++------- src/Repl.Mcp/McpSessionContext.cs | 49 ++++ .../Given_McpConcurrentSessions.cs | 72 ++++++ 4 files changed, 290 insertions(+), 131 deletions(-) create mode 100644 src/Repl.Mcp/McpSessionContext.cs diff --git a/src/Repl.Mcp/McpClientRootsService.cs b/src/Repl.Mcp/McpClientRootsService.cs index 41f0e7d4..c59b9973 100644 --- a/src/Repl.Mcp/McpClientRootsService.cs +++ b/src/Repl.Mcp/McpClientRootsService.cs @@ -9,20 +9,21 @@ 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(); - // Hard roots are SESSION state: one handler can serve several root-capable sessions, - // and serving session A's cached roots to session B would expose A's workspace URIs - // and build B's root-dependent snapshot from the wrong workspace. Entries are keyed by - // the destination server (weak keys — they die with the session); a single global - // version stamp invalidates every entry on any roots-list change (coarse, but the - // event is rare and correctness beats granularity here). - private readonly System.Runtime.CompilerServices.ConditionalWeakTable _sessionRoots = []; + private McpClientRoot[] _hardRoots = []; private McpClientRoot[] _softRoots = []; - private long _rootsVersion; + private bool _hardRootsLoaded; + private long _hardRootsVersion; public McpClientRootsService(ICoreReplApp app, McpRequestServerAccessor servers) { @@ -30,13 +31,6 @@ public McpClientRootsService(ICoreReplApp app, McpRequestServerAccessor servers) _servers = servers; } - private sealed class SessionRoots - { - public McpClientRoot[] Roots = []; - public bool Loaded; - public long LoadedVersion; - } - public bool IsSupported => _servers.Effective?.ClientCapabilities?.Roots is not null; public bool HasSoftRoots @@ -54,16 +48,9 @@ public IReadOnlyList Current { get { - var server = _servers.Effective; lock (_syncRoot) { - if (server?.ClientCapabilities?.Roots is null) - { - return _softRoots; - } - - var entry = _sessionRoots.GetOrCreateValue(server); - return entry.Loaded && entry.LoadedVersion == _rootsVersion ? entry.Roots : []; + return IsSupported ? _hardRoots : _softRoots; } } } @@ -78,18 +65,18 @@ public async ValueTask> GetAsync(CancellationToken return Current; } - var entry = _sessionRoots.GetOrCreateValue(server); long versionAtStart; lock (_syncRoot) { - versionAtStart = _rootsVersion; - if (entry.Loaded && entry.LoadedVersion == versionAtStart) + if (_hardRootsLoaded) { - return entry.Roots; + return _hardRoots; } + + versionAtStart = _hardRootsVersion; } - return await GetAndMaybeCacheRootsAsync(server, entry, versionAtStart, cancellationToken).ConfigureAwait(false); + return await GetAndMaybeCacheRootsAsync(server, versionAtStart, cancellationToken).ConfigureAwait(false); } public void SetSoftRoots(IEnumerable roots) @@ -135,7 +122,9 @@ public void HandleRootsListChanged() { lock (_syncRoot) { - _rootsVersion++; + _hardRoots = []; + _hardRootsLoaded = false; + _hardRootsVersion++; } _app.InvalidateRouting(); @@ -143,7 +132,6 @@ public void HandleRootsListChanged() private async ValueTask> GetAndMaybeCacheRootsAsync( McpServer server, - SessionRoots entry, long versionAtStart, CancellationToken cancellationToken) { @@ -153,12 +141,11 @@ private async ValueTask> GetAndMaybeCacheRootsAsync lock (_syncRoot) { - if (_rootsVersion == versionAtStart) + if (_hardRootsVersion == versionAtStart) { - entry.Roots = mappedRoots; - entry.Loaded = true; - entry.LoadedVersion = versionAtStart; - return entry.Roots; + _hardRoots = mappedRoots; + _hardRootsLoaded = true; + return _hardRoots; } return mappedRoots; diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index 571fcc93..5cbf9dab 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -28,25 +28,26 @@ internal sealed class McpServerHandler private readonly TimeProvider _timeProvider; private readonly char _separator; private readonly McpRequestServerAccessor _requestServers = new(); - private readonly McpClientRootsService _roots; private readonly McpSamplingService _sampling; private readonly McpElicitationService _elicitation; private readonly McpFeedbackService _feedback; - private readonly IServiceProvider _sessionServices; - private readonly SemaphoreSlim _snapshotGate = new(initialCount: 1, maxCount: 1); private readonly Lock _refreshLock = new(); private readonly Lock _attachLock = new(); - private McpGeneratedSnapshot? _snapshot; + // Global routing version: bumped by InvalidateRouting for every session; each session's + // context caches the snapshot it built at a given version. private SnapshotVersionState _snapshotState = new(Version: 1, LastVisibilityRetractionVersion: 0); - private long _builtSnapshotVersion; - // One handler can serve several concurrent sessions; session-scoped concerns (routing - // notifications, roots list-changed handler) track EVERY active session, not a single - // last- or first-attached server. Guarded by _attachLock. - private readonly List _sessions = []; + // One handler can serve several concurrent sessions; everything session-owned lives in + // McpSessionContext, and this list (guarded by _attachLock) tracks every ACTIVE session + // for server-initiated notifications and subscription lifetime. + private readonly List _sessions = []; + // Lazy single context for externally hosted servers (options built via + // BuildDynamicServerOptions and run by the host without RunAsync): those servers carry + // the HOST's provider, so requests cannot recover a per-session context from it — they + // share one explicit fallback context instead of racing a last-attached field. + private McpSessionContext? _externalContext; private EventHandler? _routingChangedHandler; private ITimer? _debounceTimer; - private int _compatibilityIntroServed; private static readonly TimeSpan DebounceDelay = TimeSpan.FromMilliseconds(100); // Notifications are fire-and-forget best-effort — a stuck stdio peer must not hang this @@ -64,19 +65,61 @@ public McpServerHandler( _services = services; _timeProvider = services.GetService(typeof(TimeProvider)) as TimeProvider ?? TimeProvider.System; _separator = McpToolNameFlattener.ResolveSeparator(options.ToolNamingSeparator); - _roots = new McpClientRootsService(app, _requestServers); + // Sampling/elicitation/feedback are stateless (they resolve the request-bound server + // through the accessor) and safely shared; roots and everything else session-owned + // is created per session in CreateSessionContext. _sampling = new McpSamplingService(_requestServers); _elicitation = new McpElicitationService(_requestServers); _feedback = new McpFeedbackService(_requestServers); - _sessionServices = new McpServiceProviderOverlay( - services, - new Dictionary + } + + private McpSessionContext CreateSessionContext() + { + var roots = new McpClientRootsService(_app, _requestServers); + var overlayServices = new Dictionary + { + [typeof(IMcpClientRoots)] = roots, + [typeof(IMcpSampling)] = _sampling, + [typeof(IMcpElicitation)] = _elicitation, + [typeof(IMcpFeedback)] = _feedback, + }; + var context = new McpSessionContext(roots, new McpServiceProviderOverlay(_services, overlayServices)); + // The context rides in its own overlay so request handlers can recover their + // originating session through the server's provider (the dictionary is captured by + // reference, making this two-phase registration safe). + overlayServices[typeof(McpSessionContext)] = context; + return context; + } + + // Requests recover their session through the provider handed to McpServer.Create — + // even a destination-bound per-request server exposes its session's services. Servers + // created by an external host (BuildDynamicServerOptions) carry the host's provider + // instead and share the explicit fallback context. + private McpSessionContext ResolveContext(McpServer? requestServer) + { + if (requestServer?.Services?.GetService(typeof(McpSessionContext)) is McpSessionContext context) + { + return context; + } + + lock (_attachLock) + { + if (_externalContext is null) { - [typeof(IMcpClientRoots)] = _roots, - [typeof(IMcpSampling)] = _sampling, - [typeof(IMcpElicitation)] = _elicitation, - [typeof(IMcpFeedback)] = _feedback, - }); + _externalContext = CreateSessionContext(); + _sessions.Add(_externalContext); + EnsureRoutingSubscription(); + } + + if (_externalContext.SessionServer is null && requestServer is not null) + { + _externalContext.SessionServer = requestServer; + _requestServers.AttachSession(requestServer); + EnsureRootsNotificationHandler(requestServer, _externalContext.Roots); + } + + return _externalContext; + } } [UnconditionalSuppressMessage( @@ -92,8 +135,10 @@ public async Task RunAsync(IReplIoContext io, CancellationToken ct) : new StdioServerTransport(serverName); try { - var server = McpServer.Create(transport, serverOptions, serviceProvider: _sessionServices); - AttachServer(server); + var context = CreateSessionContext(); + var server = McpServer.Create(transport, serverOptions, serviceProvider: context.Services); + context.SessionServer = server; + AttachSession(context, server); try { @@ -101,7 +146,7 @@ public async Task RunAsync(IReplIoContext io, CancellationToken ct) } finally { - DetachSession(server); + DetachSession(context); await server.DisposeAsync().ConfigureAwait(false); } } @@ -121,7 +166,7 @@ internal McpServerOptions BuildDynamicServerOptions() // then repeated for the same commands during the first discovery request. if (_options.CommandFilter is null) { - _ = CreateDocumentationModel(); + _ = CreateDocumentationModel(CreateSessionContext().Services); } return new McpServerOptions @@ -145,7 +190,7 @@ internal McpServerOptions BuildStaticServerOptions() { var serverName = _options.ServerName ?? ResolveAppName() ?? "repl-mcp-server"; var serverVersion = _options.ServerVersion ?? "1.0.0"; - var snapshot = BuildSnapshotCore(); + var snapshot = BuildSnapshotCore(CreateSessionContext()); return new McpServerOptions { @@ -157,10 +202,10 @@ internal McpServerOptions BuildStaticServerOptions() }; } - internal McpGeneratedSnapshot BuildSnapshotForTests() => BuildSnapshotCore(); + internal McpGeneratedSnapshot BuildSnapshotForTests() => BuildSnapshotCore(CreateSessionContext()); internal async Task BuildSnapshotForTestsAsync(CancellationToken cancellationToken = default) => - await GetSnapshotAsync(server: null, cancellationToken).ConfigureAwait(false); + await GetSnapshotAsync(CreateSessionContext(), cancellationToken).ConfigureAwait(false); private string? ResolveAppName() { @@ -174,10 +219,11 @@ private async ValueTask ListToolsAsync( CancellationToken cancellationToken) { BindRequestServer(request.Server); - var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); + var context = ResolveContext(request.Server); + var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); if (_options.DynamicToolCompatibility == DynamicToolCompatibilityMode.DiscoverAndCallShim - && Interlocked.CompareExchange(ref _compatibilityIntroServed, 1, 0) == 0) + && Interlocked.CompareExchange(ref context.CompatibilityIntroServed, 1, 0) == 0) { _ = SendNotificationSafeAsync(NotificationMethods.ToolListChangedNotification); return new ListToolsResult @@ -201,7 +247,8 @@ private async ValueTask CallToolAsync( CancellationToken cancellationToken) { BindRequestServer(request.Server); - var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); + var context = ResolveContext(request.Server); + var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); IDictionary arguments = request.Params.Arguments ?? EmptyArguments; var toolName = request.Params.Name ?? string.Empty; var progressToken = request.Params.ProgressToken; @@ -233,7 +280,8 @@ private async ValueTask ListResourcesAsync( CancellationToken cancellationToken) { BindRequestServer(request.Server); - var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); + var context = ResolveContext(request.Server); + var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); return new ListResourcesResult { Resources = @@ -250,7 +298,8 @@ private async ValueTask ListResourceTemplatesAsync( CancellationToken cancellationToken) { BindRequestServer(request.Server); - var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); + var context = ResolveContext(request.Server); + var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); return new ListResourceTemplatesResult { ResourceTemplates = @@ -267,7 +316,8 @@ private async ValueTask ReadResourceAsync( CancellationToken cancellationToken) { BindRequestServer(request.Server); - var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); + var context = ResolveContext(request.Server); + var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); var uri = request.Params.Uri ?? string.Empty; var resource = snapshot.Resources.FirstOrDefault(candidate => candidate.IsMatch(uri)); if (resource is null) @@ -283,7 +333,8 @@ private async ValueTask ListPromptsAsync( CancellationToken cancellationToken) { BindRequestServer(request.Server); - var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); + var context = ResolveContext(request.Server); + var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); return new ListPromptsResult { Prompts = [.. snapshot.Prompts.Select(static prompt => prompt.ProtocolPrompt)], @@ -295,7 +346,8 @@ private async ValueTask GetPromptAsync( CancellationToken cancellationToken) { BindRequestServer(request.Server); - var snapshot = await GetSnapshotAsync(request.Server, cancellationToken).ConfigureAwait(false); + var context = ResolveContext(request.Server); + var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); var promptName = request.Params.Name ?? string.Empty; var prompt = snapshot.Prompts.FirstOrDefault(candidate => string.Equals(candidate.ProtocolPrompt.Name, promptName, StringComparison.OrdinalIgnoreCase)); @@ -307,33 +359,34 @@ private async ValueTask GetPromptAsync( return await prompt.GetAsync(request, cancellationToken).ConfigureAwait(false); } + // The snapshot is SESSION state: the tool graph can be gated on session capabilities + // (roots, module presence predicates), so each context caches its own build against + // the handler-global routing version. private async ValueTask GetSnapshotAsync( - McpServer? server, + McpSessionContext context, CancellationToken cancellationToken) { - BindRequestServer(server); - var snapshotVersion = Volatile.Read(ref _snapshotState).Version; - if (Volatile.Read(ref _builtSnapshotVersion) == snapshotVersion - && _snapshot is { } cached) + if (context.BuiltSnapshotVersion == snapshotVersion + && context.Snapshot is { } cached) { return cached; } - await _snapshotGate.WaitAsync(cancellationToken).ConfigureAwait(false); + await context.SnapshotGate.WaitAsync(cancellationToken).ConfigureAwait(false); try { snapshotVersion = Volatile.Read(ref _snapshotState).Version; - if (Volatile.Read(ref _builtSnapshotVersion) == snapshotVersion - && _snapshot is { } refreshed) + if (context.BuiltSnapshotVersion == snapshotVersion + && context.Snapshot is { } refreshed) { return refreshed; } - var previousSnapshot = _snapshot; + var previousSnapshot = context.Snapshot; try { - return await BuildCurrentSnapshotAsync(snapshotVersion, cancellationToken).ConfigureAwait(false); + return await BuildCurrentSnapshotAsync(context, snapshotVersion, cancellationToken).ConfigureAwait(false); } catch (OperationCanceledException) { @@ -347,17 +400,17 @@ private async ValueTask GetSnapshotAsync( catch (Exception) when ( previousSnapshot is not null && Volatile.Read(ref _snapshotState).LastVisibilityRetractionVersion - <= Volatile.Read(ref _builtSnapshotVersion)) + <= context.BuiltSnapshotVersion) { // Preserve availability for transient projection failures, but leave the version dirty // so the next request retries without requiring another routing mutation. - _snapshot = previousSnapshot; + context.Snapshot = previousSnapshot; return previousSnapshot; } } finally { - _snapshotGate.Release(); + context.SnapshotGate.Release(); } } @@ -380,14 +433,15 @@ private static void ThrowSanitizedIfAClientAlreadyHasASchema(McpGeneratedSnapsho } private async ValueTask BuildCurrentSnapshotAsync( + McpSessionContext context, long snapshotVersion, CancellationToken cancellationToken) { while (true) { cancellationToken.ThrowIfCancellationRequested(); - await _roots.GetAsync(cancellationToken).ConfigureAwait(false); - var built = BuildSnapshotCore(); + await context.Roots.GetAsync(cancellationToken).ConfigureAwait(false); + var built = BuildSnapshotCore(context); var observedState = Volatile.Read(ref _snapshotState); // Version and retraction watermark are one atomically published state. A reader can @@ -399,32 +453,35 @@ private async ValueTask BuildCurrentSnapshotAsync( continue; } - _snapshot = built; + context.Snapshot = built; if (observedState.Version == snapshotVersion) { - Volatile.Write(ref _builtSnapshotVersion, snapshotVersion); + context.BuiltSnapshotVersion = snapshotVersion; } return built; } } - private McpGeneratedSnapshot BuildSnapshotCore() + private McpGeneratedSnapshot BuildSnapshotCore(McpSessionContext context) { // Project once here so tools/list, tools/call and prompts/list all read the same option list. - var model = McpAutomationProjection.Apply(CreateDocumentationModel()); - var adapter = new McpToolAdapter(_app, _options, _sessionServices); + var model = McpAutomationProjection.Apply(CreateDocumentationModel(context.Services)); + var adapter = new McpToolAdapter(_app, _options, context.Services); var commandsByPath = model.Commands.ToDictionary( command => command.Path, command => command, StringComparer.OrdinalIgnoreCase); var tools = GenerateAllTools(model, adapter, _separator, commandsByPath); ValidateCompatibilityToolNames(tools); - var resources = GenerateResources(model, adapter, _separator, commandsByPath); + var resources = GenerateResources(model, adapter, _separator, commandsByPath, context.Services); var prompts = CollectPrompts(model, adapter, _separator); return new McpGeneratedSnapshot(adapter, tools, resources, prompts); } - private ReplDocumentationModel CreateDocumentationModel() + // The documentation model resolves module-presence predicates against the SESSION's + // services (e.g. IMcpClientRoots), so the model — and everything generated from it — + // reflects the capabilities of the session it is built for. + private ReplDocumentationModel CreateDocumentationModel(IServiceProvider sessionServices) { var coreApp = _app as CoreReplApp ?? throw new InvalidOperationException("MCP server handler requires CoreReplApp."); @@ -433,7 +490,9 @@ private ReplDocumentationModel CreateDocumentationModel() ReplSessionIO.IsProgrammatic = true; try { - return coreApp.CreateDocumentationModel(CreateDiscoveryServices(), IsMcpCandidateBeforeValidation); + return coreApp.CreateDocumentationModel( + CreateDiscoveryServices(sessionServices), + IsMcpCandidateBeforeValidation); } finally { @@ -444,9 +503,9 @@ private ReplDocumentationModel CreateDocumentationModel() // Every MCP tool invocation overlays a concrete interaction channel before entering the // binder. Discovery must expose that guaranteed fallback even when the caller did not supply // a base provider (or supplied one without the channel). - private McpServiceProviderOverlay CreateDiscoveryServices() => + private McpServiceProviderOverlay CreateDiscoveryServices(IServiceProvider sessionServices) => new( - _sessionServices, + sessionServices, new Dictionary { [typeof(IReplInteractionChannel)] = new McpInteractionChannel( @@ -476,9 +535,8 @@ private void ValidateCompatibilityToolNames(IReadOnlyList tools) // Request-level binding: capability services resolve the flowing request's // destination-bound server through the AsyncLocal accessor, so concurrent requests // (SDK 2.0 creates one destination-bound McpServer per request) cannot cross-wire - // each other's client capabilities. Externally hosted servers (options built via - // BuildDynamicServerOptions and run by the host, without RunAsync's session attach) - // adopt the first observed server for session-level concerns. + // each other's client capabilities. Session-level concerns are handled by + // AttachSession (RunAsync) or the external fallback context (ResolveContext). private void BindRequestServer(McpServer? server) { if (server is null) @@ -487,39 +545,19 @@ private void BindRequestServer(McpServer? server) } _requestServers.BindRequest(server); - bool needsSessionAttach; - lock (_attachLock) - { - needsSessionAttach = _sessions.Count == 0; - } - - if (needsSessionAttach) - { - AttachServer(server); - } } // Session-level attach: routing-change notifications and the roots list-changed // handler belong to the session servers, registered once per session — never to the // per-request destination wrappers. - private void AttachServer(McpServer? server) + private void AttachSession(McpSessionContext context, McpServer server) { - if (server is null) - { - return; - } - lock (_attachLock) { - if (_sessions.Contains(server)) - { - return; - } - - _sessions.Add(server); + _sessions.Add(context); _requestServers.AttachSession(server); EnsureRoutingSubscription(); - EnsureRootsNotificationHandler(server); + EnsureRootsNotificationHandler(server, context.Roots); } } @@ -554,12 +592,12 @@ private void EnsureRoutingSubscription() // Removes a closing session and repairs the shared state: the accessor's session // fallback moves to a surviving session, and the routing subscription is dropped only // when the LAST session ends — a first-session close must not silence the others. - private void DetachSession(McpServer server) + private void DetachSession(McpSessionContext context) { lock (_attachLock) { - _sessions.Remove(server); - _requestServers.AttachSession(_sessions.Count > 0 ? _sessions[^1] : null); + _sessions.Remove(context); + _requestServers.AttachSession(_sessions.Count > 0 ? _sessions[^1].SessionServer : null); if (_sessions.Count == 0) { UnsubscribeFromRoutingChanges(); @@ -567,9 +605,9 @@ private void DetachSession(McpServer server) } } - private void EnsureRootsNotificationHandler(McpServer server) + private static void EnsureRootsNotificationHandler(McpServer server, McpClientRootsService roots) { - var weakSelf = new WeakReference(this); + var weakSelf = new WeakReference(roots); // Roots is deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005) but hosts still send // this notification; Repl keeps supporting it until the SDK removes the surface (#51). #pragma warning disable MCP9005 @@ -579,7 +617,7 @@ private void EnsureRootsNotificationHandler(McpServer server) { if (weakSelf.TryGetTarget(out var target)) { - target._roots.HandleRootsListChanged(); + target.HandleRootsListChanged(); } return ValueTask.CompletedTask; @@ -618,7 +656,14 @@ private void OnRoutingInvalidated(bool isVisibilityRetraction) if (_options.DynamicToolCompatibility == DynamicToolCompatibilityMode.DiscoverAndCallShim) { - Interlocked.Exchange(ref _compatibilityIntroServed, 0); + // Every active session re-serves its compatibility intro after a routing change. + lock (_attachLock) + { + foreach (var session in _sessions) + { + Interlocked.Exchange(ref session.CompatibilityIntroServed, 0); + } + } } lock (_refreshLock) @@ -644,7 +689,11 @@ private async Task SendNotificationSafeAsync(string method) McpServer[] sessions; lock (_attachLock) { - sessions = [.. _sessions]; + sessions = [ + .. _sessions + .Select(static session => session.SessionServer) + .OfType(), + ]; } foreach (var server in sessions) @@ -727,8 +776,9 @@ private bool HasMcpAppResources() ReplSessionIO.IsProgrammatic = true; try { + var sessionServices = CreateSessionContext().Services; using var runtimeStateScope = coreApp.PushRuntimeState( - CreateDiscoveryServices(), + CreateDiscoveryServices(sessionServices), isInteractiveSession: false); var activeGraph = coreApp.ResolveActiveRoutingGraph(); var commands = coreApp.ResolveDiscoverableRoutes( @@ -994,7 +1044,8 @@ private List GenerateResources( ReplDocumentationModel model, McpToolAdapter adapter, char separator, - Dictionary commandsByPath) + Dictionary commandsByPath, + IServiceProvider sessionServices) { var resources = new List(); var resourceMimeType = adapter.ForcedOutputMimeType; @@ -1047,7 +1098,7 @@ private List GenerateResources( foreach (var uiResource in _options.UiResources) { - resources.Add(new McpAppResource(uiResource, _sessionServices)); + resources.Add(new McpAppResource(uiResource, sessionServices)); } return resources; diff --git a/src/Repl.Mcp/McpSessionContext.cs b/src/Repl.Mcp/McpSessionContext.cs new file mode 100644 index 00000000..e175d7cf --- /dev/null +++ b/src/Repl.Mcp/McpSessionContext.cs @@ -0,0 +1,49 @@ +using ModelContextProtocol.Server; +using Repl.Documentation; + +namespace Repl.Mcp; + +/// +/// State owned by ONE MCP transport session. +/// +/// +/// One can serve several concurrent sessions, so anything +/// that varies per client lives here instead of on the handler: hard/soft roots, the +/// generated snapshot cache (the tool graph can be gated on session capabilities), the +/// compatibility-shim intro state, and the session's service overlay. The context is +/// registered in the provider passed to McpServer.Create, so request handlers +/// recover their originating session through request.Server.Services — never +/// through a destination-bound per-request server used as a surrogate session key. +/// Request-bound OUTBOUND capabilities (sampling, elicitation, progress) keep flowing +/// through the per-request binding, which is +/// finer-grained than the session. +/// +internal sealed class McpSessionContext +{ + public McpSessionContext(McpClientRootsService roots, IServiceProvider services) + { + Roots = roots; + Services = services; + } + + /// Session-owned hard/soft roots. + public McpClientRootsService Roots { get; } + + /// Per-session service overlay handed to McpServer.Create. + public IServiceProvider Services { get; } + + /// Session server used for server-initiated notifications. + public McpServer? SessionServer { get; set; } + + /// Serializes snapshot builds for this session. + public SemaphoreSlim SnapshotGate { get; } = new(initialCount: 1, maxCount: 1); + + /// Cached generated snapshot for this session. + public McpServerHandler.McpGeneratedSnapshot? Snapshot { get; set; } + + /// Routing version the cached snapshot was built at. + public long BuiltSnapshotVersion { get; set; } + + /// Whether this session already received the compatibility-shim intro list. + public int CompatibilityIntroServed; +} diff --git a/src/Repl.McpTests/Given_McpConcurrentSessions.cs b/src/Repl.McpTests/Given_McpConcurrentSessions.cs index 1b45ddc4..3485029d 100644 --- a/src/Repl.McpTests/Given_McpConcurrentSessions.cs +++ b/src/Repl.McpTests/Given_McpConcurrentSessions.cs @@ -163,6 +163,78 @@ public async Task When_FirstSessionCloses_Then_SurvivingSessionStillReceivesRout await ctsB.CancelAsync().ConfigureAwait(false); } + [TestMethod] + [Description("Guards snapshot isolation across sessions sharing one handler: a tool graph gated on session capabilities (module presence on IMcpClientRoots.IsSupported) must be computed per session — a shared snapshot cache would serve the roots-capable session's tools to a session without roots.")] + public async Task When_ToolGraphIsSessionGated_Then_EachSessionSeesItsOwnTools() + { + var app = ReplApp.Create(); + app.UseMcpServer(); + app.Map("always", () => "ok"); + app.MapModule(new RootsGatedModule(), (IMcpClientRoots roots) => roots.IsSupported); + + var options = new ReplMcpServerOptions + { + TransportFactory = static (serverName, io) => new StreamServerTransport( + ((McpTestFixture.PipeIoContext)io).InputStream, + ((McpTestFixture.PipeIoContext)io).OutputStream, + serverName), + }; + var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + using var cts = new CancellationTokenSource(); + + var (clientWithRoots, _) = await StartSessionAsync(handler, BuildRootsClientOptions("file:///ga"), cts.Token).ConfigureAwait(false); + var (clientWithoutRoots, _) = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + + var toolsWithRoots = await clientWithRoots.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); + var toolsWithoutRoots = await clientWithoutRoots.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); + + toolsWithRoots.Should().Contain(tool => string.Equals(tool.Name, "gated", StringComparison.Ordinal)); + toolsWithoutRoots.Should().Contain(tool => string.Equals(tool.Name, "always", StringComparison.Ordinal)); + toolsWithoutRoots.Should().NotContain(tool => string.Equals(tool.Name, "gated", StringComparison.Ordinal)); + + await clientWithRoots.DisposeAsync().ConfigureAwait(false); + await clientWithoutRoots.DisposeAsync().ConfigureAwait(false); + await cts.CancelAsync().ConfigureAwait(false); + } + + [TestMethod] + [Description("Guards the compatibility-shim intro across sessions: DiscoverAndCallShim serves the discover_tools/call_tool intro on each session's FIRST tools/list — a handler-global flag would give the intro only to whichever session listed first, leaving later sessions without the documented bootstrap.")] + public async Task When_ShimEnabledAndTwoSessionsList_Then_EachSessionGetsTheIntro() + { + var app = ReplApp.Create(); + app.UseMcpServer(); + app.Map("alpha", () => "a"); + + var options = new ReplMcpServerOptions + { + DynamicToolCompatibility = DynamicToolCompatibilityMode.DiscoverAndCallShim, + TransportFactory = static (serverName, io) => new StreamServerTransport( + ((McpTestFixture.PipeIoContext)io).InputStream, + ((McpTestFixture.PipeIoContext)io).OutputStream, + serverName), + }; + var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + using var cts = new CancellationTokenSource(); + + var (clientA, _) = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + var (clientB, _) = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + + var firstListA = await clientA.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); + var firstListB = await clientB.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); + + firstListA.Select(static tool => tool.Name).Should().BeEquivalentTo(["discover_tools", "call_tool"]); + firstListB.Select(static tool => tool.Name).Should().BeEquivalentTo(["discover_tools", "call_tool"]); + + await clientA.DisposeAsync().ConfigureAwait(false); + await clientB.DisposeAsync().ConfigureAwait(false); + await cts.CancelAsync().ConfigureAwait(false); + } + + private sealed class RootsGatedModule : IReplModule + { + public void Map(IReplMap app) => app.Map("gated", () => "roots-only"); + } + private static McpClientOptions BuildRootsClientOptions(string rootUri) => new() { Capabilities = new ClientCapabilities From 9c1b8ce61619b68ad0259cd6016d891c56277f35 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Sun, 19 Jul 2026 18:19:57 -0400 Subject: [PATCH 07/16] =?UTF-8?q?docs(mcp):=20correct=20SEP-2322=20availab?= =?UTF-8?q?ility=20=E2=80=94=20MRTR=20ships=20experimentally=20in=20SDK=20?= =?UTF-8?q?2.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deprecation pragmas and the reference doc claimed the SEP-2322 multi-round-trip successor was 'not yet consumable in the SDK'; preview.3 actually ships it experimentally (MrtrContext/MrtrContinuation/MrtrExchange). Reworded to 'shipped experimentally, not adopted by Repl yet' — adoption is a follow-up under the compliance track. --- docs/mcp-reference.md | 2 +- src/Repl.Mcp/IMcpFeedback.cs | 5 +++-- src/Repl.Mcp/McpClientRootsService.cs | 5 +++-- src/Repl.Mcp/McpFeedbackService.cs | 5 +++-- src/Repl.Mcp/McpInteractionChannel.cs | 3 ++- src/Repl.Mcp/McpSamplingService.cs | 3 ++- 6 files changed, 14 insertions(+), 9 deletions(-) diff --git a/docs/mcp-reference.md b/docs/mcp-reference.md index 845575ad..e8669261 100644 --- a/docs/mcp-reference.md +++ b/docs/mcp-reference.md @@ -515,7 +515,7 @@ Feature support varies across agents. Check [mcp-availability.com](https://mcp-a ### SDK and protocol versions - Repl.Mcp builds on the official C# SDK (`ModelContextProtocol`), currently on the **2.0 line** (`2.0.0-preview.3`). The SDK negotiates the protocol version with each client, including fallback to the legacy `initialize` handshake for older hosts. -- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577). Repl.Mcp keeps supporting them **for existing hosts and applications only** — new applications should not adopt these features (the SDK may remove them) and should prefer Repl's portable abstractions such as `IReplInteractionChannel`. The designated successor for server-initiated flows (SEP-2322, multi-round-trip requests) is not yet consumable in the SDK. +- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577). Repl.Mcp keeps supporting them **for existing hosts and applications only** — new applications should not adopt these features (the SDK may remove them) and should prefer Repl's portable abstractions such as `IReplInteractionChannel`. The designated successor for server-initiated flows (SEP-2322, multi-round-trip requests) ships experimentally in the SDK 2.0 line; Repl has not adopted it yet. - **MCP Tasks**: the SDK reorganized Tasks into `ModelContextProtocol.Extensions.Tasks` and dropped the per-tool execution augmentation (`Tool.Execution`) from the protocol surface, so `.LongRunning()` commands no longer advertise task support at the protocol level. The annotation stays in Repl's own model (help/docs); protocol-level task support can return once Repl integrates the Tasks extension, store, and get/update/cancel lifecycle (tracked in issue #72). | Feature | Claude Desktop | Claude Code | Codex | VS Code Copilot | Cursor | Continue | diff --git a/src/Repl.Mcp/IMcpFeedback.cs b/src/Repl.Mcp/IMcpFeedback.cs index 9abbf943..2deea2e0 100644 --- a/src/Repl.Mcp/IMcpFeedback.cs +++ b/src/Repl.Mcp/IMcpFeedback.cs @@ -3,8 +3,9 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on -// these features, so Repl keeps supporting them until the SDK removes the surface (#51). +// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps +// supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; diff --git a/src/Repl.Mcp/McpClientRootsService.cs b/src/Repl.Mcp/McpClientRootsService.cs index c59b9973..d0d404e3 100644 --- a/src/Repl.Mcp/McpClientRootsService.cs +++ b/src/Repl.Mcp/McpClientRootsService.cs @@ -3,8 +3,9 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on -// these features, so Repl keeps supporting them until the SDK removes the surface (#51). +// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps +// supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; diff --git a/src/Repl.Mcp/McpFeedbackService.cs b/src/Repl.Mcp/McpFeedbackService.cs index 6ad0505d..9486d742 100644 --- a/src/Repl.Mcp/McpFeedbackService.cs +++ b/src/Repl.Mcp/McpFeedbackService.cs @@ -7,8 +7,9 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on -// these features, so Repl keeps supporting them until the SDK removes the surface (#51). +// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps +// supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; diff --git a/src/Repl.Mcp/McpInteractionChannel.cs b/src/Repl.Mcp/McpInteractionChannel.cs index f1e94a7e..7adb8f34 100644 --- a/src/Repl.Mcp/McpInteractionChannel.cs +++ b/src/Repl.Mcp/McpInteractionChannel.cs @@ -7,7 +7,8 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on +// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps // these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 diff --git a/src/Repl.Mcp/McpSamplingService.cs b/src/Repl.Mcp/McpSamplingService.cs index c04d6b52..8ec4e238 100644 --- a/src/Repl.Mcp/McpSamplingService.cs +++ b/src/Repl.Mcp/McpSamplingService.cs @@ -3,7 +3,8 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests) is not yet consumable in the SDK and hosts still rely on +// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps // these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 From ddce4d408466630cacc59d5c67b53cb92f6d8653 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 13 Aug 2026 17:23:21 -0400 Subject: [PATCH 08/16] chore(mcp): update SDK to 2.2.0 --- docs/mcp-reference.md | 4 ++-- src/Directory.Packages.props | 2 +- src/Repl.Mcp/IMcpFeedback.cs | 2 +- src/Repl.Mcp/McpClientRootsService.cs | 2 +- src/Repl.Mcp/McpFeedbackService.cs | 2 +- src/Repl.Mcp/McpInteractionChannel.cs | 2 +- src/Repl.Mcp/McpSamplingService.cs | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/mcp-reference.md b/docs/mcp-reference.md index e8669261..c6e6f341 100644 --- a/docs/mcp-reference.md +++ b/docs/mcp-reference.md @@ -514,8 +514,8 @@ Feature support varies across agents. Check [mcp-availability.com](https://mcp-a ### SDK and protocol versions -- Repl.Mcp builds on the official C# SDK (`ModelContextProtocol`), currently on the **2.0 line** (`2.0.0-preview.3`). The SDK negotiates the protocol version with each client, including fallback to the legacy `initialize` handshake for older hosts. -- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577). Repl.Mcp keeps supporting them **for existing hosts and applications only** — new applications should not adopt these features (the SDK may remove them) and should prefer Repl's portable abstractions such as `IReplInteractionChannel`. The designated successor for server-initiated flows (SEP-2322, multi-round-trip requests) ships experimentally in the SDK 2.0 line; Repl has not adopted it yet. +- Repl.Mcp builds on the official C# SDK (`ModelContextProtocol`), currently at **2.2.0**. The SDK negotiates the protocol version with each client, including fallback to the legacy `initialize` handshake for older hosts. +- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577). Repl.Mcp keeps supporting them **for existing hosts and applications only** — new applications should not adopt these features (the SDK may remove them) and should prefer Repl's portable abstractions such as `IReplInteractionChannel`. The designated successor for server-initiated flows (SEP-2322, multi-round-trip requests) is available starting with SDK 2.0; Repl has not adopted it yet. - **MCP Tasks**: the SDK reorganized Tasks into `ModelContextProtocol.Extensions.Tasks` and dropped the per-tool execution augmentation (`Tool.Execution`) from the protocol surface, so `.LongRunning()` commands no longer advertise task support at the protocol level. The annotation stays in Repl's own model (help/docs); protocol-level task support can return once Repl integrates the Tasks extension, store, and get/update/cancel lifecycle (tracked in issue #72). | Feature | Claude Desktop | Claude Code | Codex | VS Code Copilot | Cursor | Continue | diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 77d7b240..fc66d5ae 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -14,7 +14,7 @@ - + diff --git a/src/Repl.Mcp/IMcpFeedback.cs b/src/Repl.Mcp/IMcpFeedback.cs index 2deea2e0..543127dd 100644 --- a/src/Repl.Mcp/IMcpFeedback.cs +++ b/src/Repl.Mcp/IMcpFeedback.cs @@ -3,7 +3,7 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) // is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps // supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 diff --git a/src/Repl.Mcp/McpClientRootsService.cs b/src/Repl.Mcp/McpClientRootsService.cs index d0d404e3..f699c709 100644 --- a/src/Repl.Mcp/McpClientRootsService.cs +++ b/src/Repl.Mcp/McpClientRootsService.cs @@ -3,7 +3,7 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) // is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps // supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 diff --git a/src/Repl.Mcp/McpFeedbackService.cs b/src/Repl.Mcp/McpFeedbackService.cs index 9486d742..5c03c241 100644 --- a/src/Repl.Mcp/McpFeedbackService.cs +++ b/src/Repl.Mcp/McpFeedbackService.cs @@ -7,7 +7,7 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) // is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps // supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 diff --git a/src/Repl.Mcp/McpInteractionChannel.cs b/src/Repl.Mcp/McpInteractionChannel.cs index 7adb8f34..4a332151 100644 --- a/src/Repl.Mcp/McpInteractionChannel.cs +++ b/src/Repl.Mcp/McpInteractionChannel.cs @@ -7,7 +7,7 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) // is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps // these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 diff --git a/src/Repl.Mcp/McpSamplingService.cs b/src/Repl.Mcp/McpSamplingService.cs index 8ec4e238..53d62b3a 100644 --- a/src/Repl.Mcp/McpSamplingService.cs +++ b/src/Repl.Mcp/McpSamplingService.cs @@ -3,7 +3,7 @@ // Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK // diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, shipped experimentally in SDK 2.0 as MrtrContext/MrtrExchange) +// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) // is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps // these features, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 From 01b2403e3f9b32f0820af9ff4b75bc34026de3a3 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Fri, 21 Aug 2026 22:11:07 -0400 Subject: [PATCH 09/16] test(mcp): own server tasks and pin the negotiated revision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The concurrent-session tests discarded every RunAsync task, so they validated assertions but never server termination. RED OBSERVED (mutation, since a discarded task cannot fail a test): appending `throw new InvalidOperationException(...)` after the teardown block of McpServerHandler.RunAsync left 4 of 7 tests GREEN — exactly the 4 that discard their server task. Only the three that observe it (When_FirstSessionCloses via `await serverTaskA`, and the two McpTestFixture-based tests) went red. After this change the same mutation turns all 9 red. - McpPipeSession owns the pipe pair, a linked CTS and the RunAsync task; DisposeAsync awaits it and accepts ONLY cancellation, so a teardown fault or hang surfaces. It carries the single copy of that logic: McpTestFixture now delegates to it instead of holding a second one, and the handshake/server race that used to live only in the fixture now protects the shared-handler sessions too. - Given_McpConcurrentSessions: drop the local StartSessionAsync, all seven discarded tasks, and the five copy-pasted TransportFactory blocks (one CreateHandler helper, and McpTestFixture.PipeTransportFactory replaces the 10 duplicated PipeIoContext casts). The file-wide MCP9005 pragma is now scoped to the two deprecated client-option builders. Two new regressions close the gap that let the sessionless-protocol defects survive four review waves — nothing anywhere asserted which revision these guarantees describe: - When_TwoSessionsShareOneHandler_Then_BothNegotiateTheModernRevision pins 2026-07-28 on both sessions. - When_ToolGraphIsSessionGated_Then_ListResultIsTaggedPrivateAndStale locks the SEP-2549 cache contract that makes a per-session tools/list legal: the result must carry cacheScope=private and ttlMs=0, since an absent cacheScope defaults to Public and would let a shared gateway serve one client's capability-gated catalog to another. Verified that the SDK stamps both on the 2026-07-28 path. Full MCP suite green (232 passed, 1 known Inspector skip). --- .../Given_McpConcurrentSessions.cs | 304 +++++++++--------- src/Repl.McpTests/McpPipeSession.cs | 118 +++++++ src/Repl.McpTests/McpTestFixture.cs | 122 ++----- 3 files changed, 281 insertions(+), 263 deletions(-) create mode 100644 src/Repl.McpTests/McpPipeSession.cs diff --git a/src/Repl.McpTests/Given_McpConcurrentSessions.cs b/src/Repl.McpTests/Given_McpConcurrentSessions.cs index 3485029d..228eec42 100644 --- a/src/Repl.McpTests/Given_McpConcurrentSessions.cs +++ b/src/Repl.McpTests/Given_McpConcurrentSessions.cs @@ -1,19 +1,37 @@ -using System.IO.Pipelines; using ModelContextProtocol; using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; -using ModelContextProtocol.Server; using Repl.Mcp; -// One test exercises Sampling, deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005) -// but still supported by Repl.Mcp until the SDK removes the surface (#51). -#pragma warning disable MCP9005 - namespace Repl.McpTests; [TestClass] public sealed class Given_McpConcurrentSessions { + // The SDK's McpProtocolVersions constants are internal, so the revisions are pinned here. + // 2026-07-28 (SEP-2567) is the sessionless, per-request-metadata revision the default client + // negotiates; every guarantee in this file is written against it, hence the explicit assertions. + private const string ModernProtocolVersion = "2026-07-28"; + + [TestMethod] + [Description("Pins the protocol revision every other guarantee in this file is written against: two sessions sharing one handler must both negotiate 2026-07-28. Without this, a silent fallback to the 2025-11-25 initialize handshake would make the per-session capability and catalog assertions below describe a revision they were never meant to characterise — which is exactly how four review waves missed the sessionless-protocol defects.")] + public async Task When_TwoSessionsShareOneHandler_Then_BothNegotiateTheModernRevision() + { + var app = ReplApp.Create(); + app.UseMcpServer(); + app.Map("alpha", () => "a"); + var handler = CreateHandler(app); + using var cts = new CancellationTokenSource(); + + var sessionA = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + await using var scopeA = sessionA.ConfigureAwait(false); + var sessionB = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + await using var scopeB = sessionB.ConfigureAwait(false); + + sessionA.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); + sessionB.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); + } + [TestMethod] [Description("Guards capability binding against cross-session interference: with one handler serving two sessions (SDK 2.0 binds a destination server per request), a paused call from a sampling-capable client must still observe ITS OWN client's capabilities after a request from a sampling-less client has been served — the capability services must bind to the flowing request, not to a shared last-attached server.")] public async Task When_TwoClientsWithDifferentCapabilitiesShareHandler_Then_CapabilityBindingIsPerRequest() @@ -32,41 +50,28 @@ public async Task When_TwoClientsWithDifferentCapabilitiesShareHandler_Then_Capa return $"{before}|{after}"; }); app.Map("poke", () => "ok"); - - var options = new ReplMcpServerOptions - { - TransportFactory = static (serverName, io) => new StreamServerTransport( - ((McpTestFixture.PipeIoContext)io).InputStream, - ((McpTestFixture.PipeIoContext)io).OutputStream, - serverName), - }; - var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + var handler = CreateHandler(app); using var cts = new CancellationTokenSource(); - var (clientA, serverTaskA) = await StartSessionAsync(handler, BuildSamplingClientOptions(), cts.Token).ConfigureAwait(false); - var (clientB, serverTaskB) = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + var sessionA = await StartSessionAsync(handler, BuildSamplingClientOptions(), cts.Token).ConfigureAwait(false); + await using var scopeA = sessionA.ConfigureAwait(false); + var sessionB = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + await using var scopeB = sessionB.ConfigureAwait(false); // Session A enters "probe" (sampling supported) and pauses on the gate; session B is // then served in full; A resumes and must STILL see its own sampling capability. - var probeTask = clientA.CallToolAsync( + var probeTask = sessionA.Client.CallToolAsync( "probe", new Dictionary(StringComparer.Ordinal), cancellationToken: cts.Token); (await entered.WaitAsync(TimeSpan.FromSeconds(10)).ConfigureAwait(false)).Should().BeTrue(); - await clientB.CallToolAsync( + await sessionB.Client.CallToolAsync( "poke", new Dictionary(StringComparer.Ordinal), cancellationToken: cts.Token) .ConfigureAwait(false); gate.Release(); var probeResult = await probeTask.ConfigureAwait(false); - var text = probeResult.Content.OfType().First().Text; - text.Should().Contain("True|True"); - - await clientA.DisposeAsync().ConfigureAwait(false); - await clientB.DisposeAsync().ConfigureAwait(false); - await cts.CancelAsync().ConfigureAwait(false); - _ = serverTaskA; - _ = serverTaskB; + probeResult.Content.OfType().First().Text.Should().Contain("True|True"); } [TestMethod] @@ -77,25 +82,19 @@ public async Task When_TwoRootCapableClientsShareHandler_Then_EachSeesOwnRoots() app.UseMcpServer(); app.Map("roots", async (IMcpClientRoots roots, CancellationToken ct) => string.Join(',', (await roots.GetAsync(ct).ConfigureAwait(false)).Select(root => root.Uri.ToString()))); - - var options = new ReplMcpServerOptions - { - TransportFactory = static (serverName, io) => new StreamServerTransport( - ((McpTestFixture.PipeIoContext)io).InputStream, - ((McpTestFixture.PipeIoContext)io).OutputStream, - serverName), - }; - var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + var handler = CreateHandler(app); using var cts = new CancellationTokenSource(); - var (clientA, _) = await StartSessionAsync(handler, BuildRootsClientOptions("file:///ga"), cts.Token).ConfigureAwait(false); - var (clientB, _) = await StartSessionAsync(handler, BuildRootsClientOptions("file:///bu"), cts.Token).ConfigureAwait(false); + var sessionA = await StartSessionAsync(handler, BuildRootsClientOptions("file:///ga"), cts.Token).ConfigureAwait(false); + await using var scopeA = sessionA.ConfigureAwait(false); + var sessionB = await StartSessionAsync(handler, BuildRootsClientOptions("file:///bu"), cts.Token).ConfigureAwait(false); + await using var scopeB = sessionB.ConfigureAwait(false); - var resultA = await clientA.CallToolAsync( + var resultA = await sessionA.Client.CallToolAsync( toolName: "roots", arguments: new Dictionary(StringComparer.Ordinal), cancellationToken: cts.Token).ConfigureAwait(false); - var resultB = await clientB.CallToolAsync( + var resultB = await sessionB.Client.CallToolAsync( toolName: "roots", arguments: new Dictionary(StringComparer.Ordinal), cancellationToken: cts.Token).ConfigureAwait(false); @@ -104,10 +103,6 @@ public async Task When_TwoRootCapableClientsShareHandler_Then_EachSeesOwnRoots() var textB = resultB.Content.OfType().First().Text; textB.Should().Contain("file:///bu"); textB.Should().NotContain("file:///ga"); - - await clientA.DisposeAsync().ConfigureAwait(false); - await clientB.DisposeAsync().ConfigureAwait(false); - await cts.CancelAsync().ConfigureAwait(false); } [TestMethod] @@ -117,50 +112,31 @@ public async Task When_FirstSessionCloses_Then_SurvivingSessionStillReceivesRout var app = ReplApp.Create(); app.UseMcpServer(); app.Map("alpha", () => "a"); + var handler = CreateHandler(app); + using var cts = new CancellationTokenSource(); - var options = new ReplMcpServerOptions - { - TransportFactory = static (serverName, io) => new StreamServerTransport( - ((McpTestFixture.PipeIoContext)io).InputStream, - ((McpTestFixture.PipeIoContext)io).OutputStream, - serverName), - }; - var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); - using var ctsA = new CancellationTokenSource(); - using var ctsB = new CancellationTokenSource(); - - var (clientA, serverTaskA) = await StartSessionAsync(handler, clientOptions: null, ctsA.Token).ConfigureAwait(false); - var (clientB, _) = await StartSessionAsync(handler, clientOptions: null, ctsB.Token).ConfigureAwait(false); + var sessionA = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + var sessionB = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + await using var scopeB = sessionB.ConfigureAwait(false); var listChanged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var registration = clientB.RegisterNotificationHandler( + var registration = sessionB.Client.RegisterNotificationHandler( NotificationMethods.ToolListChangedNotification, (_, _) => { listChanged.TrySetResult(); return ValueTask.CompletedTask; }); - await using var _ = registration.ConfigureAwait(false); + await using var scopeRegistration = registration.ConfigureAwait(false); - // Both sessions are live; close the FIRST one, then invalidate routing. - await clientA.DisposeAsync().ConfigureAwait(false); - await ctsA.CancelAsync().ConfigureAwait(false); - try - { - await serverTaskA.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); - } - catch (OperationCanceledException) - { - // Expected: session A's RunAsync ends on cancellation. - } + // Both sessions are live; close the FIRST one, then invalidate routing. Disposing the + // session awaits its RunAsync, so a teardown fault surfaces here instead of being swallowed. + await sessionA.DisposeAsync().ConfigureAwait(false); app.Map("late", () => "l"); app.Core.InvalidateRouting(); await listChanged.Task.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); - - await clientB.DisposeAsync().ConfigureAwait(false); - await ctsB.CancelAsync().ConfigureAwait(false); } [TestMethod] @@ -171,116 +147,67 @@ public async Task When_ToolGraphIsSessionGated_Then_EachSessionSeesItsOwnTools() app.UseMcpServer(); app.Map("always", () => "ok"); app.MapModule(new RootsGatedModule(), (IMcpClientRoots roots) => roots.IsSupported); - - var options = new ReplMcpServerOptions - { - TransportFactory = static (serverName, io) => new StreamServerTransport( - ((McpTestFixture.PipeIoContext)io).InputStream, - ((McpTestFixture.PipeIoContext)io).OutputStream, - serverName), - }; - var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + var handler = CreateHandler(app); using var cts = new CancellationTokenSource(); - var (clientWithRoots, _) = await StartSessionAsync(handler, BuildRootsClientOptions("file:///ga"), cts.Token).ConfigureAwait(false); - var (clientWithoutRoots, _) = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + var withRoots = await StartSessionAsync(handler, BuildRootsClientOptions("file:///ga"), cts.Token).ConfigureAwait(false); + await using var scopeWithRoots = withRoots.ConfigureAwait(false); + var withoutRoots = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + await using var scopeWithoutRoots = withoutRoots.ConfigureAwait(false); - var toolsWithRoots = await clientWithRoots.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); - var toolsWithoutRoots = await clientWithoutRoots.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); + var toolsWithRoots = await withRoots.Client.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); + var toolsWithoutRoots = await withoutRoots.Client.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); toolsWithRoots.Should().Contain(tool => string.Equals(tool.Name, "gated", StringComparison.Ordinal)); toolsWithoutRoots.Should().Contain(tool => string.Equals(tool.Name, "always", StringComparison.Ordinal)); toolsWithoutRoots.Should().NotContain(tool => string.Equals(tool.Name, "gated", StringComparison.Ordinal)); - - await clientWithRoots.DisposeAsync().ConfigureAwait(false); - await clientWithoutRoots.DisposeAsync().ConfigureAwait(false); - await cts.CancelAsync().ConfigureAwait(false); } [TestMethod] - [Description("Guards the compatibility-shim intro across sessions: DiscoverAndCallShim serves the discover_tools/call_tool intro on each session's FIRST tools/list — a handler-global flag would give the intro only to whichever session listed first, leaving later sessions without the documented bootstrap.")] - public async Task When_ShimEnabledAndTwoSessionsList_Then_EachSessionGetsTheIntro() + [Description("Locks the cache contract that makes a per-session tools/list legal on 2026-07-28: SEP-2549 permits list results to vary per client (CacheScope.Private is documented for \"filtered list results that vary per user\"), but an absent cacheScope defaults to Public, which would let a shared gateway serve one client's capability-gated catalog to another. A varying catalog must therefore be tagged private and immediately stale.")] + public async Task When_ToolGraphIsSessionGated_Then_ListResultIsTaggedPrivateAndStale() { var app = ReplApp.Create(); app.UseMcpServer(); - app.Map("alpha", () => "a"); - - var options = new ReplMcpServerOptions - { - DynamicToolCompatibility = DynamicToolCompatibilityMode.DiscoverAndCallShim, - TransportFactory = static (serverName, io) => new StreamServerTransport( - ((McpTestFixture.PipeIoContext)io).InputStream, - ((McpTestFixture.PipeIoContext)io).OutputStream, - serverName), - }; - var handler = new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + app.Map("always", () => "ok"); + app.MapModule(new RootsGatedModule(), (IMcpClientRoots roots) => roots.IsSupported); + var handler = CreateHandler(app); using var cts = new CancellationTokenSource(); - var (clientA, _) = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); - var (clientB, _) = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); - - var firstListA = await clientA.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); - var firstListB = await clientB.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); + var session = await StartSessionAsync(handler, BuildRootsClientOptions("file:///ga"), cts.Token).ConfigureAwait(false); + await using var scope = session.ConfigureAwait(false); - firstListA.Select(static tool => tool.Name).Should().BeEquivalentTo(["discover_tools", "call_tool"]); - firstListB.Select(static tool => tool.Name).Should().BeEquivalentTo(["discover_tools", "call_tool"]); + session.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); + var result = await session.Client.SendRequestAsync( + RequestMethods.ToolsList, + new ListToolsRequestParams(), + cancellationToken: cts.Token).ConfigureAwait(false); - await clientA.DisposeAsync().ConfigureAwait(false); - await clientB.DisposeAsync().ConfigureAwait(false); - await cts.CancelAsync().ConfigureAwait(false); + result.Tools.Should().Contain(tool => string.Equals(tool.Name, "gated", StringComparison.Ordinal)); + result.CacheScope.Should().Be(CacheScope.Private); + result.TimeToLive.Should().Be(TimeSpan.Zero); } - private sealed class RootsGatedModule : IReplModule + [TestMethod] + [Description("Guards the compatibility-shim intro across sessions: DiscoverAndCallShim serves the discover_tools/call_tool intro on each session's FIRST tools/list — a handler-global flag would give the intro only to whichever session listed first, leaving later sessions without the documented bootstrap.")] + public async Task When_ShimEnabledAndTwoSessionsList_Then_EachSessionGetsTheIntro() { - public void Map(IReplMap app) => app.Map("gated", () => "roots-only"); - } + var app = ReplApp.Create(); + app.UseMcpServer(); + app.Map("alpha", () => "a"); + var handler = CreateHandler(app, DynamicToolCompatibilityMode.DiscoverAndCallShim); + using var cts = new CancellationTokenSource(); - private static McpClientOptions BuildRootsClientOptions(string rootUri) => new() - { - Capabilities = new ClientCapabilities - { - Roots = new RootsCapability { ListChanged = true }, - }, - Handlers = new McpClientHandlers - { - RootsHandler = (_, _) => ValueTask.FromResult(new ListRootsResult - { - Roots = [new Root { Uri = rootUri, Name = rootUri }], - }), - }, - }; + var sessionA = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + await using var scopeA = sessionA.ConfigureAwait(false); + var sessionB = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); + await using var scopeB = sessionB.ConfigureAwait(false); - private static McpClientOptions BuildSamplingClientOptions() => new() - { - Capabilities = new ClientCapabilities { Sampling = new SamplingCapability() }, - Handlers = new McpClientHandlers - { - SamplingHandler = static (request, _, _) => ValueTask.FromResult(new CreateMessageResult - { - Content = [new TextContentBlock { Text = "ga" }], - Model = "test-model", - }), - }, - }; + var firstListA = await sessionA.Client.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); + var firstListB = await sessionB.Client.ListToolsAsync(cancellationToken: cts.Token).ConfigureAwait(false); - private static async Task<(McpClient Client, Task ServerTask)> StartSessionAsync( - McpServerHandler handler, - McpClientOptions? clientOptions, - CancellationToken cancellationToken) - { - var clientToServer = new Pipe(); - var serverToClient = new Pipe(); - var io = new McpTestFixture.PipeIoContext( - clientToServer.Reader.AsStream(), - serverToClient.Writer.AsStream()); - var serverTask = handler.RunAsync(io, cancellationToken); - - var clientTransport = new StreamClientTransport( - clientToServer.Writer.AsStream(), - serverToClient.Reader.AsStream()); - var client = await McpClient.CreateAsync(clientTransport, clientOptions, cancellationToken: cancellationToken) - .ConfigureAwait(false); - return (client, serverTask); + firstListA.Select(static tool => tool.Name).Should().BeEquivalentTo(["discover_tools", "call_tool"]); + firstListB.Select(static tool => tool.Name).Should().BeEquivalentTo(["discover_tools", "call_tool"]); } [TestMethod] @@ -347,4 +274,61 @@ public async Task When_ToolsInvokedConcurrently_Then_OutputIsIsolated() } } } + + private static Task StartSessionAsync( + McpServerHandler handler, + McpClientOptions? clientOptions, + CancellationToken cancellationToken) => + McpPipeSession.StartAsync(handler.RunAsync, clientOptions, cancellationToken); + + private static McpServerHandler CreateHandler( + ReplApp app, + DynamicToolCompatibilityMode compatibility = DynamicToolCompatibilityMode.Disabled) + { + var options = new ReplMcpServerOptions + { + DynamicToolCompatibility = compatibility, + TransportFactory = McpTestFixture.PipeTransportFactory, + }; + + return new McpServerHandler(app.Core, options, McpTestFixture.EmptyServices); + } + + private sealed class RootsGatedModule : IReplModule + { + public void Map(IReplMap app) => app.Map("gated", () => "roots-only"); + } + + // Roots is deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005) but still supported by + // Repl.Mcp until the SDK removes the surface (#51). +#pragma warning disable MCP9005 + private static McpClientOptions BuildRootsClientOptions(string rootUri) => new() + { + Capabilities = new ClientCapabilities + { + Roots = new RootsCapability { ListChanged = true }, + }, + Handlers = new McpClientHandlers + { + RootsHandler = (_, _) => ValueTask.FromResult(new ListRootsResult + { + Roots = [new Root { Uri = rootUri, Name = rootUri }], + }), + }, + }; + + // Sampling carries the same SEP-2577 deprecation as Roots above. + private static McpClientOptions BuildSamplingClientOptions() => new() + { + Capabilities = new ClientCapabilities { Sampling = new SamplingCapability() }, + Handlers = new McpClientHandlers + { + SamplingHandler = static (request, _, _) => ValueTask.FromResult(new CreateMessageResult + { + Content = [new TextContentBlock { Text = "ga" }], + Model = "test-model", + }), + }, + }; +#pragma warning restore MCP9005 } diff --git a/src/Repl.McpTests/McpPipeSession.cs b/src/Repl.McpTests/McpPipeSession.cs new file mode 100644 index 00000000..1c7cdf1c --- /dev/null +++ b/src/Repl.McpTests/McpPipeSession.cs @@ -0,0 +1,118 @@ +using System.IO.Pipelines; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; + +namespace Repl.McpTests; + +/// +/// One MCP client connected to an existing over in-process pipes. +/// +/// +/// Several sessions can share one handler, which is what the concurrent-session regressions need. +/// This type owns the transport pair, the cancellation source, and — crucially — the +/// RunAsync task: awaits it and lets any fault surface. Discarding +/// that task is what once let a server throwing during teardown leave every concurrency test green. +/// +internal sealed class McpPipeSession : IAsyncDisposable +{ + private readonly CancellationTokenSource _cts; + private readonly Pipe _clientToServer; + private readonly Pipe _serverToClient; + private readonly Task _serverTask; + + private McpPipeSession( + McpClient client, + Task serverTask, + CancellationTokenSource cts, + Pipe clientToServer, + Pipe serverToClient) + { + Client = client; + _serverTask = serverTask; + _cts = cts; + _clientToServer = clientToServer; + _serverToClient = serverToClient; + } + + public McpClient Client { get; } + + /// + /// Starts a session by handing the server side of a fresh pipe pair. + /// + /// + /// The handshake races the server task so a server that fails while starting surfaces its own + /// exception instead of an initialize timeout carrying the wrong one. + /// + public static async Task StartAsync( + Func startServer, + McpClientOptions? clientOptions, + CancellationToken cancellationToken) + { + var clientToServer = new Pipe(); + var serverToClient = new Pipe(); + var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + + var io = new McpTestFixture.PipeIoContext( + clientToServer.Reader.AsStream(), + serverToClient.Writer.AsStream()); + var serverTask = startServer(io, cts.Token); + + var clientTransport = new StreamClientTransport( + clientToServer.Writer.AsStream(), + serverToClient.Reader.AsStream()); + + try + { + var clientTask = McpClient.CreateAsync(clientTransport, clientOptions, cancellationToken: cts.Token); + if (ReferenceEquals(await Task.WhenAny(serverTask, clientTask).ConfigureAwait(false), serverTask)) + { + // Rethrows a start failure; a clean early exit means the handshake never completes. + await serverTask.ConfigureAwait(false); + + throw new InvalidOperationException( + "The MCP server stopped before the client completed its handshake."); + } + + var client = await clientTask.ConfigureAwait(false); + + return new McpPipeSession(client, serverTask, cts, clientToServer, serverToClient); + } + catch + { + await cts.CancelAsync().ConfigureAwait(false); + await clientToServer.Writer.CompleteAsync().ConfigureAwait(false); + await serverToClient.Writer.CompleteAsync().ConfigureAwait(false); + cts.Dispose(); + throw; + } + } + + /// + /// Closes the session and asserts the server terminated cleanly. + /// + /// + /// Only cancellation is an accepted outcome. A fault propagates, and so does a timeout: a server + /// that never shuts down is a defect, not noise to be swallowed. + /// + public async ValueTask DisposeAsync() + { + await Client.DisposeAsync().ConfigureAwait(false); + await _cts.CancelAsync().ConfigureAwait(false); + + await _clientToServer.Writer.CompleteAsync().ConfigureAwait(false); + await _serverToClient.Writer.CompleteAsync().ConfigureAwait(false); + + try + { + await _serverTask.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Expected: the server's RunAsync ends on cancellation. + } + finally + { + _cts.Dispose(); + } + } +} diff --git a/src/Repl.McpTests/McpTestFixture.cs b/src/Repl.McpTests/McpTestFixture.cs index ab8b3c90..0e37690f 100644 --- a/src/Repl.McpTests/McpTestFixture.cs +++ b/src/Repl.McpTests/McpTestFixture.cs @@ -1,4 +1,3 @@ -using System.IO.Pipelines; using Microsoft.Extensions.DependencyInjection; using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; @@ -14,30 +13,17 @@ namespace Repl.McpTests; /// internal sealed class McpTestFixture : IAsyncDisposable { - private readonly CancellationTokenSource _cts; - private readonly Pipe _clientToServer; - private readonly Pipe _serverToClient; - private readonly Task _serverTask; + private readonly McpPipeSession _session; private readonly ReplApp _app; - private McpTestFixture( - ReplApp app, - McpClient client, - Task serverTask, - CancellationTokenSource cts, - Pipe clientToServer, - Pipe serverToClient) + private McpTestFixture(ReplApp app, McpPipeSession session) { _app = app; - Client = client; - _serverTask = serverTask; - _cts = cts; - _clientToServer = clientToServer; - _serverToClient = serverToClient; + _session = session; } public ReplApp App => _app; - public McpClient Client { get; } + public McpClient Client => _session.Client; public static Task CreateAsync(Action configure) => CreateAsync(configure, configureOptions: null, clientOptions: null); @@ -61,96 +47,26 @@ public static async Task CreateAsync( var options = new ReplMcpServerOptions(); configureOptions?.Invoke(options); + options.TransportFactory ??= PipeTransportFactory; - var serviceProvider = app.Services; - var handler = new McpServerHandler(app.Core, options, serviceProvider); - - var clientToServer = new Pipe(); - var serverToClient = new Pipe(); - var cts = new CancellationTokenSource(); - - var inputStream = clientToServer.Reader.AsStream(); - var outputStream = serverToClient.Writer.AsStream(); - var ioContext = new PipeIoContext(inputStream, outputStream); - if (options.TransportFactory is null) - { - options.TransportFactory = static (serverName, io) => new StreamServerTransport( - ((PipeIoContext)io).InputStream, - ((PipeIoContext)io).OutputStream, - serverName); - } - var serverTask = handler.RunAsync(ioContext, cts.Token); - - var clientTransport = new StreamClientTransport( - clientToServer.Writer.AsStream(), - serverToClient.Reader.AsStream()); - - try - { - // Race the handshake against the server. Awaiting only the client means a server that - // fails while starting is observable solely as an initialize timeout carrying the wrong - // exception — which is what once pushed production code into throwing synchronously - // just to stay testable. - var clientTask = McpClient.CreateAsync(clientTransport, clientOptions); - if (ReferenceEquals(await Task.WhenAny(serverTask, clientTask).ConfigureAwait(false), serverTask)) - { - // Rethrows a start failure; a clean early exit means the handshake never completes. - await serverTask.ConfigureAwait(false); - - throw new InvalidOperationException( - "The MCP server stopped before the client completed its handshake."); - } - - var client = await clientTask.ConfigureAwait(false); - - return new McpTestFixture(app, client, serverTask, cts, clientToServer, serverToClient); - } - catch - { - await AbandonAsync(cts, clientToServer, serverToClient).ConfigureAwait(false); - throw; - } - } + var handler = new McpServerHandler(app.Core, options, app.Services); - /// - /// Releases what - /// allocated when construction fails before the fixture takes ownership. - /// - private static async Task AbandonAsync( - CancellationTokenSource cts, - Pipe clientToServer, - Pipe serverToClient) - { - await cts.CancelAsync().ConfigureAwait(false); - await clientToServer.Writer.CompleteAsync().ConfigureAwait(false); - await serverToClient.Writer.CompleteAsync().ConfigureAwait(false); - cts.Dispose(); - } + var session = await McpPipeSession + .StartAsync(handler.RunAsync, clientOptions, CancellationToken.None) + .ConfigureAwait(false); - public async ValueTask DisposeAsync() - { - await Client.DisposeAsync().ConfigureAwait(false); - await _cts.CancelAsync().ConfigureAwait(false); - - await _clientToServer.Writer.CompleteAsync().ConfigureAwait(false); - await _serverToClient.Writer.CompleteAsync().ConfigureAwait(false); - - try - { - await _serverTask.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); - } - catch (OperationCanceledException) - { - // Expected: server RunAsync cancelled during shutdown. - } - catch (TimeoutException) - { - // Server did not shut down within timeout — transport will be collected. - } - - _cts.Dispose(); + return new McpTestFixture(app, session); } + /// Builds the server transport over the pipe pair a supplies. + internal static Func PipeTransportFactory { get; } = + static (serverName, io) => new StreamServerTransport( + ((PipeIoContext)io).InputStream, + ((PipeIoContext)io).OutputStream, + serverName); + + public ValueTask DisposeAsync() => _session.DisposeAsync(); + internal static IServiceProvider EmptyServices => EmptyServiceProvider.Instance; private sealed class EmptyServiceProvider : IServiceProvider From 3f98670a6db07752646ab3ada720b08c4b8383a8 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Fri, 21 Aug 2026 22:16:50 -0400 Subject: [PATCH 10/16] fix(mcp): publish the session snapshot cache as one atomic value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving the snapshot cache onto McpSessionContext replaced a release/acquire protocol with two plain auto-properties. Before this PR (base d4f0398): _snapshot = built; // 399 Volatile.Write(ref _builtSnapshotVersion, snapshotVersion); // 402 ... if (Volatile.Read(ref _builtSnapshotVersion) == snapshotVersion // 314 && _snapshot is { } cached) At the previous head all four accesses were ordinary property reads and writes, while the fast-path reader still runs OUTSIDE SnapshotGate. The semaphore orders writers against each other, not a lock-free reader against a writer, so nothing published the (snapshot, version) pair as a unit: a reader could observe the new version while still seeing the previous snapshot and serve stale discovery state. - Snapshot and version become ONE immutable SnapshotCacheEntry swapped with Volatile.Write/Volatile.Read, so there is no ordering left to get wrong. A plain volatile write is enough here — unlike PublishSnapshotInvalidation, which races several threads and needs its CAS loop. - StaleVersion (0) replaces "set the snapshot, leave the version dirty", which was previously expressible only as two writes with one skipped. Routing versions start at 1 and only increase, so it can never collide. - The intro latch moves behind TryClaimCompatibilityIntro/ ResetCompatibilityIntro, so the Interlocked-by-ref requirement that forced a public int field is stated by the API instead of implied by it. NO NEW TEST, deliberately. I looked for an honest red and there is none: replaying the sequence by hand, the old and new code are logically identical — the stale/retry paths behave the same and are already covered by Given_McpDebounce's five snapshot-cache regressions. The only difference is the memory barrier, and a store-order violation is not deterministically reproducible on x86. The observed evidence for this fix is the diff against d4f0398 quoted above, not a test run. A tautological "publish then read returns the pair" test would assert the implementation, not the contract. Full MCP suite green (232 passed, 1 known Inspector skip); strict Release build of the solution clean. --- src/Repl.Mcp/McpServerHandler.cs | 33 ++++++++++--------- src/Repl.Mcp/McpSessionContext.cs | 54 +++++++++++++++++++++++++++---- 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index 5cbf9dab..c17f3116 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -223,7 +223,7 @@ private async ValueTask ListToolsAsync( var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); if (_options.DynamicToolCompatibility == DynamicToolCompatibilityMode.DiscoverAndCallShim - && Interlocked.CompareExchange(ref context.CompatibilityIntroServed, 1, 0) == 0) + && context.TryClaimCompatibilityIntro()) { _ = SendNotificationSafeAsync(NotificationMethods.ToolListChangedNotification); return new ListToolsResult @@ -367,23 +367,21 @@ private async ValueTask GetSnapshotAsync( CancellationToken cancellationToken) { var snapshotVersion = Volatile.Read(ref _snapshotState).Version; - if (context.BuiltSnapshotVersion == snapshotVersion - && context.Snapshot is { } cached) + if (context.SnapshotCache is { } cached && cached.Version == snapshotVersion) { - return cached; + return cached.Snapshot; } await context.SnapshotGate.WaitAsync(cancellationToken).ConfigureAwait(false); try { snapshotVersion = Volatile.Read(ref _snapshotState).Version; - if (context.BuiltSnapshotVersion == snapshotVersion - && context.Snapshot is { } refreshed) + if (context.SnapshotCache is { } refreshed && refreshed.Version == snapshotVersion) { - return refreshed; + return refreshed.Snapshot; } - var previousSnapshot = context.Snapshot; + var previousSnapshot = context.SnapshotCache?.Snapshot; try { return await BuildCurrentSnapshotAsync(context, snapshotVersion, cancellationToken).ConfigureAwait(false); @@ -400,11 +398,11 @@ private async ValueTask GetSnapshotAsync( catch (Exception) when ( previousSnapshot is not null && Volatile.Read(ref _snapshotState).LastVisibilityRetractionVersion - <= context.BuiltSnapshotVersion) + <= (context.SnapshotCache?.Version ?? McpSessionContext.SnapshotCacheEntry.StaleVersion)) { - // Preserve availability for transient projection failures, but leave the version dirty - // so the next request retries without requiring another routing mutation. - context.Snapshot = previousSnapshot; + // Preserve availability for transient projection failures, but republish as stale so the + // next request retries without requiring another routing mutation. + context.PublishStaleSnapshot(previousSnapshot); return previousSnapshot; } } @@ -453,10 +451,15 @@ private async ValueTask BuildCurrentSnapshotAsync( continue; } - context.Snapshot = built; + // One publication either way: a build that raced a routing bump is still serve-able, but + // is marked stale so the next request rebuilds it. if (observedState.Version == snapshotVersion) { - context.BuiltSnapshotVersion = snapshotVersion; + context.PublishSnapshot(built, snapshotVersion); + } + else + { + context.PublishStaleSnapshot(built); } return built; } @@ -661,7 +664,7 @@ private void OnRoutingInvalidated(bool isVisibilityRetraction) { foreach (var session in _sessions) { - Interlocked.Exchange(ref session.CompatibilityIntroServed, 0); + session.ResetCompatibilityIntro(); } } } diff --git a/src/Repl.Mcp/McpSessionContext.cs b/src/Repl.Mcp/McpSessionContext.cs index e175d7cf..97ba1a14 100644 --- a/src/Repl.Mcp/McpSessionContext.cs +++ b/src/Repl.Mcp/McpSessionContext.cs @@ -1,5 +1,4 @@ using ModelContextProtocol.Server; -using Repl.Documentation; namespace Repl.Mcp; @@ -20,6 +19,9 @@ namespace Repl.Mcp; /// internal sealed class McpSessionContext { + private SnapshotCacheEntry? _snapshotCache; + private int _compatibilityIntroServed; + public McpSessionContext(McpClientRootsService roots, IServiceProvider services) { Roots = roots; @@ -38,12 +40,50 @@ public McpSessionContext(McpClientRootsService roots, IServiceProvider services) /// Serializes snapshot builds for this session. public SemaphoreSlim SnapshotGate { get; } = new(initialCount: 1, maxCount: 1); - /// Cached generated snapshot for this session. - public McpServerHandler.McpGeneratedSnapshot? Snapshot { get; set; } + /// + /// Cached snapshot paired with the routing version it was built at, or + /// before this session's first build. + /// + public SnapshotCacheEntry? SnapshotCache => Volatile.Read(ref _snapshotCache); + + /// Publishes as current for . + public void PublishSnapshot(McpServerHandler.McpGeneratedSnapshot snapshot, long version) => + Volatile.Write(ref _snapshotCache, new SnapshotCacheEntry(snapshot, version)); + + /// + /// Publishes as serve-able but stale, so the next request rebuilds + /// without waiting for another routing mutation. + /// + public void PublishStaleSnapshot(McpServerHandler.McpGeneratedSnapshot snapshot) => + Volatile.Write(ref _snapshotCache, new SnapshotCacheEntry(snapshot, SnapshotCacheEntry.StaleVersion)); - /// Routing version the cached snapshot was built at. - public long BuiltSnapshotVersion { get; set; } + /// + /// Claims this session's one-time compatibility-shim intro; for the first + /// caller only. + /// + public bool TryClaimCompatibilityIntro() => + Interlocked.CompareExchange(ref _compatibilityIntroServed, 1, 0) == 0; - /// Whether this session already received the compatibility-shim intro list. - public int CompatibilityIntroServed; + /// Re-arms the compatibility-shim intro after a routing invalidation. + public void ResetCompatibilityIntro() => Interlocked.Exchange(ref _compatibilityIntroServed, 0); + + /// + /// A generated snapshot and the routing version it was built at, published as ONE value. + /// + /// + /// Held as two independent fields, a lock-free reader could observe the fresh version paired with + /// the previous snapshot and serve stale discovery state; one reference swapped with + /// release/acquire semantics removes the ordering question altogether. Writers are serialized by + /// , so a plain Volatile.Write suffices — unlike + /// , which races several threads and + /// therefore needs a compare-and-swap loop. + /// + internal sealed record SnapshotCacheEntry(McpServerHandler.McpGeneratedSnapshot Snapshot, long Version) + { + /// + /// Marks an entry serve-able but stale. Routing versions start at 1 and only increase, so this + /// can never equal a live version and the next request always rebuilds. + /// + public const long StaleVersion = 0; + } } From 84d73d7653dde15c24675a051e415b8a5f03e5c9 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Fri, 21 Aug 2026 22:28:26 -0400 Subject: [PATCH 11/16] fix(mcp): bind the flowing request on the pre-built options path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BuildMcpServerOptions() is public and docs/mcp-transports.md tells hosts to build it once and create a server per connection. That path goes through BuildStaticServerOptions, which registers NO Handlers: the SDK dispatches straight into the pre-built primitives, so ListToolsAsync/CallToolAsync — and with them the request-binding prologue — never run at all. RED OBSERVED, new Given_McpSharedServerOptions on one shared options instance with two McpServer.Create calls: Expected "sampling-on", actual "sampling-off" A sampling-capable client was told sampling is unavailable, because nothing on that path ever bound a request and the accessor resolved against null. This is the reviewer's exact repro. Green after the change. - McpRequestServerAccessor now binds the whole MessageContext, not just its server. RequestContext derives from MessageContext, so every call site already has one, and the per-request _meta that 2026-07-28 uses for capabilities (and, next, the log level) travels with it. - The session-level fallback field is GONE. McpServer.ClientCapabilities is documented null on the root server for 2026-07-28, so on the modern path that fallback resolved to nothing useful, and on the legacy path it handed out whichever connection attached last — the cross-wiring this type exists to prevent. Its three writers (AttachSession, DetachSession, ResolveContext) drop with it. - McpToolAdapter takes the accessor and exposes BindRequest; the four primitives that dispatch through the Repl pipeline (ReplMcpServerTool, ReplMcpServerResource, ReplMcpServerPrompt, ReplMcpServerUiResource) bind their request. ReplMcpAppLauncherTool does not: it returns static fallback text and resolves no capability service. - The seven handler prologues bind `request` instead of `request.Server`. Knock-on: two test call sites construct McpToolAdapter directly and pass a fresh accessor. Not fixed here, and deliberately: soft roots set on one connection are still visible to another on this same shared-options path. That is a different cause — one captured McpSessionContext, not an unbound request — and it is fixed where context resolution is reworked. Its regression lands with that change rather than as a known-failing test. Full MCP suite green (233 passed, 1 known Inspector skip), including the roots and agent-capability suites that would have surfaced any out-of-request reader depending on the removed fallback. Strict Release build clean. --- src/Repl.Mcp/McpRequestServerAccessor.cs | 38 +++--- src/Repl.Mcp/McpServerHandler.cs | 41 +++---- src/Repl.Mcp/McpToolAdapter.cs | 19 ++- src/Repl.Mcp/ReplMcpServerPrompt.cs | 2 + src/Repl.Mcp/ReplMcpServerResource.cs | 1 + src/Repl.Mcp/ReplMcpServerTool.cs | 1 + src/Repl.Mcp/ReplMcpServerUiResource.cs | 1 + .../Given_McpResourceParameters.cs | 3 +- .../Given_McpSharedServerOptions.cs | 108 ++++++++++++++++++ src/Repl.McpTests/Given_McpToolAdapter.cs | 3 +- 10 files changed, 171 insertions(+), 46 deletions(-) create mode 100644 src/Repl.McpTests/Given_McpSharedServerOptions.cs diff --git a/src/Repl.Mcp/McpRequestServerAccessor.cs b/src/Repl.Mcp/McpRequestServerAccessor.cs index be138772..18d3018a 100644 --- a/src/Repl.Mcp/McpRequestServerAccessor.cs +++ b/src/Repl.Mcp/McpRequestServerAccessor.cs @@ -3,29 +3,33 @@ namespace Repl.Mcp; /// -/// Resolves the a capability call must target. +/// Resolves the a capability call must target, from the flowing request. /// /// -/// SDK 2.0's 2026-07-28 protocol path hands each request a destination-bound -/// , and one handler can serve several sessions. The capability -/// services are singletons (exposed through DI to command handlers), so the effective -/// server must be the one bound to the FLOWING request — a shared mutable field would be -/// overwritten by whichever request attached last, cross-wiring capabilities between -/// concurrent calls. flows with the invocation and cannot -/// leak across requests; the session-level server remains the fallback for code running -/// outside a request (e.g. routing-change notifications). +/// On the 2026-07-28 revision there is no initialize handshake: the client declares its +/// capabilities per request in _meta, and the SDK surfaces them only on the destination-bound +/// server handed to a handler — is documented as +/// on the root server. Capability resolution is therefore a property of the +/// REQUEST, not of the connection. +/// +/// The whole is bound rather than just its server, because the same +/// per-request metadata carries more than the destination (see the log level in +/// ). flows with the invocation and cannot +/// leak across requests. There is deliberately no session-level fallback: a shared field would hand +/// out the capabilities of whichever connection attached last, which is precisely the cross-wiring +/// this type exists to prevent. +/// /// internal sealed class McpRequestServerAccessor { - private readonly AsyncLocal _current = new(); - private McpServer? _session; + private readonly AsyncLocal _current = new(); - /// Server for the flowing request, falling back to the session server. - public McpServer? Effective => _current.Value ?? _session; + /// The request currently flowing on this async context, if any. + public MessageContext? Current => _current.Value; - /// Binds the flowing async context to the request's destination server. - public void BindRequest(McpServer server) => _current.Value = server; + /// Server for the flowing request, or outside a request. + public McpServer? Effective => _current.Value?.Server; - /// Records the session-level server used outside request flows (null when the last session ends). - public void AttachSession(McpServer? server) => _session = server; + /// Binds the flowing async context to the request being served. + public void BindRequest(MessageContext request) => _current.Value = request; } diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index c17f3116..0600f4fa 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -1,4 +1,4 @@ -using System.Diagnostics.CodeAnalysis; +using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Nodes; using ModelContextProtocol; @@ -114,7 +114,6 @@ private McpSessionContext ResolveContext(McpServer? requestServer) if (_externalContext.SessionServer is null && requestServer is not null) { _externalContext.SessionServer = requestServer; - _requestServers.AttachSession(requestServer); EnsureRootsNotificationHandler(requestServer, _externalContext.Roots); } @@ -218,7 +217,7 @@ private async ValueTask ListToolsAsync( RequestContext request, CancellationToken cancellationToken) { - BindRequestServer(request.Server); + BindRequest(request); var context = ResolveContext(request.Server); var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); @@ -246,7 +245,7 @@ private async ValueTask CallToolAsync( RequestContext request, CancellationToken cancellationToken) { - BindRequestServer(request.Server); + BindRequest(request); var context = ResolveContext(request.Server); var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); IDictionary arguments = request.Params.Arguments ?? EmptyArguments; @@ -279,7 +278,7 @@ private async ValueTask ListResourcesAsync( RequestContext request, CancellationToken cancellationToken) { - BindRequestServer(request.Server); + BindRequest(request); var context = ResolveContext(request.Server); var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); return new ListResourcesResult @@ -297,7 +296,7 @@ private async ValueTask ListResourceTemplatesAsync( RequestContext request, CancellationToken cancellationToken) { - BindRequestServer(request.Server); + BindRequest(request); var context = ResolveContext(request.Server); var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); return new ListResourceTemplatesResult @@ -315,7 +314,7 @@ private async ValueTask ReadResourceAsync( RequestContext request, CancellationToken cancellationToken) { - BindRequestServer(request.Server); + BindRequest(request); var context = ResolveContext(request.Server); var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); var uri = request.Params.Uri ?? string.Empty; @@ -332,7 +331,7 @@ private async ValueTask ListPromptsAsync( RequestContext request, CancellationToken cancellationToken) { - BindRequestServer(request.Server); + BindRequest(request); var context = ResolveContext(request.Server); var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); return new ListPromptsResult @@ -345,7 +344,7 @@ private async ValueTask GetPromptAsync( RequestContext request, CancellationToken cancellationToken) { - BindRequestServer(request.Server); + BindRequest(request); var context = ResolveContext(request.Server); var snapshot = await GetSnapshotAsync(context, cancellationToken).ConfigureAwait(false); var promptName = request.Params.Name ?? string.Empty; @@ -469,7 +468,7 @@ private McpGeneratedSnapshot BuildSnapshotCore(McpSessionContext context) { // Project once here so tools/list, tools/call and prompts/list all read the same option list. var model = McpAutomationProjection.Apply(CreateDocumentationModel(context.Services)); - var adapter = new McpToolAdapter(_app, _options, context.Services); + var adapter = new McpToolAdapter(_app, _options, context.Services, _requestServers); var commandsByPath = model.Commands.ToDictionary( command => command.Path, command => command, @@ -535,20 +534,12 @@ private void ValidateCompatibilityToolNames(IReadOnlyList tools) } } - // Request-level binding: capability services resolve the flowing request's - // destination-bound server through the AsyncLocal accessor, so concurrent requests - // (SDK 2.0 creates one destination-bound McpServer per request) cannot cross-wire - // each other's client capabilities. Session-level concerns are handled by - // AttachSession (RunAsync) or the external fallback context (ResolveContext). - private void BindRequestServer(McpServer? server) - { - if (server is null) - { - return; - } - - _requestServers.BindRequest(server); - } + // Request-level binding: capability services resolve the flowing request through the AsyncLocal + // accessor, so concurrent requests (SDK 2.0 creates one destination-bound McpServer per request) + // cannot cross-wire each other's client capabilities. The whole request is bound, not just its + // server, because 2026-07-28 carries the client's capabilities and log level in per-request + // _meta. Session-level concerns are handled by AttachSession (RunAsync). + private void BindRequest(MessageContext request) => _requestServers.BindRequest(request); // Session-level attach: routing-change notifications and the roots list-changed // handler belong to the session servers, registered once per session — never to the @@ -558,7 +549,6 @@ private void AttachSession(McpSessionContext context, McpServer server) lock (_attachLock) { _sessions.Add(context); - _requestServers.AttachSession(server); EnsureRoutingSubscription(); EnsureRootsNotificationHandler(server, context.Roots); } @@ -600,7 +590,6 @@ private void DetachSession(McpSessionContext context) lock (_attachLock) { _sessions.Remove(context); - _requestServers.AttachSession(_sessions.Count > 0 ? _sessions[^1].SessionServer : null); if (_sessions.Count == 0) { UnsubscribeFromRoutingChanges(); diff --git a/src/Repl.Mcp/McpToolAdapter.cs b/src/Repl.Mcp/McpToolAdapter.cs index 5e79bf0a..c9b5ddd0 100644 --- a/src/Repl.Mcp/McpToolAdapter.cs +++ b/src/Repl.Mcp/McpToolAdapter.cs @@ -22,16 +22,33 @@ internal sealed partial class McpToolAdapter private readonly ICoreReplApp _app; private readonly ReplMcpServerOptions _options; private readonly IServiceProvider _services; + private readonly McpRequestServerAccessor _requestServers; private readonly System.Collections.Concurrent.ConcurrentDictionary _toolRoutes = new(StringComparer.OrdinalIgnoreCase); private readonly System.Collections.Concurrent.ConcurrentDictionary _staticToolResults = new(StringComparer.OrdinalIgnoreCase); - public McpToolAdapter(ICoreReplApp app, ReplMcpServerOptions options, IServiceProvider services) + public McpToolAdapter( + ICoreReplApp app, + ReplMcpServerOptions options, + IServiceProvider services, + McpRequestServerAccessor requestServers) { _app = app; _options = options; _services = services; + _requestServers = requestServers; } + /// + /// Binds the flowing async context to before dispatching a command. + /// + /// + /// The pre-built primitives ( and friends) are dispatched straight + /// by the SDK on the BuildMcpServerOptions path, bypassing 's + /// request handlers entirely. Without this, capability services resolved from DI would have no + /// request to resolve against and would report every client capability as unavailable. + /// + internal void BindRequest(MessageContext request) => _requestServers.BindRequest(request); + internal string ForcedOutputMimeType { get diff --git a/src/Repl.Mcp/ReplMcpServerPrompt.cs b/src/Repl.Mcp/ReplMcpServerPrompt.cs index 8568f010..61be9544 100644 --- a/src/Repl.Mcp/ReplMcpServerPrompt.cs +++ b/src/Repl.Mcp/ReplMcpServerPrompt.cs @@ -64,6 +64,8 @@ public override async ValueTask GetAsync( RequestContext request, CancellationToken cancellationToken = default) { + _adapter.BindRequest(request); + // Prompt arguments are already JsonElement — pass through directly. var jsonArgs = request.Params.Arguments is { } args ? new Dictionary(args, StringComparer.Ordinal) diff --git a/src/Repl.Mcp/ReplMcpServerResource.cs b/src/Repl.Mcp/ReplMcpServerResource.cs index bd979708..dfae7da3 100644 --- a/src/Repl.Mcp/ReplMcpServerResource.cs +++ b/src/Repl.Mcp/ReplMcpServerResource.cs @@ -68,6 +68,7 @@ public override async ValueTask ReadAsync( RequestContext request, CancellationToken cancellationToken = default) { + _adapter.BindRequest(request); var arguments = ExtractArguments(request.Params.Uri); var result = await _adapter.InvokeResourceAsync( diff --git a/src/Repl.Mcp/ReplMcpServerTool.cs b/src/Repl.Mcp/ReplMcpServerTool.cs index a6376770..7ab5e50f 100644 --- a/src/Repl.Mcp/ReplMcpServerTool.cs +++ b/src/Repl.Mcp/ReplMcpServerTool.cs @@ -49,6 +49,7 @@ public override async ValueTask InvokeAsync( RequestContext request, CancellationToken cancellationToken = default) { + _adapter.BindRequest(request); var arguments = request.Params.Arguments ?? new Dictionary(StringComparer.Ordinal); var progressToken = request.Params.ProgressToken; diff --git a/src/Repl.Mcp/ReplMcpServerUiResource.cs b/src/Repl.Mcp/ReplMcpServerUiResource.cs index 5c49059a..462f7792 100644 --- a/src/Repl.Mcp/ReplMcpServerUiResource.cs +++ b/src/Repl.Mcp/ReplMcpServerUiResource.cs @@ -57,6 +57,7 @@ public override async ValueTask ReadAsync( RequestContext request, CancellationToken cancellationToken = default) { + _adapter.BindRequest(request); var arguments = ExtractArguments(request.Params.Uri); var result = await _adapter.InvokeAsync( diff --git a/src/Repl.McpTests/Given_McpResourceParameters.cs b/src/Repl.McpTests/Given_McpResourceParameters.cs index 79831561..52de451f 100644 --- a/src/Repl.McpTests/Given_McpResourceParameters.cs +++ b/src/Repl.McpTests/Given_McpResourceParameters.cs @@ -255,7 +255,8 @@ public async Task When_ResourceCommandFails_Then_ReadThrowsMcpException() public async Task When_ResourceRouteIsUnknown_Then_AdapterReturnsTextError() { await using var services = new ServiceCollection().BuildServiceProvider(); - var adapter = new McpToolAdapter(ReplApp.Create().Core, new ReplMcpServerOptions(), services); + var adapter = new McpToolAdapter( + ReplApp.Create().Core, new ReplMcpServerOptions(), services, new McpRequestServerAccessor()); var result = await adapter.InvokeResourceAsync( "missing", diff --git a/src/Repl.McpTests/Given_McpSharedServerOptions.cs b/src/Repl.McpTests/Given_McpSharedServerOptions.cs new file mode 100644 index 00000000..2c84bb21 --- /dev/null +++ b/src/Repl.McpTests/Given_McpSharedServerOptions.cs @@ -0,0 +1,108 @@ +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using Repl.Mcp; + +namespace Repl.McpTests; + +/// +/// Regressions for the documented multi-connection host pattern: build +/// BuildMcpServerOptions() ONCE and create an per connection +/// (docs/mcp-transports.md). That path bypasses 's request handlers +/// entirely — the SDK dispatches straight into the pre-built primitives — so nothing that relies on +/// the handler prologue applies to it. +/// +[TestClass] +public sealed class Given_McpSharedServerOptions +{ + // The SDK's McpProtocolVersions constants are internal, so the revision is pinned here. + private const string ModernProtocolVersion = "2026-07-28"; + + [TestMethod] + [Description("Guards capability resolution on the documented reusable-options path: two connections created from ONE BuildMcpServerOptions() result must each observe their OWN client's capabilities. The pre-built primitives never run the handler's request prologue, so without per-invocation request binding a sampling-capable client is told sampling is unavailable — the capability is resolved against nothing at all.")] + public async Task When_TwoConnectionsShareOneOptionsInstance_Then_CapabilitiesAreRequestScoped() + { + var app = ReplApp.Create(); + app.UseMcpServer(); + // Distinct tokens, not "supported"/"not-supported": the tool result is JSON, so the text block + // carries quotes, and a substring assertion on the shorter word would match both answers. + app.Map("probe", (IMcpSampling sampling) => sampling.IsSupported ? "sampling-on" : "sampling-off"); + + // Built once and reused across connections, exactly as docs/mcp-transports.md prescribes. + var mcpOptions = app.BuildMcpServerOptions(); + using var cts = new CancellationTokenSource(); + + var capable = await StartAsync(mcpOptions, BuildSamplingClientOptions(), cts.Token).ConfigureAwait(false); + await using var capableScope = capable.ConfigureAwait(false); + var plain = await StartAsync(mcpOptions, clientOptions: null, cts.Token).ConfigureAwait(false); + await using var plainScope = plain.ConfigureAwait(false); + + capable.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); + plain.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); + + var capableText = await CallProbeAsync(capable, cts.Token).ConfigureAwait(false); + var plainText = await CallProbeAsync(plain, cts.Token).ConfigureAwait(false); + + capableText.Should().Contain("sampling-on"); + plainText.Should().Contain("sampling-off"); + } + + private static Task CallProbeAsync(McpPipeSession session, CancellationToken cancellationToken) => + CallAsync(session, "probe", cancellationToken); + + private static async Task CallAsync( + McpPipeSession session, + string toolName, + CancellationToken cancellationToken) + { + var result = await session.Client.CallToolAsync( + toolName: toolName, + arguments: new Dictionary(StringComparer.Ordinal), + cancellationToken: cancellationToken).ConfigureAwait(false); + + return result.Content.OfType().First().Text; + } + + /// + /// Starts one connection over the shared , mirroring the sample in + /// docs/mcp-transports.md — including passing no service provider to McpServer.Create. + /// + private static Task StartAsync( + McpServerOptions mcpOptions, + McpClientOptions? clientOptions, + CancellationToken cancellationToken) => + McpPipeSession.StartAsync( + async (io, token) => + { + var transport = new StreamServerTransport(io.InputStream, io.OutputStream, "shared-options-server"); + var server = McpServer.Create(transport, mcpOptions); + try + { + await server.RunAsync(token).ConfigureAwait(false); + } + finally + { + await server.DisposeAsync().ConfigureAwait(false); + await transport.DisposeAsync().ConfigureAwait(false); + } + }, + clientOptions, + cancellationToken); + + // Sampling is deprecated by MCP spec 2026-07-28 (SEP-2577, MCP9005) but still supported by + // Repl.Mcp until the SDK removes the surface (#51). +#pragma warning disable MCP9005 + private static McpClientOptions BuildSamplingClientOptions() => new() + { + Capabilities = new ClientCapabilities { Sampling = new SamplingCapability() }, + Handlers = new McpClientHandlers + { + SamplingHandler = static (request, _, _) => ValueTask.FromResult(new CreateMessageResult + { + Content = [new TextContentBlock { Text = "ga" }], + Model = "test-model", + }), + }, + }; +#pragma warning restore MCP9005 +} diff --git a/src/Repl.McpTests/Given_McpToolAdapter.cs b/src/Repl.McpTests/Given_McpToolAdapter.cs index 4a286695..0dc32184 100644 --- a/src/Repl.McpTests/Given_McpToolAdapter.cs +++ b/src/Repl.McpTests/Given_McpToolAdapter.cs @@ -633,7 +633,8 @@ public async Task When_StringToolValueNamesResponseFile_Then_ProgrammaticInvocat }) .WithOption("hidden", static option => option.Hidden()); await using var services = new ServiceCollection().BuildServiceProvider(); - var adapter = new McpToolAdapter(app.Core, new ReplMcpServerOptions(), services); + var adapter = new McpToolAdapter( + app.Core, new ReplMcpServerOptions(), services, new McpRequestServerAccessor()); adapter.RegisterRoute( "deploy", new ReplDocCommand( From 3190574b55887cd0f2d85c7223471088f23b7142 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Sun, 23 Aug 2026 10:36:21 -0400 Subject: [PATCH 12/16] fix(mcp): let the SDK fan out list-changed to actual subscribers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repl sent */list_changed itself, looping over session servers. A server cannot do that correctly on 2026-07-28: it has no access to the subscription registry, so it delivers every notification type to every client regardless of what each asked for, and untagged. RED OBSERVED, new Given_McpSubscriptions: a client subscribing to prompts/list_changed ONLY still received tools/list_changed ("Expected boolean to be False because the client never subscribed to tools/list_changed, but found True"), and the notification payload had no params at all — hence no _meta/subscriptionId. Both halves green after. VERIFIED FIRST, because the whole approach depends on it: subscriptions/listen does reach a stream-transport (stdio-shaped) server, and the SDK's built-in handler acknowledges the filters it grants. That probe is kept as a test. Backward compatibility is preserved by NOT pinning the server. Leaving ProtocolVersion null keeps it multi-revision, and the SDK then picks the delivery mode per client: an unsolicited session-wide broadcast for initialize-era clients, filtered and subscriptionId-tagged delivery over the listen stream for 2026-07-28 ones. Pinning the server to 2025-11-25 would have made it reject modern per-request metadata outright — compatible, but at the cost of the new protocol. - Three empty primitive collections act as pure signals: clearing an empty collection raises Changed without mutating anything a client could observe. That Clear() raises unconditionally is NOT documented, so When_ClearingAnEmptyCollection_Then_ChangedStillFires pins it — a future SDK optimising Clear() into a no-op must fail loudly, not silently stop discovery notifications. (Deriving to reach the protected RaiseChanged was the first attempt; McpServerResourceCollection is sealed, and add/remove of a sentinel would briefly expose an internal resource to a concurrent list.) - SendNotificationSafeAsync goes, and with it the sequential fan-out that cost up to 5s per wedged peer per notification (15s per invalidation across the three), and its two entirely silent catch blocks. No ILogger is introduced: the swallowed failures are gone with their owner. - McpProtocolRevisions replaces three copies of the "2026-07-28" literal across test classes. BEHAVIOUR CHANGE: a 2026-07-28 client that never opens subscriptions/listen no longer receives */list_changed. That is what SEP-2575 requires — the same sentence that forbids sending unrequested types forbids sending anything to a client with no subscription — so there is no conformant middle path. Mitigated by ttlMs:0 on list results, which stops clients caching them at all. Tests updated to say which revision they characterise: the two shim tests pin their CLIENT to 2025-11-25 (the shim exists precisely for clients that do not refresh a changing tool list), and the surviving-session lifetime test now subscribes, so it exercises the modern path rather than the legacy broadcast. Full solution green: 1464 passed, 1 known Inspector skip. --- src/Repl.Mcp/McpProtocolRevisions.cs | 22 +++ src/Repl.Mcp/McpServerHandler.cs | 69 +++---- src/Repl.Mcp/McpSessionContext.cs | 9 +- .../Given_McpConcurrentSessions.cs | 36 +++- .../Given_McpRootsAndDynamicTools.cs | 14 +- .../Given_McpSharedServerOptions.cs | 7 +- src/Repl.McpTests/Given_McpSubscriptions.cs | 173 ++++++++++++++++++ 7 files changed, 271 insertions(+), 59 deletions(-) create mode 100644 src/Repl.Mcp/McpProtocolRevisions.cs create mode 100644 src/Repl.McpTests/Given_McpSubscriptions.cs diff --git a/src/Repl.Mcp/McpProtocolRevisions.cs b/src/Repl.Mcp/McpProtocolRevisions.cs new file mode 100644 index 00000000..eda22733 --- /dev/null +++ b/src/Repl.Mcp/McpProtocolRevisions.cs @@ -0,0 +1,22 @@ +namespace Repl.Mcp; + +/// +/// MCP protocol revisions Repl reasons about explicitly. +/// +/// +/// The SDK's own McpProtocolVersions class is internal, so the literals have to live here. +/// +internal static class McpProtocolRevisions +{ + /// + /// The last revision built on the initialize handshake, and therefore the last one with + /// protocol-level sessions. + /// + public const string LastWithSessions = "2025-11-25"; + + /// + /// The revision that removed protocol sessions (SEP-2567) and moved per-call state — client + /// capabilities, log level — into per-request _meta (SEP-2575). + /// + public const string Sessionless = "2026-07-28"; +} diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index 0600f4fa..70cc7f61 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -50,10 +50,16 @@ internal sealed class McpServerHandler private ITimer? _debounceTimer; private static readonly TimeSpan DebounceDelay = TimeSpan.FromMilliseconds(100); - // Notifications are fire-and-forget best-effort — a stuck stdio peer must not hang this - // indefinitely, since nothing awaits it and it would otherwise pile up one task per - // invalidation forever. - private static readonly TimeSpan NotificationSendTimeout = TimeSpan.FromSeconds(5); + // Discovery-change signals. These collections stay EMPTY and never contribute a primitive to a + // list response: they exist only so the SDK's own fan-out runs, because that is the only code + // with access to the subscription registry. On 2026-07-28 it delivers each notification type + // ONLY to clients that requested it through subscriptions/listen, over that request's stream and + // tagged with its id, while still broadcasting session-wide to initialize-era clients. Every + // McpServer built from the options subscribes on construction and unsubscribes on dispose, which + // is what makes one shared instance correct across concurrent connections. + private readonly McpServerPrimitiveCollection _toolListChanged = new(); + private readonly McpServerResourceCollection _resourceListChanged = new(); + private readonly McpServerPrimitiveCollection _promptListChanged = new(); public McpServerHandler( ICoreReplApp app, @@ -136,7 +142,6 @@ public async Task RunAsync(IReplIoContext io, CancellationToken ct) { var context = CreateSessionContext(); var server = McpServer.Create(transport, serverOptions, serviceProvider: context.Services); - context.SessionServer = server; AttachSession(context, server); try @@ -182,6 +187,11 @@ internal McpServerOptions BuildDynamicServerOptions() ListPromptsHandler = ListPromptsAsync, GetPromptHandler = GetPromptAsync, }, + // Empty on purpose: collections augment the handlers rather than replace them, so the + // tool graph still comes entirely from the handlers above. See the field declarations. + ToolCollection = _toolListChanged, + ResourceCollection = _resourceListChanged, + PromptCollection = _promptListChanged, }; } @@ -224,7 +234,7 @@ private async ValueTask ListToolsAsync( if (_options.DynamicToolCompatibility == DynamicToolCompatibilityMode.DiscoverAndCallShim && context.TryClaimCompatibilityIntro()) { - _ = SendNotificationSafeAsync(NotificationMethods.ToolListChangedNotification); + SignalToolListChanged(); return new ListToolsResult { Tools = @@ -662,50 +672,27 @@ private void OnRoutingInvalidated(bool isVisibilityRetraction) { _debounceTimer?.Dispose(); _debounceTimer = _timeProvider.CreateTimer( - _ => _ = SendDiscoveryNotificationsSafeAsync(), + _ => SignalDiscoveryChanged(), state: null, dueTime: DebounceDelay, period: Timeout.InfiniteTimeSpan); } } - private async Task SendDiscoveryNotificationsSafeAsync() + private void SignalDiscoveryChanged() { - await SendNotificationSafeAsync(NotificationMethods.ToolListChangedNotification).ConfigureAwait(false); - await SendNotificationSafeAsync(NotificationMethods.ResourceListChangedNotification).ConfigureAwait(false); - await SendNotificationSafeAsync(NotificationMethods.PromptListChangedNotification).ConfigureAwait(false); + _toolListChanged.Clear(); + _resourceListChanged.Clear(); + _promptListChanged.Clear(); } - private async Task SendNotificationSafeAsync(string method) - { - McpServer[] sessions; - lock (_attachLock) - { - sessions = [ - .. _sessions - .Select(static session => session.SessionServer) - .OfType(), - ]; - } - - foreach (var server in sessions) - { - try - { - using var timeoutCts = new CancellationTokenSource(NotificationSendTimeout); - await server.SendNotificationAsync(method, timeoutCts.Token).ConfigureAwait(false); - } - catch (OperationCanceledException) - { - // Notifications are best-effort. Cancellation is not actionable here. - } - catch (Exception) - { - // Notifications are best-effort per session. The next list/read request - // will rebuild on demand. - } - } - } + // Clearing an already-empty primitive collection raises its Changed event without mutating + // anything, which is what lets an empty collection act as a pure signal. That the event fires + // unconditionally is NOT documented on Clear(), so it is pinned by + // Given_McpSubscriptions.When_ClearingAnEmptyCollection_Then_ChangedStillFires: if a future SDK + // turns Clear() into a no-op, that test fails loudly instead of discovery notifications silently + // disappearing. + private void SignalToolListChanged() => _toolListChanged.Clear(); private void UnsubscribeFromRoutingChanges() { diff --git a/src/Repl.Mcp/McpSessionContext.cs b/src/Repl.Mcp/McpSessionContext.cs index 97ba1a14..831fd01a 100644 --- a/src/Repl.Mcp/McpSessionContext.cs +++ b/src/Repl.Mcp/McpSessionContext.cs @@ -34,7 +34,14 @@ public McpSessionContext(McpClientRootsService roots, IServiceProvider services) /// Per-session service overlay handed to McpServer.Create. public IServiceProvider Services { get; } - /// Session server used for server-initiated notifications. + /// + /// Latches the first server observed by the externally hosted fallback context, so its + /// roots-list-changed handler is registered once. + /// + /// + /// This used to be the destination for server-initiated notifications; the SDK now owns that + /// fan-out, leaving only the latch. It disappears with the fallback context itself. + /// public McpServer? SessionServer { get; set; } /// Serializes snapshot builds for this session. diff --git a/src/Repl.McpTests/Given_McpConcurrentSessions.cs b/src/Repl.McpTests/Given_McpConcurrentSessions.cs index 228eec42..18c8de05 100644 --- a/src/Repl.McpTests/Given_McpConcurrentSessions.cs +++ b/src/Repl.McpTests/Given_McpConcurrentSessions.cs @@ -8,11 +8,6 @@ namespace Repl.McpTests; [TestClass] public sealed class Given_McpConcurrentSessions { - // The SDK's McpProtocolVersions constants are internal, so the revisions are pinned here. - // 2026-07-28 (SEP-2567) is the sessionless, per-request-metadata revision the default client - // negotiates; every guarantee in this file is written against it, hence the explicit assertions. - private const string ModernProtocolVersion = "2026-07-28"; - [TestMethod] [Description("Pins the protocol revision every other guarantee in this file is written against: two sessions sharing one handler must both negotiate 2026-07-28. Without this, a silent fallback to the 2025-11-25 initialize handshake would make the per-session capability and catalog assertions below describe a revision they were never meant to characterise — which is exactly how four review waves missed the sessionless-protocol defects.")] public async Task When_TwoSessionsShareOneHandler_Then_BothNegotiateTheModernRevision() @@ -28,8 +23,8 @@ public async Task When_TwoSessionsShareOneHandler_Then_BothNegotiateTheModernRev var sessionB = await StartSessionAsync(handler, clientOptions: null, cts.Token).ConfigureAwait(false); await using var scopeB = sessionB.ConfigureAwait(false); - sessionA.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); - sessionB.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); + sessionA.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.Sessionless); + sessionB.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.Sessionless); } [TestMethod] @@ -106,7 +101,7 @@ public async Task When_TwoRootCapableClientsShareHandler_Then_EachSeesOwnRoots() } [TestMethod] - [Description("Guards routing-notification lifetime across sessions: when the first-attached session closes, the surviving session must still receive tools/list_changed after a routing invalidation — session attachment must be reference-counted, not first-wins with a handler-wide unsubscribe on first close.")] + [Description("Guards routing-notification lifetime across sessions: when the first-attached session closes, the surviving session must still receive tools/list_changed after a routing invalidation — session attachment must be reference-counted, not first-wins with a handler-wide unsubscribe on first close. The surviving session subscribes through subscriptions/listen, which is how a 2026-07-28 client asks for the notification at all.")] public async Task When_FirstSessionCloses_Then_SurvivingSessionStillReceivesRoutingNotifications() { var app = ReplApp.Create(); @@ -129,6 +124,16 @@ public async Task When_FirstSessionCloses_Then_SurvivingSessionStillReceivesRout }); await using var scopeRegistration = registration.ConfigureAwait(false); + using var listenCts = new CancellationTokenSource(); + var listenTask = sessionB.Client.SendRequestAsync( + RequestMethods.SubscriptionsListen, + new SubscriptionsListenRequestParams + { + Notifications = new SubscriptionsListenNotifications { ToolsListChanged = true }, + }, + cancellationToken: listenCts.Token) + .AsTask(); + // Both sessions are live; close the FIRST one, then invalidate routing. Disposing the // session awaits its RunAsync, so a teardown fault surfaces here instead of being swallowed. await sessionA.DisposeAsync().ConfigureAwait(false); @@ -137,6 +142,19 @@ public async Task When_FirstSessionCloses_Then_SurvivingSessionStillReceivesRout app.Core.InvalidateRouting(); await listChanged.Task.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); + + await listenCts.CancelAsync().ConfigureAwait(false); + try + { + // Started above and awaited only after cancellation; MSTest has no sync context. +#pragma warning disable VSTHRD003 + await listenTask.ConfigureAwait(false); +#pragma warning restore VSTHRD003 + } + catch (OperationCanceledException) + { + // Expected: the listen stream ends on cancellation. + } } [TestMethod] @@ -177,7 +195,7 @@ public async Task When_ToolGraphIsSessionGated_Then_ListResultIsTaggedPrivateAnd var session = await StartSessionAsync(handler, BuildRootsClientOptions("file:///ga"), cts.Token).ConfigureAwait(false); await using var scope = session.ConfigureAwait(false); - session.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); + session.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.Sessionless); var result = await session.Client.SendRequestAsync( RequestMethods.ToolsList, new ListToolsRequestParams(), diff --git a/src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs b/src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs index 5eb7b01b..7fe62aef 100644 --- a/src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs +++ b/src/Repl.McpTests/Given_McpRootsAndDynamicTools.cs @@ -1,4 +1,4 @@ -using System.Text.Json; +using System.Text.Json; using ModelContextProtocol; using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; @@ -105,7 +105,11 @@ public async Task When_DynamicToolCompatibilityEnabled_Then_ClientCanDiscoverAnd { app.Map("echo {msg}", (string msg) => $"echo:{msg}"); }, - configureOptions: options => options.DynamicToolCompatibility = DynamicToolCompatibilityMode.DiscoverAndCallShim); + configureOptions: options => options.DynamicToolCompatibility = DynamicToolCompatibilityMode.DiscoverAndCallShim, + // The shim exists for clients that do not refresh a changing tool list — i.e. initialize-era + // clients. Pinning the CLIENT (not the server) keeps the server multi-revision while making + // this test state which revision its unsolicited-notification expectation belongs to. + clientOptions: new McpClientOptions { ProtocolVersion = McpProtocolRevisions.LastWithSessions }); await using var registration = fixture.Client.RegisterNotificationHandler( NotificationMethods.ToolListChangedNotification, @@ -162,7 +166,11 @@ public async Task When_RoutingChanges_AfterCompatibilityIntro_Then_ShimIsServedA { app.Map("echo {msg}", (string msg) => $"echo:{msg}"); }, - configureOptions: options => options.DynamicToolCompatibility = DynamicToolCompatibilityMode.DiscoverAndCallShim); + configureOptions: options => options.DynamicToolCompatibility = DynamicToolCompatibilityMode.DiscoverAndCallShim, + // The shim exists for clients that do not refresh a changing tool list — i.e. initialize-era + // clients. Pinning the CLIENT (not the server) keeps the server multi-revision while making + // this test state which revision its unsolicited-notification expectation belongs to. + clientOptions: new McpClientOptions { ProtocolVersion = McpProtocolRevisions.LastWithSessions }); await using var registration = fixture.Client.RegisterNotificationHandler( NotificationMethods.ToolListChangedNotification, diff --git a/src/Repl.McpTests/Given_McpSharedServerOptions.cs b/src/Repl.McpTests/Given_McpSharedServerOptions.cs index 2c84bb21..0354c40e 100644 --- a/src/Repl.McpTests/Given_McpSharedServerOptions.cs +++ b/src/Repl.McpTests/Given_McpSharedServerOptions.cs @@ -15,9 +15,6 @@ namespace Repl.McpTests; [TestClass] public sealed class Given_McpSharedServerOptions { - // The SDK's McpProtocolVersions constants are internal, so the revision is pinned here. - private const string ModernProtocolVersion = "2026-07-28"; - [TestMethod] [Description("Guards capability resolution on the documented reusable-options path: two connections created from ONE BuildMcpServerOptions() result must each observe their OWN client's capabilities. The pre-built primitives never run the handler's request prologue, so without per-invocation request binding a sampling-capable client is told sampling is unavailable — the capability is resolved against nothing at all.")] public async Task When_TwoConnectionsShareOneOptionsInstance_Then_CapabilitiesAreRequestScoped() @@ -37,8 +34,8 @@ public async Task When_TwoConnectionsShareOneOptionsInstance_Then_CapabilitiesAr var plain = await StartAsync(mcpOptions, clientOptions: null, cts.Token).ConfigureAwait(false); await using var plainScope = plain.ConfigureAwait(false); - capable.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); - plain.Client.NegotiatedProtocolVersion.Should().Be(ModernProtocolVersion); + capable.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.Sessionless); + plain.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.Sessionless); var capableText = await CallProbeAsync(capable, cts.Token).ConfigureAwait(false); var plainText = await CallProbeAsync(plain, cts.Token).ConfigureAwait(false); diff --git a/src/Repl.McpTests/Given_McpSubscriptions.cs b/src/Repl.McpTests/Given_McpSubscriptions.cs new file mode 100644 index 00000000..f8d8173f --- /dev/null +++ b/src/Repl.McpTests/Given_McpSubscriptions.cs @@ -0,0 +1,173 @@ +using System.Text.Json; +using ModelContextProtocol; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using Repl.Mcp; + +namespace Repl.McpTests; + +/// +/// Covers subscriptions/listen (SEP-2575) delivery over the in-process stream transport that +/// stands in for stdio, and the SDK behaviour Repl's discovery signal depends on. +/// +[TestClass] +public sealed class Given_McpSubscriptions +{ + [TestMethod] + [Description("Pins the undocumented SDK behaviour the discovery signal rests on: clearing an already-empty primitive collection must still raise Changed. McpServerHandler uses empty collections as pure list-changed signals, so if a future SDK turns Clear() into a no-op when the collection is empty, discovery notifications would silently stop; this test fails loudly instead.")] + public void When_ClearingAnEmptyCollection_Then_ChangedStillFires() + { + var resources = new McpServerResourceCollection(); + var tools = new McpServerPrimitiveCollection(); + var resourceSignals = 0; + var toolSignals = 0; + resources.Changed += (_, _) => resourceSignals++; + tools.Changed += (_, _) => toolSignals++; + + resources.Clear(); + tools.Clear(); + + resourceSignals.Should().Be(1); + toolSignals.Should().Be(1); + resources.Count.Should().Be(0, because: "the signal must not mutate anything a client could observe"); + tools.Count.Should().Be(0, because: "the signal must not mutate anything a client could observe"); + } + + [TestMethod] + [Description("Guards backward compatibility while the modern path is filtered: an initialize-era client that pins 2025-11-25 and opens NO subscription must still receive tools/list_changed as an unsolicited session-wide broadcast. Delegating fan-out to the SDK must not cost existing hosts their discovery notifications — the server stays multi-revision and the SDK picks the delivery mode per client.")] + public async Task When_LegacyClientNeverSubscribes_Then_ListChangedIsStillBroadcast() + { + await using var fixture = await McpTestFixture.CreateAsync( + app => app.Map("alpha", () => "a"), + configureOptions: null, + clientOptions: new McpClientOptions { ProtocolVersion = McpProtocolRevisions.LastWithSessions }) + .ConfigureAwait(false); + fixture.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.LastWithSessions); + + var toolsChanged = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + var registration = Capture(fixture, NotificationMethods.ToolListChangedNotification, toolsChanged); + await using var registrationScope = registration.ConfigureAwait(false); + + fixture.App.Map("late", () => "l"); + fixture.App.Core.InvalidateRouting(); + + await toolsChanged.Task.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); + } + + [TestMethod] + [Description("Confirms subscriptions/listen reaches a stream-transport (stdio-shaped) server and that the SDK's built-in handler acknowledges the filters it grants. Repl delegates list-changed fan-out to that pipeline, so this is the precondition the delegation rests on.")] + public async Task When_ClientOpensSubscriptionsListen_Then_ServerAcknowledges() + { + await using var fixture = await McpTestFixture.CreateAsync(app => app.Map("alpha", () => "a")) + .ConfigureAwait(false); + fixture.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.Sessionless); + + var acknowledged = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + var registration = Capture(fixture, NotificationMethods.SubscriptionsAcknowledgedNotification, acknowledged); + await using var registrationScope = registration.ConfigureAwait(false); + + using var listenCts = new CancellationTokenSource(); + var listenTask = OpenListenAsync( + fixture, + new SubscriptionsListenNotifications { ToolsListChanged = true }, + listenCts.Token); + + var notification = await acknowledged.Task.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); + + var granted = notification.Params + .Deserialize(McpJsonUtilities.DefaultOptions); + granted.Should().NotBeNull(); + granted!.Notifications.ToolsListChanged.Should().BeTrue(); + + await CloseListenAsync(listenTask, listenCts).ConfigureAwait(false); + } + + [TestMethod] + [Description("Guards SEP-2575 delivery filtering: a 2026-07-28 client that subscribes to prompts/list_changed only must NOT receive tools/list_changed, and the notification it does receive must carry its listen request id. A server that sends */list_changed itself has no access to the subscription registry, so it delivers every type to every client, untagged.")] + public async Task When_ClientSubscribesToPromptsOnly_Then_ToolListChangedIsNotDelivered() + { + await using var fixture = await McpTestFixture.CreateAsync(app => app.Map("alpha", () => "a")) + .ConfigureAwait(false); + fixture.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.Sessionless); + + var toolsChanged = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + var promptsChanged = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + var toolsRegistration = Capture(fixture, NotificationMethods.ToolListChangedNotification, toolsChanged); + await using var toolsScope = toolsRegistration.ConfigureAwait(false); + var promptsRegistration = Capture(fixture, NotificationMethods.PromptListChangedNotification, promptsChanged); + await using var promptsScope = promptsRegistration.ConfigureAwait(false); + + using var listenCts = new CancellationTokenSource(); + var listenTask = OpenListenAsync( + fixture, + new SubscriptionsListenNotifications { PromptsListChanged = true }, + listenCts.Token); + + fixture.App.Map("late", () => "l"); + fixture.App.Core.InvalidateRouting(); + + // Discovery signals fire tools-then-resources-then-prompts, so observing the prompts + // notification proves the tools one has already had its chance. + var prompts = await promptsChanged.Task.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); + + // Render to a string rather than asserting on the JsonNode: NotBeNull on a node reached via + // ?. is vacuous (the null-conditional result satisfies it even when the payload is absent), + // and the listen request id is a JSON-RPC id, so it may be a number as well as a string. + var subscriptionId = prompts.Params?["_meta"]?[MetaKeys.SubscriptionId]?.ToJsonString(); + subscriptionId.Should().NotBeNullOrEmpty( + because: "SEP-2575 requires every subscription notification to carry its listen request id"); + toolsChanged.Task.IsCompleted.Should().BeFalse( + because: "the client never subscribed to tools/list_changed"); + + await CloseListenAsync(listenTask, listenCts).ConfigureAwait(false); + } + + private static IAsyncDisposable Capture( + McpTestFixture fixture, + string method, + TaskCompletionSource received) => + fixture.Client.RegisterNotificationHandler( + method, + (notification, _) => + { + received.TrySetResult(notification); + return ValueTask.CompletedTask; + }); + + /// + /// subscriptions/listen is a long-lived request: the response is held open for the + /// subscription's lifetime, so it must not be awaited until the stream is cancelled. + /// + private static Task OpenListenAsync( + McpTestFixture fixture, + SubscriptionsListenNotifications filters, + CancellationToken cancellationToken) => + fixture.Client.SendRequestAsync( + RequestMethods.SubscriptionsListen, + new SubscriptionsListenRequestParams { Notifications = filters }, + cancellationToken: cancellationToken) + .AsTask(); + + private static async Task CloseListenAsync(Task listenTask, CancellationTokenSource listenCts) + { + await listenCts.CancelAsync().ConfigureAwait(false); + try + { + // The listen request is deliberately started by the caller and awaited only once its + // stream has been cancelled. MSTest runs without a synchronization context, so the + // deadlock VSTHRD003 guards against cannot arise here. +#pragma warning disable VSTHRD003 + await listenTask.ConfigureAwait(false); +#pragma warning restore VSTHRD003 + } + catch (OperationCanceledException) + { + // Expected: the listen stream ends on cancellation. + } + } +} From c6b4caeb272844012f6833fa58282ccbd11334c1 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Sun, 23 Aug 2026 23:28:20 -0400 Subject: [PATCH 13/16] fix(mcp): honour the per-request log level and own the message contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SendMessageAsync emitted notifications/message whenever any server was bound, and IsLoggingSupported was just "is there a server". The 2026-07-28 revision (SEP-2575) replaced logging/setLevel with a per-request _meta/io.modelcontextprotocol/logLevel field and states the server MUST NOT emit message notifications for a request that omitted it. The SDK projects the field onto JsonRpcMessageContext but consumes it nowhere, so the filtering was nobody's. RED OBSERVED: the two existing feedback regressions, which run a default (2026-07-28) client, kept receiving all three notifications. They now pin an initialize-era client — see below — and the modern rule has its own test. FINDING, established by probing rather than assumed: with SDK 2.2.0 a client CANNOT ask for message notifications on 2026-07-28 at all. SetLoggingLevelAsync throws ("not available on protocol version '2026-07-28'. Use the per-request '_meta/io.modelcontextprotocol/logLevel' field instead"), McpClientOptions exposes no level, and a caller's _meta is discarded — dumping what the server receives shows the SDK replaced it with exactly its own three keys: {"io.modelcontextprotocol/protocolVersion":"2026-07-28", "io.modelcontextprotocol/clientInfo":{...}, "io.modelcontextprotocol/clientCapabilities":{}} So on the modern revision this channel is effectively closed. That makes carrying feedback in the tool result the only way a modern host sees it — not a nicety. - Messages that cannot be delivered as notifications are buffered per invocation and appended to CallToolResult as trailing content blocks. The command's own payload stays the FIRST block and StructuredContent is untouched, so a caller reading the primary result is unaffected. Only requests with no usable level buffer anything, so a client that does receive notifications never sees a message twice. Resource reads get no such block: their body must match the advertised MIME type. - Legacy revisions keep session-wide logging/setLevel semantics unchanged, including sending everything when the client never set a level. Backward compatibility here is not incidental — it is the only path on which notification delivery still works at all. - McpInteractionChannel no longer sends straight to the server when it holds no IMcpFeedback: that branch bypassed both the level rule and the buffer. It is reachable only for the discovery-only channel, which has no server either. PUBLIC API BREAK: IMcpFeedback.SendMessageAsync takes a new Repl-owned McpMessageLevel instead of the SDK's LoggingLevel. LoggingLevel carries MCP9005, and the #pragma in IMcpFeedback.cs only ever covered Repl's own compilation — a consumer building with warnings as errors (as this repo itself mandates) got a hard error on a Repl signature it never chose to depend on. IMcpFeedback.cs now needs no suppression at all. Full solution green: 1466 passed, 1 known Inspector skip. --- src/Repl.Mcp/IMcpFeedback.cs | 22 +-- src/Repl.Mcp/McpFeedbackService.cs | 155 +++++++++++++++++++-- src/Repl.Mcp/McpInteractionChannel.cs | 39 ++---- src/Repl.Mcp/McpMessageLevel.cs | 37 +++++ src/Repl.Mcp/McpToolAdapter.cs | 61 ++++++-- src/Repl.Mcp/ReplMcpServerPrompt.cs | 2 +- src/Repl.Mcp/ReplMcpServerResource.cs | 2 +- src/Repl.Mcp/ReplMcpServerTool.cs | 2 +- src/Repl.Mcp/ReplMcpServerUiResource.cs | 2 +- src/Repl.McpTests/Given_McpUserFeedback.cs | 84 ++++++++++- 10 files changed, 345 insertions(+), 61 deletions(-) create mode 100644 src/Repl.Mcp/McpMessageLevel.cs diff --git a/src/Repl.Mcp/IMcpFeedback.cs b/src/Repl.Mcp/IMcpFeedback.cs index 543127dd..fda746af 100644 --- a/src/Repl.Mcp/IMcpFeedback.cs +++ b/src/Repl.Mcp/IMcpFeedback.cs @@ -1,13 +1,5 @@ -using ModelContextProtocol.Protocol; using Repl.Interaction; -// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) -// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps -// supporting them until the SDK removes the surface (#51). -#pragma warning disable MCP9005 - namespace Repl.Mcp; /// @@ -24,8 +16,15 @@ public interface IMcpFeedback bool IsProgressSupported { get; } /// - /// Gets a value indicating whether the connected MCP client can receive logging/message notifications. + /// Gets a value indicating whether a message sent for the current request would reach the + /// connected MCP client as a notification. /// + /// + /// On the 2026-07-28 revision this is unless the request declared + /// a log level in its metadata, because the specification forbids emitting message notifications + /// for a request that did not ask for them. A message sent while this is + /// is not lost: it is carried back in the tool result instead. + /// bool IsLoggingSupported { get; } /// @@ -36,10 +35,11 @@ ValueTask ReportProgressAsync( CancellationToken cancellationToken = default); /// - /// Sends a structured MCP message notification to the connected client. + /// Sends a message to the connected client, as a notification when the request asked for one and + /// otherwise as part of the tool result. /// ValueTask SendMessageAsync( - LoggingLevel level, + McpMessageLevel level, object? data, CancellationToken cancellationToken = default); } diff --git a/src/Repl.Mcp/McpFeedbackService.cs b/src/Repl.Mcp/McpFeedbackService.cs index 5c03c241..2610857a 100644 --- a/src/Repl.Mcp/McpFeedbackService.cs +++ b/src/Repl.Mcp/McpFeedbackService.cs @@ -5,27 +5,27 @@ using ModelContextProtocol.Server; using Repl.Interaction; -// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) -// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps -// supporting them until the SDK removes the surface (#51). +// Logging is deprecated by MCP spec 2026-07-28 (SEP-2577, SDK diagnostic MCP9005); the designated +// successor for server-initiated flows (SEP-2322 multi-round-trip requests, shipped in the SDK 2.0 +// line as MrtrContext/MrtrExchange) is not adopted by Repl yet, and hosts still rely on message +// notifications, so Repl keeps supporting them until the SDK removes the surface (#51). #pragma warning disable MCP9005 namespace Repl.Mcp; /// -/// Internal implementation of backed by a live session. +/// Internal implementation of backed by the flowing MCP request. /// internal sealed class McpFeedbackService(McpRequestServerAccessor servers) : IMcpFeedback { private const string LoggerName = "repl.interaction"; private readonly AsyncLocal _progressToken = new(); + private readonly AsyncLocal _undelivered = new(); public bool IsProgressSupported => servers.Effective is not null && _progressToken.Value is not null; - public bool IsLoggingSupported => servers.Effective is not null; + public bool IsLoggingSupported => ResolveThreshold() is not null; public async ValueTask ReportProgressAsync( ReplProgressEvent progress, @@ -55,12 +55,25 @@ await server.NotifyProgressAsync( } public async ValueTask SendMessageAsync( - LoggingLevel level, + McpMessageLevel level, object? data, CancellationToken cancellationToken = default) { - if (servers.Effective is not { } server) + // Single read of both the server and the threshold: a concurrent request re-binding the + // accessor must not be observed between the decision and the send. + var server = servers.Effective; + var threshold = ResolveThreshold(); + + if (server is null || threshold is null || level < threshold) { + // Either the client cannot receive notifications for this request, or the message is + // below the level it asked for. Below-threshold messages are dropped as requested; + // undeliverable ones are kept so the tool result can carry them instead. + if (threshold is null) + { + _undelivered.Value?.Add(level, data); + } + return; } @@ -68,17 +81,75 @@ await server.SendNotificationAsync( NotificationMethods.LoggingMessageNotification, new LoggingMessageNotificationParams { - Level = level, + Level = (LoggingLevel)level, Logger = LoggerName, Data = SerializeData(data), }, cancellationToken: cancellationToken).ConfigureAwait(false); } + /// + /// Resolves the severity threshold the current request asked for, or when + /// it asked for nothing and no notification may be sent. + /// + /// + /// The 2026-07-28 revision (SEP-2575) replaced logging/setLevel with a per-request + /// _meta/io.modelcontextprotocol/logLevel field and states that a server MUST NOT emit + /// message notifications for a request that omitted it. The SDK parses the field onto the message + /// context but consumes it nowhere, so the filtering is Repl's to do. + /// + /// Initialize-era revisions keep the session-wide logging/setLevel semantics, including the + /// historical behaviour of sending everything when the client never set a level — those hosts must + /// not lose feedback to a rule that does not apply to them. + /// + /// + /// Note that the SDK's own CLIENT cannot currently ask for notifications on 2026-07-28: it + /// rejects logging/setLevel on that revision, exposes no option for the level, and replaces + /// a caller's _meta with its own three keys (protocol version, client info, capabilities). + /// So in practice this returns for every SDK-client request on the modern + /// revision, which is exactly why undeliverable messages are carried back in the tool result. + /// + /// + private McpMessageLevel? ResolveThreshold() + { + if (servers.Current is not { } request) + { + return null; + } + + var context = (request.JsonRpcMessage as JsonRpcRequest)?.Context; + if (context?.LogLevel is { } requestedLevel) + { + return (McpMessageLevel)requestedLevel; + } + + if (IsSessionlessRevision(context?.ProtocolVersion)) + { + return null; + } + + return request.Server.LoggingLevel is { } sessionLevel + ? (McpMessageLevel)sessionLevel + : McpMessageLevel.Debug; + } + + private static bool IsSessionlessRevision(string? protocolVersion) => + string.Equals(protocolVersion, McpProtocolRevisions.Sessionless, StringComparison.Ordinal); internal IDisposable PushProgressToken(ProgressToken? progressToken) => new ProgressTokenScope(_progressToken, progressToken); + /// + /// Opens a scope that collects messages the current request cannot receive as notifications. + /// + /// + /// Pushed per invocation by and drained into the tool result, so a + /// client that never asked for log notifications still sees what a command reported. The buffer + /// is rather than a field on + /// because a command can inject directly and bypass the channel. + /// + internal UndeliveredMessageScope PushUndeliveredMessages() => new(_undelivered); + private static JsonElement SerializeData(object? data) => data switch { @@ -95,6 +166,70 @@ private static string BuildProgressMessage(ReplProgressEvent progress) => ? progress.Label : $"{progress.Label}: {progress.Details}"; + /// Messages collected for a request that cannot receive notifications. + internal sealed class UndeliveredMessages + { + private readonly List _lines = []; + + public void Add(McpMessageLevel level, object? data) + { + var text = data as string ?? data?.ToString(); + if (string.IsNullOrWhiteSpace(text)) + { + return; + } + + lock (_lines) + { + _lines.Add($"[{level.ToString().ToLowerInvariant()}] {text}"); + } + } + + public IReadOnlyList Drain() + { + lock (_lines) + { + if (_lines.Count == 0) + { + return []; + } + + var drained = _lines.ToArray(); + _lines.Clear(); + return drained; + } + } + } + + /// Restores the previous buffer on dispose, so nested invocations stay independent. + internal sealed class UndeliveredMessageScope : IDisposable + { + private readonly AsyncLocal _slot; + private readonly UndeliveredMessages? _previous; + private bool _disposed; + + public UndeliveredMessageScope(AsyncLocal slot) + { + _slot = slot; + _previous = slot.Value; + Messages = new UndeliveredMessages(); + slot.Value = Messages; + } + + public UndeliveredMessages Messages { get; } + + public void Dispose() + { + if (_disposed) + { + return; + } + + _slot.Value = _previous; + _disposed = true; + } + } + private sealed class ProgressTokenScope : IDisposable { private readonly AsyncLocal _progressTokenSlot; diff --git a/src/Repl.Mcp/McpInteractionChannel.cs b/src/Repl.Mcp/McpInteractionChannel.cs index 4a332151..8ed6282d 100644 --- a/src/Repl.Mcp/McpInteractionChannel.cs +++ b/src/Repl.Mcp/McpInteractionChannel.cs @@ -1,4 +1,4 @@ -using System.Text.Json; +using System.Text.Json; using System.Text.Json.Nodes; using ModelContextProtocol; using ModelContextProtocol.Protocol; @@ -228,7 +228,7 @@ await _server.NotifyProgressAsync( public async ValueTask WriteStatusAsync(string text, CancellationToken cancellationToken) { await SendFeedbackAsync( - LoggingLevel.Info, + McpMessageLevel.Info, JsonSerializer.SerializeToElement(text, McpJsonContext.Default.String), cancellationToken) .ConfigureAwait(false); @@ -247,24 +247,24 @@ public ValueTask DispatchAsync( { WriteStatusRequest status => CompleteBuiltInDispatchAsync( SendFeedbackAsync( - LoggingLevel.Info, + McpMessageLevel.Info, JsonSerializer.SerializeToElement(status.Text, McpJsonContext.Default.String), cancellationToken)), WriteProgressRequest progress => CompleteBuiltInDispatchAsync( WriteStructuredProgressAsync(progress, cancellationToken)), WriteNoticeRequest notice => CompleteBuiltInDispatchAsync( SendFeedbackAsync( - LoggingLevel.Info, + McpMessageLevel.Info, JsonSerializer.SerializeToElement(notice.Text, McpJsonContext.Default.String), cancellationToken)), WriteWarningRequest warning => CompleteBuiltInDispatchAsync( SendFeedbackAsync( - LoggingLevel.Warning, + McpMessageLevel.Warning, JsonSerializer.SerializeToElement(warning.Text, McpJsonContext.Default.String), cancellationToken)), WriteProblemRequest problem => CompleteBuiltInDispatchAsync( SendFeedbackAsync( - LoggingLevel.Error, + McpMessageLevel.Error, SerializeProblem(problem), cancellationToken)), _ => throw new NotSupportedException( @@ -273,30 +273,19 @@ public ValueTask DispatchAsync( } private async ValueTask SendFeedbackAsync( - LoggingLevel level, + McpMessageLevel level, JsonElement data, CancellationToken cancellationToken) { + // Always through IMcpFeedback: it owns the per-request log-level rule (2026-07-28 forbids + // emitting message notifications for a request that did not ask for them) and the buffer that + // carries undeliverable messages back in the tool result. Sending straight to the server here + // would bypass both. It is absent only for the discovery-only channel, which has no server + // to send to either. if (_feedback is not null) { await _feedback.SendMessageAsync(level, data, cancellationToken).ConfigureAwait(false); - return; - } - - if (_server is null) - { - return; } - - await _server.SendNotificationAsync( - NotificationMethods.LoggingMessageNotification, - new LoggingMessageNotificationParams - { - Level = level, - Logger = "repl.interaction", - Data = data, - }, - cancellationToken: cancellationToken).ConfigureAwait(false); } private async ValueTask WriteStructuredProgressAsync( @@ -317,7 +306,7 @@ await _feedback.ReportProgressAsync( if (progress.State == ReplProgressState.Warning) { await _feedback.SendMessageAsync( - LoggingLevel.Warning, + McpMessageLevel.Warning, BuildProgressPayload(progress), cancellationToken) .ConfigureAwait(false); @@ -325,7 +314,7 @@ await _feedback.SendMessageAsync( else if (progress.State == ReplProgressState.Error) { await _feedback.SendMessageAsync( - LoggingLevel.Error, + McpMessageLevel.Error, BuildProgressPayload(progress), cancellationToken) .ConfigureAwait(false); diff --git a/src/Repl.Mcp/McpMessageLevel.cs b/src/Repl.Mcp/McpMessageLevel.cs new file mode 100644 index 00000000..9bae5f76 --- /dev/null +++ b/src/Repl.Mcp/McpMessageLevel.cs @@ -0,0 +1,37 @@ +namespace Repl.Mcp; + +/// +/// Severity of a message sent to the connected MCP client through . +/// +/// +/// This mirrors the protocol's syslog-derived severities. It exists as a Repl-owned type so the +/// public surface does not expose the SDK's LoggingLevel, which the 2026-07-28 +/// specification deprecates (SEP-2577, SDK diagnostic MCP9005): a consumer building with warnings +/// as errors would otherwise fail on a Repl signature it never chose to depend on. +/// +public enum McpMessageLevel +{ + /// Detailed information, useful only when diagnosing a problem. + Debug = 0, + + /// Normal operational information. + Info = 1, + + /// A normal but significant condition. + Notice = 2, + + /// A condition that is not an error but deserves attention. + Warning = 3, + + /// An error that did not prevent the operation from continuing. + Error = 4, + + /// A condition that requires immediate attention. + Critical = 5, + + /// Action must be taken immediately. + Alert = 6, + + /// The system is unusable. + Emergency = 7, +} diff --git a/src/Repl.Mcp/McpToolAdapter.cs b/src/Repl.Mcp/McpToolAdapter.cs index c9b5ddd0..152d8aae 100644 --- a/src/Repl.Mcp/McpToolAdapter.cs +++ b/src/Repl.Mcp/McpToolAdapter.cs @@ -1,4 +1,4 @@ -using System.Text.Json; +using System.Text.Json; using System.Text.RegularExpressions; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; @@ -146,7 +146,8 @@ public async Task InvokeAsync( : $"Command failed with exit code {invocation.ExitCode}."; } - return BuildToolResult(output, invocation.ExitCode, _options.PagedResultTextMode); + return BuildToolResult( + output, invocation.ExitCode, _options.PagedResultTextMode, invocation.UndeliveredMessages); } internal async Task InvokeResourceAsync( @@ -220,8 +221,11 @@ private async Task ExecuteThroughPipelineAsync( { [typeof(IReplInteractionChannel)] = interactionChannel, }); - using var feedbackScope = (_services.GetService(typeof(IMcpFeedback)) as McpFeedbackService) - ?.PushProgressToken(progressToken); + var feedbackService = _services.GetService(typeof(IMcpFeedback)) as McpFeedbackService; + using var feedbackScope = feedbackService?.PushProgressToken(progressToken); + // Messages the client cannot receive as notifications ride back in the tool result instead, + // so no feedback is lost on a request that never asked for log notifications. + using var undeliveredScope = feedbackService?.PushUndeliveredMessages(); // Force JSON output — agents consume structured data, not human tables/banners. var effectiveTokens = new List(tokens.Count + 1) { $"--output:{ForcedOutputFormat}" }; @@ -246,21 +250,32 @@ private async Task ExecuteThroughPipelineAsync( var output = outputWriter.ToString().Trim(); var error = captureCommandOutput ? string.Empty : errorWriter.ToString().Trim(); - return new McpPipelineInvocation(output, error, exitCode); + var undelivered = undeliveredScope?.Messages.Drain() ?? []; + return new McpPipelineInvocation(output, error, exitCode, undelivered); } } internal readonly record struct McpResourceReadInvocation(string Text, string MimeType, bool IsError); - private readonly record struct McpPipelineInvocation(string Output, string Error, int ExitCode); + private readonly record struct McpPipelineInvocation( + string Output, + string Error, + int ExitCode, + IReadOnlyList UndeliveredMessages); - private static CallToolResult BuildToolResult(string output, int exitCode, McpPagedResultTextMode pagedTextMode) + private static CallToolResult BuildToolResult( + string output, + int exitCode, + McpPagedResultTextMode pagedTextMode, + IReadOnlyList undeliveredMessages) { if (exitCode == 0 && TryCreatePagedStructuredResult(output, out var structuredContent, out var summary)) { return new CallToolResult { - Content = [new TextContentBlock { Text = BuildPagedTextContent(output, summary, pagedTextMode) }], + Content = WithMessages( + new TextContentBlock { Text = BuildPagedTextContent(output, summary, pagedTextMode) }, + undeliveredMessages), StructuredContent = structuredContent, IsError = false, }; @@ -268,11 +283,39 @@ private static CallToolResult BuildToolResult(string output, int exitCode, McpPa return new CallToolResult { - Content = [new TextContentBlock { Text = output }], + Content = WithMessages(new TextContentBlock { Text = output }, undeliveredMessages), IsError = exitCode != 0, }; } + /// + /// Appends messages the client could not receive as notifications, as a trailing content block. + /// + /// + /// The command's own payload stays the FIRST block (and StructuredContent is untouched), so + /// a caller reading the primary result is unaffected. Only requests that asked for no log level + /// carry anything here — a client receiving message notifications would otherwise see each one + /// twice. Resource reads deliberately get no such block: their body must match the advertised + /// MIME type. + /// + private static List WithMessages( + TextContentBlock primary, + IReadOnlyList undeliveredMessages) + { + if (undeliveredMessages.Count == 0) + { + return [primary]; + } + + var blocks = new List(undeliveredMessages.Count + 1) { primary }; + foreach (var message in undeliveredMessages) + { + blocks.Add(new TextContentBlock { Text = message }); + } + + return blocks; + } + private static string BuildPagedTextContent( string serializedPage, string summary, diff --git a/src/Repl.Mcp/ReplMcpServerPrompt.cs b/src/Repl.Mcp/ReplMcpServerPrompt.cs index 61be9544..c002115e 100644 --- a/src/Repl.Mcp/ReplMcpServerPrompt.cs +++ b/src/Repl.Mcp/ReplMcpServerPrompt.cs @@ -1,4 +1,4 @@ -using ModelContextProtocol; +using ModelContextProtocol; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using Repl.Documentation; diff --git a/src/Repl.Mcp/ReplMcpServerResource.cs b/src/Repl.Mcp/ReplMcpServerResource.cs index dfae7da3..9b0fad87 100644 --- a/src/Repl.Mcp/ReplMcpServerResource.cs +++ b/src/Repl.Mcp/ReplMcpServerResource.cs @@ -1,4 +1,4 @@ -using System.Text.Json; +using System.Text.Json; using System.Text.RegularExpressions; using ModelContextProtocol; using ModelContextProtocol.Protocol; diff --git a/src/Repl.Mcp/ReplMcpServerTool.cs b/src/Repl.Mcp/ReplMcpServerTool.cs index 7ab5e50f..b3aa7b28 100644 --- a/src/Repl.Mcp/ReplMcpServerTool.cs +++ b/src/Repl.Mcp/ReplMcpServerTool.cs @@ -1,4 +1,4 @@ -using System.Text.Json; +using System.Text.Json; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using Repl.Documentation; diff --git a/src/Repl.Mcp/ReplMcpServerUiResource.cs b/src/Repl.Mcp/ReplMcpServerUiResource.cs index 462f7792..9ae90d75 100644 --- a/src/Repl.Mcp/ReplMcpServerUiResource.cs +++ b/src/Repl.Mcp/ReplMcpServerUiResource.cs @@ -1,4 +1,4 @@ -using System.Text.Json; +using System.Text.Json; using System.Text.RegularExpressions; using ModelContextProtocol; using ModelContextProtocol.Protocol; diff --git a/src/Repl.McpTests/Given_McpUserFeedback.cs b/src/Repl.McpTests/Given_McpUserFeedback.cs index 28869839..b9ecb1e5 100644 --- a/src/Repl.McpTests/Given_McpUserFeedback.cs +++ b/src/Repl.McpTests/Given_McpUserFeedback.cs @@ -4,6 +4,7 @@ using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; using Repl.Interaction; +using Repl.Mcp; // These tests exercise Roots/Sampling/Logging, deprecated by MCP spec 2026-07-28 // (SEP-2577, MCP9005) but still supported by Repl.Mcp until the SDK removes them. @@ -24,7 +25,7 @@ public async Task When_ToolEmitsUserFeedback_Then_McpReceivesNotifications() NotificationCaptureState.Current = captureState; try { - await using var fixture = await CreateFeedbackFixtureAsync(clientOptions: CreateClientOptions()).ConfigureAwait(false); + await using var fixture = await CreateFeedbackFixtureAsync(LegacyClientOptions()).ConfigureAwait(false); var result = await fixture.Client.CallToolAsync( toolName: "feedback", @@ -49,7 +50,7 @@ public async Task When_ToolEmitsStructuredProgress_Then_McpReceivesProgressAndMe NotificationCaptureState.Current = captureState; try { - await using var fixture = await CreateStructuredProgressFixtureAsync(CreateClientOptions()).ConfigureAwait(false); + await using var fixture = await CreateStructuredProgressFixtureAsync(LegacyClientOptions()).ConfigureAwait(false); var result = await fixture.Client.CallToolAsync( toolName: "feedback_progress", @@ -68,6 +69,85 @@ await WaitForConditionAsync(() => } } + [TestMethod] + [Description("Guards the 2026-07-28 rule that a server MUST NOT emit notifications/message for a request that declared no log level (SEP-2575) — and guards against that rule silently swallowing user feedback: the notice, warning and problem the command reported must instead ride back in the tool result, so no host loses them.")] + public async Task When_RequestDeclaresNoLogLevel_Then_FeedbackRidesInTheToolResultInstead() + { + var notifications = new List<(LoggingLevel Level, string Data)>(); + var captureState = new NotificationCaptureState(notifications); + NotificationCaptureState.Current = captureState; + try + { + await using var fixture = await CreateFeedbackFixtureAsync(CreateClientOptions()).ConfigureAwait(false); + fixture.Client.NegotiatedProtocolVersion.Should().Be(McpProtocolRevisions.Sessionless); + + var result = await fixture.Client.CallToolAsync( + toolName: "feedback", + arguments: new Dictionary(StringComparer.Ordinal)).ConfigureAwait(false); + + // Give a (forbidden) notification time to arrive before asserting that none did. + await WaitForConditionAsync(() => notifications.Count > 0, timeoutMs: 500).ConfigureAwait(false); + + notifications.Should().BeEmpty( + because: "the request declared no log level, so the server must not emit message notifications"); + var text = string.Join('\n', result.Content.OfType().Select(block => block.Text)); + text.Should().Contain("Connected"); + text.Should().Contain("Token expires soon"); + text.Should().Contain("Sync failed"); + result.IsError.Should().BeFalse(); + } + finally + { + NotificationCaptureState.Current = null; + } + } + + [TestMethod] + [Description("Guards severity filtering against the level the client asked for: after logging/setLevel(Error) the notice and warning a command reports must not be delivered, while the problem must. Emitting everything regardless of the requested threshold floods hosts that deliberately asked for errors only.")] + public async Task When_ClientRequestsErrorLevel_Then_LowerSeveritiesAreNotNotified() + { + var notifications = new List<(LoggingLevel Level, string Data)>(); + var captureState = new NotificationCaptureState(notifications); + NotificationCaptureState.Current = captureState; + try + { + await using var fixture = await CreateFeedbackFixtureAsync(LegacyClientOptions()).ConfigureAwait(false); + await fixture.Client.SetLoggingLevelAsync(LoggingLevel.Error).ConfigureAwait(false); + + await fixture.Client.CallToolAsync( + toolName: "feedback", + arguments: new Dictionary(StringComparer.Ordinal)).ConfigureAwait(false); + + await WaitForConditionAsync(() => notifications.Count >= 1).ConfigureAwait(false); + + notifications.Should().OnlyContain(entry => entry.Level == LoggingLevel.Error); + notifications.Should().ContainSingle(entry => + entry.Data.Contains("Sync failed", StringComparison.Ordinal)); + } + finally + { + NotificationCaptureState.Current = null; + } + } + + /// + /// A client pinned to the last revision on which message notifications can be requested at all. + /// + /// + /// On 2026-07-28 the SDK's client cannot ask for a log level: it rejects + /// logging/setLevel for that revision, exposes no option for the level, and replaces a + /// caller's _meta with its own keys (protocol version, client info, capabilities). Tests + /// that assert notification DELIVERY therefore have to pin the initialize-era revision. The modern + /// path is covered by + /// . + /// + private static McpClientOptions LegacyClientOptions() + { + var options = CreateClientOptions(); + options.ProtocolVersion = McpProtocolRevisions.LastWithSessions; + return options; + } + private static async Task CreateFeedbackFixtureAsync( McpClientOptions clientOptions) => await CreateFeedbackFixtureAsync( From 4c9fd428775ac2829c940ef0ae9557e7e862893e Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Sun, 23 Aug 2026 23:33:25 -0400 Subject: [PATCH 14/16] refactor(mcp): drop the unreachable external session fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ResolveContext fell back to one lazily created context shared by every caller that missed the session lookup. Two consequences: - it was added to _sessions and never removed, so _sessions.Count == 0 became unreachable and UnsubscribeFromRoutingChanges could never run — the routing subscription and its 100ms debounce timer leaked for the process lifetime; - it latched a destination-bound PER-REQUEST server as its SessionServer, contradicting the comment two methods below stating the roots handler is registered per session and "never to the per-request destination wrappers". RED OBSERVED (mutation, since dead code cannot be covered by a failing test): making the fallback branch throw unconditionally left the entire suite green — 240 tests, including every MCP path. It is genuinely unreachable: the only server built without a session provider comes from BuildStaticServerOptions, whose pre-built primitives never route through a handler. - ResolveContext is now a static pure lookup that throws with a message naming the invariant it rests on. - The five throwaway CreateSessionContext() sites (fail-fast validation, the static catalog, both snapshot test seams, HasMcpAppResources) share one _catalogContext built in the constructor, instead of allocating five roots services and five never-disposed semaphores and dropping them on the floor. - McpSessionContext is IDisposable and the per-session one is disposed by RunAsync; SessionServer is gone, its last reader having left with the SDK-owned notification fan-out. Not fixed here: soft roots set on one connection are still visible to another created from the same BuildMcpServerOptions() result, because that path deliberately has no per-connection identity — 2026-07-28 has no protocol sessions, and giving hosts per-connection services would mean new public API. Documented as a known limitation in the docs commit rather than left as a silent trap, with a follow-up to design the seam. Full MCP suite green (239 passed, 1 known Inspector skip); strict Release build of the solution clean. --- src/Repl.Mcp/McpServerHandler.cs | 72 ++++++++++++++----------------- src/Repl.Mcp/McpSessionContext.cs | 14 ++---- 2 files changed, 35 insertions(+), 51 deletions(-) diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index 70cc7f61..08d7a008 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -31,6 +31,12 @@ internal sealed class McpServerHandler private readonly McpSamplingService _sampling; private readonly McpElicitationService _elicitation; private readonly McpFeedbackService _feedback; + // Context for work that belongs to no MCP session: eager fail-fast validation, the pre-built + // catalog behind BuildMcpServerOptions, and the snapshot test seams. Each of those used to mint + // its own throwaway context — five roots services and five never-disposed semaphores dropped on + // the floor. It shares the handler's lifetime, so nothing disposes it either; unlike a session + // context there is never more than one. + private readonly McpSessionContext _catalogContext; private readonly Lock _refreshLock = new(); private readonly Lock _attachLock = new(); @@ -41,11 +47,6 @@ internal sealed class McpServerHandler // McpSessionContext, and this list (guarded by _attachLock) tracks every ACTIVE session // for server-initiated notifications and subscription lifetime. private readonly List _sessions = []; - // Lazy single context for externally hosted servers (options built via - // BuildDynamicServerOptions and run by the host without RunAsync): those servers carry - // the HOST's provider, so requests cannot recover a per-session context from it — they - // share one explicit fallback context instead of racing a last-attached field. - private McpSessionContext? _externalContext; private EventHandler? _routingChangedHandler; private ITimer? _debounceTimer; private static readonly TimeSpan DebounceDelay = TimeSpan.FromMilliseconds(100); @@ -77,6 +78,7 @@ public McpServerHandler( _sampling = new McpSamplingService(_requestServers); _elicitation = new McpElicitationService(_requestServers); _feedback = new McpFeedbackService(_requestServers); + _catalogContext = CreateSessionContext(); } private McpSessionContext CreateSessionContext() @@ -97,35 +99,25 @@ private McpSessionContext CreateSessionContext() return context; } - // Requests recover their session through the provider handed to McpServer.Create — - // even a destination-bound per-request server exposes its session's services. Servers - // created by an external host (BuildDynamicServerOptions) carry the host's provider - // instead and share the explicit fallback context. - private McpSessionContext ResolveContext(McpServer? requestServer) - { - if (requestServer?.Services?.GetService(typeof(McpSessionContext)) is McpSessionContext context) - { - return context; - } - - lock (_attachLock) - { - if (_externalContext is null) - { - _externalContext = CreateSessionContext(); - _sessions.Add(_externalContext); - EnsureRoutingSubscription(); - } - - if (_externalContext.SessionServer is null && requestServer is not null) - { - _externalContext.SessionServer = requestServer; - EnsureRootsNotificationHandler(requestServer, _externalContext.Roots); - } - - return _externalContext; - } - } + /// + /// Recovers the session a request belongs to, through the provider handed to + /// McpServer.Create — even a destination-bound per-request server exposes its session's + /// services. + /// + /// + /// A pure lookup on purpose. This used to fall back to one lazily created context shared by every + /// caller that missed the lookup, which made _sessions permanently non-empty — so the + /// routing subscription and its debounce timer could never be released — and latched a + /// destination-bound per-request server as if it were a session server. The fallback was also + /// unreachable: the only server built without a session provider comes from + /// , whose pre-built primitives never route through here. + /// + private static McpSessionContext ResolveContext(McpServer? requestServer) => + requestServer?.Services?.GetService(typeof(McpSessionContext)) as McpSessionContext + ?? throw new InvalidOperationException( + "An MCP request reached a Repl handler without its session context. Handlers are only " + + "registered by BuildDynamicServerOptions, whose server is always created with the " + + "session's own service provider."); [UnconditionalSuppressMessage( "Trimming", @@ -140,7 +132,7 @@ public async Task RunAsync(IReplIoContext io, CancellationToken ct) : new StdioServerTransport(serverName); try { - var context = CreateSessionContext(); + using var context = CreateSessionContext(); var server = McpServer.Create(transport, serverOptions, serviceProvider: context.Services); AttachSession(context, server); @@ -170,7 +162,7 @@ internal McpServerOptions BuildDynamicServerOptions() // then repeated for the same commands during the first discovery request. if (_options.CommandFilter is null) { - _ = CreateDocumentationModel(CreateSessionContext().Services); + _ = CreateDocumentationModel(_catalogContext.Services); } return new McpServerOptions @@ -199,7 +191,7 @@ internal McpServerOptions BuildStaticServerOptions() { var serverName = _options.ServerName ?? ResolveAppName() ?? "repl-mcp-server"; var serverVersion = _options.ServerVersion ?? "1.0.0"; - var snapshot = BuildSnapshotCore(CreateSessionContext()); + var snapshot = BuildSnapshotCore(_catalogContext); return new McpServerOptions { @@ -211,10 +203,10 @@ internal McpServerOptions BuildStaticServerOptions() }; } - internal McpGeneratedSnapshot BuildSnapshotForTests() => BuildSnapshotCore(CreateSessionContext()); + internal McpGeneratedSnapshot BuildSnapshotForTests() => BuildSnapshotCore(_catalogContext); internal async Task BuildSnapshotForTestsAsync(CancellationToken cancellationToken = default) => - await GetSnapshotAsync(CreateSessionContext(), cancellationToken).ConfigureAwait(false); + await GetSnapshotAsync(_catalogContext, cancellationToken).ConfigureAwait(false); private string? ResolveAppName() { @@ -755,7 +747,7 @@ private bool HasMcpAppResources() ReplSessionIO.IsProgrammatic = true; try { - var sessionServices = CreateSessionContext().Services; + var sessionServices = _catalogContext.Services; using var runtimeStateScope = coreApp.PushRuntimeState( CreateDiscoveryServices(sessionServices), isInteractiveSession: false); diff --git a/src/Repl.Mcp/McpSessionContext.cs b/src/Repl.Mcp/McpSessionContext.cs index 831fd01a..373b69b0 100644 --- a/src/Repl.Mcp/McpSessionContext.cs +++ b/src/Repl.Mcp/McpSessionContext.cs @@ -17,7 +17,7 @@ namespace Repl.Mcp; /// through the per-request binding, which is /// finer-grained than the session. /// -internal sealed class McpSessionContext +internal sealed class McpSessionContext : IDisposable { private SnapshotCacheEntry? _snapshotCache; private int _compatibilityIntroServed; @@ -34,16 +34,6 @@ public McpSessionContext(McpClientRootsService roots, IServiceProvider services) /// Per-session service overlay handed to McpServer.Create. public IServiceProvider Services { get; } - /// - /// Latches the first server observed by the externally hosted fallback context, so its - /// roots-list-changed handler is registered once. - /// - /// - /// This used to be the destination for server-initiated notifications; the SDK now owns that - /// fan-out, leaving only the latch. It disappears with the fallback context itself. - /// - public McpServer? SessionServer { get; set; } - /// Serializes snapshot builds for this session. public SemaphoreSlim SnapshotGate { get; } = new(initialCount: 1, maxCount: 1); @@ -74,6 +64,8 @@ public bool TryClaimCompatibilityIntro() => /// Re-arms the compatibility-shim intro after a routing invalidation. public void ResetCompatibilityIntro() => Interlocked.Exchange(ref _compatibilityIntroServed, 0); + public void Dispose() => SnapshotGate.Dispose(); + /// /// A generated snapshot and the routing version it was built at, published as ONE value. /// From 31953dcd5ad748f62f1e1bb482de122730851d8f Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Sun, 23 Aug 2026 23:35:01 -0400 Subject: [PATCH 15/16] docs(mcp): correct the isolation, soft-roots and notification claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - mcp-transports.md said "each connection or HTTP session is isolated: its own MCP session / its own I/O capture / its own session-aware routing state". Two of those three were wrong: I/O capture is per INVOCATION (a fresh scope per tool call), and a server built from a reused BuildMcpServerOptions() result has no session-aware routing state at all. Replaced with a table of what is isolated at which boundary — request, invocation, connection — since 2026-07-28 removed protocol sessions and the boundaries are no longer the same size. - The reuse sample keeps working and is now explicit that capabilities resolve per request on that path, with a called-out known limitation: soft roots are shared across connections built from one options instance, and what to do instead (one server per process, or an explicit command argument). - mcp-advanced.md steered new 2026-07-28 applications toward soft roots. They are the same connection-scoped state by another name, so the notice now points at explicit parameters or server-minted handles per SEP-2567 and labels soft roots a legacy compatibility feature. The SetSoftRoots row states its real scope per hosting mode instead of "the current session". - mcp-reference.md documents the delivery split for discovery notifications and user feedback, including that a modern client which opens no subscription receives no */list_changed (mitigated by ttlMs: 0) and that feedback rides in the tool result when it cannot be a notification. - mcp-reference.md's MRTR line said SEP-2322 "is available starting with SDK 2.0", which a chore commit had silently substituted for "ships experimentally" — erasing true history even though the new wording happens to hold for 2.2.0. It now states both: shipped experimentally in the 2.0 preview line, stable as of 2.2.0, not adopted by Repl. - mcp-agent-capabilities.md follows IMcpFeedback to McpMessageLevel, and stops presenting IsLoggingSupported as a gate: sending unconditionally is correct now that undeliverable messages are carried in the result. --- docs/mcp-advanced.md | 13 +++++++++---- docs/mcp-agent-capabilities.md | 17 +++++++++++------ docs/mcp-reference.md | 4 +++- docs/mcp-transports.md | 27 +++++++++++++++++++++------ 4 files changed, 44 insertions(+), 17 deletions(-) diff --git a/docs/mcp-advanced.md b/docs/mcp-advanced.md index 90ea3f62..6473a80f 100644 --- a/docs/mcp-advanced.md +++ b/docs/mcp-advanced.md @@ -22,10 +22,15 @@ If your tool list is static, stay with the default setup from [mcp-overview.md]( > **⚠️ Deprecation notice (SEP-2577):** the MCP specification (2026-07-28) deprecates the > Roots feature, and the SDK may remove it in a future version. Repl keeps supporting it -> **for existing hosts and applications only** — new applications should not build on -> native MCP roots and can use [soft roots](#soft-roots-fallback) or explicit command -> parameters instead. See +> **for existing hosts and applications only.** New applications should take the workspace as an +> **explicit command parameter**, or mint a handle from a setup command and pass it back — that is +> what SEP-2567 prescribes now that the protocol has no sessions to hang such state on. See > [mcp-reference.md](mcp-reference.md#sdk-and-protocol-versions) for the version posture. +> +> [Soft roots](#soft-roots-fallback) are **not** the modern answer: they are the same +> connection-scoped state by another name, and they are scoped to the process rather than the +> connection when a host reuses one `BuildMcpServerOptions()` result. Treat them as a legacy +> compatibility feature for clients that lack native roots. A **root** is a URI the client declares as being in scope for the session — typically an opened project folder, a working directory, or a boundary for what the agent should inspect or modify. @@ -49,7 +54,7 @@ app.Map("workspace roots", async (IMcpClientRoots roots, CancellationToken ct) = | `Current` | Current effective roots for the session | | `GetAsync()` | Refreshes native roots if supported | | `HasSoftRoots` | Fallback roots were initialized manually | -| `SetSoftRoots()` / `ClearSoftRoots()` | Manage fallback roots for the current session | +| `SetSoftRoots()` / `ClearSoftRoots()` | Manage fallback roots — per connection under `mcp serve`, per process when a host reuses one `BuildMcpServerOptions()` result | > **Why `IMcpClientRoots` is MCP-only:** Roots are session-scoped MCP data. They don't make sense as a generic `Repl.Core` concept for terminal or non-MCP execution. That's why the interface lives in `Repl.Mcp` and is injected only for MCP sessions. diff --git a/docs/mcp-agent-capabilities.md b/docs/mcp-agent-capabilities.md index f4a1e8c4..3997d062 100644 --- a/docs/mcp-agent-capabilities.md +++ b/docs/mcp-agent-capabilities.md @@ -246,12 +246,15 @@ public interface IMcpFeedback CancellationToken cancellationToken = default); ValueTask SendMessageAsync( - LoggingLevel level, + McpMessageLevel level, object? data, CancellationToken cancellationToken = default); } ``` +`McpMessageLevel` is Repl's own enum (`Debug` … `Emergency`), so this signature does not expose the +SDK's deprecated `LoggingLevel` to your build. + Use it when: - you need to control MCP progress/message notifications directly @@ -264,10 +267,10 @@ Use it when: app.Map("sync contacts", async (IMcpFeedback feedback, CancellationToken ct) => { - if (feedback.IsLoggingSupported) - { - await feedback.SendMessageAsync(LoggingLevel.Info, "Starting sync.", ct); - } + // Sending unconditionally is fine: a message the client cannot receive as a notification + // is carried back in the tool result instead. Check IsLoggingSupported only when you want + // to skip work that would otherwise be wasted. + await feedback.SendMessageAsync(McpMessageLevel.Info, "Starting sync.", ct); if (feedback.IsProgressSupported) { @@ -330,7 +333,9 @@ if (!elicitation.IsSupported) For `IMcpFeedback`, the same idea applies: - check `IsProgressSupported` before sending MCP-only progress directly -- check `IsLoggingSupported` before sending MCP-only messages directly +- `IsLoggingSupported` tells you whether a message would arrive as a **notification**; it is `false` + on `2026-07-28` unless the request declared a log level. Messages are never dropped for that + reason — they ride back in the tool result — so treat it as a hint, not a gate - prefer `IReplInteractionChannel` when the feedback should still render well outside MCP ## Client compatibility diff --git a/docs/mcp-reference.md b/docs/mcp-reference.md index c6e6f341..c70e5bd4 100644 --- a/docs/mcp-reference.md +++ b/docs/mcp-reference.md @@ -515,7 +515,9 @@ Feature support varies across agents. Check [mcp-availability.com](https://mcp-a ### SDK and protocol versions - Repl.Mcp builds on the official C# SDK (`ModelContextProtocol`), currently at **2.2.0**. The SDK negotiates the protocol version with each client, including fallback to the legacy `initialize` handshake for older hosts. -- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577). Repl.Mcp keeps supporting them **for existing hosts and applications only** — new applications should not adopt these features (the SDK may remove them) and should prefer Repl's portable abstractions such as `IReplInteractionChannel`. The designated successor for server-initiated flows (SEP-2322, multi-round-trip requests) is available starting with SDK 2.0; Repl has not adopted it yet. +- **Roots, Sampling, and Logging** are deprecated by MCP specification 2026-07-28 (SEP-2577). Repl.Mcp keeps supporting them **for existing hosts and applications only** — new applications should not adopt these features (the SDK may remove them) and should prefer Repl's portable abstractions such as `IReplInteractionChannel`. The designated successor for server-initiated flows (SEP-2322, multi-round-trip requests) shipped experimentally in the SDK 2.0 preview line and is stable as of 2.2.0; Repl has not adopted it yet. +- **Discovery notifications** follow the negotiated revision. Repl drives the SDK's own fan-out rather than broadcasting itself, so an initialize-era client keeps receiving unsolicited `*/list_changed` while a `2026-07-28` client receives only the notification types it requested through `subscriptions/listen`, each tagged with its listen request id (SEP-2575). A modern client that opens no subscription receives none — which is what the specification requires. List results carry `ttlMs: 0`, so such a client re-lists on demand rather than caching. +- **User feedback** (`notice` / `warning` / `problem`, and `IMcpFeedback.SendMessageAsync`) follows the same split. On `2026-07-28`, `logging/setLevel` is gone and a server must not emit `notifications/message` for a request that declared no `_meta/io.modelcontextprotocol/logLevel`. Messages that cannot be delivered as notifications are appended to the **tool result** instead, after the command's own payload, so no host loses them. Initialize-era clients keep the session-wide `logging/setLevel` behaviour unchanged. Note that the SDK's own client cannot request a level on `2026-07-28` at all, so in practice modern hosts see feedback in the tool result. - **MCP Tasks**: the SDK reorganized Tasks into `ModelContextProtocol.Extensions.Tasks` and dropped the per-tool execution augmentation (`Tool.Execution`) from the protocol surface, so `.LongRunning()` commands no longer advertise task support at the protocol level. The annotation stays in Repl's own model (help/docs); protocol-level task support can return once Repl integrates the Tasks extension, store, and get/update/cancel lifecycle (tracked in issue #72). | Feature | Claude Desktop | Claude Code | Codex | VS Code Copilot | Cursor | Continue | diff --git a/docs/mcp-transports.md b/docs/mcp-transports.md index e8d71c4d..5ed42a89 100644 --- a/docs/mcp-transports.md +++ b/docs/mcp-transports.md @@ -46,6 +46,16 @@ async Task HandleConnectionAsync(Stream input, Stream output, CancellationToken } ``` +Client capabilities (sampling, elicitation, roots) resolve per **request** on this path, so each +connection sees its own — that is a property of the request, not of the options instance. + +> **Known limitation:** cross-call state does not. The options carry one command catalog, so +> [soft roots](mcp-advanced.md#soft-roots-fallback) set by one connection are visible to every other +> connection built from the same options. The `2026-07-28` revision removed protocol-level sessions, +> and this path has no per-connection identity to hang that state on. If your commands rely on soft +> roots, host one server per process (`mcp serve`) or pass the workspace as an explicit command +> argument. + ## Scenario B: MCP-over-HTTP The MCP spec also defines an HTTP transport. For that, you typically host MCP inside ASP.NET Core rather than through `mcp serve`. @@ -64,14 +74,19 @@ var mcpOptions = app.Core.BuildMcpServerOptions(configure: o => You can then pass those options to the MCP SDK's HTTP integration. -## Session isolation +## What is isolated, and at which boundary -Each connection or HTTP session is isolated: +The `2026-07-28` revision removed protocol-level sessions: the client declares its capabilities on +every request rather than once per connection. So the boundaries are not all the same size. -- its own MCP session -- its own I/O capture -- its own session-aware routing state +| Isolated per | What | +|---|---| +| Request | Client capabilities, the requested log level, and the destination for sampling, elicitation and progress | +| Invocation | I/O capture — each tool call gets its own capture scope, not one per connection | +| Connection (`mcp serve` only) | The MCP session object, the native roots cache, soft roots, and session-aware routing state | -That matters especially when using dynamic tools, roots, or session-specific modules. +A server created from a reused `BuildMcpServerOptions()` result has the first two but not the third; +see the known limitation above. That matters especially when using dynamic tools, roots, or +session-specific modules. For those higher-level patterns, see [mcp-advanced.md](mcp-advanced.md). From f561022a12d240e666e8c81520306bff5460bc59 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Sun, 23 Aug 2026 23:40:55 -0400 Subject: [PATCH 16/16] chore(mcp): repair the deprecation banners and record the SDK migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The MCP9005 justification banner existed as four diverging copies of the same six lines, and two of them (McpSamplingService, McpInteractionChannel) carried a copy-paste corruption: "…so Repl keeps / these features, so Repl keeps supporting them…". Rather than fix the wording in place and leave five copies to drift again, all of them collapse to two lines plus a pointer to docs/mcp-reference.md#sdk-and-protocol-versions, where the rationale now lives once. - When_SerializingLongRunningTool asserted NotContain("execution") and NotContain("taskSupport") against a Tool type that no longer HAS those members, so neither assertion could fail; its only positive assertion (openWorldHint) came from .OpenWorld(), meaning deleting .LongRunning() left it green. It now serializes two tools differing only by .LongRunning() and requires identical payloads, which is what "emits nothing on the protocol surface" actually means. Verified live: adding .ReadOnly() to one of the pair turns it red. - CHANGELOG records the consumer-facing surface: the 2.x requirement, the McpMessageLevel break and why the internal #pragma never covered a consumer's build, the revision-dependent delivery of both feedback and discovery notifications, and the soft-roots known limitation on the reused options path. Full solution green: 1466 passed, 1 known Inspector skip. --- CHANGELOG.md | 37 +++++++++++++++++++++++ src/Repl.Mcp/McpClientRootsService.cs | 7 ++--- src/Repl.Mcp/McpFeedbackService.cs | 6 ++-- src/Repl.Mcp/McpInteractionChannel.cs | 7 ++--- src/Repl.Mcp/McpSamplingService.cs | 7 ++--- src/Repl.McpTests/Given_McpIntegration.cs | 26 ++++++++++------ 6 files changed, 62 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57bcb2fb..623bbfd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,43 @@ Nerdbank.GitVersioning at pack time; this file groups changes by theme instead o ## Unreleased +### Changed — MCP SDK 2.2.0 + +- `Repl.Mcp` now builds on `ModelContextProtocol` **2.2.0** (from 1.4.1). The SDK is a transitively + public dependency, so a consumer referencing `Repl.Mcp` must move to the 2.x line. The server stays + multi-revision: it keeps the `initialize` handshake for existing hosts while also serving the + sessionless `2026-07-28` revision. +- **Breaking:** `IMcpFeedback.SendMessageAsync` takes a Repl-owned `McpMessageLevel` instead of the + SDK's `LoggingLevel`. `LoggingLevel` carries the SDK's `MCP9005` deprecation, and Repl's internal + `#pragma` never covered a *consumer's* compilation — anyone building with warnings as errors got a + hard error on a Repl signature. The new enum has the same members and numeric values. +- **User feedback delivery depends on the negotiated revision.** `2026-07-28` removed + `logging/setLevel` and forbids emitting `notifications/message` for a request that declared no + `_meta/io.modelcontextprotocol/logLevel` (SEP-2575), so Repl now honours that. Messages that cannot + be sent as notifications are appended to the **tool result** after the command's own payload, so no + host loses feedback. Initialize-era clients keep the previous session-wide behaviour, unchanged. + A caller reading only the first content block, or `StructuredContent`, is unaffected. +- **Discovery notifications are delivered by the SDK's own fan-out** rather than broadcast by Repl. + Initialize-era clients keep receiving unsolicited `*/list_changed`; a `2026-07-28` client receives + only the types it requested through `subscriptions/listen`, tagged with its listen request id. A + modern client that opens no subscription receives none — as the specification requires. List + results carry `ttlMs: 0`, so such a client re-lists on demand instead of caching. +- `.LongRunning()` remains Repl-local metadata and emits nothing on the protocol surface; SDK 2.x + removed the per-tool `Tool.Execution` augmentation. Protocol-level task support returns once Repl + integrates `ModelContextProtocol.Extensions.Tasks` (tracked in issue #72). + +### Compatibility notes — MCP + +- **Known limitation.** Soft roots ([`docs/mcp-advanced.md`](docs/mcp-advanced.md#soft-roots-fallback)) + set by one connection are visible to every other connection created from the same + `BuildMcpServerOptions()` result. Client capabilities *are* isolated per request on that path; + cross-call state is not, because `2026-07-28` removed protocol sessions and that path has no + per-connection identity. Host one server per process (`mcp serve`), or take the workspace as an + explicit command argument. +- `docs/mcp-transports.md` previously claimed each connection has "its own I/O capture" and "its own + session-aware routing state". I/O capture is per *invocation*, and session-aware routing state + exists only under `mcp serve`. The doc now states what is isolated at which boundary. + ### Added — option visibility - `.Hidden(bool isHidden = true)` on the option builder (`WithOption(name, option => option.Hidden())`) diff --git a/src/Repl.Mcp/McpClientRootsService.cs b/src/Repl.Mcp/McpClientRootsService.cs index f699c709..6c84d4ab 100644 --- a/src/Repl.Mcp/McpClientRootsService.cs +++ b/src/Repl.Mcp/McpClientRootsService.cs @@ -1,11 +1,8 @@ using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; -// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) -// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps -// supporting them until the SDK removes the surface (#51). +// 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; diff --git a/src/Repl.Mcp/McpFeedbackService.cs b/src/Repl.Mcp/McpFeedbackService.cs index 2610857a..c1d2a902 100644 --- a/src/Repl.Mcp/McpFeedbackService.cs +++ b/src/Repl.Mcp/McpFeedbackService.cs @@ -5,10 +5,8 @@ using ModelContextProtocol.Server; using Repl.Interaction; -// Logging is deprecated by MCP spec 2026-07-28 (SEP-2577, SDK diagnostic MCP9005); the designated -// successor for server-initiated flows (SEP-2322 multi-round-trip requests, shipped in the SDK 2.0 -// line as MrtrContext/MrtrExchange) is not adopted by Repl yet, and hosts still rely on message -// notifications, so Repl keeps supporting them until the SDK removes the surface (#51). +// 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; diff --git a/src/Repl.Mcp/McpInteractionChannel.cs b/src/Repl.Mcp/McpInteractionChannel.cs index 8ed6282d..af8bd10a 100644 --- a/src/Repl.Mcp/McpInteractionChannel.cs +++ b/src/Repl.Mcp/McpInteractionChannel.cs @@ -5,11 +5,8 @@ using ModelContextProtocol.Server; using Repl.Interaction; -// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) -// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps -// these features, so Repl keeps supporting them until the SDK removes the surface (#51). +// 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; diff --git a/src/Repl.Mcp/McpSamplingService.cs b/src/Repl.Mcp/McpSamplingService.cs index 53d62b3a..568420e7 100644 --- a/src/Repl.Mcp/McpSamplingService.cs +++ b/src/Repl.Mcp/McpSamplingService.cs @@ -1,11 +1,8 @@ using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; -// Roots, Sampling, and Logging are deprecated by MCP spec 2026-07-28 (SEP-2577, SDK -// diagnostic MCP9005); the designated successor for server-initiated flows (SEP-2322, -// multi-round-trip requests, available in SDK 2.0 as MrtrContext/MrtrExchange) -// is not adopted by Repl yet, and hosts still rely on these features, so Repl keeps -// these features, so Repl keeps supporting them until the SDK removes the surface (#51). +// 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; diff --git a/src/Repl.McpTests/Given_McpIntegration.cs b/src/Repl.McpTests/Given_McpIntegration.cs index 22cd245d..44b0bb8f 100644 --- a/src/Repl.McpTests/Given_McpIntegration.cs +++ b/src/Repl.McpTests/Given_McpIntegration.cs @@ -1,4 +1,4 @@ -using ModelContextProtocol.Client; +using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; using Repl.Mcp; @@ -151,24 +151,32 @@ public async Task When_ClientPinsLegacyProtocolVersion_Then_InitializeHandshakeA } [TestMethod] - [Description("Locks the SDK-2.0 tools/list wire shape for .LongRunning() commands: annotations survive serialization, and no task/execution augmentation is emitted — Repl deliberately does not advertise MCP task support until the Tasks runtime is implemented end-to-end (issue #51).")] - public void When_SerializingLongRunningTool_Then_NoTaskAugmentationIsEmitted() + [Description("Locks the tools/list wire shape for .LongRunning(): the annotation is Repl-local metadata and must leave no trace on the protocol surface until Repl integrates the SDK Tasks extension (issue #72). Asserted by serializing two otherwise identical tools that differ only by .LongRunning() and requiring byte-identical payloads — a NotContain(\"execution\") assertion could not fail, since SDK 2.x removed Tool.Execution entirely.")] + public void When_SerializingLongRunningTool_Then_WireShapeIsUnchanged() { var app = ReplApp.Create(); app.Map("deploy", () => "deployed") .WithDescription("Deploy application") .LongRunning() .OpenWorld(); + app.Map("release", () => "released") + .WithDescription("Deploy application") + .OpenWorld(); var options = app.BuildMcpServerOptions(); + + SerializeTool(options, "deploy").Should().Be( + SerializeTool(options, "release").Replace("\"release\"", "\"deploy\"", StringComparison.Ordinal), + because: ".LongRunning() must not change anything a client can observe"); + } + + private static string SerializeTool(ModelContextProtocol.Server.McpServerOptions options, string name) + { var tool = options.ToolCollection!.Single(tool => - string.Equals(tool.ProtocolTool.Name, "deploy", StringComparison.Ordinal)); - var json = System.Text.Json.JsonSerializer.Serialize( - tool.ProtocolTool, ModelContextProtocol.McpJsonUtilities.DefaultOptions); + string.Equals(tool.ProtocolTool.Name, name, StringComparison.Ordinal)); - json.Should().Contain("\"openWorldHint\""); - json.Should().NotContain("execution"); - json.Should().NotContain("taskSupport"); + return System.Text.Json.JsonSerializer.Serialize( + tool.ProtocolTool, ModelContextProtocol.McpJsonUtilities.DefaultOptions); } [TestMethod]