From 09709dc05363c6f5a687f53ae7e75c70a6bd76fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 1 Oct 2026 07:42:50 +0000 Subject: [PATCH 01/12] Upgrade ModelContextProtocol SDK to 2.2.0 and keep stateful HTTP sessions Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs | 6 +++++- Directory.Packages.props | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs index 1193cea1..5018048e 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs @@ -14,6 +14,7 @@ namespace Azure.Data.Cosmos.Shell.Mcp; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; +using ModelContextProtocol.AspNetCore; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; @@ -80,7 +81,10 @@ private static void ConfigureMcpServer(IServiceCollection services) var mcpServerBuilder = services.AddMcpServer(); mcpServerBuilder.WithResources(); - mcpServerBuilder.WithHttpTransport(); + + // Destructive-command confirmation uses elicitation, a server-to-client request that + // requires a session. SDK 2.x defaults to stateless, so keep sessions enabled. + mcpServerBuilder.WithHttpTransport(options => options.SessionMode = HttpServerSessionMode.Stateful); } private static string LoadServerInstructions() diff --git a/Directory.Packages.props b/Directory.Packages.props index c555da62..b379a907 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -24,8 +24,8 @@ - - + + From 17de1b370a5997ef2e8d1771c730ed21f7e00c1d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:09:40 +0000 Subject: [PATCH 02/12] Tie MCP location subscriptions to session lifetime and fix test port race Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- .../McpLocationSubscriptionTests.cs | 72 ++++++++++++--- .../LocationResourceSubscriptions.cs | 89 +++++++++++++++---- .../Azure.Data.Cosmos.Shell.Mcp/McpServer.cs | 20 ++++- .../ToolOperations.cs | 4 +- docs/mcp.md | 2 +- 5 files changed, 151 insertions(+), 36 deletions(-) diff --git a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs index 2115838c..cfcb055e 100644 --- a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs +++ b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs @@ -4,13 +4,15 @@ namespace CosmosShell.Tests; -using System.Net; -using System.Net.Sockets; using System.Text.Json; using Azure.Data.Cosmos.Shell.Core; using Azure.Data.Cosmos.Shell.Mcp; using Azure.Data.Cosmos.Shell.States; +using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Hosting.Server.Features; using Microsoft.Azure.Cosmos; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using ModelContextProtocol; using ModelContextProtocol.Client; @@ -22,20 +24,11 @@ public async Task SubscribedClient_ReceivesInteractiveLocationChange() { using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); timeout.CancelAfter(TimeSpan.FromSeconds(10)); - using var listener = new TcpListener(IPAddress.Loopback, 0); - listener.Start(); - var port = ((IPEndPoint)listener.LocalEndpoint).Port; - listener.Stop(); - - using var host = McpServer.CreateHost(new Program.CosmosShellOptions { McpPort = port }); + using var host = McpServer.CreateHost(new Program.CosmosShellOptions { McpPort = 0 }); await host.StartAsync(timeout.Token); try { - var transport = new HttpClientTransport(new HttpClientTransportOptions - { - Endpoint = new Uri($"http://127.0.0.1:{port}/"), - }); - await using var client = await McpClient.CreateAsync(transport, cancellationToken: timeout.Token); + await using var client = await ConnectAsync(host, timeout.Token); var resources = await client.ListResourcesAsync(cancellationToken: timeout.Token); Assert.Contains(resources, resource => resource.Uri == ResourceOperations.CurrentLocationUri); @@ -61,6 +54,9 @@ public async Task SubscribedClient_ReceivesInteractiveLocationChange() new CosmosClientOptions { ConnectionMode = ConnectionMode.Gateway }); try { + // The subscription must not depend on per-request objects that the GC can reclaim. + GC.Collect(); + GC.WaitForPendingFinalizers(); ShellInterpreter.Instance.State = new DatabaseState("McpNotificationTest", cosmosClient); Assert.Equal(ResourceOperations.CurrentLocationUri, await updated.Task.WaitAsync(timeout.Token)); @@ -80,4 +76,54 @@ public async Task SubscribedClient_ReceivesInteractiveLocationChange() await host.StopAsync(TestContext.Current.CancellationToken); } } + + [Fact] + public async Task EndedSession_RemovesLocationSubscription() + { + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + timeout.CancelAfter(TimeSpan.FromSeconds(10)); + using var host = McpServer.CreateHost(new Program.CosmosShellOptions { McpPort = 0 }); + await host.StartAsync(timeout.Token); + try + { + var subscriptions = host.Services.GetRequiredService(); + var client = await ConnectAsync(host, timeout.Token); + await client.SubscribeToResourceAsync(ResourceOperations.CurrentLocationUri, cancellationToken: timeout.Token); + Assert.Equal(1, subscriptions.SubscriberCount); + + // Disposing the client ends the session with DELETE, which must release the subscription. + await client.DisposeAsync(); + while (subscriptions.SubscriberCount != 0) + { + await Task.Delay(20, timeout.Token); + } + } + finally + { + await host.StopAsync(TestContext.Current.CancellationToken); + } + } + + [Fact] + public void HttpTransport_UsesStatefulSessionsWithBoundedIdleTimeout() + { + var options = new ModelContextProtocol.AspNetCore.HttpServerTransportOptions(); + McpServer.ConfigureHttpTransport(options); + + Assert.Equal(ModelContextProtocol.AspNetCore.HttpServerSessionMode.Stateful, options.SessionMode); +#pragma warning disable MCP9006, MCPEXP002 + Assert.Equal(McpServer.SessionIdleTimeout, options.IdleTimeout); + Assert.NotNull(options.RunSessionHandler); +#pragma warning restore MCP9006, MCPEXP002 + } + + private static async Task ConnectAsync(IHost host, CancellationToken cancellationToken) + { + var address = host.Services.GetRequiredService().Features.Get()!.Addresses.Single(); + var transport = new HttpClientTransport(new HttpClientTransportOptions + { + Endpoint = new Uri(address.TrimEnd('/') + "/"), + }); + return await McpClient.CreateAsync(transport, cancellationToken: cancellationToken); + } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs index 5759d729..b892c0d6 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs @@ -16,7 +16,11 @@ internal sealed class LocationResourceSubscriptions : BackgroundService { private readonly object sync = new(); - private readonly List> subscribers = []; + // Session servers keyed by session ID. Entries are added when a session starts running and + // removed when it ends (DELETE, idle timeout, or shutdown), so no dead session is retained. + private readonly Dictionary sessions = new(StringComparer.Ordinal); + + private readonly HashSet subscribedSessionIds = new(StringComparer.Ordinal); // Notifications carry only the URI, so pending changes are coalesced into one. private readonly Channel changes = Channel.CreateBounded( @@ -30,25 +34,81 @@ public LocationResourceSubscriptions(ILogger logg ShellInterpreter.Instance.LocationChanged += this.OnLocationChanged; } - public void Subscribe(ModelContextProtocol.Server.McpServer server, string uri) + internal int SubscriberCount + { + get + { + lock (this.sync) + { + return this.subscribedSessionIds.Count; + } + } + } + + /// + /// Runs an MCP session and keeps it available for notifications until the session ends. + /// + public async Task RunSessionAsync(ModelContextProtocol.Server.McpServer server, CancellationToken cancellationToken) + { + var sessionId = server.SessionId; + if (sessionId is not null) + { + lock (this.sync) + { + this.sessions[sessionId] = server; + } + } + + try + { + await server.RunAsync(cancellationToken); + } + finally + { + if (sessionId is not null) + { + lock (this.sync) + { + if (this.sessions.TryGetValue(sessionId, out var current) && ReferenceEquals(current, server)) + { + this.sessions.Remove(sessionId); + this.subscribedSessionIds.Remove(sessionId); + } + } + } + } + } + + public void Subscribe(string? sessionId, string uri) { ValidateUri(uri); + if (sessionId is null) + { + throw new McpProtocolException( + "Resource subscriptions require a stateful MCP session.", + McpErrorCode.InvalidRequest); + } + lock (this.sync) { - this.PruneSubscribers(); - if (!this.subscribers.Any(reference => reference.TryGetTarget(out var target) && ReferenceEquals(target, server))) + if (this.sessions.ContainsKey(sessionId)) { - this.subscribers.Add(new WeakReference(server)); + this.subscribedSessionIds.Add(sessionId); } } } - public void Unsubscribe(ModelContextProtocol.Server.McpServer server, string uri) + public void Unsubscribe(string? sessionId, string uri) { ValidateUri(uri); + if (sessionId is null) + { + return; + } + lock (this.sync) { - this.subscribers.RemoveAll(reference => !reference.TryGetTarget(out var target) || ReferenceEquals(target, server)); + this.subscribedSessionIds.Remove(sessionId); } } @@ -59,9 +119,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) ModelContextProtocol.Server.McpServer[] servers; lock (this.sync) { - this.PruneSubscribers(); - servers = this.subscribers - .Select(reference => reference.TryGetTarget(out var server) ? server : null) + servers = this.subscribedSessionIds + .Select(sessionId => this.sessions.TryGetValue(sessionId, out var server) ? server : null) .OfType() .ToArray(); } @@ -81,11 +140,8 @@ await server.SendNotificationAsync( } catch (Exception ex) when (!stoppingToken.IsCancellationRequested) { + // Session cleanup is tied to the session lifetime; a failed send only affects this notification. this.logger.LogWarning(ex, "Could not notify an MCP client about the shell location change."); - lock (this.sync) - { - this.subscribers.RemoveAll(reference => !reference.TryGetTarget(out var target) || ReferenceEquals(target, server)); - } } } } @@ -111,9 +167,4 @@ private void OnLocationChanged() { this.changes.Writer.TryWrite(true); } - - private void PruneSubscribers() - { - this.subscribers.RemoveAll(reference => !reference.TryGetTarget(out _)); - } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs index 38c7933c..18a9282b 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs @@ -27,6 +27,8 @@ namespace Azure.Data.Cosmos.Shell.Mcp; /// internal class McpServer { + internal static readonly TimeSpan SessionIdleTimeout = TimeSpan.FromMinutes(10); + public static IHost CreateHost(CosmosShellOptions serverArguments) { var builder = WebApplication.CreateBuilder([]); @@ -88,7 +90,23 @@ private static void ConfigureMcpServer(IServiceCollection services) // Destructive-command confirmation uses elicitation, a server-to-client request that // requires a session. SDK 2.x defaults to stateless, so keep sessions enabled. - mcpServerBuilder.WithHttpTransport(options => options.SessionMode = HttpServerSessionMode.Stateful); + mcpServerBuilder.WithHttpTransport(ConfigureHttpTransport); + } + + internal static void ConfigureHttpTransport(HttpServerTransportOptions options) + { + options.SessionMode = HttpServerSessionMode.Stateful; + + // Sessions with an open GET stream never go idle. Once a client disconnects without DELETE, + // the session is disposed after this timeout, which also ends its location subscription. +#pragma warning disable MCP9006 // Stateful Streamable HTTP options are required for session-bound features. + options.IdleTimeout = SessionIdleTimeout; +#pragma warning restore MCP9006 + +#pragma warning disable MCPEXP002 // RunSessionHandler is the only hook that observes the session lifetime. + options.RunSessionHandler = (httpContext, server, cancellationToken) => + httpContext.RequestServices.GetRequiredService().RunSessionAsync(server, cancellationToken); +#pragma warning restore MCPEXP002 } private static string LoadServerInstructions() diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs index 612779e3..2fca20dd 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs @@ -54,13 +54,13 @@ public ToolOperations(ILogger logger, LocationResourceSubscripti private ValueTask SubscribeToResourcesAsync(RequestContext context, CancellationToken cancellationToken) { - this.locationSubscriptions.Subscribe(context.Server, context.Params?.Uri ?? string.Empty); + this.locationSubscriptions.Subscribe(context.Server.SessionId, context.Params?.Uri ?? string.Empty); return ValueTask.FromResult(new EmptyResult()); } private ValueTask UnsubscribeFromResourcesAsync(RequestContext context, CancellationToken cancellationToken) { - this.locationSubscriptions.Unsubscribe(context.Server, context.Params?.Uri ?? string.Empty); + this.locationSubscriptions.Unsubscribe(context.Server.SessionId, context.Params?.Uri ?? string.Empty); return ValueTask.FromResult(new EmptyResult()); } diff --git a/docs/mcp.md b/docs/mcp.md index 80f18c8f..26766803 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -99,7 +99,7 @@ Clients can read the `cosmos://shell/current-location` MCP resource. Its JSON co Only `cosmos://shell/current-location` supports subscriptions; subscribing to any other URI, including the documentation resources, returns an invalid-params error. -This server uses the subscription protocol supported by its MCP SDK; clients must support subscriptions and server-to-client notifications over the HTTP connection. A notification does not guarantee that a client refreshes the model's context. Every tool response also includes `currentLocation`, and explicit `database` / `container` arguments remain the reliable way to target independent operations. +This server uses the subscription protocol supported by its MCP SDK; clients must support subscriptions and server-to-client notifications over the HTTP connection. Notifications are delivered on the session's GET stream. A subscription lasts as long as its MCP session: it ends when the client deletes the session or when the session has had no open request (including the GET stream) for 10 minutes. After that the server returns 404 for the session and the client must start a new session and subscribe again. A notification does not guarantee that a client refreshes the model's context. Every tool response also includes `currentLocation`, and explicit `database` / `container` arguments remain the reliable way to target independent operations. ### Data Exposure From 1e76cf68198df9350971bf8bcfd6ac0621b914ec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 1 Oct 2026 10:56:57 +0000 Subject: [PATCH 03/12] Confirm destructive MCP commands with multi-round-trip requests Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- CosmosDBShell.Tests/McpConfirmationTests.cs | 65 ++++++++ .../ToolOperationsCallToolTests.cs | 155 +++++++++++++++++- .../ConfirmationRequestState.cs | 67 ++++++++ .../ToolOperations.cs | 120 +++++++++++--- docs/mcp.md | 6 +- 5 files changed, 384 insertions(+), 29 deletions(-) create mode 100644 CosmosDBShell.Tests/McpConfirmationTests.cs create mode 100644 CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs diff --git a/CosmosDBShell.Tests/McpConfirmationTests.cs b/CosmosDBShell.Tests/McpConfirmationTests.cs new file mode 100644 index 00000000..ec4bea72 --- /dev/null +++ b/CosmosDBShell.Tests/McpConfirmationTests.cs @@ -0,0 +1,65 @@ +// ------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +// ------------------------------------------------------------ + +namespace CosmosShell.Tests; + +using System.Text.Json; +using Azure.Data.Cosmos.Shell.Mcp; +using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; + +[Collection(CosmosShell.Tests.Shell.ThemeStateTestCollection.Name)] +public class McpConfirmationTests +{ + [Fact] + public async Task DestructiveCommand_OverHttp_AsksClientAndHonorsDecline() + { + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + timeout.CancelAfter(TimeSpan.FromSeconds(10)); + using var host = McpServer.CreateHost(new Program.CosmosShellOptions { McpPort = 0 }); + await host.StartAsync(timeout.Token); + try + { + var prompts = new List(); + var address = host.Services.GetRequiredService().Features.Get()!.Addresses.Single(); + var transport = new HttpClientTransport(new HttpClientTransportOptions + { + Endpoint = new Uri(address.TrimEnd('/') + "/"), + }); + await using var client = await McpClient.CreateAsync( + transport, + new McpClientOptions + { + Handlers = new McpClientHandlers + { + ElicitationHandler = (request, _) => + { + prompts.Add(request?.Message); + return ValueTask.FromResult(new ElicitResult { Action = "decline" }); + }, + }, + }, + cancellationToken: timeout.Token); + + var result = await client.CallToolAsync( + "rmdb", + new Dictionary { ["name"] = "McpConfirmationTestDb" }, + cancellationToken: timeout.Token); + + var prompt = Assert.Single(prompts); + Assert.Contains("rmdb \"McpConfirmationTestDb\"", prompt); + Assert.True(result.IsError); + using var document = JsonDocument.Parse(Assert.IsType(Assert.Single(result.Content)).Text); + Assert.Contains("was not approved by the user", document.RootElement.GetProperty("error").GetString()); + } + finally + { + await host.StopAsync(TestContext.Current.CancellationToken); + } + } +} diff --git a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs index d1e89c65..bac15612 100644 --- a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs +++ b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs @@ -197,7 +197,7 @@ public async Task CallTool_RmWithJsonNullSafetyOption_ConfirmedCommandFailsWitho factory, command, "rm \"order-123\" --key \"id\" --partition-key \"customer-42\"", - (_, _) => new ValueTask(new ElicitResult { Action = "accept" }), + (_, _, _) => new ValueTask(new ElicitResult { Action = "accept" }), TestContext.Current.CancellationToken); Assert.True(result.IsError); @@ -212,7 +212,7 @@ public async Task ConfirmDestructive_UserAccepts_ReturnsNull() var tool = CreateToolOperations(); var result = await tool.ConfirmDestructiveAsync( - (_, _) => new ValueTask(new ElicitResult { Action = "accept" }), + (_, _, _) => new ValueTask(new ElicitResult { Action = "accept" }), "rmdb", "rmdb mydb", CancellationToken.None); @@ -230,7 +230,7 @@ public async Task ExecuteTool_ContextChangesDuringConfirmation_RefusesExecution( { var result = await CreateToolOperations().ExecuteToolAsync( shell.App.Commands["rm"], command, "rm test-*", - (request, _) => + (request, _, _) => { Assert.Contains("Account:", request.Message); Assert.Contains("Current location:", request.Message); @@ -266,7 +266,7 @@ public async Task ExecuteTool_UnchangedContextAfterConfirmation_ExecutesCommand( var command = new TrackingCommand(); var result = await CreateToolOperations().ExecuteToolAsync( ShellInterpreter.Instance.App.Commands["rm"], command, "rm test-*", - (_, _) => new ValueTask(new ElicitResult { Action = "accept" }), + (_, _, _) => new ValueTask(new ElicitResult { Action = "accept" }), TestContext.Current.CancellationToken); Assert.False(result.IsError == true); Assert.True(command.Executed); @@ -292,7 +292,7 @@ public async Task ExecuteTool_WithoutAnsiTerminal_EchoesPlainlyAndStillExecutes( var result = await CreateToolOperations().ExecuteToolAsync( ShellInterpreter.Instance.App.Commands["rm"], command, "rm test-*", - (_, _) => new ValueTask(new ElicitResult { Action = "accept" }), + (_, _, _) => new ValueTask(new ElicitResult { Action = "accept" }), TestContext.Current.CancellationToken); Assert.False(result.IsError == true); @@ -316,7 +316,7 @@ public async Task ConfirmDestructive_UserDeclines_ReturnsErrorAndDoesNotExecute( var tool = CreateToolOperations(); var result = await tool.ConfirmDestructiveAsync( - (_, _) => new ValueTask(new ElicitResult { Action = action }), + (_, _, _) => new ValueTask(new ElicitResult { Action = action }), "rmdb", "rmdb mydb", CancellationToken.None); @@ -338,7 +338,7 @@ public async Task ConfirmDestructive_ElicitationThrows_ReturnsErrorAndDoesNotExe var tool = CreateToolOperations(); var result = await tool.ConfirmDestructiveAsync( - (_, _) => throw new InvalidOperationException("boom"), + (_, _, _) => throw new InvalidOperationException("boom"), "rmdb", "rmdb mydb", CancellationToken.None); @@ -674,4 +674,145 @@ public async Task ListTools_ReturnsRegisteredTools() Assert.Contains(result.Tools, t => t.Name == "query"); Assert.Contains(result.Tools, t => t.Name == "echo"); } + + [Fact] + public void ConfirmationRequestState_RoundTripsOnlyForTheSameCommand() + { + var state = ConfirmationRequestState.Create("rmdb mydb", 42); + + Assert.True(ConfirmationRequestState.TryRead(state, "rmdb mydb", out var version)); + Assert.Equal(42, version); + Assert.False(ConfirmationRequestState.TryRead(state, "rmdb otherdb", out _)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("not-a-state")] + [InlineData("Zm9v.YmFy")] + public void ConfirmationRequestState_RejectsMissingOrForgedState(string? state) + { + Assert.False(ConfirmationRequestState.TryRead(state, "rmdb mydb", out _)); + } + + [Fact] + public void ConfirmationRequestState_RejectsTamperedPayload() + { + var state = ConfirmationRequestState.Create("rmdb mydb", 1); + var signature = state[(state.IndexOf('.') + 1)..]; + var forgedPayload = Microsoft.AspNetCore.WebUtilities.WebEncoders.Base64UrlEncode( + System.Text.Encoding.UTF8.GetBytes("1\nrmdb otherdb")); + + Assert.False(ConfirmationRequestState.TryRead(forgedPayload + "." + signature, "rmdb otherdb", out _)); + } + + [Fact] + public async Task ExecuteTool_MrtrPrompt_RequestsConfirmationWithoutExecuting() + { + var command = new TrackingCommand(); + var shell = ShellInterpreter.Instance; + + var exception = await Assert.ThrowsAsync(() => CreateToolOperations().ExecuteToolAsync( + shell.App.Commands["rm"], command, "rm test-*", + (request, stateVersion, _) => throw new InputRequiredException( + inputRequests: new Dictionary { ["confirm"] = InputRequest.ForElicitation(request) }, + requestState: ConfirmationRequestState.Create("rm test-*", stateVersion)), + TestContext.Current.CancellationToken)); + + Assert.False(command.Executed); + var inputRequest = Assert.Single(exception.Result.InputRequests!); + Assert.Equal("confirm", inputRequest.Key); + Assert.Contains("rm test-*", inputRequest.Value.ElicitationParams!.Message); + Assert.True(ConfirmationRequestState.TryRead(exception.Result.RequestState, "rm test-*", out var version)); + Assert.Equal(shell.StateVersion, version); + } + + [Fact] + public async Task ExecuteTool_AcceptedConfirmationResponse_ExecutesCommand() + { + var command = new TrackingCommand(); + var shell = ShellInterpreter.Instance; + + var result = await CreateToolOperations().ExecuteToolAsync( + shell.App.Commands["rm"], command, "rm test-*", null, + TestContext.Current.CancellationToken, + ConfirmationRetry("accept", ConfirmationRequestState.Create("rm test-*", shell.StateVersion))); + + Assert.False(result.IsError == true); + Assert.True(command.Executed); + } + + [Theory] + [InlineData("decline")] + [InlineData("cancel")] + public async Task ExecuteTool_DeclinedConfirmationResponse_DoesNotExecute(string action) + { + var command = new TrackingCommand(); + var shell = ShellInterpreter.Instance; + + var result = await CreateToolOperations().ExecuteToolAsync( + shell.App.Commands["rm"], command, "rm test-*", null, + TestContext.Current.CancellationToken, + ConfirmationRetry(action, ConfirmationRequestState.Create("rm test-*", shell.StateVersion))); + + Assert.True(result.IsError); + Assert.False(command.Executed); + Assert.Contains("was not approved by the user", Assert.IsType(Assert.Single(result.Content)).Text); + } + + [Fact] + public async Task ExecuteTool_ConfirmationResponseForOtherCommand_DoesNotExecute() + { + var command = new TrackingCommand(); + var shell = ShellInterpreter.Instance; + + var result = await CreateToolOperations().ExecuteToolAsync( + shell.App.Commands["rm"], command, "rm *", null, + TestContext.Current.CancellationToken, + ConfirmationRetry("accept", ConfirmationRequestState.Create("rm test-*", shell.StateVersion))); + + Assert.True(result.IsError); + Assert.False(command.Executed); + Assert.Contains("does not match this command", Assert.IsType(Assert.Single(result.Content)).Text); + } + + [Fact] + public async Task ExecuteTool_ContextChangedBetweenConfirmationRounds_DoesNotExecute() + { + var shell = ShellInterpreter.Instance; + var originalState = shell.State; + var command = new TrackingCommand(); + var requestState = ConfirmationRequestState.Create("rm test-*", shell.StateVersion); + try + { + shell.State = new DisconnectedState(); + shell.State = originalState; + + var result = await CreateToolOperations().ExecuteToolAsync( + shell.App.Commands["rm"], command, "rm test-*", null, + TestContext.Current.CancellationToken, + ConfirmationRetry("accept", requestState)); + + Assert.True(result.IsError); + Assert.False(command.Executed); + Assert.Contains("context changed", Assert.IsType(Assert.Single(result.Content)).Text); + } + finally + { + shell.State = originalState; + } + } + + private static CallToolRequestParams ConfirmationRetry(string action, string requestState) + { + return new CallToolRequestParams + { + Name = "rm", + RequestState = requestState, + InputResponses = new Dictionary + { + ["confirm"] = InputResponse.FromElicitResult(new ElicitResult { Action = action }), + }, + }; + } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs new file mode 100644 index 00000000..8f2b282b --- /dev/null +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs @@ -0,0 +1,67 @@ +// ------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +// ------------------------------------------------------------ + +namespace Azure.Data.Cosmos.Shell.Mcp; + +using System.Globalization; +using System.Security.Cryptography; +using System.Text; +using Microsoft.AspNetCore.WebUtilities; + +/// +/// Creates and validates the MRTR requestState for destructive-command confirmation. +/// The state binds an approval to the exact command line and shell state version it was +/// requested for, and is signed so a client cannot fabricate or retarget it. +/// +internal static class ConfirmationRequestState +{ + // Per-process key: confirmations do not survive a server restart. + private static readonly byte[] Key = RandomNumberGenerator.GetBytes(32); + + public static string Create(string commandLine, long stateVersion) + { + var payload = Encoding.UTF8.GetBytes(stateVersion.ToString(CultureInfo.InvariantCulture) + "\n" + commandLine); + return WebEncoders.Base64UrlEncode(payload) + "." + WebEncoders.Base64UrlEncode(Sign(payload)); + } + + public static bool TryRead(string? requestState, string commandLine, out long stateVersion) + { + stateVersion = 0; + if (string.IsNullOrEmpty(requestState)) + { + return false; + } + + var separator = requestState.IndexOf('.'); + if (separator <= 0) + { + return false; + } + + byte[] payload; + byte[] signature; + try + { + payload = WebEncoders.Base64UrlDecode(requestState[..separator]); + signature = WebEncoders.Base64UrlDecode(requestState[(separator + 1)..]); + } + catch (FormatException) + { + return false; + } + + if (!CryptographicOperations.FixedTimeEquals(signature, Sign(payload))) + { + return false; + } + + var text = Encoding.UTF8.GetString(payload); + var newline = text.IndexOf('\n'); + return newline > 0 + && string.Equals(text[(newline + 1)..], commandLine, StringComparison.Ordinal) + && long.TryParse(text[..newline], NumberStyles.None, CultureInfo.InvariantCulture, out stateVersion); + } + + private static byte[] Sign(byte[] payload) => HMACSHA256.HashData(Key, payload); +} diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs index 2fca20dd..dd2c9cbd 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs @@ -25,6 +25,11 @@ internal class ToolOperations private const string MaxArgument = "max"; + private const string ConfirmationInputKey = "confirm"; + + private const string ContextChangedMessage = + "The shell context changed while awaiting confirmation. Nothing was executed. Retry the command and confirm its current target."; + private const string ContinuationDescription = "Non-null continuation token returned by a previous call to this tool. Pass it back to fetch the next page, or omit this argument to start from the beginning. A null output token means the result is exhausted and no further call should be made. The value is opaque; do not modify it."; @@ -44,6 +49,11 @@ public ToolOperations(ILogger logger, LocationResourceSubscripti LazyThreadSafetyMode.ExecutionAndPublication); } + internal delegate ValueTask ConfirmationPrompt( + ElicitRequestParams request, + long stateVersion, + CancellationToken cancellationToken); + public McpRequestHandler ListToolsHandler => this.OnListToolsAsync; public McpRequestHandler CallToolHandler => this.OnCallToolsAsync; @@ -643,18 +653,31 @@ private async ValueTask OnCallToolsAsync( // MCP argument order is not semantic, so render positionals in the order the shell binds them. sb.Append(FormatPositionalsForHistory(command.Parameters, positionalValues)); sb.Append(optionText); + var commandLine = sb.ToString(); var server = parameters.Server; - Func>? elicit = - server?.ClientCapabilities?.Elicitation != null ? server.ElicitAsync : null; - return await this.ExecuteToolAsync(command, cmd, sb.ToString(), elicit, cancellationToken); + ConfirmationPrompt? prompt = null; + if (server?.ClientCapabilities?.Elicitation != null && server.IsMrtrSupported) + { + // Multi-round-trip request: the client prompts the user and retries this call with the answer. + // On sessions that predate MRTR, the SDK sends an elicitation request and retries for us. + prompt = (request, stateVersion, _) => throw new InputRequiredException( + inputRequests: new Dictionary + { + [ConfirmationInputKey] = InputRequest.ForElicitation(request), + }, + requestState: ConfirmationRequestState.Create(commandLine, stateVersion)); + } + + return await this.ExecuteToolAsync(command, cmd, commandLine, prompt, cancellationToken, parameters.Params); } internal async Task ExecuteToolAsync( CommandFactory command, CosmosCommand cmd, string commandLine, - Func>? elicit, - CancellationToken cancellationToken) + ConfirmationPrompt? prompt, + CancellationToken cancellationToken, + RequestParams? requestParams = null) { var shell = ShellInterpreter.Instance; long? confirmedVersion = null; @@ -662,8 +685,18 @@ internal async Task ExecuteToolAsync( { var snapshot = await shell.RunSerializedAsync( () => Task.FromResult((Version: shell.StateVersion, Context: DescribeContext(shell.State))), cancellationToken); - var confirmation = await this.ConfirmDestructiveAsync( - elicit, command.CommandName, commandLine, cancellationToken, snapshot.Context); + CallToolResult? confirmation; + if (requestParams?.InputResponses?.TryGetValue(ConfirmationInputKey, out var response) == true) + { + confirmation = this.EvaluateConfirmationResponse( + response, requestParams.RequestState, command.CommandName, commandLine, snapshot.Version); + } + else + { + confirmation = await this.ConfirmDestructiveAsync( + prompt, command.CommandName, commandLine, cancellationToken, snapshot.Context, snapshot.Version); + } + if (confirmation != null) { return confirmation; @@ -681,8 +714,7 @@ internal async Task ExecuteToolAsync( { if (confirmedVersion.HasValue && confirmedVersion.Value != shell.StateVersion) { - return McpResponseFactory.CreateError( - "The shell context changed while awaiting confirmation. Nothing was executed. Retry the command and confirm its current target.", shell.State); + return McpResponseFactory.CreateError(ContextChangedMessage, shell.State); } shell.PrintCommand(commandLine); @@ -713,19 +745,58 @@ private static string DescribeContext(State state) return $"Account: {endpoint}\nCurrent location: {McpResponseFactory.GetCurrentLocation(state) ?? "(none)"}\nExplicit database/container arguments in the command override this location."; } - // Gates a destructive command behind an MCP elicitation confirmation. Returns - // null when the operation is approved and should proceed; otherwise returns the - // CallToolResult to send back (refusal, denial, or a failed confirmation). - // Fails closed: when the client cannot elicit, the command is refused rather - // than executed. + // Validates the client's answer to a confirmation input request. Returns null when the + // operation is approved for the current shell context; otherwise returns the refusal. + internal CallToolResult? EvaluateConfirmationResponse( + InputResponse response, + string? requestState, + string commandName, + string commandLine, + long currentStateVersion) + { + if (!ConfirmationRequestState.TryRead(requestState, commandLine, out var confirmedStateVersion)) + { + this.logger?.LogWarning( + "Rejected a confirmation response for '{Command}' whose request state does not match the command.", + commandName); + return McpResponseFactory.CreateError( + $"The confirmation for '{commandName}' does not match this command. Nothing was executed. Retry the command and confirm it again.", + ShellInterpreter.Instance.State); + } + + if (confirmedStateVersion != currentStateVersion) + { + return McpResponseFactory.CreateError(ContextChangedMessage, ShellInterpreter.Instance.State); + } + + ElicitResult? result; + try + { + result = response.Deserialize(InputResponse.ElicitResultJsonTypeInfo); + } + catch (JsonException) + { + result = null; + } + + return this.EvaluateElicitResult(result, commandName); + } + + // Gates a destructive command behind a user confirmation. Returns null when the + // operation is approved and should proceed; otherwise returns the CallToolResult + // to send back (refusal, denial, or a failed confirmation). A prompt may instead + // throw InputRequiredException, which ends this call and asks the client to retry + // with the user's answer. Fails closed: when the client cannot confirm, the + // command is refused rather than executed. internal async ValueTask ConfirmDestructiveAsync( - Func>? elicit, + ConfirmationPrompt? prompt, string commandName, string commandLine, CancellationToken cancellationToken, - string? context = null) + string? context = null, + long stateVersion = 0) { - if (elicit == null) + if (prompt == null) { this.logger?.LogWarning( "Destructive command '{Command}' requires confirmation, but the MCP client does not support elicitation.", @@ -747,12 +818,16 @@ private static string DescribeContext(State state) ElicitResult result; try { - result = await elicit(request, cancellationToken); + result = await prompt(request, stateVersion, cancellationToken); } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { throw; } + catch (InputRequiredException) + { + throw; + } catch (Exception ex) when (ex is not OperationCanceledException) { this.logger?.LogWarning(ex, "Confirmation prompt for destructive command '{Command}' failed.", commandName); @@ -761,9 +836,14 @@ private static string DescribeContext(State state) ShellInterpreter.Instance.State); } - if (!result.IsAccepted) + return this.EvaluateElicitResult(result, commandName); + } + + private CallToolResult? EvaluateElicitResult(ElicitResult? result, string commandName) + { + if (result?.IsAccepted != true) { - var action = string.IsNullOrEmpty(result.Action) ? "cancel" : result.Action; + var action = string.IsNullOrEmpty(result?.Action) ? "cancel" : result.Action; this.logger?.LogInformation( "User did not approve destructive command '{Command}' (action={Action}).", commandName, diff --git a/docs/mcp.md b/docs/mcp.md index 26766803..93e32c0b 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -73,15 +73,17 @@ Transactional batches invoked through MCP must use the one-shot `batch run` subc ### Destructive Command Confirmation -Destructive commands (`delete`, `rm`, `rmcon`, `rmdb`) are gated behind an explicit user confirmation. When a client invokes one, the server sends an MCP elicitation prompt describing the exact command line before anything runs: +Destructive commands (`delete`, `rm`, `rmcon`, `rmdb`) are gated behind an explicit user confirmation. When a client invokes one, the server asks the client for an elicitation prompt describing the exact command line before anything runs: - **Approved** — the command executes normally. - **Declined or cancelled** — nothing is executed and the tool call returns an error explaining that the user did not approve. - **Client cannot confirm** — if the connected client does not support elicitation, the command is refused (fail-closed) and the response suggests running it manually in the shell. +The prompt is sent as a multi-round-trip request: the tool call returns an input-required result, and the client shows the prompt and retries the call with the answer. For clients on protocol revisions before `2026-07-28`, the server sends a standard `elicitation/create` request on the session and retries the call itself, so those clients see the same prompt as before. The retry carries a server-signed state that ties the answer to the exact command line and shell context. An answer for a different command, or with a missing or altered state, is refused without executing. + This replaces any opt-in write flag: destructive commands are always allowed to be invoked, but always require confirmation. -Confirmation includes the connected account endpoint and current navigation location alongside the command and its explicit target arguments. If the connection or navigation state changes while confirmation is pending, the approved command is refused without executing; retry it to confirm the new context. Even navigating away and back invalidates the pending confirmation. +Confirmation includes the connected account endpoint and current navigation location alongside the command and its explicit target arguments. If the connection or navigation state changes while confirmation is pending, the approved command is refused without executing; retry it to confirm the new context. Even navigating away and back invalidates the pending confirmation. A pending confirmation also expires when the MCP server restarts. Shell and MCP command execution is serialized against the shared interpreter. Confirmation prompts do not hold the execution lock, so the shell remains usable while waiting. Clients still share a connection and navigation context: pass explicit `database` and `container` arguments for independent operations rather than relying on an earlier `cd` call. From 0f8ca4aa0b437d4606dc538d4e24bf98cf78935e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 1 Oct 2026 13:39:30 +0000 Subject: [PATCH 04/12] Serve 2026-07-28 clients statelessly and stream location updates via subscriptions/listen Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- CosmosDBShell.Tests/McpConfirmationTests.cs | 8 +- .../McpLocationSubscriptionTests.cs | 107 ++++++++++++++- .../LocationResourceSubscriptions.cs | 124 +++++++++++++++++- .../Azure.Data.Cosmos.Shell.Mcp/McpServer.cs | 9 +- .../ToolOperations.cs | 6 +- docs/mcp.md | 11 +- 6 files changed, 247 insertions(+), 18 deletions(-) diff --git a/CosmosDBShell.Tests/McpConfirmationTests.cs b/CosmosDBShell.Tests/McpConfirmationTests.cs index ec4bea72..238e847b 100644 --- a/CosmosDBShell.Tests/McpConfirmationTests.cs +++ b/CosmosDBShell.Tests/McpConfirmationTests.cs @@ -16,8 +16,10 @@ namespace CosmosShell.Tests; [Collection(CosmosShell.Tests.Shell.ThemeStateTestCollection.Name)] public class McpConfirmationTests { - [Fact] - public async Task DestructiveCommand_OverHttp_AsksClientAndHonorsDecline() + [Theory] + [InlineData(null, "2026-07-28")] // Stateless request; confirmation uses native multi-round-trip requests. + [InlineData("2025-11-25", "2025-11-25")] // Initialize handshake; confirmation is sent over the session. + public async Task DestructiveCommand_OverHttp_AsksClientAndHonorsDecline(string? requestedVersion, string negotiatedVersion) { using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); timeout.CancelAfter(TimeSpan.FromSeconds(10)); @@ -35,6 +37,7 @@ public async Task DestructiveCommand_OverHttp_AsksClientAndHonorsDecline() transport, new McpClientOptions { + ProtocolVersion = requestedVersion, Handlers = new McpClientHandlers { ElicitationHandler = (request, _) => @@ -45,6 +48,7 @@ public async Task DestructiveCommand_OverHttp_AsksClientAndHonorsDecline() }, }, cancellationToken: timeout.Token); + Assert.Equal(negotiatedVersion, client.NegotiatedProtocolVersion); var result = await client.CallToolAsync( "rmdb", diff --git a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs index cfcb055e..a93bfde5 100644 --- a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs +++ b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs @@ -15,6 +15,7 @@ namespace CosmosShell.Tests; using Microsoft.Extensions.Hosting; using ModelContextProtocol; using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; [Collection(CosmosShell.Tests.Shell.ThemeStateTestCollection.Name)] public class McpLocationSubscriptionTests @@ -105,25 +106,123 @@ public async Task EndedSession_RemovesLocationSubscription() } [Fact] - public void HttpTransport_UsesStatefulSessionsWithBoundedIdleTimeout() + public async Task ListeningClient_ReceivesLocationChangeOnListenStream() + { + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + timeout.CancelAfter(TimeSpan.FromSeconds(10)); + using var host = McpServer.CreateHost(new Program.CosmosShellOptions { McpPort = 0 }); + await host.StartAsync(timeout.Token); + try + { + var subscriptions = host.Services.GetRequiredService(); + await using var client = await ConnectAsync(host, timeout.Token, protocolVersion: null); + Assert.Equal("2026-07-28", client.NegotiatedProtocolVersion); + + var acknowledged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var updated = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await using var acknowledgedHandler = client.RegisterNotificationHandler( + NotificationMethods.SubscriptionsAcknowledgedNotification, + (notification, _) => + { + acknowledged.TrySetResult(notification); + return ValueTask.CompletedTask; + }); + await using var updatedHandler = client.RegisterNotificationHandler( + NotificationMethods.ResourceUpdatedNotification, + (notification, _) => + { + updated.TrySetResult(notification); + return ValueTask.CompletedTask; + }); + + using var listenCancellation = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token); + var listen = client.SendRequestAsync( + new JsonRpcRequest + { + Id = new RequestId("location-listen"), + Method = RequestMethods.SubscriptionsListen, + Params = JsonSerializer.SerializeToNode(new SubscriptionsListenRequestParams + { + Notifications = new SubscriptionsListenNotifications + { + ResourceSubscriptions = [ResourceOperations.CurrentLocationUri, "cosmos://docs/scripting"], + ToolsListChanged = true, + }, + }), + }, + listenCancellation.Token); + + // Only the current-location resource is honored. + var acknowledgement = await acknowledged.Task.WaitAsync(timeout.Token); + var granted = acknowledgement.Params!["notifications"]!.AsObject(); + Assert.Equal(ResourceOperations.CurrentLocationUri, Assert.Single(granted["resourceSubscriptions"]!.AsArray())!.GetValue()); + Assert.False(granted.ContainsKey("toolsListChanged")); + Assert.Equal("location-listen", acknowledgement.Params["_meta"]![MetaKeys.SubscriptionId]!.GetValue()); + while (subscriptions.ListenerCount != 1) + { + await Task.Delay(20, timeout.Token); + } + + var originalState = ShellInterpreter.Instance.State; + using var cosmosClient = new CosmosClient( + "https://localhost:8081", + Convert.ToBase64String(new byte[64]), + new CosmosClientOptions { ConnectionMode = ConnectionMode.Gateway }); + try + { + ShellInterpreter.Instance.State = new DatabaseState("McpListenTest", cosmosClient); + var notification = await updated.Task.WaitAsync(timeout.Token); + Assert.Equal(ResourceOperations.CurrentLocationUri, notification.Params!["uri"]!.GetValue()); + Assert.Equal("location-listen", notification.Params["_meta"]![MetaKeys.SubscriptionId]!.GetValue()); + } + finally + { + ShellInterpreter.Instance.State = originalState; + } + + // Cancelling the listen request ends the stream and releases the listener. + await listenCancellation.CancelAsync(); + try + { + await listen; + Assert.Fail("The listen request should end only when cancelled."); + } + catch (OperationCanceledException) + { + } + + while (subscriptions.ListenerCount != 0) + { + await Task.Delay(20, timeout.Token); + } + } + finally + { + await host.StopAsync(TestContext.Current.CancellationToken); + } + } + + [Fact] + public void HttpTransport_UsesSessionsOnlyForInitializeClientsWithBoundedIdleTimeout() { var options = new ModelContextProtocol.AspNetCore.HttpServerTransportOptions(); McpServer.ConfigureHttpTransport(options); - Assert.Equal(ModelContextProtocol.AspNetCore.HttpServerSessionMode.Stateful, options.SessionMode); + Assert.Equal(ModelContextProtocol.AspNetCore.HttpServerSessionMode.StatefulForInitializeClients, options.SessionMode); #pragma warning disable MCP9006, MCPEXP002 Assert.Equal(McpServer.SessionIdleTimeout, options.IdleTimeout); Assert.NotNull(options.RunSessionHandler); #pragma warning restore MCP9006, MCPEXP002 } - private static async Task ConnectAsync(IHost host, CancellationToken cancellationToken) + // resources/subscribe and session lifetimes exist only for clients that use the initialize handshake. + private static async Task ConnectAsync(IHost host, CancellationToken cancellationToken, string? protocolVersion = "2025-11-25") { var address = host.Services.GetRequiredService().Features.Get()!.Addresses.Single(); var transport = new HttpClientTransport(new HttpClientTransportOptions { Endpoint = new Uri(address.TrimEnd('/') + "/"), }); - return await McpClient.CreateAsync(transport, cancellationToken: cancellationToken); + return await McpClient.CreateAsync(transport, new McpClientOptions { ProtocolVersion = protocolVersion }, cancellationToken: cancellationToken); } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs index b892c0d6..1a8b1078 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs @@ -4,6 +4,8 @@ namespace Azure.Data.Cosmos.Shell.Mcp; +using System.Text.Json; +using System.Text.Json.Nodes; using System.Threading.Channels; using Azure.Data.Cosmos.Shell.Core; using Microsoft.Extensions.Hosting; @@ -22,6 +24,12 @@ internal sealed class LocationResourceSubscriptions : BackgroundService private readonly HashSet subscribedSessionIds = new(StringComparer.Ordinal); + // Open subscriptions/listen streams (2026-07-28). Each lives as long as its listen request. + private readonly HashSet listenStreams = []; + + // Listen requests are held-open POSTs, which the transport does not end on shutdown. + private readonly CancellationTokenSource stopping = new(); + // Notifications carry only the URI, so pending changes are coalesced into one. private readonly Channel changes = Channel.CreateBounded( new BoundedChannelOptions(1) { FullMode = BoundedChannelFullMode.DropWrite, SingleReader = true }); @@ -45,12 +53,24 @@ internal int SubscriberCount } } + internal int ListenerCount + { + get + { + lock (this.sync) + { + return this.listenStreams.Count; + } + } + } + /// /// Runs an MCP session and keeps it available for notifications until the session ends. /// public async Task RunSessionAsync(ModelContextProtocol.Server.McpServer server, CancellationToken cancellationToken) { - var sessionId = server.SessionId; + // Stateless 2026-07-28 requests have no session to register. + var sessionId = string.IsNullOrEmpty(server.SessionId) ? null : server.SessionId; if (sessionId is not null) { lock (this.sync) @@ -82,7 +102,7 @@ public async Task RunSessionAsync(ModelContextProtocol.Server.McpServer server, public void Subscribe(string? sessionId, string uri) { ValidateUri(uri); - if (sessionId is null) + if (string.IsNullOrEmpty(sessionId)) { throw new McpProtocolException( "Resource subscriptions require a stateful MCP session.", @@ -101,7 +121,7 @@ public void Subscribe(string? sessionId, string uri) public void Unsubscribe(string? sessionId, string uri) { ValidateUri(uri); - if (sessionId is null) + if (string.IsNullOrEmpty(sessionId)) { return; } @@ -112,17 +132,78 @@ public void Unsubscribe(string? sessionId, string uri) } } + /// + /// Handles a subscriptions/listen request: acknowledges the honored filters and streams + /// current-location updates over the request until it is cancelled. + /// + public async ValueTask ListenAsync(RequestContext request, CancellationToken cancellationToken) + { + // Only the current-location resource supports updates; tools and resources lists are static. + var honorsLocation = request.Params?.Notifications?.ResourceSubscriptions?.Contains(ResourceOperations.CurrentLocationUri, StringComparer.Ordinal) == true; + var stream = new ListenStream(request.Server, request.JsonRpcRequest.Id); + + var acknowledgement = JsonSerializer.SerializeToNode( + new SubscriptionsAcknowledgedNotificationParams + { + Notifications = new SubscriptionsListenNotifications + { + ResourceSubscriptions = honorsLocation ? [ResourceOperations.CurrentLocationUri] : null, + }, + }, + McpJsonUtilities.DefaultOptions)!.AsObject(); + acknowledgement["_meta"] = stream.CreateMeta(); + await request.Server.SendMessageAsync( + new JsonRpcNotification { Method = NotificationMethods.SubscriptionsAcknowledgedNotification, Params = acknowledgement }, + cancellationToken); + + if (!honorsLocation) + { + return new EmptyResult(); + } + + using var listenCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, this.stopping.Token); + lock (this.sync) + { + this.listenStreams.Add(stream); + } + + try + { + await Task.Delay(Timeout.Infinite, listenCancellation.Token); + } + catch (OperationCanceledException) when (listenCancellation.IsCancellationRequested) + { + } + finally + { + lock (this.sync) + { + this.listenStreams.Remove(stream); + } + } + + return new EmptyResult(); + } + + public override async Task StopAsync(CancellationToken cancellationToken) + { + await this.stopping.CancelAsync(); + await base.StopAsync(cancellationToken); + } + protected override async Task ExecuteAsync(CancellationToken stoppingToken) { await foreach (var change in this.changes.Reader.ReadAllAsync(stoppingToken)) { ModelContextProtocol.Server.McpServer[] servers; + ListenStream[] streams; lock (this.sync) { servers = this.subscribedSessionIds .Select(sessionId => this.sessions.TryGetValue(sessionId, out var server) ? server : null) .OfType() .ToArray(); + streams = [.. this.listenStreams]; } foreach (var server in servers) @@ -144,12 +225,33 @@ await server.SendNotificationAsync( this.logger.LogWarning(ex, "Could not notify an MCP client about the shell location change."); } } + + foreach (var stream in streams) + { + try + { + await stream.Server.SendNotificationAsync( + NotificationMethods.ResourceUpdatedNotification, + new ResourceUpdatedNotificationParams { Uri = ResourceOperations.CurrentLocationUri, Meta = stream.CreateMeta() }, + cancellationToken: stoppingToken); + } + catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) + { + return; + } + catch (Exception ex) when (!stoppingToken.IsCancellationRequested) + { + // The stream is removed when its listen request ends; a failed send only affects this notification. + this.logger.LogWarning(ex, "Could not notify an MCP client about the shell location change."); + } + } } } public override void Dispose() { ShellInterpreter.Instance.LocationChanged -= this.OnLocationChanged; + this.stopping.Dispose(); base.Dispose(); } @@ -167,4 +269,20 @@ private void OnLocationChanged() { this.changes.Writer.TryWrite(true); } + + private sealed class ListenStream(ModelContextProtocol.Server.McpServer server, RequestId id) + { + public ModelContextProtocol.Server.McpServer Server { get; } = server; + + // Notifications on a listen stream are tagged with the listen request ID so clients can demultiplex them. + public JsonObject CreateMeta() => new() + { + [MetaKeys.SubscriptionId] = id.Id switch + { + string stringId => JsonValue.Create(stringId), + long longId => JsonValue.Create(longId), + _ => null, + }, + }; + } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs index 18a9282b..d07021d1 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/McpServer.cs @@ -56,7 +56,7 @@ private static void ConfigureMcpServer(IServiceCollection services) services.AddSingleton(); services.AddHostedService(services => services.GetRequiredService()); services.AddOptions() - .Configure((mcpServerOptions, toolOperations) => + .Configure((mcpServerOptions, toolOperations, locationSubscriptions) => { var entryAssembly = Assembly.GetEntryAssembly(); var assemblyName = entryAssembly?.GetName(); @@ -80,6 +80,7 @@ private static void ConfigureMcpServer(IServiceCollection services) ListToolsHandler = toolOperations.ListToolsHandler, SubscribeToResourcesHandler = toolOperations.SubscribeToResourcesHandler, UnsubscribeFromResourcesHandler = toolOperations.UnsubscribeFromResourcesHandler, + SubscriptionsListenHandler = locationSubscriptions.ListenAsync, }; mcpServerOptions.ServerInstructions = LoadServerInstructions(); @@ -88,14 +89,14 @@ private static void ConfigureMcpServer(IServiceCollection services) var mcpServerBuilder = services.AddMcpServer(); mcpServerBuilder.WithResources(); - // Destructive-command confirmation uses elicitation, a server-to-client request that - // requires a session. SDK 2.x defaults to stateless, so keep sessions enabled. mcpServerBuilder.WithHttpTransport(ConfigureHttpTransport); } internal static void ConfigureHttpTransport(HttpServerTransportOptions options) { - options.SessionMode = HttpServerSessionMode.Stateful; + // 2026-07-28 clients are served statelessly (confirmation via MRTR, updates via subscriptions/listen). + // Clients that use the initialize handshake still get a session for elicitation and resources/subscribe. + options.SessionMode = HttpServerSessionMode.StatefulForInitializeClients; // Sessions with an open GET stream never go idle. Once a client disconnects without DELETE, // the session is disposed after this timeout, which also ends its location subscription. diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs index dd2c9cbd..a2f7bdb3 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs @@ -656,7 +656,11 @@ private async ValueTask OnCallToolsAsync( var commandLine = sb.ToString(); var server = parameters.Server; ConfirmationPrompt? prompt = null; - if (server?.ClientCapabilities?.Elicitation != null && server.IsMrtrSupported) + + // Stateless 2026-07-28 requests declare capabilities per request, which the SDK does not + // surface through ClientCapabilities. + var clientCapabilities = server?.ClientCapabilities ?? parameters.JsonRpcRequest?.Context?.ClientCapabilities; + if (clientCapabilities?.Elicitation != null && server!.IsMrtrSupported) { // Multi-round-trip request: the client prompts the user and retries this call with the answer. // On sessions that predate MRTR, the SDK sends an elicitation request and retries for us. diff --git a/docs/mcp.md b/docs/mcp.md index 93e32c0b..d77855cb 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -79,7 +79,7 @@ Destructive commands (`delete`, `rm`, `rmcon`, `rmdb`) are gated behind an expli - **Declined or cancelled** — nothing is executed and the tool call returns an error explaining that the user did not approve. - **Client cannot confirm** — if the connected client does not support elicitation, the command is refused (fail-closed) and the response suggests running it manually in the shell. -The prompt is sent as a multi-round-trip request: the tool call returns an input-required result, and the client shows the prompt and retries the call with the answer. For clients on protocol revisions before `2026-07-28`, the server sends a standard `elicitation/create` request on the session and retries the call itself, so those clients see the same prompt as before. The retry carries a server-signed state that ties the answer to the exact command line and shell context. An answer for a different command, or with a missing or altered state, is refused without executing. +The prompt is sent as a multi-round-trip request: the tool call returns an input-required result, and the client shows the prompt and retries the call with the answer. For clients that use the `initialize` handshake, the server sends a standard `elicitation/create` request on the session and retries the call itself, so those clients see the same prompt as before. The retry carries a server-signed state that ties the answer to the exact command line and shell context. An answer for a different command, or with a missing or altered state, is refused without executing. This replaces any opt-in write flag: destructive commands are always allowed to be invoked, but always require confirmation. @@ -97,11 +97,14 @@ For deterministic ARM routing in multi-subscription environments, start the shel ### Shell Location Updates -Clients can read the `cosmos://shell/current-location` MCP resource. Its JSON content has a `currentLocation` field (`null` when disconnected, `/` at the account root, or `/database[/container]`) and a separate `currentAccountEndpoint` field (the connected Cosmos DB account URL, or `null` when disconnected). For example: `{"currentLocation":"/myDb/myContainer","currentAccountEndpoint":"https://myaccount.documents.azure.com/"}`. Clients that support resource subscriptions can subscribe to this URI with `resources/subscribe` and receive `notifications/resources/updated` when the shared shell location or connection changes, including changes made interactively. On notification, read the resource again for the new values; the notification itself contains only the URI. Rapid consecutive changes may be coalesced into a single notification. Unsubscribe with `resources/unsubscribe` when no longer needed. +Clients can read the `cosmos://shell/current-location` MCP resource. Its JSON content has a `currentLocation` field (`null` when disconnected, `/` at the account root, or `/database[/container]`) and a separate `currentAccountEndpoint` field (the connected Cosmos DB account URL, or `null` when disconnected). For example: `{"currentLocation":"/myDb/myContainer","currentAccountEndpoint":"https://myaccount.documents.azure.com/"}`. Clients that support resource subscriptions receive `notifications/resources/updated` when the shared shell location or connection changes, including changes made interactively. On notification, read the resource again for the new values; the notification itself contains only the URI. Rapid consecutive changes may be coalesced into a single notification. -Only `cosmos://shell/current-location` supports subscriptions; subscribing to any other URI, including the documentation resources, returns an invalid-params error. +How a client subscribes depends on its protocol revision: -This server uses the subscription protocol supported by its MCP SDK; clients must support subscriptions and server-to-client notifications over the HTTP connection. Notifications are delivered on the session's GET stream. A subscription lasts as long as its MCP session: it ends when the client deletes the session or when the session has had no open request (including the GET stream) for 10 minutes. After that the server returns 404 for the session and the client must start a new session and subscribe again. A notification does not guarantee that a client refreshes the model's context. Every tool response also includes `currentLocation`, and explicit `database` / `container` arguments remain the reliable way to target independent operations. +- **`2026-07-28` clients** send `subscriptions/listen` with `resourceSubscriptions` containing `cosmos://shell/current-location`. The server first sends `notifications/subscriptions/acknowledged` listing the subscriptions it honors, then streams updates on the same response. Every notification on the stream carries the listen request ID in `_meta["io.modelcontextprotocol/subscriptionId"]`. Only `cosmos://shell/current-location` is honored; other resource URIs and list-changed filters are left out of the acknowledgement. If nothing is honored, the listen request completes right after the acknowledgement. Otherwise the subscription lasts until the client cancels the request or closes the connection. +- **Clients that use the `initialize` handshake** subscribe with `resources/subscribe` and unsubscribe with `resources/unsubscribe`. Subscribing to any URI other than `cosmos://shell/current-location`, including the documentation resources, returns an invalid-params error. Notifications are delivered on the session's GET stream. A subscription lasts as long as its MCP session: it ends when the client deletes the session or when the session has had no open request (including the GET stream) for 10 minutes. After that the server returns 404 for the session and the client must start a new session and subscribe again. + +The HTTP server serves `2026-07-28` requests without a session and gives a session only to clients that use the `initialize` handshake. A notification does not guarantee that a client refreshes the model's context. Every tool response also includes `currentLocation`, and explicit `database` / `container` arguments remain the reliable way to target independent operations. ### Data Exposure From f8e6c260376dbd3f619be97bbf224683289d3994 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 1 Oct 2026 13:47:26 +0000 Subject: [PATCH 05/12] Address code quality findings in MCP confirmation and listen handling Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- .../McpLocationSubscriptionTests.cs | 1 + .../LocationResourceSubscriptions.cs | 6 ++---- .../Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs | 14 ++++---------- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs index a93bfde5..118eb1a2 100644 --- a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs +++ b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs @@ -189,6 +189,7 @@ public async Task ListeningClient_ReceivesLocationChangeOnListenStream() } catch (OperationCanceledException) { + Assert.True(listenCancellation.IsCancellationRequested); } while (subscriptions.ListenerCount != 0) diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs index 1a8b1078..397c3f40 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs @@ -169,10 +169,8 @@ await request.Server.SendMessageAsync( try { - await Task.Delay(Timeout.Infinite, listenCancellation.Token); - } - catch (OperationCanceledException) when (listenCancellation.IsCancellationRequested) - { + // Cancellation is the normal end of a listen stream. + await Task.Delay(Timeout.Infinite, listenCancellation.Token).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing); } finally { diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs index a2f7bdb3..829dfea9 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs @@ -689,17 +689,11 @@ internal async Task ExecuteToolAsync( { var snapshot = await shell.RunSerializedAsync( () => Task.FromResult((Version: shell.StateVersion, Context: DescribeContext(shell.State))), cancellationToken); - CallToolResult? confirmation; - if (requestParams?.InputResponses?.TryGetValue(ConfirmationInputKey, out var response) == true) - { - confirmation = this.EvaluateConfirmationResponse( - response, requestParams.RequestState, command.CommandName, commandLine, snapshot.Version); - } - else - { - confirmation = await this.ConfirmDestructiveAsync( + var confirmation = requestParams?.InputResponses?.TryGetValue(ConfirmationInputKey, out var response) == true + ? this.EvaluateConfirmationResponse( + response, requestParams.RequestState, command.CommandName, commandLine, snapshot.Version) + : await this.ConfirmDestructiveAsync( prompt, command.CommandName, commandLine, cancellationToken, snapshot.Context, snapshot.Version); - } if (confirmation != null) { From 4e5f22091a718f3be34d2d852d656a4ac1c8bfd5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:08:30 +0000 Subject: [PATCH 06/12] Make confirmation state single-use, canonicalize option order, and close the listen activation race Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- .../ToolOperationsCallToolTests.cs | 63 +++++++++++++++++++ .../ConfirmationRequestState.cs | 47 +++++++++++--- .../LocationResourceSubscriptions.cs | 55 +++++++++++++--- .../ToolOperations.cs | 12 +++- docs/mcp.md | 2 +- 5 files changed, 159 insertions(+), 20 deletions(-) diff --git a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs index bac15612..d03518f8 100644 --- a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs +++ b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs @@ -170,6 +170,33 @@ public async Task CallTool_RmWithPartitionKeyAndETag_ConfirmationShowsTargetCond } } + [Fact] + public async Task CallTool_RmArgumentOrder_DoesNotChangeConfirmedCommandLine() + { + var tool = CreateToolOperations(); + var forward = new Dictionary + { + ["pattern"] = Json("\"order-123\""), + ["key"] = Json("\"id\""), + ["partition-key"] = Json("\"customer-42\""), + ["etag"] = Json("\"etag-1\""), + }; + var reversed = forward.Reverse().ToDictionary(); + + var errors = new List(); + foreach (var arguments in new[] { forward, reversed }) + { + var (_, root, document) = ReadResult(await tool.CallToolHandler(CallContext("rm", arguments), CancellationToken.None)); + using (document) + { + errors.Add(root.GetProperty("error").GetString()); + } + } + + Assert.Contains("rm \"order-123\" --key \"id\" --partition-key \"customer-42\" --etag \"etag-1\"", errors[0]); + Assert.Equal(errors[0], errors[1]); + } + [Theory] [InlineData("etag", "command-rm-error-etag_empty")] [InlineData("partition-key", "command-rm-error-partition_key_missing_value")] @@ -685,6 +712,24 @@ public void ConfirmationRequestState_RoundTripsOnlyForTheSameCommand() Assert.False(ConfirmationRequestState.TryRead(state, "rmdb otherdb", out _)); } + [Fact] + public void ConfirmationRequestState_CanBeReadOnlyOnce() + { + var state = ConfirmationRequestState.Create("rmdb mydb", 42); + + Assert.True(ConfirmationRequestState.TryRead(state, "rmdb mydb", out _)); + Assert.False(ConfirmationRequestState.TryRead(state, "rmdb mydb", out _)); + } + + [Fact] + public void ConfirmationRequestState_ExpiresAfterLifetime() + { + var state = ConfirmationRequestState.Create("rmdb mydb", 42); + + Assert.False(ConfirmationRequestState.TryRead( + state, "rmdb mydb", DateTimeOffset.UtcNow + ConfirmationRequestState.Lifetime + TimeSpan.FromSeconds(1), out _)); + } + [Theory] [InlineData(null)] [InlineData("")] @@ -742,6 +787,24 @@ public async Task ExecuteTool_AcceptedConfirmationResponse_ExecutesCommand() Assert.True(command.Executed); } + [Fact] + public async Task ExecuteTool_ReplayedConfirmationResponse_DoesNotExecuteAgain() + { + var shell = ShellInterpreter.Instance; + var retry = ConfirmationRetry("accept", ConfirmationRequestState.Create("rm test-*", shell.StateVersion)); + var first = new TrackingCommand(); + var replayed = new TrackingCommand(); + + await CreateToolOperations().ExecuteToolAsync( + shell.App.Commands["rm"], first, "rm test-*", null, TestContext.Current.CancellationToken, retry); + var result = await CreateToolOperations().ExecuteToolAsync( + shell.App.Commands["rm"], replayed, "rm test-*", null, TestContext.Current.CancellationToken, retry); + + Assert.True(first.Executed); + Assert.True(result.IsError); + Assert.False(replayed.Executed); + } + [Theory] [InlineData("decline")] [InlineData("cancel")] diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs index 8f2b282b..4e7c460d 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs @@ -4,6 +4,7 @@ namespace Azure.Data.Cosmos.Shell.Mcp; +using System.Collections.Concurrent; using System.Globalization; using System.Security.Cryptography; using System.Text; @@ -12,20 +13,43 @@ namespace Azure.Data.Cosmos.Shell.Mcp; /// /// Creates and validates the MRTR requestState for destructive-command confirmation. /// The state binds an approval to the exact command line and shell state version it was -/// requested for, and is signed so a client cannot fabricate or retarget it. +/// requested for, and is signed so a client cannot fabricate or retarget it. Each state +/// carries a nonce that can be read once and expires after , so a +/// captured answer cannot be replayed. /// internal static class ConfirmationRequestState { + internal static readonly TimeSpan Lifetime = TimeSpan.FromMinutes(10); + // Per-process key: confirmations do not survive a server restart. private static readonly byte[] Key = RandomNumberGenerator.GetBytes(32); + // Outstanding nonces and their expiry. A nonce is removed when it is read or has expired. + private static readonly ConcurrentDictionary PendingNonces = new(StringComparer.Ordinal); + public static string Create(string commandLine, long stateVersion) { - var payload = Encoding.UTF8.GetBytes(stateVersion.ToString(CultureInfo.InvariantCulture) + "\n" + commandLine); + var now = DateTimeOffset.UtcNow; + foreach (var entry in PendingNonces) + { + if (entry.Value <= now) + { + PendingNonces.TryRemove(entry); + } + } + + var nonce = WebEncoders.Base64UrlEncode(RandomNumberGenerator.GetBytes(16)); + PendingNonces[nonce] = now + Lifetime; + var payload = Encoding.UTF8.GetBytes(nonce + "\n" + stateVersion.ToString(CultureInfo.InvariantCulture) + "\n" + commandLine); return WebEncoders.Base64UrlEncode(payload) + "." + WebEncoders.Base64UrlEncode(Sign(payload)); } public static bool TryRead(string? requestState, string commandLine, out long stateVersion) + { + return TryRead(requestState, commandLine, DateTimeOffset.UtcNow, out stateVersion); + } + + internal static bool TryRead(string? requestState, string commandLine, DateTimeOffset now, out long stateVersion) { stateVersion = 0; if (string.IsNullOrEmpty(requestState)) @@ -56,11 +80,20 @@ public static bool TryRead(string? requestState, string commandLine, out long st return false; } - var text = Encoding.UTF8.GetString(payload); - var newline = text.IndexOf('\n'); - return newline > 0 - && string.Equals(text[(newline + 1)..], commandLine, StringComparison.Ordinal) - && long.TryParse(text[..newline], NumberStyles.None, CultureInfo.InvariantCulture, out stateVersion); + var parts = Encoding.UTF8.GetString(payload).Split('\n', 3); + if (parts.Length != 3) + { + return false; + } + + // Any signed state is consumed on first use, whether or not it matches this command. + if (!PendingNonces.TryRemove(parts[0], out var expiry) || expiry <= now) + { + return false; + } + + return string.Equals(parts[2], commandLine, StringComparison.Ordinal) + && long.TryParse(parts[1], NumberStyles.None, CultureInfo.InvariantCulture, out stateVersion); } private static byte[] Sign(byte[] payload) => HMACSHA256.HashData(Key, payload); diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs index 397c3f40..c6f820a8 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs @@ -152,16 +152,16 @@ public async ValueTask ListenAsync(RequestContext this.sessions.TryGetValue(sessionId, out var server) ? server : null) .OfType() .ToArray(); - streams = [.. this.listenStreams]; + foreach (var stream in this.listenStreams.Where(stream => !stream.Acknowledged)) + { + stream.PendingUpdate = true; + } + + streams = [.. this.listenStreams.Where(stream => stream.Acknowledged)]; } foreach (var server in servers) @@ -228,10 +248,7 @@ await server.SendNotificationAsync( { try { - await stream.Server.SendNotificationAsync( - NotificationMethods.ResourceUpdatedNotification, - new ResourceUpdatedNotificationParams { Uri = ResourceOperations.CurrentLocationUri, Meta = stream.CreateMeta() }, - cancellationToken: stoppingToken); + await stream.SendUpdateAsync(stoppingToken); } catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) { @@ -263,6 +280,13 @@ internal static void ValidateUri(string uri) } } + private static Task SendAcknowledgementAsync(ModelContextProtocol.Server.McpServer server, JsonObject acknowledgement, CancellationToken cancellationToken) + { + return server.SendMessageAsync( + new JsonRpcNotification { Method = NotificationMethods.SubscriptionsAcknowledgedNotification, Params = acknowledgement }, + cancellationToken); + } + private void OnLocationChanged() { this.changes.Writer.TryWrite(true); @@ -272,6 +296,19 @@ private sealed class ListenStream(ModelContextProtocol.Server.McpServer server, { public ModelContextProtocol.Server.McpServer Server { get; } = server; + // Guarded by the owning LocationResourceSubscriptions' sync lock. + public bool Acknowledged { get; set; } + + public bool PendingUpdate { get; set; } + + public Task SendUpdateAsync(CancellationToken cancellationToken) + { + return this.Server.SendNotificationAsync( + NotificationMethods.ResourceUpdatedNotification, + new ResourceUpdatedNotificationParams { Uri = ResourceOperations.CurrentLocationUri, Meta = this.CreateMeta() }, + cancellationToken: cancellationToken); + } + // Notifications on a listen stream are tagged with the listen request ID so clients can demultiplex them. public JsonObject CreateMeta() => new() { diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs index 829dfea9..2f0f47df 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs @@ -540,7 +540,7 @@ private async ValueTask OnCallToolsAsync( ConfigurePaging(cmd); var suppliedParameters = new HashSet(StringComparer.OrdinalIgnoreCase); var positionalValues = new Dictionary(); - var optionText = new StringBuilder(); + var optionTexts = new List<(int Order, string Text)>(); if (parameters.Params.Arguments != null) { @@ -561,7 +561,7 @@ private async ValueTask OnCallToolsAsync( memberKind: "option", memberDisplay: $"--{option.Name[0]}", commandName: command.CommandName, - appendToHistory: value => optionText.Append(FormatOptionForHistory(option, value))); + appendToHistory: value => optionTexts.Add((command.Options.IndexOf(option), FormatOptionForHistory(option, value)))); if (bindError != null) { return bindError; @@ -652,7 +652,13 @@ private async ValueTask OnCallToolsAsync( // MCP argument order is not semantic, so render positionals in the order the shell binds them. sb.Append(FormatPositionalsForHistory(command.Parameters, positionalValues)); - sb.Append(optionText); + + // Likewise render options in declaration order, so the confirmed command line does not depend on argument order. + foreach (var (_, text) in optionTexts.OrderBy(entry => entry.Order)) + { + sb.Append(text); + } + var commandLine = sb.ToString(); var server = parameters.Server; ConfirmationPrompt? prompt = null; diff --git a/docs/mcp.md b/docs/mcp.md index d77855cb..0f81f578 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -79,7 +79,7 @@ Destructive commands (`delete`, `rm`, `rmcon`, `rmdb`) are gated behind an expli - **Declined or cancelled** — nothing is executed and the tool call returns an error explaining that the user did not approve. - **Client cannot confirm** — if the connected client does not support elicitation, the command is refused (fail-closed) and the response suggests running it manually in the shell. -The prompt is sent as a multi-round-trip request: the tool call returns an input-required result, and the client shows the prompt and retries the call with the answer. For clients that use the `initialize` handshake, the server sends a standard `elicitation/create` request on the session and retries the call itself, so those clients see the same prompt as before. The retry carries a server-signed state that ties the answer to the exact command line and shell context. An answer for a different command, or with a missing or altered state, is refused without executing. +The prompt is sent as a multi-round-trip request: the tool call returns an input-required result, and the client shows the prompt and retries the call with the answer. For clients that use the `initialize` handshake, the server sends a standard `elicitation/create` request on the session and retries the call itself, so those clients see the same prompt as before. The retry carries a server-signed state that ties the answer to the exact command line and shell context. Each state can be answered once and expires after 10 minutes. An answer for a different command, a reused or expired state, or a missing or altered state is refused without executing. Argument order does not matter: the command line is built with positionals in shell order and options in declaration order. This replaces any opt-in write flag: destructive commands are always allowed to be invoked, but always require confirmation. From 5d29d140212f71b15f4b78400b82b32f1ef64154 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:14:54 +0000 Subject: [PATCH 07/12] Filter expired confirmation nonces with Where Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- .../ConfirmationRequestState.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs index 4e7c460d..f9102649 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs @@ -30,12 +30,9 @@ internal static class ConfirmationRequestState public static string Create(string commandLine, long stateVersion) { var now = DateTimeOffset.UtcNow; - foreach (var entry in PendingNonces) + foreach (var expired in PendingNonces.Where(entry => entry.Value <= now)) { - if (entry.Value <= now) - { - PendingNonces.TryRemove(entry); - } + PendingNonces.TryRemove(expired); } var nonce = WebEncoders.Base64UrlEncode(RandomNumberGenerator.GetBytes(16)); From 17579dce18361e396da75f18c858cbc32f3ff551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 1 Oct 2026 16:59:20 +0200 Subject: [PATCH 08/12] End listen streams on ApplicationStopping and make echo history test order-independent The web server stops before LocationResourceSubscriptions and waits for open requests until the host shutdown timeout, so cancelling held-open listen POSTs in StopAsync came too late and shutdown took 30 s. Cancel them when the application starts stopping instead. CallTool_EchoCommand_ReturnsSuccessResult relied on its history entry being new; history drops duplicates and another test records the same echo line, so the new tests' execution order made it fail. Use a unique message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../McpLocationSubscriptionTests.cs | 35 +++++++++++++++++++ .../ToolOperationsCallToolTests.cs | 9 +++-- .../LocationResourceSubscriptions.cs | 9 +++-- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs index 118eb1a2..191b7ca8 100644 --- a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs +++ b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs @@ -203,6 +203,41 @@ public async Task ListeningClient_ReceivesLocationChangeOnListenStream() } } + [Fact] + public async Task HostStop_EndsOpenListenStreamWithoutWaitingForShutdownTimeout() + { + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + timeout.CancelAfter(TimeSpan.FromSeconds(10)); + using var host = McpServer.CreateHost(new Program.CosmosShellOptions { McpPort = 0 }); + await host.StartAsync(timeout.Token); + var subscriptions = host.Services.GetRequiredService(); + await using var client = await ConnectAsync(host, timeout.Token, protocolVersion: null); + + _ = client.SendRequestAsync( + new JsonRpcRequest + { + Id = new RequestId("shutdown-listen"), + Method = RequestMethods.SubscriptionsListen, + Params = JsonSerializer.SerializeToNode(new SubscriptionsListenRequestParams + { + Notifications = new SubscriptionsListenNotifications + { + ResourceSubscriptions = [ResourceOperations.CurrentLocationUri], + }, + }), + }, + timeout.Token); + while (subscriptions.ListenerCount != 1) + { + await Task.Delay(20, timeout.Token); + } + + // The web server stops first and waits for open requests until the host shutdown timeout (30 s by default). + await host.StopAsync(CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + + Assert.Equal(0, subscriptions.ListenerCount); + } + [Fact] public void HttpTransport_UsesSessionsOnlyForInitializeClientsWithBoundedIdleTimeout() { diff --git a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs index d03518f8..8c856566 100644 --- a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs +++ b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs @@ -607,9 +607,12 @@ public async Task CallTool_EchoCommand_ReturnsSuccessResult() var tool = CreateToolOperations(); var history = ShellInterpreter.Instance.History.ToArray(); using var output = new StringWriter(); + + // History drops duplicates, so the entry must be unique to this test to grow the history. + var unique = "world-" + Guid.NewGuid().ToString("N"); var arguments = new Dictionary { - ["messages"] = Json("[\"hello\", \"world\"]"), + ["messages"] = Json($"[\"hello\", \"{unique}\"]"), }; var saved = AnsiConsole.Console; @@ -627,14 +630,14 @@ public async Task CallTool_EchoCommand_ReturnsSuccessResult() Assert.Contains("echo", output.ToString(), StringComparison.Ordinal); var recorded = ShellInterpreter.Instance.History.ToArray(); Assert.Equal(history.Length + 1, recorded.Length); - Assert.Equal("echo \"hello\" \"world\"", recorded[^1]); + Assert.Equal($"echo \"hello\" \"{unique}\"", recorded[^1]); Assert.Single(recorded, entry => entry == recorded[^1]); var (isError, root, document) = ReadResult(result); using (document) { Assert.False(isError); - Assert.Equal("hello world", root.GetProperty("result").GetString()); + Assert.Equal($"hello {unique}", root.GetProperty("result").GetString()); Assert.True(root.TryGetProperty("currentLocation", out _)); } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs index c6f820a8..0f67f912 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs @@ -27,18 +27,22 @@ internal sealed class LocationResourceSubscriptions : BackgroundService // Open subscriptions/listen streams (2026-07-28). Each lives as long as its listen request. private readonly HashSet listenStreams = []; - // Listen requests are held-open POSTs, which the transport does not end on shutdown. + // Listen requests are held-open POSTs, which the transport does not end on shutdown. They must end + // on ApplicationStopping: the web server stops before this service and waits for open requests. private readonly CancellationTokenSource stopping = new(); + private readonly CancellationTokenRegistration stoppingRegistration; + // Notifications carry only the URI, so pending changes are coalesced into one. private readonly Channel changes = Channel.CreateBounded( new BoundedChannelOptions(1) { FullMode = BoundedChannelFullMode.DropWrite, SingleReader = true }); private readonly ILogger logger; - public LocationResourceSubscriptions(ILogger logger) + public LocationResourceSubscriptions(ILogger logger, IHostApplicationLifetime? lifetime = null) { this.logger = logger; + this.stoppingRegistration = lifetime?.ApplicationStopping.Register(this.stopping.Cancel) ?? default; ShellInterpreter.Instance.LocationChanged += this.OnLocationChanged; } @@ -266,6 +270,7 @@ await server.SendNotificationAsync( public override void Dispose() { ShellInterpreter.Instance.LocationChanged -= this.OnLocationChanged; + this.stoppingRegistration.Dispose(); this.stopping.Dispose(); base.Dispose(); } From c94dafe355f6223b32366c3a659aecd96b699a32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 1 Oct 2026 18:29:54 +0200 Subject: [PATCH 09/12] Isolate listen streams and bound pending confirmation nonces Each subscriptions/listen request now drains its own coalescing queue and sends with its own request-linked token, so a slow or stalled stream delays only itself. The acknowledgement also uses the linked token, so shutdown releases it. Pending confirmation nonces are tracked in creation order and pruned from the front, capped at 1,024 entries; beyond that the oldest is dropped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ToolOperationsCallToolTests.cs | 12 ++++ .../ConfirmationRequestState.cs | 39 ++++++++--- .../LocationResourceSubscriptions.cs | 68 +++++++------------ docs/mcp.md | 2 +- 4 files changed, 68 insertions(+), 53 deletions(-) diff --git a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs index 8c856566..a83ffafb 100644 --- a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs +++ b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs @@ -724,6 +724,18 @@ public void ConfirmationRequestState_CanBeReadOnlyOnce() Assert.False(ConfirmationRequestState.TryRead(state, "rmdb mydb", out _)); } + [Fact] + public void ConfirmationRequestState_DropsOldestBeyondMaxPending() + { + var oldest = ConfirmationRequestState.Create("rmdb oldest", 1); + var states = Enumerable.Range(0, ConfirmationRequestState.MaxPending) + .Select(index => ConfirmationRequestState.Create($"rmdb db{index}", 1)) + .ToArray(); + + Assert.False(ConfirmationRequestState.TryRead(oldest, "rmdb oldest", out _)); + Assert.True(ConfirmationRequestState.TryRead(states[^1], $"rmdb db{ConfirmationRequestState.MaxPending - 1}", out _)); + } + [Fact] public void ConfirmationRequestState_ExpiresAfterLifetime() { diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs index f9102649..a967ad65 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ConfirmationRequestState.cs @@ -4,7 +4,6 @@ namespace Azure.Data.Cosmos.Shell.Mcp; -using System.Collections.Concurrent; using System.Globalization; using System.Security.Cryptography; using System.Text; @@ -19,24 +18,39 @@ namespace Azure.Data.Cosmos.Shell.Mcp; /// internal static class ConfirmationRequestState { + // Upper bound on tracked nonces. Beyond it the oldest is dropped, so its confirmation must be repeated. + internal const int MaxPending = 1024; + internal static readonly TimeSpan Lifetime = TimeSpan.FromMinutes(10); // Per-process key: confirmations do not survive a server restart. private static readonly byte[] Key = RandomNumberGenerator.GetBytes(32); - // Outstanding nonces and their expiry. A nonce is removed when it is read or has expired. - private static readonly ConcurrentDictionary PendingNonces = new(StringComparer.Ordinal); + private static readonly object Sync = new(); + + // Outstanding nonces and their expiry. A nonce is removed when it is read, expires, or is evicted. + private static readonly Dictionary PendingNonces = new(StringComparer.Ordinal); + + // Nonces in creation order. All share one lifetime, so the oldest expires first and pruning stops at the + // first live entry. Entries already read stay queued until pruned; the queue never exceeds MaxPending. + private static readonly Queue<(string Nonce, DateTimeOffset Expiry)> NonceOrder = new(); public static string Create(string commandLine, long stateVersion) { var now = DateTimeOffset.UtcNow; - foreach (var expired in PendingNonces.Where(entry => entry.Value <= now)) + var nonce = WebEncoders.Base64UrlEncode(RandomNumberGenerator.GetBytes(16)); + lock (Sync) { - PendingNonces.TryRemove(expired); + while (NonceOrder.TryPeek(out var oldest) && (oldest.Expiry <= now || NonceOrder.Count >= MaxPending)) + { + NonceOrder.Dequeue(); + PendingNonces.Remove(oldest.Nonce); + } + + PendingNonces[nonce] = now + Lifetime; + NonceOrder.Enqueue((nonce, now + Lifetime)); } - var nonce = WebEncoders.Base64UrlEncode(RandomNumberGenerator.GetBytes(16)); - PendingNonces[nonce] = now + Lifetime; var payload = Encoding.UTF8.GetBytes(nonce + "\n" + stateVersion.ToString(CultureInfo.InvariantCulture) + "\n" + commandLine); return WebEncoders.Base64UrlEncode(payload) + "." + WebEncoders.Base64UrlEncode(Sign(payload)); } @@ -84,7 +98,16 @@ internal static bool TryRead(string? requestState, string commandLine, DateTimeO } // Any signed state is consumed on first use, whether or not it matches this command. - if (!PendingNonces.TryRemove(parts[0], out var expiry) || expiry <= now) + DateTimeOffset expiry; + lock (Sync) + { + if (!PendingNonces.Remove(parts[0], out expiry)) + { + return false; + } + } + + if (expiry <= now) { return false; } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs index 0f67f912..e038769f 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/LocationResourceSubscriptions.cs @@ -156,16 +156,15 @@ public async ValueTask ListenAsync(RequestContext ListenAsync(RequestContext this.sessions.TryGetValue(sessionId, out var server) ? server : null) .OfType() .ToArray(); - foreach (var stream in this.listenStreams.Where(stream => !stream.Acknowledged)) + foreach (var stream in this.listenStreams) { - stream.PendingUpdate = true; + stream.Updates.Writer.TryWrite(true); } - - streams = [.. this.listenStreams.Where(stream => stream.Acknowledged)]; } foreach (var server in servers) @@ -247,23 +245,6 @@ await server.SendNotificationAsync( this.logger.LogWarning(ex, "Could not notify an MCP client about the shell location change."); } } - - foreach (var stream in streams) - { - try - { - await stream.SendUpdateAsync(stoppingToken); - } - catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) - { - return; - } - catch (Exception ex) when (!stoppingToken.IsCancellationRequested) - { - // The stream is removed when its listen request ends; a failed send only affects this notification. - this.logger.LogWarning(ex, "Could not notify an MCP client about the shell location change."); - } - } } } @@ -301,10 +282,9 @@ private sealed class ListenStream(ModelContextProtocol.Server.McpServer server, { public ModelContextProtocol.Server.McpServer Server { get; } = server; - // Guarded by the owning LocationResourceSubscriptions' sync lock. - public bool Acknowledged { get; set; } - - public bool PendingUpdate { get; set; } + // Notifications carry only the URI, so pending changes for this listener are coalesced into one. + public Channel Updates { get; } = Channel.CreateBounded( + new BoundedChannelOptions(1) { FullMode = BoundedChannelFullMode.DropWrite, SingleReader = true, SingleWriter = true }); public Task SendUpdateAsync(CancellationToken cancellationToken) { diff --git a/docs/mcp.md b/docs/mcp.md index 0f81f578..d1742f37 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -79,7 +79,7 @@ Destructive commands (`delete`, `rm`, `rmcon`, `rmdb`) are gated behind an expli - **Declined or cancelled** — nothing is executed and the tool call returns an error explaining that the user did not approve. - **Client cannot confirm** — if the connected client does not support elicitation, the command is refused (fail-closed) and the response suggests running it manually in the shell. -The prompt is sent as a multi-round-trip request: the tool call returns an input-required result, and the client shows the prompt and retries the call with the answer. For clients that use the `initialize` handshake, the server sends a standard `elicitation/create` request on the session and retries the call itself, so those clients see the same prompt as before. The retry carries a server-signed state that ties the answer to the exact command line and shell context. Each state can be answered once and expires after 10 minutes. An answer for a different command, a reused or expired state, or a missing or altered state is refused without executing. Argument order does not matter: the command line is built with positionals in shell order and options in declaration order. +The prompt is sent as a multi-round-trip request: the tool call returns an input-required result, and the client shows the prompt and retries the call with the answer. For clients that use the `initialize` handshake, the server sends a standard `elicitation/create` request on the session and retries the call itself, so those clients see the same prompt as before. The retry carries a server-signed state that ties the answer to the exact command line and shell context. Each state can be answered once and expires after 10 minutes. At most 1,024 confirmations are tracked at a time; beyond that the oldest pending one is dropped and must be confirmed again. An answer for a different command, a reused or expired state, or a missing or altered state is refused without executing. Argument order does not matter: the command line is built with positionals in shell order and options in declaration order. This replaces any opt-in write flag: destructive commands are always allowed to be invoked, but always require confirmation. From c3764a74b89825bdfd264d05249ecbc60bffcae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 1 Oct 2026 18:40:10 +0200 Subject: [PATCH 10/12] Test that listen requests with only unsupported filters complete immediately Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../McpLocationSubscriptionTests.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs index 191b7ca8..6196cc75 100644 --- a/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs +++ b/CosmosDBShell.Tests/McpLocationSubscriptionTests.cs @@ -203,6 +203,57 @@ public async Task ListeningClient_ReceivesLocationChangeOnListenStream() } } + [Fact] + public async Task ListenWithOnlyUnsupportedFilters_AcknowledgesNothingAndCompletes() + { + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + timeout.CancelAfter(TimeSpan.FromSeconds(10)); + using var host = McpServer.CreateHost(new Program.CosmosShellOptions { McpPort = 0 }); + await host.StartAsync(timeout.Token); + try + { + var subscriptions = host.Services.GetRequiredService(); + await using var client = await ConnectAsync(host, timeout.Token, protocolVersion: null); + + var acknowledged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await using var acknowledgedHandler = client.RegisterNotificationHandler( + NotificationMethods.SubscriptionsAcknowledgedNotification, + (notification, _) => + { + acknowledged.TrySetResult(notification); + return ValueTask.CompletedTask; + }); + + var response = await client.SendRequestAsync( + new JsonRpcRequest + { + Id = new RequestId("unsupported-listen"), + Method = RequestMethods.SubscriptionsListen, + Params = JsonSerializer.SerializeToNode(new SubscriptionsListenRequestParams + { + Notifications = new SubscriptionsListenNotifications + { + ResourceSubscriptions = ["cosmos://docs/scripting"], + ToolsListChanged = true, + }, + }), + }, + timeout.Token); + + Assert.Equal("unsupported-listen", response.Id.ToString()); + var acknowledgement = await acknowledged.Task.WaitAsync(timeout.Token); + var granted = acknowledgement.Params!["notifications"]!.AsObject(); + Assert.False(granted.ContainsKey("resourceSubscriptions")); + Assert.False(granted.ContainsKey("toolsListChanged")); + Assert.Equal("unsupported-listen", acknowledgement.Params["_meta"]![MetaKeys.SubscriptionId]!.GetValue()); + Assert.Equal(0, subscriptions.ListenerCount); + } + finally + { + await host.StopAsync(TestContext.Current.CancellationToken); + } + } + [Fact] public async Task HostStop_EndsOpenListenStreamWithoutWaitingForShutdownTimeout() { From 7dcefc4d44438d797f192da5431d792b966d6947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 1 Oct 2026 20:33:11 +0200 Subject: [PATCH 11/12] Describe the confirmation prompt as sent by the server Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/mcp.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/mcp.md b/docs/mcp.md index d1742f37..ff30916c 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -73,7 +73,7 @@ Transactional batches invoked through MCP must use the one-shot `batch run` subc ### Destructive Command Confirmation -Destructive commands (`delete`, `rm`, `rmcon`, `rmdb`) are gated behind an explicit user confirmation. When a client invokes one, the server asks the client for an elicitation prompt describing the exact command line before anything runs: +Destructive commands (`delete`, `rm`, `rmcon`, `rmdb`) are gated behind an explicit user confirmation. When a client invokes one, the server sends the client an elicitation prompt describing the exact command line and waits for the user's answer before anything runs: - **Approved** — the command executes normally. - **Declined or cancelled** — nothing is executed and the tool call returns an error explaining that the user did not approve. From 20afa300c808a77294cc8d533a8372a10326173d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Fri, 2 Oct 2026 08:56:45 +0200 Subject: [PATCH 12/12] Added commit instructions for copilot --- .github/copilot-instructions.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b78787e6..24abf69f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -63,3 +63,10 @@ - Fix root causes instead of patching symptoms. - Preserve public behavior unless the task explicitly changes the CLI or output contract. - If behavior changes, update tests and docs in the same change. + +## Commits + +- When creating Git commits, do not add Copilot or AI `Co-authored-by` trailers. +- Ensure commit messages are clear, concise, and follow the project's existing style. +- Use the imperative mood in commit messages (e.g., "Add feature" instead of "Added feature"). +- Reference relevant issues or pull requests in the commit message when applicable.