From c557f78ab2b9eabd1dfe925846f2df6878a5d6e9 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Wed, 21 Oct 2026 19:57:42 +0100 Subject: [PATCH] .NET: Rename and restructure MCP skills cache Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eeaa072d-79cf-4092-8f3f-9931e6c1a4e0 --- .../Skills/Listed/McpListedSkillsCache.cs | 40 ++++---- .../Listed/McpListedSkillsDiscoveryResult.cs | 32 ------ .../Skills/Listed/McpListedSkillsLoader.cs | 8 +- .../Skills/Listed/McpListedSkillsSnapshot.cs | 32 ++++++ .../Listed/McpListedSkillsCacheTests.cs | 98 +++++++++++-------- 5 files changed, 113 insertions(+), 97 deletions(-) delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsDiscoveryResult.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsSnapshot.cs diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsCache.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsCache.cs index c7c4bdc2dff..71c1b693af9 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsCache.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsCache.cs @@ -14,34 +14,36 @@ namespace Microsoft.Agents.AI; internal sealed class McpListedSkillsCache : IDisposable { private readonly TimeProvider _timeProvider; + private readonly Func> _discoverSkills; private readonly SemaphoreSlim _discoveryGate = new(1, 1); - private volatile McpListedSkillsDiscoveryResult? _cachedSkills; + private volatile McpListedSkillsSnapshot? _cachedSnapshot; private bool _disposed; /// /// Initializes a cache using the clock that records discovery page receipt timestamps. /// /// The clock for checking page freshness. - public McpListedSkillsCache(TimeProvider timeProvider) + /// Discovers and validates all pages, including their receipt timestamps and TTLs. + public McpListedSkillsCache(TimeProvider timeProvider, Func> discoverSkills) { this._timeProvider = Throw.IfNull(timeProvider); + this._discoverSkills = Throw.IfNull(discoverSkills); } /// /// Gets fresh cached skills or invokes discovery to replace the catalog. /// - /// Discovers and validates all pages, including their receipt timestamps and TTLs. /// The token used to cancel waiting or discovery. /// The cached or newly discovered skills. - public async Task> GetOrCreateAsync(Func> discoverSkills, CancellationToken cancellationToken) + public async Task> GetOrCreateAsync(CancellationToken cancellationToken) { ObjectDisposedException.ThrowIf(this._disposed, this); cancellationToken.ThrowIfCancellationRequested(); // Reuse the catalog only while every page is fresh. - if (this.GetCachedSkills() is { } cached) + if (this.GetCachedSnapshot() is { } cached) { - return cached; + return cached.Skills; } // Allow only one caller to discover or refresh the catalog at a time. @@ -52,20 +54,20 @@ public async Task> GetOrCreateAsync(Func> GetOrCreateAsync(Func - /// Gets the cached skills, or if the cache is absent or expired. + /// Gets the cached snapshot, or if it is absent or any listing page has expired. /// - private List? GetCachedSkills() + private McpListedSkillsSnapshot? GetCachedSnapshot() { - if (this._cachedSkills is not { } cached) + if (this._cachedSnapshot is not { } cached) { return null; } @@ -94,17 +96,17 @@ public async Task> GetOrCreateAsync(Func - /// Caches the discovered skills together with their page lifetimes. + /// Stores the snapshot. /// - /// The complete discovery result. - private void CacheSkills(McpListedSkillsDiscoveryResult result) + /// The skill list and its listing-page lifetimes. + private void CacheSnapshot(McpListedSkillsSnapshot snapshot) { // Publish the complete catalog and page lifetimes together only after discovery and validation succeed. - this._cachedSkills = result; + this._cachedSnapshot = snapshot; } /// diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsDiscoveryResult.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsDiscoveryResult.cs deleted file mode 100644 index cdd3efeb26a..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsDiscoveryResult.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System.Collections.Generic; - -namespace Microsoft.Agents.AI; - -/// -/// Discovered and validated skills together with the lifetime of each discovery page. -/// -internal sealed class McpListedSkillsDiscoveryResult -{ - /// - /// Initializes a discovery result with its skills and page lifetimes. - /// - /// The complete list of discovered skills. - /// The receipt timestamp and TTL of each discovery page. - public McpListedSkillsDiscoveryResult(List skills, List pageLifetimes) - { - this.Skills = skills; - this.PageLifetimes = pageLifetimes; - } - - /// - /// Gets the complete list of discovered skills. - /// - public List Skills { get; } - - /// - /// Gets the receipt timestamp and TTL of each discovery page. - /// - public List PageLifetimes { get; } -} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs index 8ff7ae04bfe..acfc502b033 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs @@ -60,7 +60,7 @@ public McpListedSkillsLoader(McpClient client, ILogger? logger = null, AgentMcpS ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxResourceSizeBytes, nameof(options.ListedResourceMaxSizeBytes)); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxPageCount, nameof(options.ListedMaxPageCount)); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxSkillCount, nameof(options.ListedMaxSkillCount)); - this._cache = new McpListedSkillsCache(this._timeProvider); + this._cache = new McpListedSkillsCache(this._timeProvider, this.DiscoverCoreAsync); } /// @@ -76,10 +76,10 @@ public McpListedSkillsLoader(McpClient client, ILogger? logger = null, AgentMcpS /// The loader has been disposed. public Task> DiscoverAsync(CancellationToken cancellationToken) { - return this._cache.GetOrCreateAsync(this.DiscoverCoreAsync, cancellationToken); + return this._cache.GetOrCreateAsync(cancellationToken); } - private async Task DiscoverCoreAsync(CancellationToken cancellationToken) + private async Task DiscoverCoreAsync(CancellationToken cancellationToken) { var (skillEntries, pageLifetimes) = await this.ListSkillEntriesAsync(cancellationToken).ConfigureAwait(false); @@ -95,7 +95,7 @@ private async Task DiscoverCoreAsync(Cancellatio skills.Add(new AgentMcpListedSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes)); } - return new McpListedSkillsDiscoveryResult(skills, pageLifetimes); + return new McpListedSkillsSnapshot(skills, pageLifetimes); } private async Task<(List Entries, List PageLifetimes)> ListSkillEntriesAsync(CancellationToken cancellationToken) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsSnapshot.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsSnapshot.cs new file mode 100644 index 00000000000..93c9b30e537 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsSnapshot.cs @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; + +namespace Microsoft.Agents.AI; + +/// +/// A skill list and its listing-page lifetimes at a point in time. +/// +internal sealed class McpListedSkillsSnapshot +{ + /// + /// Initializes a snapshot with its skills and listing-page lifetimes. + /// + /// The complete skill list for this snapshot. + /// The receipt timestamp and TTL of each listing page. + public McpListedSkillsSnapshot(List skills, List pageLifetimes) + { + this.Skills = skills; + this.PageLifetimes = pageLifetimes; + } + + /// + /// Gets the complete skill list for this snapshot. + /// + public List Skills { get; } + + /// + /// Gets the receipt timestamp and TTL of each listing page. + /// + public List PageLifetimes { get; } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/McpListedSkillsCacheTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/McpListedSkillsCacheTests.cs index a238e46123d..1b8aa7bdf78 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/McpListedSkillsCacheTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/McpListedSkillsCacheTests.cs @@ -10,25 +10,36 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; public sealed class McpListedSkillsCacheTests { + [Fact] + public void Constructor_NullDiscoverSkills_ThrowsArgumentNullException() + { + // Arrange + var clock = new ManualTimeProvider(); + + // Act & Assert + Assert.Throws("discoverSkills", () => new McpListedSkillsCache(clock, null!)); + } + [Fact] public async Task GetOrCreateAsync_PositiveTtl_ReturnsOriginalListUntilExactExpirationAsync() { // Arrange var clock = new ManualTimeProvider(); - using var cache = new McpListedSkillsCache(clock); int calls = 0; - McpListedSkillsDiscoveryResult? discovered = null; - Task DiscoverAsync(CancellationToken token) + McpListedSkillsSnapshot? discovered = null; + Task DiscoverAsync(CancellationToken token) { calls++; - discovered = CreateResult(clock, 1000); + discovered = CreateSnapshot(clock, 1000); return Task.FromResult(discovered); } + using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + // Act - var first = await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + var first = await cache.GetOrCreateAsync(CancellationToken.None); clock.Advance(TimeSpan.FromMilliseconds(999)); - var fresh = await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + var fresh = await cache.GetOrCreateAsync(CancellationToken.None); // Assert Assert.NotNull(discovered); @@ -39,7 +50,7 @@ Task DiscoverAsync(CancellationToken token) // Act clock.Advance(TimeSpan.FromMilliseconds(1)); - var refreshed = await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + var refreshed = await cache.GetOrCreateAsync(CancellationToken.None); // Assert Assert.NotSame(first, refreshed); @@ -55,21 +66,21 @@ public async Task GetOrCreateAsync_AnyExpiredPage_RefreshesEntireCatalogAsync(in { // Arrange var clock = new ManualTimeProvider(); - using var cache = new McpListedSkillsCache(clock); int calls = 0; - Task DiscoverAsync(CancellationToken token) + Task DiscoverAsync(CancellationToken token) { calls++; long[] ttls = [5000, 5000, 5000]; ttls[expiredPage] = 1000; - return Task.FromResult(CreateResult(clock, ttls)); + return Task.FromResult(CreateSnapshot(clock, ttls)); } - var first = await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + var first = await cache.GetOrCreateAsync(CancellationToken.None); // Act clock.Advance(TimeSpan.FromSeconds(1)); - var refreshed = await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + var refreshed = await cache.GetOrCreateAsync(CancellationToken.None); // Assert Assert.NotSame(first, refreshed); @@ -83,17 +94,17 @@ public async Task GetOrCreateAsync_ConcurrentCalls_ReuseOnlyFreshResultsAsync(lo { // Arrange var clock = new ManualTimeProvider(); - using var cache = new McpListedSkillsCache(clock); - var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); int calls = 0; - Task DiscoverAsync(CancellationToken token) => - ++calls == 1 ? release.Task : Task.FromResult(CreateResult(clock, ttl)); + Task DiscoverAsync(CancellationToken token) => + ++calls == 1 ? release.Task : Task.FromResult(CreateSnapshot(clock, ttl)); + using var cache = new McpListedSkillsCache(clock, DiscoverAsync); // Act Task>[] pending = Enumerable.Range(0, 8) - .Select(_ => cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None)).ToArray(); + .Select(_ => cache.GetOrCreateAsync(CancellationToken.None)).ToArray(); Assert.Equal(1, calls); - release.SetResult(CreateResult(clock, ttl)); + release.SetResult(CreateSnapshot(clock, ttl)); var results = await Task.WhenAll(pending).WaitAsync(TimeSpan.FromSeconds(10)); // Assert @@ -114,14 +125,14 @@ public async Task GetOrCreateAsync_FailedDiscovery_PreservesErrorAndReleasesWait { // Arrange var clock = new ManualTimeProvider(); - using var cache = new McpListedSkillsCache(clock); - var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var error = new InvalidOperationException("Discovery failed."); int calls = 0; - Task DiscoverAsync(CancellationToken token) => - ++calls == 1 ? release.Task : Task.FromResult(CreateResult(clock, 1000)); - var active = cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); - var waiting = cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + Task DiscoverAsync(CancellationToken token) => + ++calls == 1 ? release.Task : Task.FromResult(CreateSnapshot(clock, 1000)); + using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + var active = cache.GetOrCreateAsync(CancellationToken.None); + var waiting = cache.GetOrCreateAsync(CancellationToken.None); // Act release.SetException(error); @@ -130,7 +141,7 @@ Task DiscoverAsync(CancellationToken token) => Assert.Same(error, await Assert.ThrowsAsync(() => active.WaitAsync(TimeSpan.FromSeconds(10)))); var skills = await waiting.WaitAsync(TimeSpan.FromSeconds(10)); Assert.Single(skills); - Assert.Same(skills, await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None)); + Assert.Same(skills, await cache.GetOrCreateAsync(CancellationToken.None)); Assert.Equal(2, calls); } @@ -139,10 +150,9 @@ public async Task GetOrCreateAsync_CanceledAfterDiscovery_DoesNotCacheResultAsyn { // Arrange var clock = new ManualTimeProvider(); - using var cache = new McpListedSkillsCache(clock); using var cts = new CancellationTokenSource(); int calls = 0; - Task DiscoverAsync(CancellationToken token) + Task DiscoverAsync(CancellationToken token) { if (++calls == 1) { @@ -150,15 +160,17 @@ Task DiscoverAsync(CancellationToken token) cts.Cancel(); } - return Task.FromResult(CreateResult(clock, 1000)); + return Task.FromResult(CreateSnapshot(clock, 1000)); } + using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + // Act & Assert - await Assert.ThrowsAnyAsync(() => cache.GetOrCreateAsync(DiscoverAsync, cts.Token)); - var skills = await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + await Assert.ThrowsAnyAsync(() => cache.GetOrCreateAsync(cts.Token)); + var skills = await cache.GetOrCreateAsync(CancellationToken.None); Assert.Single(skills); Assert.Equal(2, calls); - Assert.Same(skills, await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None)); + Assert.Same(skills, await cache.GetOrCreateAsync(CancellationToken.None)); } [Theory] @@ -168,24 +180,25 @@ public async Task GetOrCreateAsync_CanceledToken_DoesNotDiscoverOrReturnCachedSk { // Arrange var clock = new ManualTimeProvider(); - using var cache = new McpListedSkillsCache(clock); int calls = 0; - Task DiscoverAsync(CancellationToken token) + Task DiscoverAsync(CancellationToken token) { calls++; - return Task.FromResult(CreateResult(clock, 1000)); + return Task.FromResult(CreateSnapshot(clock, 1000)); } + using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + if (cached) { - await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + await cache.GetOrCreateAsync(CancellationToken.None); } using var cts = new CancellationTokenSource(); await cts.CancelAsync(); // Act & Assert - await Assert.ThrowsAnyAsync(() => cache.GetOrCreateAsync(DiscoverAsync, cts.Token)); + await Assert.ThrowsAnyAsync(() => cache.GetOrCreateAsync(cts.Token)); Assert.Equal(cached ? 1 : 0, calls); } @@ -196,17 +209,18 @@ public async Task GetOrCreateAsync_DisposedCache_ThrowsWithoutInvokingDiscoveryA { // Arrange var clock = new ManualTimeProvider(); - using var cache = new McpListedSkillsCache(clock); int calls = 0; - Task DiscoverAsync(CancellationToken token) + Task DiscoverAsync(CancellationToken token) { calls++; - return Task.FromResult(CreateResult(clock, 1000)); + return Task.FromResult(CreateSnapshot(clock, 1000)); } + using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + if (cached) { - await cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None); + await cache.GetOrCreateAsync(CancellationToken.None); } // Act @@ -214,11 +228,11 @@ Task DiscoverAsync(CancellationToken token) cache.Dispose(); // Assert - await Assert.ThrowsAsync(() => cache.GetOrCreateAsync(DiscoverAsync, CancellationToken.None)); + await Assert.ThrowsAsync(() => cache.GetOrCreateAsync(CancellationToken.None)); Assert.Equal(cached ? 1 : 0, calls); } - private static McpListedSkillsDiscoveryResult CreateResult(ManualTimeProvider clock, params long[] ttls) => + private static McpListedSkillsSnapshot CreateSnapshot(ManualTimeProvider clock, params long[] ttls) => new([new AgentInlineSkill("test-skill", "A test skill.", "Test instructions.")], ttls.Select(ttl => new McpListedSkillsPageLifetime(clock.GetTimestamp(), TimeSpan.FromMilliseconds(ttl))).ToList());