Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/Aspire.Cli/Commands/AgentMcpCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,12 @@ private async ValueTask<ListToolsResult> HandleListToolsAsync(RequestContext<Lis
// Refresh resource tools if needed (e.g., AppHost selection changed or invalidated)
if (!_resourceToolRefreshService.TryGetResourceToolMap(out var resourceToolMap))
{
resourceToolMap = await _resourceToolRefreshService.RefreshResourceToolMapAsync(cancellationToken);
await _resourceToolRefreshService.SendToolsListChangedNotificationAsync(cancellationToken).ConfigureAwait(false);
// Don't send tools/list_changed here — the client already called tools/list
// and will receive the up-to-date result. Sending a notification during the
// list handler would cause the client to call tools/list again, creating an
// infinite loop when tool availability is unstable (e.g., container MCP tools
// oscillating between available/unavailable).
(resourceToolMap, _) = await _resourceToolRefreshService.RefreshResourceToolMapAsync(cancellationToken);
}

tools.AddRange(resourceToolMap.Select(x => new Tool
Expand Down Expand Up @@ -193,8 +197,12 @@ private async ValueTask<CallToolResult> HandleCallToolAsync(RequestContext<CallT
// Refresh resource tools if needed (e.g., AppHost selection changed or invalidated)
if (!_resourceToolRefreshService.TryGetResourceToolMap(out var resourceToolMap))
{
resourceToolMap = await _resourceToolRefreshService.RefreshResourceToolMapAsync(cancellationToken);
await _resourceToolRefreshService.SendToolsListChangedNotificationAsync(cancellationToken).ConfigureAwait(false);
bool changed;
(resourceToolMap, changed) = await _resourceToolRefreshService.RefreshResourceToolMapAsync(cancellationToken);
if (changed)
{
await _resourceToolRefreshService.SendToolsListChangedNotificationAsync(cancellationToken).ConfigureAwait(false);
}
toolsRefreshed = true;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Aspire.Cli/Mcp/IMcpResourceToolRefreshService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ internal interface IMcpResourceToolRefreshService
/// Refreshes the resource tool map by discovering MCP tools from connected resources.
/// </summary>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The refreshed resource tool map.</returns>
Task<IReadOnlyDictionary<string, ResourceToolEntry>> RefreshResourceToolMapAsync(CancellationToken cancellationToken);
/// <returns>A tuple containing the refreshed resource tool map and a flag indicating whether the tool set changed.</returns>
Task<(IReadOnlyDictionary<string, ResourceToolEntry> ToolMap, bool Changed)> RefreshResourceToolMapAsync(CancellationToken cancellationToken);

/// <summary>
/// Sends a tools list changed notification to connected MCP clients.
Expand Down
7 changes: 5 additions & 2 deletions src/Aspire.Cli/Mcp/McpResourceToolRefreshService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public async Task SendToolsListChangedNotificationAsync(CancellationToken cancel
}

/// <inheritdoc/>
public async Task<IReadOnlyDictionary<string, ResourceToolEntry>> RefreshResourceToolMapAsync(CancellationToken cancellationToken)
public async Task<(IReadOnlyDictionary<string, ResourceToolEntry> ToolMap, bool Changed)> RefreshResourceToolMapAsync(CancellationToken cancellationToken)
{
_logger.LogDebug("Refreshing resource tool map.");

Expand Down Expand Up @@ -117,10 +117,13 @@ public async Task<IReadOnlyDictionary<string, ResourceToolEntry>> RefreshResourc

lock (_lock)
{
var changed = _resourceToolMap.Count != refreshedMap.Count
|| !_resourceToolMap.Keys.ToHashSet(StringComparer.Ordinal).SetEquals(refreshedMap.Keys);
Comment thread
sebastienros marked this conversation as resolved.
Outdated

_resourceToolMap = refreshedMap;
_selectedAppHostPath = selectedAppHostPath;
_invalidated = false;
return _resourceToolMap;
return (_resourceToolMap, changed);
}
}
}
2 changes: 1 addition & 1 deletion src/Aspire.Cli/Mcp/Tools/RefreshToolsTool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public override JsonElement GetInputSchema()

public override async ValueTask<CallToolResult> 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;
Expand Down
67 changes: 67 additions & 0 deletions tests/Aspire.Cli.Tests/Commands/AgentMcpCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Comment thread
sebastienros marked this conversation as resolved.
Outdated
// 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()
{
Expand Down
Loading