From 25b99a60f5d276d5b8925f6fc13933ce05440cf6 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Wed, 18 Feb 2026 13:39:06 +1100 Subject: [PATCH] Fix TryGetResourceToolMap cache miss causing perpetual tools/list refresh loop TryGetResourceToolMap always returned false because it compared _selectedAppHostPath against _auxiliaryBackchannelMonitor.SelectedAppHostPath, which is only set by explicit select_apphost calls (usually null). After RefreshResourceToolMapAsync sets _selectedAppHostPath to the connection's actual path, the comparison null != "/path/to/AppHost" always failed, so every tools/list call triggered a full refresh instead of using the cached map. Fix: Add ResolvedAppHostPath property to IAuxiliaryBackchannelMonitor that returns SelectedConnection?.AppHostInfo?.AppHostPath, and compare against that. Rename field to _lastRefreshedAppHostPath for clarity. Fixes #14538 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../IAuxiliaryBackchannelMonitor.cs | 5 ++ .../Mcp/McpResourceToolRefreshService.cs | 6 +- .../Commands/AgentMcpCommandTests.cs | 60 +++++++++++++++++++ .../TestAppHostAuxiliaryBackchannel.cs | 11 ++++ 4 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/Aspire.Cli/Backchannel/IAuxiliaryBackchannelMonitor.cs b/src/Aspire.Cli/Backchannel/IAuxiliaryBackchannelMonitor.cs index 36f0a90d257..5ab0d332686 100644 --- a/src/Aspire.Cli/Backchannel/IAuxiliaryBackchannelMonitor.cs +++ b/src/Aspire.Cli/Backchannel/IAuxiliaryBackchannelMonitor.cs @@ -31,6 +31,11 @@ internal interface IAuxiliaryBackchannelMonitor /// IAppHostAuxiliaryBackchannel? SelectedConnection { get; } + /// + /// Gets the AppHost path of the currently resolved connection, or null if no connection is available. + /// + string? ResolvedAppHostPath => SelectedConnection?.AppHostInfo?.AppHostPath; + /// /// Gets all connections that are within the scope of the specified working directory. /// diff --git a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs index e3d3c368333..31e633058d2 100644 --- a/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs +++ b/src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs @@ -20,7 +20,7 @@ internal sealed class McpResourceToolRefreshService : IMcpResourceToolRefreshSer private McpServer? _server; private Dictionary _resourceToolMap = new(StringComparer.Ordinal); private bool _invalidated = true; - private string? _selectedAppHostPath; + private string? _lastRefreshedAppHostPath; public McpResourceToolRefreshService( IAuxiliaryBackchannelMonitor auxiliaryBackchannelMonitor, @@ -35,7 +35,7 @@ public bool TryGetResourceToolMap(out IReadOnlyDictionary + { + Interlocked.Increment(ref getResourceSnapshotsCallCount); + return Task.FromResult(new List + { + new ResourceSnapshot + { + Name = "db-mcp-xyz", + DisplayName = "db-mcp", + ResourceType = "Container", + State = "Running", + McpServer = new ResourceSnapshotMcpServer + { + EndpointUrl = "http://localhost:8080/mcp", + Tools = + [ + new Tool + { + Name = "query_db", + Description = "Query the database" + } + ] + } + } + }); + } + }; + + _backchannelMonitor.AddConnection(mockBackchannel.Hash, mockBackchannel.SocketPath, mockBackchannel); + + // Act - Call ListTools twice + var tools1 = await _mcpClient.ListToolsAsync(cancellationToken: _cts.Token).DefaultTimeout(); + var tools2 = await _mcpClient.ListToolsAsync(cancellationToken: _cts.Token).DefaultTimeout(); + + // Assert - Both calls return the resource tool + Assert.Contains(tools1, t => t.Name == "db_mcp_query_db"); + Assert.Contains(tools2, t => t.Name == "db_mcp_query_db"); + + // The resource tool map should be cached after the first call, + // so GetResourceSnapshotsAsync should only be called once (during the first refresh). + // Before the fix, TryGetResourceToolMap always returned false due to + // SelectedAppHostPath vs SelectedConnection path mismatch, causing every + // ListTools call to trigger a full refresh. + Assert.Equal(1, getResourceSnapshotsCallCount); + } + [Fact] public async Task McpServer_CallTool_UnknownTool_ReturnsError() { diff --git a/tests/Aspire.Cli.Tests/TestServices/TestAppHostAuxiliaryBackchannel.cs b/tests/Aspire.Cli.Tests/TestServices/TestAppHostAuxiliaryBackchannel.cs index 31c7ea06dd8..fe266efc374 100644 --- a/tests/Aspire.Cli.Tests/TestServices/TestAppHostAuxiliaryBackchannel.cs +++ b/tests/Aspire.Cli.Tests/TestServices/TestAppHostAuxiliaryBackchannel.cs @@ -46,6 +46,12 @@ internal sealed class TestAppHostAuxiliaryBackchannel : IAppHostAuxiliaryBackcha /// public Func?, CancellationToken, Task>? CallResourceMcpToolHandler { get; set; } + /// + /// Gets or sets the function to call when GetResourceSnapshotsAsync is invoked. + /// If null, returns the ResourceSnapshots list. + /// + public Func>>? GetResourceSnapshotsHandler { get; set; } + public Task GetDashboardUrlsAsync(CancellationToken cancellationToken = default) { return Task.FromResult(DashboardUrlsState); @@ -53,6 +59,11 @@ internal sealed class TestAppHostAuxiliaryBackchannel : IAppHostAuxiliaryBackcha public Task> GetResourceSnapshotsAsync(CancellationToken cancellationToken = default) { + if (GetResourceSnapshotsHandler is not null) + { + return GetResourceSnapshotsHandler(cancellationToken); + } + return Task.FromResult(ResourceSnapshots); }