diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c883638..8e957a25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,11 @@ Both `Agent365.Observability.OtelWrite` (Delegated) and `Agent365.Observability. ## [Unreleased] ### Added +- **Microsoft.Agents.A365.Tooling** - MCP connection-readiness gating + - Parses per-server and aggregate connection metadata (`allConnectionsUrl`, `missingConnectionsUrl`, `connectivityStatus`) from the tooling gateway discovery response, supporting both the legacy bare-array and wrapped `{ mcpServers, ... }` shapes + - `ListToolServersAsync` now throws the new public `McpConnectionsRequiredException` (exposing `MissingConnectionsUrl`, `ConnectivityStatus`, and `ServerNames`) when the aggregate connectivity status is present and not `Ready`; legacy responses and dev manifests are never gated + - The exception propagates through `EnumerateToolsFromServersAsync` / `EnumerateAllToolsAsync` and the framework extensions so callers can surface the setup URL to the user + - `MCPServerConfig` extended with `allConnectionsUrl`, `missingConnectionsUrl`, and `connectivityStatus` - **Microsoft.Agents.A365.Tooling** - V1/V2 per-audience token support for MCP servers - `MCPServerConfig` extended with `audience`, `scope`, `publisher`, and `Headers` fields - `IMcpTokenProvider` interface for pluggable OAuth token acquisition diff --git a/src/Tests/Microsoft.Agents.A365.Tooling.Core.Tests/McpToolServerConfigurationService_ConnectionGatingTests.cs b/src/Tests/Microsoft.Agents.A365.Tooling.Core.Tests/McpToolServerConfigurationService_ConnectionGatingTests.cs new file mode 100644 index 00000000..0990e4f4 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.Tooling.Core.Tests/McpToolServerConfigurationService_ConnectionGatingTests.cs @@ -0,0 +1,382 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Net; +using System.Net.Http; +using System.Text.Json; +using FluentAssertions; +using Microsoft.Agents.A365.Tooling.Models; +using Microsoft.Agents.A365.Tooling.Services; +using Microsoft.Agents.Builder; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Microsoft.Agents.A365.Tooling.Core.Tests; + +/// +/// Tests for MCP connection gating: gateway response parsing (wrapped + legacy), aggregate/per-server +/// connection metadata, and the raised by +/// ListToolServersAsync when configured servers are not connection-ready. +/// +public class McpToolServerConfigurationService_ConnectionGatingTests +{ + private const string WrappedPendingJson = """ + { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "id": "id-1", + "url": "https://s/mcp_Salesforce", + "scope": "McpServers.Salesforce.All", + "audience": "", + "allConnectionsUrl": "https://all/sf", + "missingConnectionsUrl": "https://missing/sf", + "connectivityStatus": "Pending" + } + ], + "allConnectionsUrl": "https://all", + "missingConnectionsUrl": "https://missing", + "connectivityStatus": "Pending" + } + """; + + private const string WrappedReadyJson = """ + { + "mcpServers": [ + { + "mcpServerName": "s", + "id": "id-1", + "url": "https://s", + "allConnectionsUrl": "https://all", + "missingConnectionsUrl": "https://missing", + "connectivityStatus": "Ready" + } + ], + "allConnectionsUrl": "https://all", + "missingConnectionsUrl": "https://missing", + "connectivityStatus": "Ready" + } + """; + + private const string LegacyArrayJson = """ + [ { "mcpServerName": "s", "id": "id-1", "url": "https://s" } ] + """; + + // ─── ParseGatewayResponse ──────────────────────────────────────────────── + + [Fact] + public void ParseGatewayResponse_Wrapped_ParsesServersAndAggregateMetadata() + { + var result = ParseGateway(WrappedPendingJson); + + var server = result.Servers.Should().ContainSingle().Subject; + server.mcpServerName.Should().Be("mcp_Salesforce"); + server.allConnectionsUrl.Should().Be("https://all/sf"); + server.missingConnectionsUrl.Should().Be("https://missing/sf"); + server.connectivityStatus.Should().Be("Pending"); + + result.AllConnectionsUrl.Should().Be("https://all"); + result.MissingConnectionsUrl.Should().Be("https://missing"); + result.ConnectivityStatus.Should().Be("Pending"); + } + + [Fact] + public void ParseGatewayResponse_Ready_NullsMissingConnectionsUrl() + { + var result = ParseGateway(WrappedReadyJson); + + var server = result.Servers.Should().ContainSingle().Subject; + server.connectivityStatus.Should().Be("Ready"); + server.missingConnectionsUrl.Should().BeNull(); + + result.ConnectivityStatus.Should().Be("Ready"); + result.MissingConnectionsUrl.Should().BeNull(); + result.AllConnectionsUrl.Should().Be("https://all"); + } + + [Fact] + public void ParseGatewayResponse_LegacyArray_ReturnsServersWithoutAggregate() + { + var result = ParseGateway(LegacyArrayJson); + + result.Servers.Should().ContainSingle(); + result.ConnectivityStatus.Should().BeNull(); + result.AllConnectionsUrl.Should().BeNull(); + result.MissingConnectionsUrl.Should().BeNull(); + } + + [Fact] + public void ParseGatewayResponse_EmptyMcpServers_ReturnsEmptyWithAggregate() + { + var result = ParseGateway("""{ "mcpServers": [], "connectivityStatus": "Ready" }"""); + + result.Servers.Should().BeEmpty(); + result.ConnectivityStatus.Should().Be("Ready"); + } + + [Fact] + public void ParseGatewayResponse_UnexpectedStructure_Throws() + { + Action act = () => ParseGateway("""{ "foo": 1 }"""); + + act.Should().Throw().WithMessage("*Unexpected JSON structure*"); + } + + // ─── EnforceConnectionReadiness ────────────────────────────────────────── + + [Fact] + public void EnforceConnectionReadiness_Pending_ThrowsWithDetails() + { + var service = CreateService(); + var discovery = new McpDiscoveryResult( + new List + { + new() { mcpServerName = "pending-srv", id = "id-1", url = "http://p", connectivityStatus = "Pending" }, + new() { mcpServerName = "ready-srv", id = "id-2", url = "http://r", connectivityStatus = "Ready" }, + }, + allConnectionsUrl: "https://all", + missingConnectionsUrl: "https://missing", + connectivityStatus: "Pending"); + + var act = () => service.EnforceConnectionReadiness(discovery); + + var ex = act.Should().Throw().Which; + ex.ConnectivityStatus.Should().Be("Pending"); + ex.MissingConnectionsUrl.Should().Be("https://missing"); + ex.ServerNames.Should().ContainSingle().Which.Should().Be("pending-srv"); + } + + [Fact] + public void EnforceConnectionReadiness_Ready_DoesNotThrow() + { + var service = CreateService(); + var discovery = new McpDiscoveryResult(new List(), connectivityStatus: "Ready"); + + var act = () => service.EnforceConnectionReadiness(discovery); + + act.Should().NotThrow(); + } + + [Fact] + public void EnforceConnectionReadiness_NullStatus_DoesNotThrow() + { + var service = CreateService(); + var discovery = new McpDiscoveryResult(new List(), connectivityStatus: null); + + var act = () => service.EnforceConnectionReadiness(discovery); + + act.Should().NotThrow(); + } + + [Fact] + public void EnforceConnectionReadiness_UnknownStatus_Throws() + { + var service = CreateService(); + var discovery = new McpDiscoveryResult(new List(), connectivityStatus: "Frozen"); + + var act = () => service.EnforceConnectionReadiness(discovery); + + act.Should().Throw() + .Which.ConnectivityStatus.Should().Be("Frozen"); + } + + [Fact] + public void EnforceConnectionReadiness_AggregateReadyButPerServerPending_DoesNotThrow() + { + // Gating is aggregate-only by design: a per-server Pending does not block the turn when the + // gateway reports the aggregate connectivity status as Ready. + var service = CreateService(); + var discovery = new McpDiscoveryResult( + new List + { + new() { mcpServerName = "srv", id = "id-1", url = "http://s", connectivityStatus = "Pending" }, + }, + connectivityStatus: "Ready"); + + var act = () => service.EnforceConnectionReadiness(discovery); + + act.Should().NotThrow(); + } + + [Theory] + [InlineData("READY")] + [InlineData(" ready ")] + public void EnforceConnectionReadiness_ReadyStatusVariants_DoNotThrow(string status) + { + // Pins IsReadyStatus: comparison is case-insensitive and whitespace-tolerant. + var service = CreateService(); + var discovery = new McpDiscoveryResult(new List(), connectivityStatus: status); + + var act = () => service.EnforceConnectionReadiness(discovery); + + act.Should().NotThrow(); + } + + [Fact] + public void EnforceConnectionReadiness_PendingWithoutPerServerStatus_ThrowsWithEmptyServerNames() + { + var service = CreateService(); + var discovery = new McpDiscoveryResult( + new List + { + new() { mcpServerName = "srv", id = "id-1", url = "http://s" }, + }, + connectivityStatus: "Pending"); + + var act = () => service.EnforceConnectionReadiness(discovery); + + var ex = act.Should().Throw().Which; + ex.ServerNames.Should().BeEmpty(); + ex.Message.Should().Contain("(unknown)"); + } + + [Fact] + public void EnforceConnectionReadiness_PendingWithoutMissingUrl_ThrowsWithNullUrl() + { + // Every consumer's handler must survive a null MissingConnectionsUrl. + var service = CreateService(); + var discovery = new McpDiscoveryResult( + new List + { + new() { mcpServerName = "srv", id = "id-1", url = "http://s", connectivityStatus = "Pending" }, + }, + connectivityStatus: "Pending"); + + var act = () => service.EnforceConnectionReadiness(discovery); + + act.Should().Throw() + .Which.MissingConnectionsUrl.Should().BeNull(); + } + + // ─── ListToolServersAsync (end-to-end via gateway) ─────────────────────── + + [Fact] + public async Task ListToolServersAsync_PendingGateway_ThrowsConnectionsRequired() + { + var service = CreateService(Respond(WrappedPendingJson)); + + var ex = await Assert.ThrowsAsync( + () => service.ListToolServersAsync("agent-123", "tok", new ToolOptions())); + + ex.ConnectivityStatus.Should().Be("Pending"); + ex.MissingConnectionsUrl.Should().Be("https://missing"); + ex.ServerNames.Should().Contain("mcp_Salesforce"); + } + + [Fact] + public async Task ListToolServersAsync_ReadyGateway_ReturnsServers() + { + var service = CreateService(Respond(WrappedReadyJson)); + + var servers = await service.ListToolServersAsync("agent-123", "tok", new ToolOptions()); + + var server = servers.Should().ContainSingle().Subject; + server.connectivityStatus.Should().Be("Ready"); + server.missingConnectionsUrl.Should().BeNull(); + server.allConnectionsUrl.Should().Be("https://all"); + } + + [Fact] + public async Task ListToolServersAsync_LegacyArrayGateway_ReturnsServersWithoutGating() + { + var service = CreateService(Respond(LegacyArrayJson)); + + var servers = await service.ListToolServersAsync("agent-123", "tok", new ToolOptions()); + + servers.Should().ContainSingle(); + } + + // ─── Exception propagation through enumeration ─────────────────────────── + + [Fact] + public async Task EnumerateToolsFromServersAsync_NoTokenProvider_PropagatesConnectionsRequired() + { + var mockService = CreatePartialMock(); + mockService + .Setup(x => x.ListToolServersAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new McpConnectionsRequiredException("https://missing", "Pending", new[] { "srv" })); + + await Assert.ThrowsAsync(() => + mockService.Object.EnumerateToolsFromServersAsync( + "agent-id", "auth-token", new Mock().Object, new ToolOptions())); + } + + [Fact] + public async Task EnumerateToolsFromServersAsync_WithTokenProvider_PropagatesConnectionsRequired() + { + var mockService = CreatePartialMock(); + mockService + .Setup(x => x.ListToolServersAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new McpConnectionsRequiredException("https://missing", "Pending", new[] { "srv" })); + + await Assert.ThrowsAsync(() => + mockService.Object.EnumerateToolsFromServersAsync( + "agent-id", "auth-token", new Mock().Object, + new Mock().Object, new ToolOptions())); + } + + [Fact] + public async Task EnumerateToolsFromServersAsync_GenericFailure_StillReturnsEmpty() + { + // Regression guard: non-gating failures remain swallowed (return empty) — unchanged behavior. + var mockService = CreatePartialMock(); + mockService + .Setup(x => x.ListToolServersAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new Exception("network")); + + var (servers, toolsByServer) = await mockService.Object.EnumerateToolsFromServersAsync( + "agent-id", "auth-token", new Mock().Object, new ToolOptions()); + + servers.Should().BeEmpty(); + toolsByServer.Should().BeEmpty(); + } + + // ─── Helpers ───────────────────────────────────────────────────────────── + + private static McpDiscoveryResult ParseGateway(string json) + { + using var doc = JsonDocument.Parse(json); + return McpToolServerConfigurationService.ParseGatewayResponse(doc.RootElement); + } + + private static Mock CreatePartialMock() => + new Mock( + MockBehavior.Default, + new Mock>().Object, + new Mock().Object, + new Mock().Object, + new Mock().Object) + { CallBase = true }; + + private static FakeHttpMessageHandler Respond(string json) => + new(_ => new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(json) }); + + private static McpToolServerConfigurationService CreateService(FakeHttpMessageHandler? handler = null) + { + var config = new Mock(); + config.Setup(c => c["MCP_PLATFORM_ENDPOINT"]).Returns("https://test.endpoint"); + + handler ??= Respond("[]"); + var httpClient = new HttpClient(handler); + var factory = new Mock(); + factory.Setup(f => f.CreateClient(It.IsAny())).Returns(httpClient); + + return new McpToolServerConfigurationService( + new Mock>().Object, + config.Object, + new Mock().Object, + factory.Object); + } + + private sealed class FakeHttpMessageHandler : HttpMessageHandler + { + private readonly Func _responder; + + public FakeHttpMessageHandler(Func responder) => _responder = responder; + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => Task.FromResult(_responder(request)); + } +} diff --git a/src/Tooling/Core/Exceptions/McpConnectionsRequiredException.cs b/src/Tooling/Core/Exceptions/McpConnectionsRequiredException.cs new file mode 100644 index 00000000..637ab58d --- /dev/null +++ b/src/Tooling/Core/Exceptions/McpConnectionsRequiredException.cs @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Agents.A365.Tooling +{ + using System; + using System.Collections.Generic; + + /// + /// Thrown when one or more configured MCP servers are not yet connection-ready. + /// The tooling gateway reports an aggregate connectivity status other than Ready + /// (for example, Pending) when the agent's MCP servers have downstream connections that the + /// user has not yet established. Callers should surface to the + /// user to complete setup, then retry on a later turn once the connections are in place. + /// + public class McpConnectionsRequiredException : Exception + { + /// + /// Initializes a new instance of the class. + /// + /// URL the user can visit to set up the missing connections, when provided by the gateway. + /// The aggregate connectivity status reported by the gateway (for example, Pending). + /// The names of the MCP servers that are not yet connection-ready. + public McpConnectionsRequiredException( + string? missingConnectionsUrl, + string? connectivityStatus, + IReadOnlyList serverNames) + : base(BuildMessage(missingConnectionsUrl, connectivityStatus, serverNames)) + { + this.MissingConnectionsUrl = missingConnectionsUrl; + this.ConnectivityStatus = connectivityStatus; + this.ServerNames = serverNames; + } + + /// Gets the URL the user can visit to set up the missing connections, or null when not provided. + public string? MissingConnectionsUrl { get; } + + /// Gets the aggregate connectivity status reported by the gateway, or null when not provided. + public string? ConnectivityStatus { get; } + + /// + /// Gets the names of the MCP servers that are not yet connection-ready. May be empty when the + /// gateway reports only an aggregate connectivity status without per-server detail. + /// + public IReadOnlyList ServerNames { get; } + + private static string BuildMessage(string? missingConnectionsUrl, string? connectivityStatus, IReadOnlyList serverNames) + { + string serversText = serverNames is { Count: > 0 } ? string.Join(", ", serverNames) : "(unknown)"; + return $"MCP servers [{serversText}] require connection setup (connectivityStatus={connectivityStatus}). " + + $"Set up missing connections at: {missingConnectionsUrl}"; + } + } +} diff --git a/src/Tooling/Core/Models/MCPServerConfig.cs b/src/Tooling/Core/Models/MCPServerConfig.cs index eb65c727..2d499e4d 100644 --- a/src/Tooling/Core/Models/MCPServerConfig.cs +++ b/src/Tooling/Core/Models/MCPServerConfig.cs @@ -42,6 +42,26 @@ public class MCPServerConfig /// public string? publisher { get; set; } + /// + /// Gets or sets the URL containing the deduped union of all connectors required for this MCP + /// server to operate. Populated by the discovery endpoint (discoverMCPServers); null + /// when not supplied (for example, on the basic server-list path). + /// + public string? allConnectionsUrl { get; set; } + + /// + /// Gets or sets the URL containing the deduped union of only the connectors required for this + /// MCP server that the connected user has not yet set up. Null when connectivityStatus + /// is Ready (no setup needed) or when not supplied. + /// + public string? missingConnectionsUrl { get; set; } + + /// + /// Gets or sets the connectivity status for this MCP server (for example, Ready when all + /// required connectors are already connected). Compare case-insensitively. Null when not supplied. + /// + public string? connectivityStatus { get; set; } + /// /// Gets or sets per-server HTTP headers, including the Authorization header populated /// by AttachPerAudienceTokensAsync before tool connections are established. diff --git a/src/Tooling/Core/Models/McpDiscoveryResult.cs b/src/Tooling/Core/Models/McpDiscoveryResult.cs new file mode 100644 index 00000000..1c63f2cc --- /dev/null +++ b/src/Tooling/Core/Models/McpDiscoveryResult.cs @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Agents.A365.Tooling.Models +{ + using System.Collections.Generic; + + /// + /// Internal result of MCP server discovery from the tooling gateway. Carries the parsed server + /// list plus the response-level (aggregate) connection metadata used for connection gating. + /// Sources that predate the connection fields (legacy bare-array gateway responses, dev-mode + /// manifests) leave the aggregate fields null. + /// + internal sealed class McpDiscoveryResult + { + /// + /// Initializes a new instance of the class. + /// + /// The parsed MCP server configurations. + /// Aggregate URL for all required connectors, or null. + /// Aggregate URL for missing connectors, or null. + /// Aggregate connectivity status, or null when not supplied. + public McpDiscoveryResult( + List servers, + string? allConnectionsUrl = null, + string? missingConnectionsUrl = null, + string? connectivityStatus = null) + { + this.Servers = servers; + this.AllConnectionsUrl = allConnectionsUrl; + this.MissingConnectionsUrl = missingConnectionsUrl; + this.ConnectivityStatus = connectivityStatus; + } + + /// Gets the parsed MCP server configurations. + public List Servers { get; } + + /// Gets the aggregate URL covering all connectors required across all servers, or null. + public string? AllConnectionsUrl { get; } + + /// Gets the aggregate URL covering only the connectors still missing across all servers, or null. + public string? MissingConnectionsUrl { get; } + + /// Gets the aggregate connectivity status across all servers, or null when not supplied. + public string? ConnectivityStatus { get; } + } +} diff --git a/src/Tooling/Core/Services/IMcpToolServerConfigurationService.cs b/src/Tooling/Core/Services/IMcpToolServerConfigurationService.cs index 9a7f2515..87b3d444 100644 --- a/src/Tooling/Core/Services/IMcpToolServerConfigurationService.cs +++ b/src/Tooling/Core/Services/IMcpToolServerConfigurationService.cs @@ -19,6 +19,7 @@ public interface IMcpToolServerConfigurationService /// Agent instance Id for the agent. /// Auth token to access the MCP servers /// Returns the list of MCP Servers that are configured. + /// Thrown when configured MCP servers report missing downstream connections (aggregate connectivity status other than "Ready"). Task> ListToolServersAsync(string agentInstanceId, string authToken); /// @@ -28,6 +29,7 @@ public interface IMcpToolServerConfigurationService /// Auth token to access the MCP servers /// Tool options for listing servers. /// Returns the list of MCP Servers that are configured. + /// Thrown when configured MCP servers report missing downstream connections (aggregate connectivity status other than "Ready"). Task> ListToolServersAsync(string agentInstanceId, string authToken, ToolOptions toolOptions); /// @@ -82,6 +84,7 @@ public interface IMcpToolServerConfigurationService /// Turn context for the current request. /// Tool options including user agent configuration. /// A tuple containing server configurations and a dictionary mapping server names to their available tools. + /// Thrown when configured MCP servers report missing downstream connections (aggregate connectivity status other than "Ready"). Task<(List Servers, Dictionary> ToolsByServer)> EnumerateToolsFromServersAsync(string agentInstanceId, string authToken, ITurnContext turnContext, ToolOptions toolOptions); /// @@ -95,6 +98,7 @@ public interface IMcpToolServerConfigurationService /// Turn context for the current request. /// Tool options including user agent configuration. /// A tuple containing server configurations and a dictionary mapping server names to their available tools. + /// Thrown when configured MCP servers report missing downstream connections (aggregate connectivity status other than "Ready"). Task<(List Servers, Dictionary> ToolsByServer)> EnumerateToolsFromServersAsync(string agentInstanceId, string authToken, IMcpTokenProvider tokenProvider, ITurnContext turnContext, ToolOptions toolOptions); /// @@ -105,6 +109,7 @@ public interface IMcpToolServerConfigurationService /// Turn context for the current request. /// Tool options including user agent configuration. /// A flat list of all MCP tools from all configured servers. + /// Thrown when configured MCP servers report missing downstream connections (aggregate connectivity status other than "Ready"). Task> EnumerateAllToolsAsync(string agentInstanceId, string authToken, ITurnContext turnContext, ToolOptions toolOptions); } } diff --git a/src/Tooling/Core/Services/McpToolServerConfigurationService.ToolEnumeration.cs b/src/Tooling/Core/Services/McpToolServerConfigurationService.ToolEnumeration.cs index bc6e7044..ce579254 100644 --- a/src/Tooling/Core/Services/McpToolServerConfigurationService.ToolEnumeration.cs +++ b/src/Tooling/Core/Services/McpToolServerConfigurationService.ToolEnumeration.cs @@ -38,6 +38,12 @@ public partial class McpToolServerConfigurationService tokenProvider, toolOptions).ConfigureAwait(false); } + catch (McpConnectionsRequiredException) + { + // Connection-readiness gating must reach the agent's turn handler so it can prompt + // the user with the setup URL. Never swallow it as a generic listing failure. + throw; + } catch (Exception ex) { _logger.LogError(ex, "Failed to list MCP tool servers for AgentInstanceId={AgentInstanceId}", agentInstanceId); @@ -131,6 +137,12 @@ ex is TaskCanceledException || authToken, toolOptions).ConfigureAwait(false); } + catch (McpConnectionsRequiredException) + { + // Connection-readiness gating must reach the agent's turn handler so it can prompt + // the user with the setup URL. Never swallow it as a generic listing failure. + throw; + } catch (Exception ex) { _logger.LogError(ex, "Failed to list MCP tool servers for AgentInstanceId={AgentInstanceId}", agentInstanceId); diff --git a/src/Tooling/Core/Services/McpToolServerConfigurationService.cs b/src/Tooling/Core/Services/McpToolServerConfigurationService.cs index f5179dab..5e7b2821 100644 --- a/src/Tooling/Core/Services/McpToolServerConfigurationService.cs +++ b/src/Tooling/Core/Services/McpToolServerConfigurationService.cs @@ -28,6 +28,11 @@ namespace Microsoft.Agents.A365.Tooling.Services /// public partial class McpToolServerConfigurationService : IMcpToolServerConfigurationService { + /// + /// The connectivity status value indicating that all required connectors are already connected. + /// + private const string ReadyConnectivityStatus = "Ready"; + private readonly ILogger _logger; private readonly IConfiguration _configuration; private readonly ILoggerFactory? _loggerFactory; @@ -57,7 +62,19 @@ public virtual async Task> ListToolServersAsync(string age /// public virtual async Task> ListToolServersAsync(string agentInstanceId, string authToken, ToolOptions toolOptions) { - return IsDevScenario() ? GetMCPServersFromManifest() : await GetMCPServerFromToolingGatewayAsync(agentInstanceId, authToken, toolOptions); + if (IsDevScenario()) + { + // Dev manifests carry no connection metadata, so connection gating never applies. + return GetMCPServersFromManifest(); + } + + McpDiscoveryResult discovery = await GetMCPServerFromToolingGatewayAsync(agentInstanceId, authToken, toolOptions); + + // Gate execution when configured MCP servers are not connection-ready. Runs before token + // attachment because readiness is independent of tokens. + EnforceConnectionReadiness(discovery); + + return discovery.Servers; } /// @@ -177,7 +194,7 @@ public async Task SendChatHistoryAsync(ITurnContext turnContext } } - private async Task> GetMCPServerFromToolingGatewayAsync( + private async Task GetMCPServerFromToolingGatewayAsync( string agentInstanceId, string authToken, ToolOptions toolOptions) { string configEndpoint = Utility.GetToolingGatewayForDigitalWorker(agentInstanceId, this._configuration); @@ -196,28 +213,11 @@ private async Task> GetMCPServerFromToolingGatewayAsync( var response = await httpClient.GetStringAsync(configEndpoint); - var options = new JsonSerializerOptions - { - PropertyNameCaseInsensitive = true - }; + // PropertyNameCaseInsensitive does not affect JsonElement materialization, so parse + // into a JsonElement directly; case-insensitive lookups are handled in ParseGatewayResponse. + var jsonDoc = JsonSerializer.Deserialize(response); - // Single parse approach - var jsonDoc = JsonSerializer.Deserialize(response, options); - - IEnumerable serverElements = jsonDoc.ValueKind switch - { - JsonValueKind.Array => jsonDoc.EnumerateArray(), - JsonValueKind.Object when jsonDoc.TryGetProperty("mcpServers", out var servers) - && servers.ValueKind == JsonValueKind.Array - => servers.EnumerateArray(), - _ => throw new InvalidOperationException( - $"Unexpected JSON structure. Expected array or object with 'mcpServers' property, got {jsonDoc.ValueKind}") - }; - - return serverElements - .Select(ParseServerConfig) - .Where(config => config != null) - .ToList()!; + return ParseGatewayResponse(jsonDoc); } catch (HttpRequestException httpEx) { @@ -285,6 +285,14 @@ JsonValueKind.Object when jsonDoc.TryGetProperty("mcpServers", out var servers) publisher = publisherElement.GetString(); } + string? allConnectionsUrl = GetStringPropertyCaseInsensitive(serverElement, "allConnectionsUrl"); + string? missingConnectionsUrl = GetStringPropertyCaseInsensitive(serverElement, "missingConnectionsUrl"); + string? connectivityStatus = GetStringPropertyCaseInsensitive(serverElement, "connectivityStatus"); + if (IsReadyStatus(connectivityStatus)) + { + missingConnectionsUrl = null; + } + // Both Name and Endpoint are required if (string.IsNullOrWhiteSpace(name) || string.IsNullOrWhiteSpace(endpoint)) { @@ -298,7 +306,10 @@ JsonValueKind.Object when jsonDoc.TryGetProperty("mcpServers", out var servers) id = id ?? string.Empty, scope = scope, audience = audience, - publisher = publisher + publisher = publisher, + allConnectionsUrl = allConnectionsUrl, + missingConnectionsUrl = missingConnectionsUrl, + connectivityStatus = connectivityStatus }; } catch (Exception) @@ -308,6 +319,136 @@ JsonValueKind.Object when jsonDoc.TryGetProperty("mcpServers", out var servers) } } + /// + /// Raises when the aggregate connectivity status + /// indicates that one or more required downstream connections are missing. + /// + /// The discovery result carrying aggregate and per-server status. + /// + /// Blocks only when the response-level connectivity status is present and not Ready + /// (for example, Pending). Absent status (legacy raw-array gateway responses and + /// dev-mode manifests) is always treated as ready, so those paths are never gated. The + /// non-Ready check is intentionally defensive against any unexpected future status value. + /// + internal void EnforceConnectionReadiness(McpDiscoveryResult discovery) + { + string? status = discovery.ConnectivityStatus; + if (string.IsNullOrWhiteSpace(status) || IsReadyStatus(status)) + { + return; + } + + List serverNames = discovery.Servers + .Where(s => !string.IsNullOrWhiteSpace(s.connectivityStatus) && !IsReadyStatus(s.connectivityStatus)) + .Select(s => string.IsNullOrEmpty(s.mcpServerName) ? s.id : s.mcpServerName) + .ToList(); + + this._logger.LogWarning( + "MCP connection gate blocking turn: connectivityStatus={ConnectivityStatus}, servers={ServerNames}", + status, + string.Join(", ", serverNames)); + + throw new McpConnectionsRequiredException(discovery.MissingConnectionsUrl, status, serverNames); + } + + /// + /// Parses a tooling gateway response into an . Supports both the + /// legacy bare-array shape (servers only, no connection metadata) and the wrapped object shape + /// { "mcpServers": [...], allConnectionsUrl, missingConnectionsUrl, connectivityStatus }. + /// + /// The root JSON element of the gateway response. + /// The parsed discovery result with servers and aggregate connection metadata. + internal static McpDiscoveryResult ParseGatewayResponse(JsonElement root) + { + if (root.ValueKind == JsonValueKind.Array) + { + // Legacy bare-array: servers only, no aggregate connection metadata. + List legacyServers = root.EnumerateArray() + .Select(ParseServerConfig) + .Where(config => config != null) + .ToList()!; + return new McpDiscoveryResult(legacyServers); + } + + if (root.ValueKind == JsonValueKind.Object && + TryGetPropertyCaseInsensitive(root, "mcpServers", out var serversElement) && + serversElement.ValueKind == JsonValueKind.Array) + { + List servers = serversElement.EnumerateArray() + .Select(ParseServerConfig) + .Where(config => config != null) + .ToList()!; + + string? connectivityStatus = GetStringPropertyCaseInsensitive(root, "connectivityStatus"); + string? allConnectionsUrl = GetStringPropertyCaseInsensitive(root, "allConnectionsUrl"); + string? missingConnectionsUrl = GetStringPropertyCaseInsensitive(root, "missingConnectionsUrl"); + if (IsReadyStatus(connectivityStatus)) + { + missingConnectionsUrl = null; + } + + return new McpDiscoveryResult(servers, allConnectionsUrl, missingConnectionsUrl, connectivityStatus); + } + + throw new InvalidOperationException( + $"Unexpected JSON structure. Expected array or object with 'mcpServers' property, got {root.ValueKind}"); + } + + /// + /// Determines whether a connectivity status value represents the ready state (case-insensitive). + /// + /// The connectivity status value to evaluate. + /// true when the value equals Ready ignoring case and surrounding whitespace. + private static bool IsReadyStatus(string? connectivityStatus) => + !string.IsNullOrWhiteSpace(connectivityStatus) && + connectivityStatus.Trim().Equals(ReadyConnectivityStatus, StringComparison.OrdinalIgnoreCase); + + /// + /// Attempts to read a property from a JSON object using a case-insensitive (ordinal) name match, + /// matching JsonSerializerOptions.PropertyNameCaseInsensitive semantics. + /// + /// The JSON object to read from. + /// The property name to match (case-insensitive). + /// The matched property value, or default when not found. + /// true when a matching property is found; otherwise false. + private static bool TryGetPropertyCaseInsensitive(JsonElement element, string propertyName, out JsonElement value) + { + value = default; + if (element.ValueKind != JsonValueKind.Object) + { + return false; + } + + foreach (var property in element.EnumerateObject()) + { + if (string.Equals(property.Name, propertyName, StringComparison.OrdinalIgnoreCase)) + { + value = property.Value; + return true; + } + } + + return false; + } + + /// + /// Reads a string property from a JSON object using a case-insensitive name match, or null when + /// the property is absent or not a string. + /// + /// The JSON object to read from. + /// The property name to match (case-insensitive). + /// The string value, or null when not present. + private static string? GetStringPropertyCaseInsensitive(JsonElement element, string propertyName) + { + if (TryGetPropertyCaseInsensitive(element, propertyName, out var value) && + value.ValueKind == JsonValueKind.String) + { + return value.GetString(); + } + + return null; + } + /// /// Parses a JSON element into an MCPServerConfig object from manifest, constructing full URL. /// diff --git a/src/Tooling/Extensions/AgentFramework/CHANGELOG.md b/src/Tooling/Extensions/AgentFramework/CHANGELOG.md index 7381f08f..82d13392 100644 --- a/src/Tooling/Extensions/AgentFramework/CHANGELOG.md +++ b/src/Tooling/Extensions/AgentFramework/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **Microsoft.Agents.A365.Tooling.Extensions.AgentFramework** - V2 per-audience token support - `McpToolRegistrationService.AddToolServersToAgent` and `GetMcpToolsAsync` now instantiate `AgenticMcpTokenProvider` and use the V2-aware `EnumerateToolsFromServersAsync` overload, so each V2 MCP server receives its own audience-scoped Bearer token instead of the shared ATG token +- **Microsoft.Agents.A365.Tooling.Extensions.AgentFramework** - MCP connection-readiness gating now propagates + - `AddToolServersToAgent` and `GetMcpToolsAsync` allow `McpConnectionsRequiredException` to propagate when configured MCP servers are not connection-ready, so callers can surface the setup URL to the user instead of receiving an empty tool list ## [1.0.0] - 2025-01-16 diff --git a/src/Tooling/Extensions/AzureAIFoundry/CHANGELOG.md b/src/Tooling/Extensions/AzureAIFoundry/CHANGELOG.md index c75038ed..72b15ee8 100644 --- a/src/Tooling/Extensions/AzureAIFoundry/CHANGELOG.md +++ b/src/Tooling/Extensions/AzureAIFoundry/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **Microsoft.Agents.A365.Tooling.Extensions.AzureAIFoundry** - V2 per-audience token support - `McpToolRegistrationService.AddToolServersToAgentAsync` now instantiates `AgenticMcpTokenProvider` and uses the V2-aware `GetMcpToolDefinitionsAndResourcesAsync` overload; each V2 MCP server's `MCPToolResource` is populated with its audience-scoped Bearer token instead of the shared ATG token +- **Microsoft.Agents.A365.Tooling.Extensions.AzureAIFoundry** - MCP connection-readiness gating now propagates + - `AddToolServersToAgentAsync` allows `McpConnectionsRequiredException` to propagate when configured MCP servers are not connection-ready, so callers can surface the setup URL to the user instead of receiving an empty tool list ### Added - **Microsoft.Agents.A365.Tooling.AzureFoundry** - Azure Foundry integration tooling for MCP server management diff --git a/src/Tooling/Extensions/SemanticKernel/CHANGELOG.md b/src/Tooling/Extensions/SemanticKernel/CHANGELOG.md index 852970b3..cfdf0bfc 100644 --- a/src/Tooling/Extensions/SemanticKernel/CHANGELOG.md +++ b/src/Tooling/Extensions/SemanticKernel/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Microsoft.Agents.A365.Tooling.Extensions.SemanticKernel** - V2 per-audience token support - `McpToolRegistrationService.AddToolServersToAgentAsync` now instantiates `AgenticMcpTokenProvider` and uses the V2-aware `EnumerateToolsFromServersAsync` overload, so each V2 MCP server receives its own audience-scoped Bearer token instead of the shared ATG token - OBO token acquisition is deferred until after the dev-mode check; in `Development` environments the `DevMcpTokenProvider` supplies tokens from environment variables (`BEARER_TOKEN_` / `BEARER_TOKEN`) without requiring a working auth setup +- **Microsoft.Agents.A365.Tooling.Extensions.SemanticKernel** - MCP connection-readiness gating now propagates + - `AddToolServersToAgentAsync` allows `McpConnectionsRequiredException` to propagate when configured MCP servers are not connection-ready, so callers can surface the setup URL to the user instead of receiving an empty tool list ### Added - **Microsoft.Agents.A365.Tooling.Extensions.SemanticKernel** - Semantic Kernel integration tooling for MCP server management