From 46be96d803647e6f859417ea745abdf161c11a0b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 19:11:17 +0000 Subject: [PATCH 1/5] Fix tools/list infinite loop by removing notification from ListTools handler and adding change detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP server was entering an infinite tools/list loop because: 1. HandleListToolsAsync sent tools/list_changed notification after refreshing 2. The client responded with another tools/list request 3. This created a feedback loop: list → changed → list → changed Fix: - Remove SendToolsListChangedNotificationAsync from HandleListToolsAsync (the client already gets the fresh list since it requested it) - Add change detection to RefreshResourceToolMapAsync (returns bool Changed) - Only send tools/list_changed in HandleCallToolAsync when tools actually changed - RefreshToolsTool always sends notification (explicit user action) Co-authored-by: maddymontaquila <12660687+maddymontaquila@users.noreply.github.com> --- src/Aspire.Cli/Commands/AgentMcpCommand.cs | 16 +++-- .../Mcp/IMcpResourceToolRefreshService.cs | 4 +- .../Mcp/McpResourceToolRefreshService.cs | 7 +- src/Aspire.Cli/Mcp/Tools/RefreshToolsTool.cs | 2 +- .../Commands/AgentMcpCommandTests.cs | 67 +++++++++++++++++++ 5 files changed, 87 insertions(+), 9 deletions(-) diff --git a/src/Aspire.Cli/Commands/AgentMcpCommand.cs b/src/Aspire.Cli/Commands/AgentMcpCommand.cs index bb239d4ee48..3aff0911c2a 100644 --- a/src/Aspire.Cli/Commands/AgentMcpCommand.cs +++ b/src/Aspire.Cli/Commands/AgentMcpCommand.cs @@ -147,8 +147,12 @@ private async ValueTask HandleListToolsAsync(RequestContext new Tool @@ -193,8 +197,12 @@ private async ValueTask HandleCallToolAsync(RequestContext /// The cancellation token. - /// The refreshed resource tool map. - Task> RefreshResourceToolMapAsync(CancellationToken cancellationToken); + /// A tuple containing the refreshed resource tool map and a flag indicating whether the tool set changed. + Task<(IReadOnlyDictionary ToolMap, bool Changed)> RefreshResourceToolMapAsync(CancellationToken cancellationToken); /// /// Sends a tools list changed notification to connected MCP clients. diff --git a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs index 95ecf656214..7efe95ec9f7 100644 --- a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs +++ b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs @@ -71,7 +71,7 @@ public async Task SendToolsListChangedNotificationAsync(CancellationToken cancel } /// - public async Task> RefreshResourceToolMapAsync(CancellationToken cancellationToken) + public async Task<(IReadOnlyDictionary ToolMap, bool Changed)> RefreshResourceToolMapAsync(CancellationToken cancellationToken) { _logger.LogDebug("Refreshing resource tool map."); @@ -117,10 +117,13 @@ public async Task> RefreshResourc lock (_lock) { + var changed = !_resourceToolMap.Keys.OrderBy(k => k, StringComparer.Ordinal) + .SequenceEqual(refreshedMap.Keys.OrderBy(k => k, StringComparer.Ordinal), StringComparer.Ordinal); + _resourceToolMap = refreshedMap; _selectedAppHostPath = selectedAppHostPath; _invalidated = false; - return _resourceToolMap; + return (_resourceToolMap, changed); } } } diff --git a/src/Aspire.Cli/Mcp/Tools/RefreshToolsTool.cs b/src/Aspire.Cli/Mcp/Tools/RefreshToolsTool.cs index c8d3d5c4fef..e0a919c7333 100644 --- a/src/Aspire.Cli/Mcp/Tools/RefreshToolsTool.cs +++ b/src/Aspire.Cli/Mcp/Tools/RefreshToolsTool.cs @@ -19,7 +19,7 @@ public override JsonElement GetInputSchema() public override async ValueTask CallToolAsync(CallToolContext context, CancellationToken cancellationToken) { - var resourceToolMap = await refreshService.RefreshResourceToolMapAsync(cancellationToken).ConfigureAwait(false); + var (resourceToolMap, _) = await refreshService.RefreshResourceToolMapAsync(cancellationToken).ConfigureAwait(false); await refreshService.SendToolsListChangedNotificationAsync(cancellationToken).ConfigureAwait(false); var totalToolCount = KnownMcpTools.All.Count + resourceToolMap.Count; diff --git a/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs b/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs index dd7cb3e3587..d7168279ddd 100644 --- a/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs +++ b/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs @@ -347,6 +347,73 @@ public async Task McpServer_CallTool_RefreshTools_ReturnsResult() Assert.Equal(NotificationMethods.ToolListChangedNotification, notification.Method); } + [Fact] + public async Task McpServer_ListTools_DoesNotSendToolsListChangedNotification() + { + // Arrange - Create a mock backchannel with a resource that has MCP tools + // This simulates the db-mcp scenario where resource tools become available + var mockBackchannel = new TestAppHostAuxiliaryBackchannel + { + Hash = "test-apphost-hash", + IsInScope = true, + AppHostInfo = new AppHostInformation + { + AppHostPath = Path.Combine(_workspace.WorkspaceRoot.FullName, "TestAppHost", "TestAppHost.csproj"), + ProcessId = 12345 + }, + ResourceSnapshots = + [ + new ResourceSnapshot + { + Name = "db-mcp", + DisplayName = "DB MCP", + ResourceType = "Container", + State = "Running", + McpServer = new ResourceSnapshotMcpServer + { + EndpointUrl = "http://localhost:8080/mcp", + Tools = + [ + new Tool + { + Name = "query_database", + Description = "Query a database" + } + ] + } + } + ] + }; + + // Register the mock backchannel so resource tools will be discovered + _backchannelMonitor.AddConnection(mockBackchannel.Hash, mockBackchannel.SocketPath, mockBackchannel); + + // Set up a channel to detect any tools/list_changed notifications + var notificationCount = 0; + await using var notificationHandler = _mcpClient.RegisterNotificationHandler( + NotificationMethods.ToolListChangedNotification, + (notification, cancellationToken) => + { + Interlocked.Increment(ref notificationCount); + return default; + }); + + // Act - Call ListTools which should discover the resource tools via refresh + // but should NOT send a tools/list_changed notification (that would cause an infinite loop) + var tools = await _mcpClient.ListToolsAsync(cancellationToken: _cts.Token).DefaultTimeout(); + + // Give a small window for any spurious notification to arrive + await Task.Delay(200, _cts.Token); + + // Assert - tools should include the resource tool + Assert.NotNull(tools); + var dbMcpTool = tools.FirstOrDefault(t => t.Name == "db_mcp_query_database"); + Assert.NotNull(dbMcpTool); + + // Assert - no tools/list_changed notification should have been sent + Assert.Equal(0, notificationCount); + } + [Fact] public async Task McpServer_CallTool_UnknownTool_ReturnsError() { From 9c1730e4f186be4edc4fd5b13194862d2ddb0d9f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 19:12:36 +0000 Subject: [PATCH 2/5] Use HashSet.SetEquals for more efficient tool change detection Co-authored-by: maddymontaquila <12660687+maddymontaquila@users.noreply.github.com> --- src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs index 7efe95ec9f7..9269a95709b 100644 --- a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs +++ b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs @@ -117,8 +117,8 @@ public async Task SendToolsListChangedNotificationAsync(CancellationToken cancel lock (_lock) { - var changed = !_resourceToolMap.Keys.OrderBy(k => k, StringComparer.Ordinal) - .SequenceEqual(refreshedMap.Keys.OrderBy(k => k, StringComparer.Ordinal), StringComparer.Ordinal); + var changed = _resourceToolMap.Count != refreshedMap.Count + || !_resourceToolMap.Keys.ToHashSet(StringComparer.Ordinal).SetEquals(refreshedMap.Keys); _resourceToolMap = refreshedMap; _selectedAppHostPath = selectedAppHostPath; From fdddc9fbe58f10db158d1aee9069bf5ebb041ee5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 16:50:01 +0000 Subject: [PATCH 3/5] Address review feedback: use no-allocation key comparison and bounded channel wait in test - Replace HashSet.SetEquals with count check + iterate keys + ContainsKey to avoid allocation under lock in McpResourceToolRefreshService. - Replace Task.Delay(200) in test with Channel.ReadAsync + CancellationTokenSource timeout for more deterministic negative assertion. Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com> --- .../Mcp/McpResourceToolRefreshService.cs | 14 +++++++-- .../Commands/AgentMcpCommandTests.cs | 29 ++++++++++++++++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs index 9269a95709b..7ae5d78daed 100644 --- a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs +++ b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs @@ -117,8 +117,18 @@ public async Task SendToolsListChangedNotificationAsync(CancellationToken cancel lock (_lock) { - var changed = _resourceToolMap.Count != refreshedMap.Count - || !_resourceToolMap.Keys.ToHashSet(StringComparer.Ordinal).SetEquals(refreshedMap.Keys); + var changed = _resourceToolMap.Count != refreshedMap.Count; + if (!changed) + { + foreach (var key in _resourceToolMap.Keys) + { + if (!refreshedMap.ContainsKey(key)) + { + changed = true; + break; + } + } + } _resourceToolMap = refreshedMap; _selectedAppHostPath = selectedAppHostPath; diff --git a/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs b/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs index d7168279ddd..916d5290270 100644 --- a/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs +++ b/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs @@ -402,15 +402,36 @@ public async Task McpServer_ListTools_DoesNotSendToolsListChangedNotification() // but should NOT send a tools/list_changed notification (that would cause an infinite loop) var tools = await _mcpClient.ListToolsAsync(cancellationToken: _cts.Token).DefaultTimeout(); - // Give a small window for any spurious notification to arrive - await Task.Delay(200, _cts.Token); - // Assert - tools should include the resource tool Assert.NotNull(tools); var dbMcpTool = tools.FirstOrDefault(t => t.Name == "db_mcp_query_database"); Assert.NotNull(dbMcpTool); - // Assert - no tools/list_changed notification should have been sent + // Assert - no tools/list_changed notification should have been sent. + // Use a bounded wait via TryRead to catch any late-arriving notification + // without relying on an arbitrary Task.Delay. + using var timeoutCts = new CancellationTokenSource(TimeSpan.FromMilliseconds(200)); + var notificationChannel = Channel.CreateUnbounded(); + await using var channelHandler = _mcpClient.RegisterNotificationHandler( + NotificationMethods.ToolListChangedNotification, + (notification, _) => + { + notificationChannel.Writer.TryWrite(notification); + return default; + }); + + var received = false; + try + { + await notificationChannel.Reader.ReadAsync(timeoutCts.Token); + received = true; + } + catch (OperationCanceledException) + { + // Expected — no notification arrived within the timeout + } + + Assert.False(received, "tools/list_changed notification should not be sent during tools/list handling"); Assert.Equal(0, notificationCount); } From 1ec1d23f27697037b56631df5086813f0826f47d Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Fri, 13 Feb 2026 10:44:27 -0800 Subject: [PATCH 4/5] Check for both new and deleted tools in change detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous change detection only iterated old→new keys, missing the case where tools are swapped (same count but different keys). Now also checks new→old to detect newly added tools. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Mcp/McpResourceToolRefreshService.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs index 7ae5d78daed..5f504ede496 100644 --- a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs +++ b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs @@ -120,6 +120,7 @@ public async Task SendToolsListChangedNotificationAsync(CancellationToken cancel var changed = _resourceToolMap.Count != refreshedMap.Count; if (!changed) { + // Check for deleted tools (in old but not in new). foreach (var key in _resourceToolMap.Keys) { if (!refreshedMap.ContainsKey(key)) @@ -128,6 +129,19 @@ public async Task SendToolsListChangedNotificationAsync(CancellationToken cancel break; } } + + // Check for new tools (in new but not in old). + if (!changed) + { + foreach (var key in refreshedMap.Keys) + { + if (!_resourceToolMap.ContainsKey(key)) + { + changed = true; + break; + } + } + } } _resourceToolMap = refreshedMap; From 6535ab8ebfc0bf0549a146a60c22758a199087a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= Date: Fri, 13 Feb 2026 11:33:19 -0800 Subject: [PATCH 5/5] Clean up comments in AgentMcpCommandTests Removed comments about using TryRead for notifications. --- tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs b/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs index 916d5290270..25c01088c5c 100644 --- a/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs +++ b/tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs @@ -408,8 +408,6 @@ public async Task McpServer_ListTools_DoesNotSendToolsListChangedNotification() Assert.NotNull(dbMcpTool); // Assert - no tools/list_changed notification should have been sent. - // Use a bounded wait via TryRead to catch any late-arriving notification - // without relying on an arbitrary Task.Delay. using var timeoutCts = new CancellationTokenSource(TimeSpan.FromMilliseconds(200)); var notificationChannel = Channel.CreateUnbounded(); await using var channelHandler = _mcpClient.RegisterNotificationHandler(