From 927bbe96c1be387cc3e2dfc51700233aa19d8103 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Nov 2026 12:48:53 -0500 Subject: [PATCH 1/5] fix(dotnet): revalidate MCP tool approvals Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 427a0082-3d8b-44df-adbf-e5407e36b6ac --- .../IMcpToolHandler.cs | 3 + .../ObjectModel/InvokeMcpToolExecutor.cs | 117 ++++++- .../ObjectModel/InvokeMcpToolExecutorTest.cs | 317 +++++++++++++++--- 3 files changed, 392 insertions(+), 45 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs index 56b1c3deb4c..3af75dd15e3 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs @@ -13,6 +13,9 @@ namespace Microsoft.Agents.AI.Workflows.Declarative; /// /// This interface allows the MCP tool invocation to be abstracted, enabling /// different implementations for local development, hosted workflows, and testing scenarios. +/// Implementations used by approval-gated MCP actions must support the reserved tools/list operation +/// and return a text output containing a structured tools array; approval fails closed when the tool definition +/// cannot be verified. /// public interface IMcpToolHandler { diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs index 02dafb8ce2c..88f82b429ec 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs @@ -4,6 +4,8 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using System.Security.Cryptography; +using System.Text; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -32,6 +34,7 @@ internal sealed class InvokeMcpToolExecutor( { private const string ApprovalSnapshotStateKey = nameof(_approvalSnapshots); private const string LegacyApprovalSnapshotStateKey = "_approvalSnapshot"; + private const string ListToolsToolName = "tools/list"; /// /// Snapshots of evaluated parameters captured at approval-request time, keyed by @@ -192,6 +195,35 @@ public async ValueTask CaptureResponseAsync( } string? conversationId = this.GetConversationId(); + string currentToolDefinitionFingerprint = await this.GetToolDefinitionFingerprintAsync( + snapshot.ServerUrl, + snapshot.ServerLabel, + snapshot.ToolName, + headers, + snapshot.ConnectionName, + cancellationToken).ConfigureAwait(false); + if (!string.Equals( + snapshot.ToolDefinitionFingerprint, + currentToolDefinitionFingerprint, + StringComparison.Ordinal)) + { + string reapprovalNotice = snapshot.ToolDefinitionFingerprint is null + ? "The previous approval did not include the MCP tool definition. Review the request before approving." + : "The MCP tool definition changed since the previous approval. Review the request before approving."; + await this.RequestApprovalAsync( + context, + snapshot.ServerUrl, + snapshot.ServerLabel, + snapshot.ToolName, + snapshot.Arguments, + headers, + snapshot.ConnectionName, + snapshot.AutoSend ?? this.GetAutoSendValue(), + cancellationToken, + reapprovalNotice).ConfigureAwait(false); + return; + } + bool autoSend = snapshot.AutoSend ?? this.GetAutoSendValue(); McpServerToolResultContent resultContent = await mcpToolHandler.InvokeToolAsync( snapshot.ServerUrl, @@ -289,8 +321,16 @@ private async ValueTask RequestApprovalAsync( Dictionary? headers, string? connectionName, bool autoSend, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + string? reapprovalNotice = null) { + string toolDefinitionFingerprint = await this.GetToolDefinitionFingerprintAsync( + serverUrl, + serverLabel, + toolName, + headers, + connectionName, + cancellationToken).ConfigureAwait(false); string requestId = Guid.NewGuid().ToString("N"); this._approvalSnapshots[requestId] = new ApprovalSnapshot(serverUrl, serverLabel, toolName, arguments, connectionName) @@ -298,6 +338,7 @@ private async ValueTask RequestApprovalAsync( ConnectionNameValidated = true, RequiresHeaderReapprovalAfterRestore = this.Model.Headers is { Count: > 0 }, AutoSend = autoSend, + ToolDefinitionFingerprint = toolDefinitionFingerprint, }; this._approvalHeaderSnapshots[requestId] = new HeaderSnapshot(headers); @@ -308,10 +349,80 @@ private async ValueTask RequestApprovalAsync( }; ToolApprovalRequestContent approvalRequest = new(requestId, toolCall); - ChatMessage requestMessage = new(ChatRole.Assistant, [approvalRequest]); + ChatMessage requestMessage = reapprovalNotice is not null + ? new ChatMessage(ChatRole.Assistant, [new TextContent(reapprovalNotice), approvalRequest]) + : new ChatMessage(ChatRole.Assistant, [approvalRequest]); await context.SendMessageAsync(new ExternalInputRequest(new AgentResponse([requestMessage])), cancellationToken).ConfigureAwait(false); } + private async ValueTask GetToolDefinitionFingerprintAsync( + string serverUrl, + string? serverLabel, + string toolName, + IDictionary? headers, + string? connectionName, + CancellationToken cancellationToken) + { + McpServerToolResultContent toolList = await mcpToolHandler.InvokeToolAsync( + serverUrl, + serverLabel, + ListToolsToolName, + null, + headers, + connectionName, + cancellationToken).ConfigureAwait(false); + + string? matchingToolJson = null; + bool foundToolsArray = false; + if (toolList.Outputs is not null) + { + foreach (TextContent textContent in toolList.Outputs.OfType()) + { + string serialized = textContent.Text.TrimStart(); + if (serialized.Length == 0 || serialized[0] != '{') + { + continue; + } + + using JsonDocument document = JsonDocument.Parse(serialized); + if (!document.RootElement.TryGetProperty("tools", out JsonElement tools) || + tools.ValueKind != JsonValueKind.Array) + { + continue; + } + + foundToolsArray = true; + foreach (JsonElement tool in tools.EnumerateArray()) + { + if (tool.TryGetProperty("name", out JsonElement name) && + name.ValueKind == JsonValueKind.String && + string.Equals(name.GetString(), toolName, StringComparison.Ordinal)) + { + if (matchingToolJson is not null) + { + throw this.Exception($"The MCP server returned multiple definitions for tool '{toolName}'."); + } + + matchingToolJson = tool.GetRawText(); + } + } + } + } + + if (!foundToolsArray) + { + throw this.Exception($"The MCP server did not return a valid tool list while checking approval for tool '{toolName}'."); + } + + if (matchingToolJson is null) + { + throw this.Exception($"The MCP server did not return a definition for tool '{toolName}'."); + } + + byte[] definitionBytes = Encoding.UTF8.GetBytes(matchingToolJson); + return Convert.ToHexString(SHA256.HashData(definitionBytes)); + } + private async ValueTask ProcessResultAsync( IWorkflowContext context, McpServerToolResultContent resultContent, @@ -543,6 +654,8 @@ internal sealed record ApprovalSnapshot( public bool ConnectionNameValidated { get; init; } public bool RequiresHeaderReapprovalAfterRestore { get; init; } + + public string? ToolDefinitionFingerprint { get; init; } } private sealed record HeaderSnapshot(Dictionary? Headers); diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs index b3cd7d8c8d1..fcbe9b173cc 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs @@ -5,7 +5,9 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Linq.Expressions; using System.Reflection; +using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.Agents.AI.Workflows.Declarative.Events; @@ -17,6 +19,7 @@ using Microsoft.Extensions.AI; using Microsoft.PowerFx.Types; using Moq; +using Moq.Language.Flow; using ApprovalSnapshot = Microsoft.Agents.AI.Workflows.Declarative.ObjectModel.InvokeMcpToolExecutor.ApprovalSnapshot; namespace Microsoft.Agents.AI.Workflows.Declarative.UnitTests.ObjectModel; @@ -29,6 +32,7 @@ public sealed class InvokeMcpToolExecutorTest(ITestOutputHelper output) : Workfl private const string TestServerUrl = "https://mcp.example.com"; private const string TestServerLabel = "TestMcpServer"; private const string TestToolName = "test_tool"; + private const string ListToolsToolName = "tools/list"; #region Step Naming Convention Tests @@ -36,7 +40,7 @@ public sealed class InvokeMcpToolExecutorTest(ITestOutputHelper output) : Workfl public void InvokeMcpToolThrowsWhenModelInvalid() { // Arrange - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); MockAgentProvider mockAgentProvider = new(); // Act & Assert @@ -561,7 +565,7 @@ await this.ExecuteAsync( Assert.True(additionalProperties is null || additionalProperties.Count == 0); // Defense in depth: the credential value must not appear anywhere in the serialized approval content. - string serializedApproval = System.Text.Json.JsonSerializer.Serialize(capturedRequest.AgentResponse); + string serializedApproval = JsonSerializer.Serialize(capturedRequest.AgentResponse); Assert.DoesNotContain("Authorization", serializedApproval); Assert.DoesNotContain("super-secret-token", serializedApproval); @@ -589,7 +593,7 @@ public async Task InvokeMcpToolInvocationForwardsHeadersToTransportAsync() headerValue: HeaderValue); IDictionary? capturedHeaders = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider .Setup(provider => provider.InvokeToolAsync( It.IsAny(), @@ -632,7 +636,7 @@ public async Task InvokeMcpToolApprovedCaptureResponseForwardsHeadersToTransport headerValue: HeaderValue); IDictionary? capturedHeaders = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider .Setup(provider => provider.InvokeToolAsync( It.IsAny(), @@ -888,7 +892,7 @@ public async Task InvokeMcpToolCaptureResponseWithNewlySensitiveConversationIdTh RequireApproval = new BoolExpression.Builder(BoolExpression.Literal(true)), }; InvokeMcpTool model = AssignParent(builder); - Mock mockProvider = new(); + MockMcpToolProvider mockProvider = new(); MockAgentProvider mockAgentProvider = new(); InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); List emittedRequests = []; @@ -910,7 +914,7 @@ Task CaptureResponseAsync() => action.CaptureResponseAsync( mockProvider.Verify(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), - It.IsAny(), + It.Is(toolName => !string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)), It.IsAny?>(), It.IsAny?>(), It.IsAny(), @@ -1125,6 +1129,128 @@ public async Task InvokeMcpToolCaptureResponseWithApprovedAndConversationIdAsync #region Approval Snapshot Security Tests + [Fact] + public async Task InvokeMcpToolRejectsApprovalWhenToolDefinitionChangesAsync() + { + // Arrange + int definitionVersion = 1; + int invocationCount = 0; + this.State.InitializeSystem(); + InvokeMcpTool model = this.CreateModel( + displayName: nameof(InvokeMcpToolRejectsApprovalWhenToolDefinitionChangesAsync), + serverUrl: TestServerUrl, + toolName: TestToolName, + requireApproval: true); + Mock mockProvider = new(); + mockProvider.Setup(provider => provider.InvokeToolAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny?>(), + It.IsAny?>(), + It.IsAny(), + It.IsAny())) + .Returns?, IDictionary?, string?, CancellationToken>( + (_, _, toolName, _, _, _, _) => + { + if (string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)) + { + string description = definitionVersion == 1 ? "Read-only tool" : "Updated tool"; + return Task.FromResult(new McpServerToolResultContent("list-tools") + { + Outputs = [new TextContent( + $"{{\"tools\":[{{\"name\":\"{TestToolName}\",\"description\":\"{description}\",\"inputSchema\":{{\"type\":\"object\"}}}}]}}")] + }); + } + + invocationCount++; + return Task.FromResult(new McpServerToolResultContent("tool-call") + { + Outputs = [new TextContent("result")] + }); + }); + MockAgentProvider mockAgentProvider = new(); + InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); + List requests = []; + Dictionary stateStore = []; + Mock context = CreateMockWorkflowContextWithStateStore(requests, stateStore); + await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); + await InvokeProtectedMethodAsync(action, "OnCheckpointingAsync", context.Object, CancellationToken.None); + action = new InvokeMcpToolExecutor(model, mockProvider.Object, mockAgentProvider.Object, this.State); + await InvokeProtectedMethodAsync(action, "OnCheckpointRestoredAsync", context.Object, CancellationToken.None); + definitionVersion = 2; + + // Act + await action.CaptureResponseAsync( + context.Object, + CreateApprovalResponseFor(requests, approved: true), + CancellationToken.None); + + // Assert + Assert.Equal(0, invocationCount); + Assert.Equal(2, requests.Count); + ToolApprovalRequestContent firstRequest = Assert.Single( + requests[0].AgentResponse.Messages.SelectMany(message => message.Contents).OfType()); + ToolApprovalRequestContent refreshedRequest = Assert.Single( + requests[1].AgentResponse.Messages.SelectMany(message => message.Contents).OfType()); + Assert.NotEqual(firstRequest.RequestId, refreshedRequest.RequestId); + Assert.Contains( + requests[1].AgentResponse.Messages.SelectMany(message => message.Contents).OfType(), + text => text.Text.Contains("definition changed", StringComparison.Ordinal)); + } + + [Fact] + public async Task InvokeMcpToolRejectsApprovalWhenToolDefinitionIsUnavailableAsync() + { + // Arrange + int invocationCount = 0; + this.State.InitializeSystem(); + InvokeMcpTool model = this.CreateModel( + displayName: nameof(InvokeMcpToolRejectsApprovalWhenToolDefinitionIsUnavailableAsync), + serverUrl: TestServerUrl, + toolName: TestToolName, + requireApproval: true); + Mock mockProvider = new(); + mockProvider.Setup(provider => provider.InvokeToolAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny?>(), + It.IsAny?>(), + It.IsAny(), + It.IsAny())) + .Returns?, IDictionary?, string?, CancellationToken>( + (_, _, toolName, _, _, _, _) => + { + if (string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)) + { + return Task.FromResult(new McpServerToolResultContent("list-tools") + { + Outputs = [new TextContent("Tool metadata is unavailable.")] + }); + } + + invocationCount++; + return Task.FromResult(new McpServerToolResultContent("tool-call")); + }); + MockAgentProvider mockAgentProvider = new(); + InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); + List requests = []; + Mock context = CreateMockWorkflowContext(requests); + + // Act + Task ExecuteAsync() => action.HandleAsync( + new ActionExecutorResult(action.Id), + context.Object, + CancellationToken.None).AsTask(); + + // Assert + DeclarativeActionException exception = await Assert.ThrowsAsync(ExecuteAsync); + Assert.Contains("valid tool list", exception.Message); + Assert.Empty(requests); + Assert.Equal(0, invocationCount); + } + [Fact] public async Task InvokeMcpToolCaptureResponseUsesApprovedHeadersNotMutatedAsync() { @@ -1144,7 +1270,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedHeadersNotMutatedAsync variableName: "HeaderValue"); IDictionary? capturedHeaders = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1202,7 +1328,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedHeaderPresenceAsync( variableName: "HeaderValue"); IDictionary? capturedHeaders = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1259,7 +1385,7 @@ public async Task InvokeMcpToolApprovedHeadersUseCaseInsensitiveNamesIndependent ("X-First", "FirstHeader")); IDictionary? capturedHeaders = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1314,7 +1440,7 @@ public async Task InvokeMcpToolHeaderSnapshotsAreRequestIsolatedAndReplaySafeAsy variableName: "HeaderValue"); List capturedHeaderValues = []; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1324,7 +1450,13 @@ public async Task InvokeMcpToolHeaderSnapshotsAreRequestIsolatedAndReplaySafeAsy It.IsAny(), It.IsAny())) .Callback?, IDictionary?, string?, CancellationToken>( - (_, _, _, _, headers, _, _) => capturedHeaderValues.Add(headers!["X-Test-Header"])) + (_, _, toolName, _, headers, _, _) => + { + if (!string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)) + { + capturedHeaderValues.Add(headers!["X-Test-Header"]); + } + }) .ReturnsAsync(new McpServerToolResultContent("capture-call-id") { Outputs = [new TextContent("result")] @@ -1378,7 +1510,7 @@ public async Task InvokeMcpToolRestoredApprovalReissuesWithoutPersistingHeadersA InvokeMcpTool model = AssignParent(builder); List capturedHeaderValues = []; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1388,7 +1520,13 @@ public async Task InvokeMcpToolRestoredApprovalReissuesWithoutPersistingHeadersA It.IsAny(), It.IsAny())) .Callback?, IDictionary?, string?, CancellationToken>( - (_, _, _, _, headers, _, _) => capturedHeaderValues.Add(headers!["Authorization"])) + (_, _, toolName, _, headers, _, _) => + { + if (!string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)) + { + capturedHeaderValues.Add(headers!["Authorization"]); + } + }) .ReturnsAsync(new McpServerToolResultContent("capture-call-id") { Outputs = [new TextContent("result")] @@ -1407,7 +1545,7 @@ public async Task InvokeMcpToolRestoredApprovalReissuesWithoutPersistingHeadersA ExternalInputResponse staleResponse = CreateApprovalResponseForRequest(emittedRequests[0], approved: true); await InvokeProtectedMethodAsync(action, "OnCheckpointingAsync", mockContext.Object, CancellationToken.None); - string serializedCheckpoint = System.Text.Json.JsonSerializer.Serialize(stateStore); + string serializedCheckpoint = JsonSerializer.Serialize(stateStore); Assert.DoesNotContain("Authorization", serializedCheckpoint); Assert.DoesNotContain(BeforeRestoreHeader, serializedCheckpoint); @@ -1438,7 +1576,7 @@ public async Task InvokeMcpToolRestoredApprovalReissuesWithoutPersistingHeadersA .RequestId; Assert.NotEqual(staleRequestId, freshRequestId); - string serializedFreshApproval = System.Text.Json.JsonSerializer.Serialize(emittedRequests[1].AgentResponse); + string serializedFreshApproval = JsonSerializer.Serialize(emittedRequests[1].AgentResponse); Assert.DoesNotContain("Authorization", serializedFreshApproval); Assert.DoesNotContain(AfterRestoreHeader, serializedFreshApproval); @@ -1471,7 +1609,7 @@ public async Task InvokeMcpToolLegacyRestoredApprovalRevalidatesConnectionNameAs RequireApproval = new BoolExpression.Builder(BoolExpression.Literal(true)), }; InvokeMcpTool model = AssignParent(builder); - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); MockAgentProvider mockAgentProvider = new(); InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); List emittedRequests = []; @@ -1509,7 +1647,7 @@ Task CaptureResponseAsync() => action.CaptureResponseAsync( mockProvider.Verify(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), - It.IsAny(), + It.Is(toolName => !string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)), It.IsAny?>(), It.IsAny?>(), It.IsAny(), @@ -1544,7 +1682,7 @@ public async Task InvokeMcpToolLegacyRestoredApprovalReissuesValidatedConnection }; InvokeMcpTool model = AssignParent(builder); string? capturedConnectionName = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1604,7 +1742,7 @@ public async Task InvokeMcpToolLegacyRestoredApprovalReissuesValidatedConnection mockProvider.Verify(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), - It.IsAny(), + It.Is(toolName => !string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)), It.IsAny?>(), It.IsAny?>(), It.IsAny(), @@ -1692,7 +1830,7 @@ public async Task InvokeMcpToolWorkflowRestoreRoutesReapprovalToExternalInputAsy """); List capturedHeaderValues = []; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1702,7 +1840,13 @@ public async Task InvokeMcpToolWorkflowRestoreRoutesReapprovalToExternalInputAsy It.IsAny(), It.IsAny())) .Callback?, IDictionary?, string?, CancellationToken>( - (_, _, _, _, headers, _, _) => capturedHeaderValues.Add(headers!["X-Test-Header"])) + (_, _, toolName, _, headers, _, _) => + { + if (!string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)) + { + capturedHeaderValues.Add(headers!["X-Test-Header"]); + } + }) .ReturnsAsync(new McpServerToolResultContent("capture-call-id") { Outputs = [new TextContent("result")] @@ -1843,7 +1987,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedToolNameNotMutatedAsyn variableName: "TargetTool"); string? capturedToolName = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1904,7 +2048,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedArgumentsNotMutatedAsy variableName: "SqlQuery"); IDictionary? capturedArguments = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -1963,7 +2107,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedServerUrlNotMutatedAsy toolName: TestToolName); string? capturedServerUrl = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -2022,7 +2166,7 @@ public async Task InvokeMcpToolCaptureResponseUsesSnapshotAfterCheckpointRestore variableName: "TargetTool"); string? capturedToolName = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(provider => provider.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -2086,7 +2230,7 @@ public async Task InvokeMcpToolEmitsUniqueRequestIdPerInvocationAsync() displayName: nameof(InvokeMcpToolEmitsUniqueRequestIdPerInvocationAsync), serverUrl: TestServerUrl, toolName: TestToolName); - Mock mockProvider = new(); + MockMcpToolProvider mockProvider = new(); MockAgentProvider mockAgentProvider = new(); InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); @@ -2132,7 +2276,7 @@ public async Task InvokeMcpToolConcurrentPendingApprovalsDoNotSwapAsync() toolName: ToolB); List capturedToolNames = []; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(p => p.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -2142,7 +2286,13 @@ public async Task InvokeMcpToolConcurrentPendingApprovalsDoNotSwapAsync() It.IsAny(), It.IsAny())) .Callback?, IDictionary?, string?, CancellationToken>( - (_, _, toolName, _, _, _, _) => capturedToolNames.Add(toolName)) + (_, _, toolName, _, _, _, _) => + { + if (!string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)) + { + capturedToolNames.Add(toolName); + } + }) .ReturnsAsync(new McpServerToolResultContent("capture-call-id") { Outputs = [new TextContent("ok")] @@ -2183,7 +2333,7 @@ public async Task InvokeMcpToolMissingSnapshotAssignsErrorAsync() displayName: nameof(InvokeMcpToolMissingSnapshotAssignsErrorAsync), serverUrl: TestServerUrl, toolName: TestToolName); - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); MockAgentProvider mockAgentProvider = new(); InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); @@ -2209,9 +2359,8 @@ public async Task InvokeMcpToolMissingSnapshotAssignsErrorAsync() } /// - /// A snapshot persisted at the legacy "_approvalSnapshot" key must be migrated - /// under this.Id after restore so an approval response carrying - /// RequestId == this.Id resumes with the snapshot's tool name. + /// A snapshot persisted at the legacy "_approvalSnapshot" key is migrated after + /// restore, but cannot authorize invocation without a persisted tool definition fingerprint. /// [Fact] public async Task InvokeMcpToolLegacySingleSnapshotCheckpointIsMigratedAsync() @@ -2227,7 +2376,7 @@ public async Task InvokeMcpToolLegacySingleSnapshotCheckpointIsMigratedAsync() toolName: TestToolName); string? capturedToolName = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(p => p.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -2251,7 +2400,8 @@ public async Task InvokeMcpToolLegacySingleSnapshotCheckpointIsMigratedAsync() ["_approvalSnapshot"] = new ApprovalSnapshot( TestServerUrl, null, LegacyApprovedToolName, new Dictionary(), null), }; - Mock mockContext = CreateMockWorkflowContextWithStateStoreSeeded(stateStore); + List requests = []; + Mock mockContext = CreateMockWorkflowContextWithStateStoreSeeded(stateStore, requests); // Act - restore migrates the legacy snapshot under this.Id. await InvokeProtectedMethodAsync(action, "OnCheckpointRestoredAsync", mockContext.Object, CancellationToken.None); @@ -2269,8 +2419,17 @@ public async Task InvokeMcpToolLegacySingleSnapshotCheckpointIsMigratedAsync() await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None); - // Assert - the MCP tool was invoked with the snapshot's tool name. - Assert.Equal(LegacyApprovedToolName, capturedToolName); + // Assert - legacy snapshots require a fresh approval before invocation. + Assert.Null(capturedToolName); + ExternalInputRequest refreshedRequest = Assert.Single(requests); + ToolApprovalRequestContent approvalRequest = Assert.Single( + refreshedRequest.AgentResponse.Messages.SelectMany(message => message.Contents).OfType()); + Assert.NotEqual(action.Id, approvalRequest.RequestId); + McpServerToolCallContent refreshedToolCall = Assert.IsType(approvalRequest.ToolCall); + Assert.Equal(LegacyApprovedToolName, refreshedToolCall.Name); + Assert.Contains( + refreshedRequest.AgentResponse.Messages.SelectMany(message => message.Contents).OfType(), + text => text.Text.Contains("did not include", StringComparison.Ordinal)); } /// @@ -2287,7 +2446,7 @@ public async Task InvokeMcpToolLegacyKeyIsClearedAfterMigrationAsync() displayName: nameof(InvokeMcpToolLegacyKeyIsClearedAfterMigrationAsync), serverUrl: TestServerUrl, toolName: TestToolName); - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); MockAgentProvider mockAgentProvider = new(); InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); @@ -2309,7 +2468,9 @@ public async Task InvokeMcpToolLegacyKeyIsClearedAfterMigrationAsync() /// Variant of CreateMockWorkflowContextWithStateStore that accepts a pre-seeded state /// store and supports the read/write operations exercised by the legacy-migration path. /// - private static Mock CreateMockWorkflowContextWithStateStoreSeeded(Dictionary stateStore) + private static Mock CreateMockWorkflowContextWithStateStoreSeeded( + Dictionary stateStore, + List? emittedRequests = null) { Mock mockContext = new(); mockContext.Setup(c => c.AddEventAsync(It.IsAny(), It.IsAny())) @@ -2335,6 +2496,15 @@ private static Mock CreateMockWorkflowContextWithStateStoreSee new ValueTask(stateStore.TryGetValue(key, out object? val) ? val as ApprovalSnapshot : null)); mockContext.Setup(c => c.ReadStateKeysAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new HashSet()); + mockContext.Setup(c => c.SendMessageAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((message, _, _) => + { + if (emittedRequests is not null && message is ExternalInputRequest request) + { + emittedRequests.Add(request); + } + }) + .Returns(default(ValueTask)); return mockContext; } @@ -2359,7 +2529,7 @@ public async Task InvokeMcpToolResumeAfterResetUsesPersistedSnapshotAsync() variableName: "TargetTool"); string? capturedToolName = null; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(p => p.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -2424,7 +2594,7 @@ public async Task InvokeMcpToolMultiplePendingInvocationsSurviveCheckpointResetR variableName: "TargetTool"); List capturedToolNames = []; - Mock mockProvider = new(); + MetadataAwareMockMcpToolHandler mockProvider = new(); mockProvider.Setup(p => p.InvokeToolAsync( It.IsAny(), It.IsAny(), @@ -2434,7 +2604,13 @@ public async Task InvokeMcpToolMultiplePendingInvocationsSurviveCheckpointResetR It.IsAny(), It.IsAny())) .Callback?, IDictionary?, string?, CancellationToken>( - (_, _, toolName, _, _, _, _) => capturedToolNames.Add(toolName)) + (_, _, toolName, _, _, _, _) => + { + if (!string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)) + { + capturedToolNames.Add(toolName); + } + }) .ReturnsAsync(new McpServerToolResultContent("capture-call-id") { Outputs = [new TextContent("ok")] @@ -2839,6 +3015,53 @@ public enum McpInvocationValueLocation #region Mock MCP Tool Provider + private sealed class MetadataAwareMockMcpToolHandler : Mock + { + private static readonly string[] s_toolNames = + [ + TestToolName, + "safe_readonly_query", + "tool_alpha", + "tool_beta", + "legacy_approved_tool", + "approved_tool", + ]; + + public MetadataAwareMockMcpToolHandler() + { + this.SetupToolList(); + } + + public ISetup> Setup( + Expression>> expression) + { + ISetup> setup = base.Setup(expression); + this.SetupToolList(); + return setup; + } + + private void SetupToolList() + { + string definitions = string.Join( + ",", + s_toolNames.Select(name => $"{{\"name\":{JsonSerializer.Serialize(name)},\"description\":\"Test tool\"}}")); + string toolList = $"{{\"tools\":[{definitions}]}}"; + + base.Setup(provider => provider.InvokeToolAsync( + It.IsAny(), + It.IsAny(), + It.Is(name => string.Equals(name, ListToolsToolName, StringComparison.Ordinal)), + It.IsAny?>(), + It.IsAny?>(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new McpServerToolResultContent("mock-list-tools") + { + Outputs = [new TextContent(toolList)] + }); + } + } + /// /// Mock implementation of for unit testing purposes. /// @@ -2862,11 +3085,19 @@ public MockMcpToolProvider( It.IsAny(), It.IsAny())) .Returns?, IDictionary?, string?, CancellationToken>( - (_, _, _, _, _, _, _) => + (_, _, toolName, _, _, _, _) => { McpServerToolResultContent result = new("mock-call-id"); - if (returnNullOutput) + if (string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal)) + { + result.Outputs = + [ + new TextContent( + $"{{\"tools\":[{{\"name\":{JsonSerializer.Serialize(TestToolName)},\"description\":\"Test tool\"}}]}}") + ]; + } + else if (returnNullOutput) { result.Outputs = null; } From 8d266ca11e45a196dcd8ce1d9a6dd09eabd280b5 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Nov 2026 13:47:20 -0500 Subject: [PATCH 2/5] fix(dotnet): preserve MCP approval boundaries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 427a0082-3d8b-44df-adbf-e5407e36b6ac --- ...tnet-declarative-mcp-discovery-approval.md | 85 +++++ .../DefaultMcpToolHandler.cs | 17 +- .../IMcpToolHandler.cs | 4 +- .../ObjectModel/InvokeMcpToolExecutor.cs | 145 ++++++-- .../DefaultMcpToolHandlerTests.cs | 12 +- .../ObjectModel/InvokeMcpToolExecutorTest.cs | 350 ++++++++++++++++-- 6 files changed, 523 insertions(+), 90 deletions(-) create mode 100644 docs/decisions/0056-dotnet-declarative-mcp-discovery-approval.md diff --git a/docs/decisions/0056-dotnet-declarative-mcp-discovery-approval.md b/docs/decisions/0056-dotnet-declarative-mcp-discovery-approval.md new file mode 100644 index 00000000000..32fc1d0b5e0 --- /dev/null +++ b/docs/decisions/0056-dotnet-declarative-mcp-discovery-approval.md @@ -0,0 +1,85 @@ +--- +status: proposed +contact: baywet +date: 2026-11-10 +deciders: [baywet, westey-m] +informed: [rhurey] +--- + +# Separate MCP discovery consent from tool invocation approval + +## Context and Problem Statement + +An approval-gated declarative MCP action needs the current tool definition before +it can bind an invocation approval to that definition. Obtaining the definition +is itself a network operation using the configured endpoint and transport +settings. Approval to invoke a tool and permission to contact its server are +different decisions. + +The executor cannot determine whether discovery will carry credentials merely by +checking workflow headers: managed connections, custom handlers, and configured +HTTP clients may also supply them. Workflow expressions can change while an +approval is pending, and headers must not become durable checkpoint state. + +## Decision Drivers + +- Obtain consent before contacting a server for an approval-gated action. +- Bind each consent to its captured destination, workflow headers, and connection reference. +- Require invocation approval after discovering the current published definition. +- Preserve request isolation, replay handling, and credential nonpersistence. +- Support custom handlers and all supported .NET targets. + +## Considered Options + +- Discover with configured transport settings before requesting tool approval. + Good: one approval interaction and discovery works with authenticated servers. + Neutral: metadata discovery remains a separate handler invocation. + Bad: discovery contacts the destination before any consent. +- Discover without workflow headers before requesting tool approval. + Good: explicit workflow headers are not sent during discovery. + Neutral: the executor still performs a network operation before consent. + Bad: authenticated discovery can fail, and other credential sources remain. +- Obtain discovery consent, then obtain a definition-bound invocation approval. + Good: no handler invocation occurs before discovery consent, including when + authentication is supplied outside workflow headers. + Neutral: discovery and invocation remain separate, invocation-owned MCP calls. + Bad: approval-gated tool calls require two interactions rather than one. + +## Decision Outcome + +Chosen option: "Obtain discovery consent, then obtain a definition-bound invocation +approval", because it does not depend on the executor detecting every possible +credential source. + +For approval-gated actions, the initial request authorizes `tools/list` against +the displayed destination using the configured transport settings. The destination, +workflow headers, and connection reference are captured; host-managed transport +credentials remain subject to the handler's lifecycle. Discovery consent does not +authorize the requested tool invocation. After consent, discovery produces a +separate approval for the captured tool name and arguments. That approval includes +a fingerprint of the complete published tool definition in its internal snapshot. +Before invocation, the executor discovers the definition again and requests fresh +invocation approval if it differs or the previous snapshot lacks a fingerprint. +Changed metadata does not authorize an invocation. + +The reserved `tools/list` action is discovery itself: its single approval +authorizes that operation, without requiring it to appear in the tool catalog. +Actions without approval retain their existing direct-invocation behavior. + +Each stage uses a distinct request identifier. Responses use captured invocation +parameters rather than reevaluating them. Headers remain in memory only. If a +restore loses required headers, the executor reevaluates the configuration and +starts a fresh discovery approval before contacting the newly evaluated +destination. Denied or consumed requests cannot authorize a later stage. + +Handlers must return structured `tools/list` output containing complete protocol +tool definitions, including annotations and metadata. Object properties are +compared in ordinally sorted order recursively; array order and scalar values are +preserved. Missing or ambiguous definitions stop the action rather than bypassing +approval. This compares published metadata, not server implementation identity, +and does not eliminate the interval between discovery and invocation. + +Existing approval UIs must handle the additional discovery request and its notice. +Older snapshots without discovery consent restart discovery approval, as do +snapshots requiring unavailable headers. The ADR remains +proposed pending the listed deciders' PR review. diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index 5ba5bf31052..bec6ce425d9 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -350,22 +350,7 @@ private static string SerializeToolsList(IEnumerable tools) foreach (Tool tool in tools) { - writer.WriteStartObject(); - writer.WriteString("name", tool.Name); - writer.WriteString("description", tool.Description); - writer.WritePropertyName("inputSchema"); - tool.InputSchema.WriteTo(writer); - writer.WritePropertyName("outputSchema"); - if (tool.OutputSchema is JsonElement outputSchema) - { - outputSchema.WriteTo(writer); - } - else - { - writer.WriteNullValue(); - } - - writer.WriteEndObject(); + JsonSerializer.Serialize(writer, tool, McpJsonUtilities.DefaultOptions.GetTypeInfo()); } writer.WriteEndArray(); diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs index 3af75dd15e3..ac6ad2a6c87 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs @@ -14,7 +14,9 @@ namespace Microsoft.Agents.AI.Workflows.Declarative; /// This interface allows the MCP tool invocation to be abstracted, enabling /// different implementations for local development, hosted workflows, and testing scenarios. /// Implementations used by approval-gated MCP actions must support the reserved tools/list operation -/// and return a text output containing a structured tools array; approval fails closed when the tool definition +/// and return a text output containing a structured tools array with complete published tool definitions, +/// including annotations and metadata. Discovery is invoked only after explicit discovery approval; +/// a separate approval binds the tool invocation to its definition. Approval fails closed when the definition /// cannot be verified. /// public interface IMcpToolHandler diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs index 88f82b429ec..648db719d06 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs @@ -3,9 +3,9 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Security.Cryptography; -using System.Text; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -194,22 +194,13 @@ public async ValueTask CaptureResponseAsync( headers = null; } - string? conversationId = this.GetConversationId(); - string currentToolDefinitionFingerprint = await this.GetToolDefinitionFingerprintAsync( - snapshot.ServerUrl, - snapshot.ServerLabel, - snapshot.ToolName, - headers, - snapshot.ConnectionName, - cancellationToken).ConfigureAwait(false); - if (!string.Equals( - snapshot.ToolDefinitionFingerprint, - currentToolDefinitionFingerprint, - StringComparison.Ordinal)) + if (!snapshot.IsDiscoveryApproval && + !snapshot.DiscoveryApproved && + !string.Equals(snapshot.ToolName, ListToolsToolName, StringComparison.Ordinal)) { string reapprovalNotice = snapshot.ToolDefinitionFingerprint is null ? "The previous approval did not include the MCP tool definition. Review the request before approving." - : "The MCP tool definition changed since the previous approval. Review the request before approving."; + : "The previous approval did not include separate MCP tool discovery consent. Review the request before approving."; await this.RequestApprovalAsync( context, snapshot.ServerUrl, @@ -224,6 +215,40 @@ await this.RequestApprovalAsync( return; } + string? conversationId = this.GetConversationId(); + if (!string.Equals(snapshot.ToolName, ListToolsToolName, StringComparison.Ordinal)) + { + string currentToolDefinitionFingerprint = await this.GetToolDefinitionFingerprintAsync( + snapshot.ServerUrl, + snapshot.ServerLabel, + snapshot.ToolName, + headers, + snapshot.ConnectionName, + cancellationToken).ConfigureAwait(false); + if (snapshot.IsDiscoveryApproval || + !string.Equals(snapshot.ToolDefinitionFingerprint, currentToolDefinitionFingerprint, StringComparison.Ordinal)) + { + string? reapprovalNotice = snapshot.IsDiscoveryApproval + ? null + : snapshot.ToolDefinitionFingerprint is null + ? "The previous approval did not include the MCP tool definition. Review the request before approving." + : "The MCP tool definition changed since the previous approval. Review the request before approving."; + await this.RequestApprovalAsync( + context, + snapshot.ServerUrl, + snapshot.ServerLabel, + snapshot.ToolName, + snapshot.Arguments, + headers, + snapshot.ConnectionName, + snapshot.AutoSend ?? this.GetAutoSendValue(), + cancellationToken, + reapprovalNotice, + currentToolDefinitionFingerprint).ConfigureAwait(false); + return; + } + } + bool autoSend = snapshot.AutoSend ?? this.GetAutoSendValue(); McpServerToolResultContent resultContent = await mcpToolHandler.InvokeToolAsync( snapshot.ServerUrl, @@ -322,15 +347,11 @@ private async ValueTask RequestApprovalAsync( string? connectionName, bool autoSend, CancellationToken cancellationToken, - string? reapprovalNotice = null) + string? reapprovalNotice = null, + string? toolDefinitionFingerprint = null) { - string toolDefinitionFingerprint = await this.GetToolDefinitionFingerprintAsync( - serverUrl, - serverLabel, - toolName, - headers, - connectionName, - cancellationToken).ConfigureAwait(false); + bool isListTools = string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal); + bool isDiscoveryApproval = toolDefinitionFingerprint is null && !isListTools; string requestId = Guid.NewGuid().ToString("N"); this._approvalSnapshots[requestId] = new ApprovalSnapshot(serverUrl, serverLabel, toolName, arguments, connectionName) @@ -339,16 +360,30 @@ private async ValueTask RequestApprovalAsync( RequiresHeaderReapprovalAfterRestore = this.Model.Headers is { Count: > 0 }, AutoSend = autoSend, ToolDefinitionFingerprint = toolDefinitionFingerprint, + IsDiscoveryApproval = isDiscoveryApproval, + DiscoveryApproved = toolDefinitionFingerprint is not null, }; this._approvalHeaderSnapshots[requestId] = new HeaderSnapshot(headers); // Transport headers are intentionally excluded from externally-surfaced approval content. - McpServerToolCallContent toolCall = new(requestId, toolName, serverLabel ?? serverUrl) + string approvalServerName = isDiscoveryApproval || isListTools ? serverUrl : serverLabel ?? serverUrl; + McpServerToolCallContent toolCall = new(requestId, isDiscoveryApproval ? ListToolsToolName : toolName, approvalServerName) { - Arguments = arguments + Arguments = isDiscoveryApproval ? null : arguments }; ToolApprovalRequestContent approvalRequest = new(requestId, toolCall); + if (isDiscoveryApproval || isListTools) + { + string discoveryNotice = $"Allow MCP tool discovery at '{serverUrl}' using the configured transport settings."; + if (isDiscoveryApproval) + { + discoveryNotice += $" This does not approve invoking '{toolName}'; a separate tool approval will follow discovery."; + } + + reapprovalNotice = reapprovalNotice is null ? discoveryNotice : $"{reapprovalNotice} {discoveryNotice}"; + } + ChatMessage requestMessage = reapprovalNotice is not null ? new ChatMessage(ChatRole.Assistant, [new TextContent(reapprovalNotice), approvalRequest]) : new ChatMessage(ChatRole.Assistant, [approvalRequest]); @@ -372,7 +407,7 @@ private async ValueTask GetToolDefinitionFingerprintAsync( connectionName, cancellationToken).ConfigureAwait(false); - string? matchingToolJson = null; + string? matchingToolFingerprint = null; bool foundToolsArray = false; if (toolList.Outputs is not null) { @@ -398,12 +433,12 @@ private async ValueTask GetToolDefinitionFingerprintAsync( name.ValueKind == JsonValueKind.String && string.Equals(name.GetString(), toolName, StringComparison.Ordinal)) { - if (matchingToolJson is not null) + if (matchingToolFingerprint is not null) { throw this.Exception($"The MCP server returned multiple definitions for tool '{toolName}'."); } - matchingToolJson = tool.GetRawText(); + matchingToolFingerprint = ComputeToolDefinitionFingerprint(tool); } } } @@ -414,13 +449,59 @@ private async ValueTask GetToolDefinitionFingerprintAsync( throw this.Exception($"The MCP server did not return a valid tool list while checking approval for tool '{toolName}'."); } - if (matchingToolJson is null) + if (matchingToolFingerprint is null) { throw this.Exception($"The MCP server did not return a definition for tool '{toolName}'."); } - byte[] definitionBytes = Encoding.UTF8.GetBytes(matchingToolJson); - return Convert.ToHexString(SHA256.HashData(definitionBytes)); + return matchingToolFingerprint; + } + + private static string ComputeToolDefinitionFingerprint(JsonElement tool) + { + using MemoryStream stream = new(); + using (Utf8JsonWriter writer = new(stream)) + { + WriteCanonicalJson(writer, tool); + } + +#if NET + return Convert.ToHexString(SHA256.HashData(stream.ToArray())); +#else + using SHA256 sha256 = SHA256.Create(); + return BitConverter.ToString(sha256.ComputeHash(stream.ToArray())).Replace("-", string.Empty); +#endif + } + + private static void WriteCanonicalJson(Utf8JsonWriter writer, JsonElement value) + { + switch (value.ValueKind) + { + case JsonValueKind.Object: + writer.WriteStartObject(); + foreach (JsonProperty property in value.EnumerateObject().OrderBy(property => property.Name, StringComparer.Ordinal)) + { + writer.WritePropertyName(property.Name); + WriteCanonicalJson(writer, property.Value); + } + + writer.WriteEndObject(); + break; + + case JsonValueKind.Array: + writer.WriteStartArray(); + foreach (JsonElement element in value.EnumerateArray()) + { + WriteCanonicalJson(writer, element); + } + + writer.WriteEndArray(); + break; + + default: + value.WriteTo(writer); + break; + } } private async ValueTask ProcessResultAsync( @@ -656,6 +737,10 @@ internal sealed record ApprovalSnapshot( public bool RequiresHeaderReapprovalAfterRestore { get; init; } public string? ToolDefinitionFingerprint { get; init; } + + public bool IsDiscoveryApproval { get; init; } + + public bool DiscoveryApproved { get; init; } } private sealed record HeaderSnapshot(Dictionary? Headers); diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index 13f0c65fffa..819ad21c9c7 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -5,6 +5,7 @@ using System.Net.Http; using System.Text; using System.Text.Json; +using System.Text.Json.Nodes; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.AI; @@ -145,7 +146,9 @@ public async Task CreateListToolsResultContent_WithTools_ShouldSerializeToolMeta { Name = "search", Description = "Searches documentation.", - InputSchema = inputSchema + InputSchema = inputSchema, + Annotations = new ToolAnnotations { ReadOnlyHint = true, DestructiveHint = false }, + Meta = new JsonObject { ["revision"] = "initial" } }; // Act @@ -158,6 +161,9 @@ public async Task CreateListToolsResultContent_WithTools_ShouldSerializeToolMeta Assert.Equal("search", listedTool.GetProperty("name").GetString()); Assert.Equal("Searches documentation.", listedTool.GetProperty("description").GetString()); Assert.Equal("string", listedTool.GetProperty("inputSchema").GetProperty("properties").GetProperty("query").GetProperty("type").GetString()); + Assert.True(listedTool.GetProperty("annotations").GetProperty("readOnlyHint").GetBoolean()); + Assert.False(listedTool.GetProperty("annotations").GetProperty("destructiveHint").GetBoolean()); + Assert.Equal("initial", listedTool.GetProperty("_meta").GetProperty("revision").GetString()); } #endregion @@ -354,7 +360,7 @@ public void ConvertContentBlock_ResourceLinkBlock_WithMeta_ShouldPropagateToAddi Uri = "https://example.com/resource.bin", Name = string.Empty, MimeType = "application/zip", - Meta = new System.Text.Json.Nodes.JsonObject + Meta = new JsonObject { ["traceId"] = "abc-123", ["priority"] = 7, @@ -467,7 +473,7 @@ public void ConvertContentBlock_BlockWithMeta_ShouldPropagateToAdditionalPropert TextContentBlock block = new() { Text = "hello", - Meta = new System.Text.Json.Nodes.JsonObject + Meta = new JsonObject { ["traceId"] = "abc-123", ["priority"] = 7, diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs index fcbe9b173cc..9a4028f12ff 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs @@ -299,7 +299,7 @@ public async Task AutoSendUsesMcpApprovalSnapshotAsync(bool autoSend, bool resto this.State.Bind(); // Act - await action.CaptureResponseAsync(context.Object, CreateApprovalResponseFor(requests, approved: true), CancellationToken.None); + await action.CaptureResponseAsync(context.Object, await ApproveDiscoveryAsync(action, context.Object, requests), CancellationToken.None); // Assert Assert.Equal(autoSend ? 1 : 0, events.OfType().Count()); @@ -658,7 +658,7 @@ public async Task InvokeMcpToolApprovedCaptureResponseForwardsHeadersToTransport await action.HandleAsync(new ActionExecutorResult(action.Id), mockContext.Object, CancellationToken.None); // Build the matching approved response from the emitted request. - ExternalInputResponse response = CreateApprovalResponseFor(emittedRequests, approved: true); + ExternalInputResponse response = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests); // Act - call CaptureResponseAsync so the post-approval branch actually executes. await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None); @@ -898,7 +898,7 @@ public async Task InvokeMcpToolCaptureResponseWithNewlySensitiveConversationIdTh List emittedRequests = []; Mock mockContext = CreateMockWorkflowContext(emittedRequests); await action.HandleAsync(new ActionExecutorResult(action.Id), mockContext.Object, CancellationToken.None); - ExternalInputResponse response = CreateApprovalResponseFor(emittedRequests, approved: true); + ExternalInputResponse response = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests); this.State.Set("ConversationId", FormulaValue.New("sensitive-conversation"), sensitivity: SensitivityLevel.Sensitive); this.State.Bind(); @@ -1175,6 +1175,7 @@ public async Task InvokeMcpToolRejectsApprovalWhenToolDefinitionChangesAsync() Dictionary stateStore = []; Mock context = CreateMockWorkflowContextWithStateStore(requests, stateStore); await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); + ExternalInputResponse approvedResponse = await ApproveDiscoveryAsync(action, context.Object, requests); await InvokeProtectedMethodAsync(action, "OnCheckpointingAsync", context.Object, CancellationToken.None); action = new InvokeMcpToolExecutor(model, mockProvider.Object, mockAgentProvider.Object, this.State); await InvokeProtectedMethodAsync(action, "OnCheckpointRestoredAsync", context.Object, CancellationToken.None); @@ -1183,19 +1184,19 @@ public async Task InvokeMcpToolRejectsApprovalWhenToolDefinitionChangesAsync() // Act await action.CaptureResponseAsync( context.Object, - CreateApprovalResponseFor(requests, approved: true), + approvedResponse, CancellationToken.None); // Assert Assert.Equal(0, invocationCount); - Assert.Equal(2, requests.Count); + Assert.Equal(3, requests.Count); ToolApprovalRequestContent firstRequest = Assert.Single( - requests[0].AgentResponse.Messages.SelectMany(message => message.Contents).OfType()); - ToolApprovalRequestContent refreshedRequest = Assert.Single( requests[1].AgentResponse.Messages.SelectMany(message => message.Contents).OfType()); + ToolApprovalRequestContent refreshedRequest = Assert.Single( + requests[2].AgentResponse.Messages.SelectMany(message => message.Contents).OfType()); Assert.NotEqual(firstRequest.RequestId, refreshedRequest.RequestId); Assert.Contains( - requests[1].AgentResponse.Messages.SelectMany(message => message.Contents).OfType(), + requests[2].AgentResponse.Messages.SelectMany(message => message.Contents).OfType(), text => text.Text.Contains("definition changed", StringComparison.Ordinal)); } @@ -1237,17 +1238,18 @@ public async Task InvokeMcpToolRejectsApprovalWhenToolDefinitionIsUnavailableAsy InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); List requests = []; Mock context = CreateMockWorkflowContext(requests); + await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); // Act - Task ExecuteAsync() => action.HandleAsync( - new ActionExecutorResult(action.Id), + Task ExecuteAsync() => action.CaptureResponseAsync( context.Object, + CreateApprovalResponseFor(requests, approved: true), CancellationToken.None).AsTask(); // Assert DeclarativeActionException exception = await Assert.ThrowsAsync(ExecuteAsync); Assert.Contains("valid tool list", exception.Message); - Assert.Empty(requests); + Assert.Single(requests); Assert.Equal(0, invocationCount); } @@ -1298,7 +1300,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedHeadersNotMutatedAsync // Act await action.CaptureResponseAsync( mockContext.Object, - CreateApprovalResponseFor(emittedRequests, approved: true), + await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests), CancellationToken.None); // Assert @@ -1356,7 +1358,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedHeaderPresenceAsync( // Act await action.CaptureResponseAsync( mockContext.Object, - CreateApprovalResponseFor(emittedRequests, approved: true), + await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests), CancellationToken.None); // Assert @@ -1414,7 +1416,7 @@ public async Task InvokeMcpToolApprovedHeadersUseCaseInsensitiveNamesIndependent // Act await action.CaptureResponseAsync( mockContext.Object, - CreateApprovalResponseFor(emittedRequests, approved: true), + await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests), CancellationToken.None); // Assert @@ -1472,8 +1474,8 @@ public async Task InvokeMcpToolHeaderSnapshotsAreRequestIsolatedAndReplaySafeAsy this.State.Bind(); await action.HandleAsync(new ActionExecutorResult(action.Id), mockContext.Object, CancellationToken.None); - ExternalInputResponse responseA = CreateApprovalResponseForRequest(emittedRequests[0], approved: true); - ExternalInputResponse responseB = CreateApprovalResponseForRequest(emittedRequests[1], approved: true); + ExternalInputResponse responseA = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests, emittedRequests[0]); + ExternalInputResponse responseB = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests, emittedRequests[1]); // Act await action.CaptureResponseAsync(mockContext.Object, responseB, CancellationToken.None); @@ -1485,11 +1487,14 @@ public async Task InvokeMcpToolHeaderSnapshotsAreRequestIsolatedAndReplaySafeAsy } [Theory] - [InlineData(true, false)] - [InlineData(false, false)] - [InlineData(true, true)] - [InlineData(false, true)] - public async Task InvokeMcpToolRestoredApprovalReissuesWithoutPersistingHeadersAsync(bool autoSend, bool sensitiveCurrentSetting) + [InlineData(true, false, false)] + [InlineData(false, false, false)] + [InlineData(true, true, false)] + [InlineData(false, true, false)] + [InlineData(true, false, true)] + [InlineData(false, true, true)] + public async Task InvokeMcpToolRestoredApprovalReissuesWithoutPersistingHeadersAsync( + bool autoSend, bool sensitiveCurrentSetting, bool checkpointAfterDiscovery) { // Arrange const string BeforeRestoreHeader = "before-restore-secret"; @@ -1542,7 +1547,15 @@ public async Task InvokeMcpToolRestoredApprovalReissuesWithoutPersistingHeadersA .Callback((e, _) => events.Add(e)) .Returns(default(ValueTask)); await action.HandleAsync(new ActionExecutorResult(action.Id), mockContext.Object, CancellationToken.None); - ExternalInputResponse staleResponse = CreateApprovalResponseForRequest(emittedRequests[0], approved: true); + if (checkpointAfterDiscovery) + { + await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests); + } + + ExternalInputRequest staleRequest = emittedRequests[emittedRequests.Count - 1]; + ExternalInputResponse staleResponse = CreateApprovalResponseForRequest(staleRequest, approved: true); + int callsBeforeRestore = mockProvider.Invocations.Count; + int requestsBeforeRestore = emittedRequests.Count; await InvokeProtectedMethodAsync(action, "OnCheckpointingAsync", mockContext.Object, CancellationToken.None); string serializedCheckpoint = JsonSerializer.Serialize(stateStore); @@ -1562,27 +1575,29 @@ public async Task InvokeMcpToolRestoredApprovalReissuesWithoutPersistingHeadersA // Assert Assert.Empty(capturedHeaderValues); - Assert.Equal(2, emittedRequests.Count); + Assert.Equal(callsBeforeRestore, mockProvider.Invocations.Count); + Assert.Equal(requestsBeforeRestore + 1, emittedRequests.Count); + ExternalInputRequest freshRequest = emittedRequests[emittedRequests.Count - 1]; - string staleRequestId = emittedRequests[0].AgentResponse.Messages + string staleRequestId = staleRequest.AgentResponse.Messages .SelectMany(message => message.Contents) .OfType() .Single() .RequestId; - string freshRequestId = emittedRequests[1].AgentResponse.Messages + string freshRequestId = freshRequest.AgentResponse.Messages .SelectMany(message => message.Contents) .OfType() .Single() .RequestId; Assert.NotEqual(staleRequestId, freshRequestId); - string serializedFreshApproval = JsonSerializer.Serialize(emittedRequests[1].AgentResponse); + string serializedFreshApproval = JsonSerializer.Serialize(freshRequest.AgentResponse); Assert.DoesNotContain("Authorization", serializedFreshApproval); Assert.DoesNotContain(AfterRestoreHeader, serializedFreshApproval); await action.CaptureResponseAsync( mockContext.Object, - CreateApprovalResponseForRequest(emittedRequests[1], approved: true), + await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests, freshRequest), CancellationToken.None); Assert.Equal([AfterRestoreHeader], capturedHeaderValues); Assert.Equal(autoSend ? 1 : 0, events.OfType().Count()); @@ -1750,7 +1765,7 @@ public async Task InvokeMcpToolLegacyRestoredApprovalReissuesValidatedConnection await action.CaptureResponseAsync( mockContext.Object, - CreateApprovalResponseForRequest(freshRequest, approved: true), + await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests, freshRequest), CancellationToken.None); Assert.Equal(CurrentConnectionName, capturedConnectionName); Assert.Equal((autoSend ?? currentAutoSend) ? 1 : 0, events.OfType().Count()); @@ -2015,7 +2030,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedToolNameNotMutatedAsyn this.State.Bind(); // User clicks approve (they saw "safe_readonly_query" in the approval UI) - ExternalInputResponse response = CreateApprovalResponseFor(emittedRequests, approved: true); + ExternalInputResponse response = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests); // Resume after approval await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None); @@ -2076,7 +2091,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedArgumentsNotMutatedAsy this.State.Bind(); // User clicks approve - ExternalInputResponse response = CreateApprovalResponseFor(emittedRequests, approved: true); + ExternalInputResponse response = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests); // Resume after approval await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None); @@ -2135,7 +2150,7 @@ public async Task InvokeMcpToolCaptureResponseUsesApprovedServerUrlNotMutatedAsy this.State.Bind(); // User clicks approve - ExternalInputResponse response = CreateApprovalResponseFor(emittedRequests, approved: true); + ExternalInputResponse response = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests); // Resume after approval await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None); @@ -2206,7 +2221,7 @@ public async Task InvokeMcpToolCaptureResponseUsesSnapshotAfterCheckpointRestore this.State.Bind(); // User clicks approve - ExternalInputResponse response = CreateApprovalResponseFor(emittedRequests, approved: true); + ExternalInputResponse response = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests); // Resume after approval await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None); @@ -2312,8 +2327,8 @@ public async Task InvokeMcpToolConcurrentPendingApprovalsDoNotSwapAsync() await actionB.HandleAsync(new ActionExecutorResult(actionB.Id), ctxB.Object, CancellationToken.None); // Deliver responses out of order - await actionB.CaptureResponseAsync(ctxB.Object, CreateApprovalResponseFor(emittedB, approved: true), CancellationToken.None); - await actionA.CaptureResponseAsync(ctxA.Object, CreateApprovalResponseFor(emittedA, approved: true), CancellationToken.None); + await actionB.CaptureResponseAsync(ctxB.Object, await ApproveDiscoveryAsync(actionB, ctxB.Object, emittedB), CancellationToken.None); + await actionA.CaptureResponseAsync(ctxA.Object, await ApproveDiscoveryAsync(actionA, ctxA.Object, emittedA), CancellationToken.None); // Assert - each invocation invoked its own approved tool name Assert.Equal([ToolB, ToolA], capturedToolNames); @@ -2426,10 +2441,65 @@ public async Task InvokeMcpToolLegacySingleSnapshotCheckpointIsMigratedAsync() refreshedRequest.AgentResponse.Messages.SelectMany(message => message.Contents).OfType()); Assert.NotEqual(action.Id, approvalRequest.RequestId); McpServerToolCallContent refreshedToolCall = Assert.IsType(approvalRequest.ToolCall); - Assert.Equal(LegacyApprovedToolName, refreshedToolCall.Name); + Assert.Equal(ListToolsToolName, refreshedToolCall.Name); Assert.Contains( refreshedRequest.AgentResponse.Messages.SelectMany(message => message.Contents).OfType(), text => text.Text.Contains("did not include", StringComparison.Ordinal)); + ExternalInputResponse freshResponse = await ApproveDiscoveryAsync(action, mockContext.Object, requests, refreshedRequest); + ToolApprovalRequestContent freshApproval = requests[1].AgentResponse.Messages + .SelectMany(message => message.Contents).OfType().Single(); + Assert.Equal(LegacyApprovedToolName, Assert.IsType(freshApproval.ToolCall).Name); + await action.CaptureResponseAsync(mockContext.Object, freshResponse, CancellationToken.None); + Assert.Equal(LegacyApprovedToolName, capturedToolName); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task InvokeMcpToolRestoredLegacyApprovalRequiresDiscoveryConsentAsync(bool hasFingerprint) + { + // Arrange + const string RequestId = "legacy-request"; + this.State.InitializeSystem(); + InvokeMcpTool model = this.CreateModelWithApproval( + nameof(InvokeMcpToolRestoredLegacyApprovalRequiresDiscoveryConsentAsync), TestServerUrl, TestToolName); + List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; + Mock provider = CreateMetadataHandler(() => """{"name":"test_tool"}""", calls); + InvokeMcpToolExecutor action = new(model, provider.Object, new MockAgentProvider().Object, this.State); + List requests = []; + Dictionary stateStore = new() + { + ["_approvalSnapshots"] = new Dictionary + { + [RequestId] = new(TestServerUrl, TestServerLabel, TestToolName, null, null) + { + ConnectionNameValidated = true, + ToolDefinitionFingerprint = hasFingerprint ? "previous-fingerprint" : null, + } + } + }; + Mock context = CreateMockWorkflowContextWithStateStore(requests, stateStore); + await InvokeProtectedMethodAsync(action, "OnCheckpointRestoredAsync", context.Object, CancellationToken.None); + ToolApprovalRequestContent previousApproval = + new(RequestId, new McpServerToolCallContent(RequestId, TestToolName, TestServerLabel)); + ExternalInputResponse previousResponse = + new(new ChatMessage(ChatRole.User, [previousApproval.CreateResponse(approved: true)])); + + // Act + await action.CaptureResponseAsync(context.Object, previousResponse, CancellationToken.None); + + // Assert + Assert.Empty(calls); + ExternalInputRequest discoveryRequest = Assert.Single(requests); + ToolApprovalRequestContent discovery = discoveryRequest.AgentResponse.Messages + .SelectMany(message => message.Contents).OfType().Single(); + McpServerToolCallContent discoveryCall = Assert.IsType(discovery.ToolCall); + Assert.Equal(ListToolsToolName, discoveryCall.Name); + Assert.Equal(TestServerUrl, discoveryCall.ServerName); + ExternalInputResponse invocationResponse = await ApproveDiscoveryAsync(action, context.Object, requests, discoveryRequest); + Assert.Equal(ListToolsToolName, Assert.Single(calls).ToolName); + await action.CaptureResponseAsync(context.Object, invocationResponse, CancellationToken.None); + Assert.Single(calls, call => string.Equals(call.ToolName, TestToolName, StringComparison.Ordinal)); } /// @@ -2564,7 +2634,7 @@ public async Task InvokeMcpToolResumeAfterResetUsesPersistedSnapshotAsync() await InvokeProtectedMethodAsync(action, "OnCheckpointRestoredAsync", mockContext.Object, CancellationToken.None); Assert.Single(liveSnapshots); - ExternalInputResponse response = CreateApprovalResponseFor(emittedRequests, approved: true); + ExternalInputResponse response = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests); await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None); // Assert - the originally-approved tool name was used and the entry was removed. @@ -2647,7 +2717,7 @@ public async Task InvokeMcpToolMultiplePendingInvocationsSurviveCheckpointResetR // Capture A's response. State has been mutated to ToolB but the per-invocation // snapshot must still drive invocation with ToolA. Assert.Equal(2, emittedRequests.Count); - ExternalInputResponse responseA = CreateApprovalResponseForRequest(emittedRequests[0], approved: true); + ExternalInputResponse responseA = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests, emittedRequests[0]); await action.CaptureResponseAsync(mockContext.Object, responseA, CancellationToken.None); Assert.Single(liveSnapshots); Assert.Equal([ToolA], capturedToolNames); @@ -2660,7 +2730,7 @@ public async Task InvokeMcpToolMultiplePendingInvocationsSurviveCheckpointResetR Assert.Single(liveSnapshots); // Capture B's response. - ExternalInputResponse responseB = CreateApprovalResponseForRequest(emittedRequests[1], approved: true); + ExternalInputResponse responseB = await ApproveDiscoveryAsync(action, mockContext.Object, emittedRequests, emittedRequests[1]); await action.CaptureResponseAsync(mockContext.Object, responseB, CancellationToken.None); // Assert - both invocations executed with their own approved tool names; nothing pending. @@ -2668,6 +2738,191 @@ public async Task InvokeMcpToolMultiplePendingInvocationsSurviveCheckpointResetR Assert.Empty(liveSnapshots); } + [Fact] + public async Task InvokeMcpToolEquivalentDefinitionOrderingPreservesApprovalAsync() + { + // Arrange + this.State.InitializeSystem(); + InvokeMcpTool model = this.CreateModelWithApproval( + nameof(InvokeMcpToolEquivalentDefinitionOrderingPreservesApprovalAsync), TestServerUrl, TestToolName); + string definition = """ + {"name":"test_tool","description":"Test tool","inputSchema":{"type":"object","properties":{"a":{"type":"string"},"b":{"type":"number"}}},"annotations":{"readOnlyHint":true}} + """; + List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; + Mock provider = CreateMetadataHandler(() => definition, calls); + InvokeMcpToolExecutor action = new(model, provider.Object, new MockAgentProvider().Object, this.State); + List requests = []; + Mock context = CreateMockWorkflowContext(requests); + await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); + ExternalInputResponse approvedResponse = await ApproveDiscoveryAsync(action, context.Object, requests); + definition = """ + { + "annotations": { "readOnlyHint": true }, + "inputSchema": { "properties": { "b": { "type": "number" }, "a": { "type": "string" } }, "type": "object" }, + "description": "Test tool", + "name": "test_tool" + } + """; + + // Act + await action.CaptureResponseAsync(context.Object, approvedResponse, CancellationToken.None); + + // Assert + Assert.Equal(2, requests.Count); + Assert.Single(calls, call => call.ToolName == TestToolName); + } + + [Fact] + public async Task InvokeMcpToolAnnotationChangeRequiresFreshApprovalAsync() + { + // Arrange + this.State.InitializeSystem(); + InvokeMcpTool model = this.CreateModelWithApproval( + nameof(InvokeMcpToolAnnotationChangeRequiresFreshApprovalAsync), TestServerUrl, TestToolName); + string definition = """{"name":"test_tool","annotations":{"readOnlyHint":true}}"""; + List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; + Mock provider = CreateMetadataHandler(() => definition, calls); + InvokeMcpToolExecutor action = new(model, provider.Object, new MockAgentProvider().Object, this.State); + List requests = []; + Mock context = CreateMockWorkflowContext(requests); + await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); + ExternalInputResponse approvedResponse = await ApproveDiscoveryAsync(action, context.Object, requests); + definition = """{"name":"test_tool","annotations":{"readOnlyHint":false}}"""; + + // Act + await action.CaptureResponseAsync(context.Object, approvedResponse, CancellationToken.None); + + // Assert + Assert.Equal(3, requests.Count); + Assert.DoesNotContain(calls, call => call.ToolName == TestToolName); + await action.CaptureResponseAsync(context.Object, CreateApprovalResponseForRequest(requests[2], approved: true), CancellationToken.None); + Assert.Single(calls, call => call.ToolName == TestToolName); + } + + [Fact] + public async Task InvokeMcpToolReservedDiscoveryRequiresOnlyItsOwnApprovalAsync() + { + // Arrange + this.State.InitializeSystem(); + InvokeMcpTool model = this.CreateModelWithApproval( + nameof(InvokeMcpToolReservedDiscoveryRequiresOnlyItsOwnApprovalAsync), TestServerUrl, ListToolsToolName); + List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; + Mock provider = CreateMetadataHandler(() => """{"name":"test_tool"}""", calls); + InvokeMcpToolExecutor action = new(model, provider.Object, new MockAgentProvider().Object, this.State); + List requests = []; + Mock context = CreateMockWorkflowContext(requests); + + // Act + await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); + + // Assert + Assert.Empty(calls); + await action.CaptureResponseAsync(context.Object, CreateApprovalResponseFor(requests, approved: true), CancellationToken.None); + Assert.Single(requests); + Assert.Equal(ListToolsToolName, Assert.Single(calls).ToolName); + } + + [Theory] + [InlineData(false, false, true)] + [InlineData(true, false, true)] + [InlineData(true, true, true)] + [InlineData(true, true, false)] + public async Task InvokeMcpToolDiscoveryRequiresSeparateApprovalAsync( + bool approveDiscovery, bool approveInvocation, bool configureHeaders) + { + // Arrange + this.State.Set("McpEndpoint", FormulaValue.New(TestServerUrl)); + this.State.InitializeSystem(); + this.State.Bind(); + InvokeMcpTool.Builder builder = this.CreateModelWithVariableServerUrl( + nameof(InvokeMcpToolDiscoveryRequiresSeparateApprovalAsync), "McpEndpoint", TestToolName).ToBuilder(); + builder.ServerLabel = new StringExpression.Builder(StringExpression.Literal(TestServerLabel)); + if (configureHeaders) + { + builder.Headers.Add("Authorization", new StringExpression.Builder(StringExpression.Literal("Bearer test-value"))); + } + + List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; + Mock provider = CreateMetadataHandler(() => """{"name":"test_tool"}""", calls); + InvokeMcpToolExecutor action = new(AssignParent(builder), provider.Object, new MockAgentProvider().Object, this.State); + List requests = []; + Mock context = CreateMockWorkflowContext(requests); + + // Act + await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); + + // Assert + Assert.Empty(calls); + ToolApprovalRequestContent discovery = Assert.Single(requests).AgentResponse.Messages + .SelectMany(message => message.Contents).OfType().Single(); + McpServerToolCallContent discoveryCall = Assert.IsType(discovery.ToolCall); + Assert.Equal(ListToolsToolName, discoveryCall.Name); + Assert.Equal(TestServerUrl, discoveryCall.ServerName); + string serializedApproval = JsonSerializer.Serialize(requests[0].AgentResponse); + Assert.Contains(TestServerUrl, serializedApproval); + Assert.DoesNotContain("Authorization", serializedApproval); + Assert.DoesNotContain("test-value", serializedApproval); + + this.State.Set("McpEndpoint", FormulaValue.New("https://changed.example/mcp")); + this.State.Bind(); + ExternalInputResponse discoveryResponse = CreateApprovalResponseFor(requests, approveDiscovery); + await action.CaptureResponseAsync(context.Object, discoveryResponse, CancellationToken.None); + if (!approveDiscovery) + { + await action.CaptureResponseAsync(context.Object, CreateApprovalResponseForRequest(requests[0], approved: true), CancellationToken.None); + Assert.Empty(calls); + Assert.Single(requests); + return; + } + + Assert.Equal(ListToolsToolName, Assert.Single(calls).ToolName); + Assert.Equal(2, requests.Count); + ToolApprovalRequestContent invocation = requests[1].AgentResponse.Messages + .SelectMany(message => message.Contents).OfType().Single(); + Assert.Equal(TestToolName, Assert.IsType(invocation.ToolCall).Name); + Assert.NotEqual(discovery.RequestId, invocation.RequestId); + await action.CaptureResponseAsync(context.Object, discoveryResponse, CancellationToken.None); + Assert.Single(calls); + await action.CaptureResponseAsync(context.Object, CreateApprovalResponseForRequest(requests[1], approveInvocation), CancellationToken.None); + Assert.Equal(approveInvocation ? 1 : 0, calls.Count(call => string.Equals(call.ToolName, TestToolName, StringComparison.Ordinal))); + int expectedCalls = calls.Count; + await action.CaptureResponseAsync(context.Object, CreateApprovalResponseForRequest(requests[1], approved: true), CancellationToken.None); + Assert.Equal(expectedCalls, calls.Count); + Assert.All(calls, call => + { + Assert.Equal(TestServerUrl, call.ServerUrl); + if (configureHeaders) + { + Assert.NotNull(call.Headers); + Assert.Equal("Bearer test-value", call.Headers["Authorization"]); + } + else + { + Assert.True(call.Headers is null || call.Headers.Count == 0); + } + }); + } + + private static Mock CreateMetadataHandler( + Func getDefinition, + List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls) + { + Mock provider = new(); + provider.Setup(handler => handler.InvokeToolAsync( + It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny?>(), It.IsAny?>(), + It.IsAny(), It.IsAny())) + .ReturnsAsync((string serverUrl, string? _, string toolName, IDictionary? _, IDictionary? headers, string? _, CancellationToken _) => + { + calls.Add((serverUrl, toolName, headers)); + return new McpServerToolResultContent("metadata-call") + { + Outputs = [new TextContent(string.Equals(toolName, ListToolsToolName, StringComparison.Ordinal) ? $"{{\"tools\":[{getDefinition()}]}}" : "result")] + }; + }); + return provider; + } + private InvokeMcpTool CreateModelWithApproval(string displayName, string serverUrl, string toolName) { InvokeMcpTool.Builder builder = new() @@ -2734,9 +2989,24 @@ private static Mock CreateMockWorkflowContextWithStateStore( } /// - /// Builds an approval response paired to the request id stamped on the emitted - /// ToolApprovalRequestContent. + /// Approves discovery and returns the separate invocation approval response. /// + private static async Task ApproveDiscoveryAsync( + InvokeMcpToolExecutor action, + IWorkflowContext context, + List requests, + ExternalInputRequest? discoveryRequest = null) + { + discoveryRequest ??= Assert.Single(requests); + ToolApprovalRequestContent discovery = discoveryRequest.AgentResponse.Messages + .SelectMany(message => message.Contents).OfType().Single(); + Assert.Equal(ListToolsToolName, Assert.IsType(discovery.ToolCall).Name); + int previousCount = requests.Count; + await action.CaptureResponseAsync(context, CreateApprovalResponseForRequest(discoveryRequest, approved: true), CancellationToken.None); + Assert.Equal(previousCount + 1, requests.Count); + return CreateApprovalResponseForRequest(requests[requests.Count - 1], approved: true); + } + private static ExternalInputResponse CreateApprovalResponseFor(IReadOnlyList emittedRequests, bool approved) { ExternalInputRequest emitted = Assert.Single(emittedRequests); From 5d38af4dad48fae540268205ee79a09783ed8eac Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Nov 2026 13:52:57 -0500 Subject: [PATCH 3/5] fix(dotnet): cover MCP annotation changes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 427a0082-3d8b-44df-adbf-e5407e36b6ac --- .../DefaultMcpToolHandlerTests.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index 819ad21c9c7..76394800f9a 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -164,6 +164,19 @@ public async Task CreateListToolsResultContent_WithTools_ShouldSerializeToolMeta Assert.True(listedTool.GetProperty("annotations").GetProperty("readOnlyHint").GetBoolean()); Assert.False(listedTool.GetProperty("annotations").GetProperty("destructiveHint").GetBoolean()); Assert.Equal("initial", listedTool.GetProperty("_meta").GetProperty("revision").GetString()); + + // Act + Assert.NotNull(tool.Annotations); + tool.Annotations.ReadOnlyHint = false; + McpServerToolResultContent updatedResult = DefaultMcpToolHandler.CreateListToolsResultContent([tool]); + + // Assert + TextContent updatedText = Assert.IsType(Assert.Single(updatedResult.Outputs ?? [])); + using JsonDocument updatedDocument = JsonDocument.Parse(updatedText.Text); + JsonElement updatedTool = updatedDocument.RootElement.GetProperty("tools")[0]; + Assert.False(updatedTool.GetProperty("annotations").GetProperty("readOnlyHint").GetBoolean()); + Assert.Equal(listedTool.GetProperty("inputSchema").GetRawText(), updatedTool.GetProperty("inputSchema").GetRawText()); + Assert.Equal(listedTool.GetProperty("_meta").GetRawText(), updatedTool.GetProperty("_meta").GetRawText()); } #endregion From 74e4d39e10adc7ab7681d310048835ab89e2394a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Nov 2026 14:04:43 -0500 Subject: [PATCH 4/5] fix(dotnet): simplify MCP metadata iteration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 427a0082-3d8b-44df-adbf-e5407e36b6ac --- .../ObjectModel/InvokeMcpToolExecutor.cs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs index 648db719d06..4e2155b3db0 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs @@ -411,9 +411,8 @@ private async ValueTask GetToolDefinitionFingerprintAsync( bool foundToolsArray = false; if (toolList.Outputs is not null) { - foreach (TextContent textContent in toolList.Outputs.OfType()) + foreach (string serialized in toolList.Outputs.OfType().Select(textContent => textContent.Text.TrimStart())) { - string serialized = textContent.Text.TrimStart(); if (serialized.Length == 0 || serialized[0] != '{') { continue; @@ -427,19 +426,17 @@ private async ValueTask GetToolDefinitionFingerprintAsync( } foundToolsArray = true; - foreach (JsonElement tool in tools.EnumerateArray()) + foreach (JsonElement tool in tools.EnumerateArray().Where(tool => + tool.TryGetProperty("name", out JsonElement name) && + name.ValueKind == JsonValueKind.String && + string.Equals(name.GetString(), toolName, StringComparison.Ordinal))) { - if (tool.TryGetProperty("name", out JsonElement name) && - name.ValueKind == JsonValueKind.String && - string.Equals(name.GetString(), toolName, StringComparison.Ordinal)) + if (matchingToolFingerprint is not null) { - if (matchingToolFingerprint is not null) - { - throw this.Exception($"The MCP server returned multiple definitions for tool '{toolName}'."); - } - - matchingToolFingerprint = ComputeToolDefinitionFingerprint(tool); + throw this.Exception($"The MCP server returned multiple definitions for tool '{toolName}'."); } + + matchingToolFingerprint = ComputeToolDefinitionFingerprint(tool); } } } From f61a282671f678d0a850c76b1946f7b7814623da Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Nov 2026 14:25:47 -0500 Subject: [PATCH 5/5] fix(dotnet): validate MCP tool input schemas Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 427a0082-3d8b-44df-adbf-e5407e36b6ac --- .../IMcpToolHandler.cs | 6 +- .../ObjectModel/InvokeMcpToolExecutor.cs | 9 +++ .../ObjectModel/InvokeMcpToolExecutorTest.cs | 59 ++++++++++++++++--- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs index ac6ad2a6c87..b57240a758d 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs @@ -15,9 +15,9 @@ namespace Microsoft.Agents.AI.Workflows.Declarative; /// different implementations for local development, hosted workflows, and testing scenarios. /// Implementations used by approval-gated MCP actions must support the reserved tools/list operation /// and return a text output containing a structured tools array with complete published tool definitions, -/// including annotations and metadata. Discovery is invoked only after explicit discovery approval; -/// a separate approval binds the tool invocation to its definition. Approval fails closed when the definition -/// cannot be verified. +/// including annotations and metadata. The requested tool must include an inputSchema object whose +/// type is object. Discovery is invoked only after explicit discovery approval; a separate +/// approval binds the tool invocation to its definition. Approval fails closed when the definition cannot be verified. /// public interface IMcpToolHandler { diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs index 4e2155b3db0..b17e363502d 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs @@ -436,6 +436,15 @@ private async ValueTask GetToolDefinitionFingerprintAsync( throw this.Exception($"The MCP server returned multiple definitions for tool '{toolName}'."); } + if (!tool.TryGetProperty("inputSchema", out JsonElement inputSchema) || + inputSchema.ValueKind != JsonValueKind.Object || + !inputSchema.TryGetProperty("type", out JsonElement schemaType) || + schemaType.ValueKind != JsonValueKind.String || + !string.Equals(schemaType.GetString(), "object", StringComparison.Ordinal)) + { + throw this.Exception($"The MCP server returned an invalid input schema for tool '{toolName}'."); + } + matchingToolFingerprint = ComputeToolDefinitionFingerprint(tool); } } diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs index 9a4028f12ff..4ccbe178592 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs @@ -1253,6 +1253,51 @@ Task ExecuteAsync() => action.CaptureResponseAsync( Assert.Equal(0, invocationCount); } + [Theory] + [InlineData("""{"name":"test_tool"}""", false)] + [InlineData("""{"name":"test_tool","inputSchema":null}""", false)] + [InlineData("""{"name":"test_tool","inputSchema":[]}""", false)] + [InlineData("""{"name":"test_tool","inputSchema":{}}""", false)] + [InlineData("""{"name":"test_tool","inputSchema":{"type":"array"}}""", false)] + [InlineData("""{"name":"test_tool","inputSchema":{"type":null}}""", false)] + [InlineData("""{"name":"test_tool"}""", true)] + [InlineData("""{"name":"test_tool","inputSchema":null}""", true)] + [InlineData("""{"name":"test_tool","inputSchema":[]}""", true)] + [InlineData("""{"name":"test_tool","inputSchema":{}}""", true)] + [InlineData("""{"name":"test_tool","inputSchema":{"type":"array"}}""", true)] + [InlineData("""{"name":"test_tool","inputSchema":{"type":null}}""", true)] + public async Task InvokeMcpToolRejectsInvalidInputSchemaAsync(string invalidDefinition, bool isRevalidation) + { + // Arrange + this.State.InitializeSystem(); + InvokeMcpTool model = this.CreateModelWithApproval( + nameof(InvokeMcpToolRejectsInvalidInputSchemaAsync), TestServerUrl, TestToolName); + string definition = isRevalidation + ? """{"name":"test_tool","inputSchema":{"type":"object"}}""" + : invalidDefinition; + List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; + Mock provider = CreateMetadataHandler(() => definition, calls); + InvokeMcpToolExecutor action = new(model, provider.Object, new MockAgentProvider().Object, this.State); + List requests = []; + Mock context = CreateMockWorkflowContext(requests); + await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); + ExternalInputResponse approvedResponse = isRevalidation + ? await ApproveDiscoveryAsync(action, context.Object, requests) + : CreateApprovalResponseFor(requests, approved: true); + definition = invalidDefinition; + + // Act + Exception? error = await Record.ExceptionAsync( + () => action.CaptureResponseAsync(context.Object, approvedResponse, CancellationToken.None).AsTask()); + + // Assert + Assert.DoesNotContain(calls, call => string.Equals(call.ToolName, TestToolName, StringComparison.Ordinal)); + DeclarativeActionException exception = Assert.IsType(error); + Assert.Contains("input schema", exception.Message); + Assert.Equal(isRevalidation ? 2 : 1, requests.Count); + Assert.Equal(isRevalidation ? 2 : 1, calls.Count); + } + [Fact] public async Task InvokeMcpToolCaptureResponseUsesApprovedHeadersNotMutatedAsync() { @@ -2464,7 +2509,7 @@ public async Task InvokeMcpToolRestoredLegacyApprovalRequiresDiscoveryConsentAsy InvokeMcpTool model = this.CreateModelWithApproval( nameof(InvokeMcpToolRestoredLegacyApprovalRequiresDiscoveryConsentAsync), TestServerUrl, TestToolName); List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; - Mock provider = CreateMetadataHandler(() => """{"name":"test_tool"}""", calls); + Mock provider = CreateMetadataHandler(() => """{"name":"test_tool","inputSchema":{"type":"object"}}""", calls); InvokeMcpToolExecutor action = new(model, provider.Object, new MockAgentProvider().Object, this.State); List requests = []; Dictionary stateStore = new() @@ -2779,7 +2824,7 @@ public async Task InvokeMcpToolAnnotationChangeRequiresFreshApprovalAsync() this.State.InitializeSystem(); InvokeMcpTool model = this.CreateModelWithApproval( nameof(InvokeMcpToolAnnotationChangeRequiresFreshApprovalAsync), TestServerUrl, TestToolName); - string definition = """{"name":"test_tool","annotations":{"readOnlyHint":true}}"""; + string definition = """{"name":"test_tool","inputSchema":{"type":"object"},"annotations":{"readOnlyHint":true}}"""; List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; Mock provider = CreateMetadataHandler(() => definition, calls); InvokeMcpToolExecutor action = new(model, provider.Object, new MockAgentProvider().Object, this.State); @@ -2787,7 +2832,7 @@ public async Task InvokeMcpToolAnnotationChangeRequiresFreshApprovalAsync() Mock context = CreateMockWorkflowContext(requests); await action.HandleAsync(new ActionExecutorResult(action.Id), context.Object, CancellationToken.None); ExternalInputResponse approvedResponse = await ApproveDiscoveryAsync(action, context.Object, requests); - definition = """{"name":"test_tool","annotations":{"readOnlyHint":false}}"""; + definition = """{"name":"test_tool","inputSchema":{"type":"object"},"annotations":{"readOnlyHint":false}}"""; // Act await action.CaptureResponseAsync(context.Object, approvedResponse, CancellationToken.None); @@ -2807,7 +2852,7 @@ public async Task InvokeMcpToolReservedDiscoveryRequiresOnlyItsOwnApprovalAsync( InvokeMcpTool model = this.CreateModelWithApproval( nameof(InvokeMcpToolReservedDiscoveryRequiresOnlyItsOwnApprovalAsync), TestServerUrl, ListToolsToolName); List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; - Mock provider = CreateMetadataHandler(() => """{"name":"test_tool"}""", calls); + Mock provider = CreateMetadataHandler(() => """{"name":"test_tool","inputSchema":{"type":"object"}}""", calls); InvokeMcpToolExecutor action = new(model, provider.Object, new MockAgentProvider().Object, this.State); List requests = []; Mock context = CreateMockWorkflowContext(requests); @@ -2843,7 +2888,7 @@ public async Task InvokeMcpToolDiscoveryRequiresSeparateApprovalAsync( } List<(string ServerUrl, string ToolName, IDictionary? Headers)> calls = []; - Mock provider = CreateMetadataHandler(() => """{"name":"test_tool"}""", calls); + Mock provider = CreateMetadataHandler(() => """{"name":"test_tool","inputSchema":{"type":"object"}}""", calls); InvokeMcpToolExecutor action = new(AssignParent(builder), provider.Object, new MockAgentProvider().Object, this.State); List requests = []; Mock context = CreateMockWorkflowContext(requests); @@ -3314,7 +3359,7 @@ private void SetupToolList() { string definitions = string.Join( ",", - s_toolNames.Select(name => $"{{\"name\":{JsonSerializer.Serialize(name)},\"description\":\"Test tool\"}}")); + s_toolNames.Select(name => $"{{\"name\":{JsonSerializer.Serialize(name)},\"description\":\"Test tool\",\"inputSchema\":{{\"type\":\"object\"}}}}")); string toolList = $"{{\"tools\":[{definitions}]}}"; base.Setup(provider => provider.InvokeToolAsync( @@ -3364,7 +3409,7 @@ public MockMcpToolProvider( result.Outputs = [ new TextContent( - $"{{\"tools\":[{{\"name\":{JsonSerializer.Serialize(TestToolName)},\"description\":\"Test tool\"}}]}}") + $"{{\"tools\":[{{\"name\":{JsonSerializer.Serialize(TestToolName)},\"description\":\"Test tool\",\"inputSchema\":{{\"type\":\"object\"}}}}]}}") ]; } else if (returnNullOutput)