From 8b69fe48f3604d7e294ff4e3ac23604f6407b141 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Mon, 2 Nov 2026 18:04:44 +0000 Subject: [PATCH 01/12] .NET: Support MCP skills cacheScope with per-key cache partitions Validate cacheScope on skills/list, skills/get and resources/read responses. Treat missing or null scopes as public, and reject private responses unless CachePartitionKeySelector is configured. Keep a separate skills cache per partition key in a MemoryCache with optional count and expiration limits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Agent_Step06_McpBasedSkills/Program.cs | 19 +- .../Agent_Step06_McpBasedSkills/README.md | 61 ++ .../Microsoft.Agents.AI.Mcp.csproj | 1 + .../Skills/AgentMcpSkill.cs | 8 +- .../Skills/AgentMcpSkillsSource.cs | 72 +- .../Skills/AgentMcpSkillsSourceOptions.cs | 79 ++ ...AgentSkillsProviderBuilderMcpExtensions.cs | 4 +- .../Skills/McpSkillCacheScopeConverter.cs | 51 + .../Skills/McpSkillsCatalog.cs | 50 +- .../Skills/McpSkillsGetResult.cs | 7 + .../Skills/McpSkillsListResult.cs | 7 + .../Skills/AgentMcpSkillTests.cs | 9 +- .../AgentMcpSkillsSerializationTests.cs | 73 ++ .../Skills/AgentMcpSkillsTests.CacheScope.cs | 975 ++++++++++++++++++ .../Skills/AgentMcpSkillsTests.Refresh.cs | 4 +- .../Skills/AgentMcpSkillsTests.Ttl.cs | 3 + .../Skills/AgentMcpSkillsTests.cs | 14 +- 17 files changed, 1417 insertions(+), 20 deletions(-) create mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillCacheScopeConverter.cs create mode 100644 dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/Program.cs b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/Program.cs index c1986cc3cc4..96c390a3755 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/Program.cs +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/Program.cs @@ -52,6 +52,7 @@ var skillsProvider = new AgentSkillsProviderBuilder() .UseMcpSkills(client) + .DisableCaching() .Build(); // --- Agent --- @@ -186,7 +187,9 @@ 3. Present the converted value clearly with both units "digest": "{{ComputeDigest(ConversionTable)}}", "size": {{Encoding.UTF8.GetByteCount(ConversionTable)}} } - ] + ], + "ttlMs": 300000, + "cacheScope": "public" } ] } @@ -205,12 +208,22 @@ private static string ComputeDigest(string content) => /// [McpServerResource(UriTemplate = SkillUri, Name = "Unit Converter Skill", MimeType = "text/markdown")] [Description("Unit converter skill instructions")] - public static string GetSkillMd() => SkillMd; + public static ReadResourceResult GetSkillMd() => new() + { + Contents = [new TextResourceContents { Uri = SkillUri, Text = SkillMd, MimeType = "text/markdown" }], + TimeToLive = TimeSpan.FromMinutes(5), + CacheScope = CacheScope.Public, + }; /// /// Returns the conversion factors used by the unit-converter skill. /// [McpServerResource(UriTemplate = ConversionTableUri, Name = "Conversion Table", MimeType = "text/markdown")] [Description("Unit conversion factors")] - public static string GetConversionTable() => ConversionTable; + public static ReadResourceResult GetConversionTable() => new() + { + Contents = [new TextResourceContents { Uri = ConversionTableUri, Text = ConversionTable, MimeType = "text/markdown" }], + TimeToLive = TimeSpan.FromMinutes(5), + CacheScope = CacheScope.Public, + }; } diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md index 77381067517..6e1d5914d53 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md @@ -75,6 +75,67 @@ still applies. ## Security Considerations +### Sharing a source across authorization contexts + +According to the MCP specification, each `skills/list`, `skills/get`, and `resources/read` response must include +a `cacheScope` field that tells the client who may reuse the cached response: + +- `public` - the response is the same for every caller and may be shared between them. +- `private` - the response is specific to the caller's credentials (for example, a user or tenant) + and must not be shared with other callers. + +Not all servers follow the specification yet, so a missing or `null` `cacheScope` is treated as `public`. + +To keep private responses from leaking between callers, the source needs to know who the current caller is. +By default, it doesn't: all callers share one cache, so the source accepts only public responses and throws +`InvalidOperationException` for a private one. Each response is checked on its own, so a skill from a public +list can still fail if its `SKILL.md` or another file is returned as private. + +To accept private responses, set `CachePartitionKeySelector` to return a key for the current caller, such as +a user or tenant ID. Each key gets its own cache partition, so cached skills are never shared between keys. +Set it whenever the server can return private responses, even if your app has only one user: + +```csharp +var options = new AgentMcpSkillsSourceOptions +{ + CachePartitionKeySelector = context => GetApplicationAuthorizationKey(context), + MaxCachedPartitions = 1000, + PartitionSlidingExpiration = TimeSpan.FromMinutes(30), + PartitionAbsoluteExpiration = TimeSpan.FromHours(8), +}; + +using var provider = new AgentSkillsProviderBuilder() + .UseMcpSkills(sharedMcpClient, options) + .DisableCaching() + .Build(); +``` + +`GetApplicationAuthorizationKey` is your own helper, not a framework API. It should return a non-empty ID +for the caller whose credentials the MCP client sends, for example a user or tenant ID. +Use an opaque ID, not an access token. + +The source only separates caches; it does not handle authentication. Your app is responsible for: + +- configuring the MCP client to send the current caller's credentials; +- keeping the same caller's credentials when a returned skill later reads `SKILL.md` or other files; +- not sharing returned skills between callers. + +`DisableCaching()` turns off the provider's own cache, so the source, and therefore the key selector, +runs on every call. + +By default, a partition is kept for every key until the source is disposed. In services with many users, +use the other options to limit memory: + +- `MaxCachedPartitions` - the maximum number of partitions to keep. +- `PartitionSlidingExpiration` - removes a partition that has not been used for this long. +- `PartitionAbsoluteExpiration` - removes a partition this long after it was created, even if it is still in use. + +When a partition is removed, the next call for its key fetches skills from the server again. +Skills that were already returned keep working. These options don't change how long MCP responses +are cached; use `TtlResolver` for that. + +### Trust and size limits + Discovering skills over MCP means an external MCP server controls what skill content (including instructions and supporting resources) reaches the agent. A compromised or untrustworthy server could return adversarial content designed to manipulate the agent (indirect diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Microsoft.Agents.AI.Mcp.csproj b/dotnet/src/Microsoft.Agents.AI.Mcp/Microsoft.Agents.AI.Mcp.csproj index 9275dba930a..f130e16528c 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Microsoft.Agents.AI.Mcp.csproj +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Microsoft.Agents.AI.Mcp.csproj @@ -30,6 +30,7 @@ + diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs index 3eb7d430944..564b21ff09e 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs @@ -107,7 +107,8 @@ public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyLis /// A size or content hash mismatch returns previously verified content if available and queues a skill refresh. /// /// The SKILL.md resource is missing or invalid, - /// exceeds the size limit, has a different name or description, or fails the size or content hash check without cached content to return. + /// exceeds the size limit, has a different name or description, fails the size or content hash check without cached content to return, + /// or is private and no cache partition key is set. public override async ValueTask GetContentAsync(CancellationToken cancellationToken = default) { McpSkillResourceEntry? skillMdResource = this._resources.FirstOrDefault(resource => string.Equals(resource.Uri, this.Uri, StringComparison.Ordinal)); @@ -135,7 +136,8 @@ public override async ValueTask GetContentAsync(CancellationToken cancel /// A size or content hash mismatch returns previously verified content if available and queues a skill refresh. /// /// The resource response is invalid, exceeds the size limit, - /// or fails the size or content hash check without cached content to return. + /// fails the size or content hash check without cached content to return, + /// or is private and no cache partition key is set. public override async ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) { Throw.IfNullOrWhitespace(name); @@ -195,6 +197,8 @@ private async ValueTask> FetchResourceContentAsy // Validation time consumes the TTL; network request time does not. long receivedTimestamp = this._timeProvider.GetTimestamp(); + response.CacheScope = this._skillsCatalog.ValidateAndNormalizeCacheScope(response.CacheScope, "resources/read"); + ResourceContents content = this.GetValidatedResourceContent(resource, response); if (!this.TryVerifyResourceIntegrity(resource, content, out string? mismatch)) { diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index c2eb9274cec..a6d97f1bdba 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -5,6 +5,8 @@ using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Shared.Diagnostics; @@ -32,6 +34,22 @@ namespace Microsoft.Agents.AI; /// , so this source can check expiration on each call. /// /// +/// By default, all callers share one cache. If this source is shared by several users or tenants, +/// set so each of them gets its own cache partition. +/// +/// +/// According to the MCP specification, each skills/list, skills/get, and resources/read response +/// must declare a cacheScope. Not all servers do, so the source handles it as follows: +/// +/// Missing or : treated as public. +/// public: always accepted. +/// private: accepted only when is set; +/// otherwise, is thrown. +/// +/// Each response is checked on its own. For example, a skill from a public list still fails if its +/// SKILL.md is returned as private and no selector is set. +/// +/// /// Listing a skill does not fetch its files. The returned skill object fetches SKILL.md /// and supporting resources only when their content is requested. Only resources in its manifest are available. /// Each resource response must contain one text or binary content block; SKILL.md must contain text. @@ -93,12 +111,17 @@ public sealed partial class AgentMcpSkillsSource : AgentSkillsSource private readonly McpClient _client; private readonly ILogger _logger; private readonly McpSkillsCatalog _catalog; + private readonly AgentMcpSkillsSourceOptions? _options; + private readonly TimeProvider _timeProvider; + private readonly MemoryCache _catalogs; + private readonly MemoryCacheEntryOptions? _catalogEntryOptions; + private bool _disposed; /// /// Initializes a new instance of the class. /// /// An MCP client connected to a server that supports skills/list. The caller retains ownership of the client and is responsible for disposing it. - /// Optional TTL resolver and limits for listing pages, skill counts, and resource sizes. + /// Optional cache partitioning and retention settings, TTL resolver, and limits for listing pages, skill counts, and resource sizes. /// Optional factory for diagnostic logging. /// is . /// A configured limit is not positive. @@ -112,7 +135,17 @@ internal AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? opt this._client = Throw.IfNull(client); loggerFactory ??= NullLoggerFactory.Instance; this._logger = loggerFactory.CreateLogger(); - this._catalog = new McpSkillsCatalog(this._client, options, this._logger, Throw.IfNull(timeProvider)); + this._timeProvider = Throw.IfNull(timeProvider); + this._options = options; + + this._catalog = new McpSkillsCatalog(this._client, this._options, this._logger, this._timeProvider); + + var cacheOptions = options?.ToMemoryCacheOptions() ?? new MemoryCacheOptions(); + cacheOptions.Clock = new CatalogClock(this._timeProvider); + + this._catalogs = new MemoryCache(cacheOptions); + + this._catalogEntryOptions = options?.ToMemoryCacheEntryOptions(); } /// @@ -130,19 +163,50 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo return []; } - IList skills = await this._catalog.GetSkillsAsync(cancellationToken).ConfigureAwait(false); + string? key = this._options?.CachePartitionKeySelector is { } selector + ? Throw.IfNullOrWhitespace(selector(context)) + : null; + McpSkillsCatalog catalog = key is null + ? this._catalog + : this.GetOrCreateCatalog(key); + + IList skills = await catalog.GetSkillsAsync(cancellationToken).ConfigureAwait(false); LogSkillsLoadedTotal(this._logger, skills.Count); return skills; } + private McpSkillsCatalog GetOrCreateCatalog(string key) + { + if (this._catalogs.TryGetValue(key, out McpSkillsCatalog? catalog)) + { + return catalog!; + } + + // MemoryCache.GetOrCreate may run the factory concurrently; the lock ensures one partition per key. + lock (this._catalogs) + { + return this._catalogs.GetOrCreate( + key, + _ => new McpSkillsCatalog(this._client, this._options, this._logger, this._timeProvider, hasIsolationKey: true), + this._catalogEntryOptions)!; + } + } + + private sealed class CatalogClock(TimeProvider timeProvider) : ISystemClock + { + public DateTimeOffset UtcNow => timeProvider.GetUtcNow(); + } + /// protected override void Dispose(bool disposing) { - if (disposing) + if (disposing && !this._disposed) { + this._disposed = true; this._catalog.Dispose(); + this._catalogs.Dispose(); } base.Dispose(disposing); diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs index b95ff0b21fc..a78fadeaf78 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System; +using Microsoft.Extensions.Caching.Memory; namespace Microsoft.Agents.AI; @@ -17,6 +18,66 @@ namespace Microsoft.Agents.AI; /// public sealed class AgentMcpSkillsSourceOptions { + /// + /// Gets or sets a callback that returns a key identifying the current caller, such as a user or tenant ID. + /// The source keeps a separate skill cache for each key. + /// + /// + /// + /// MCP servers mark each response as public (the same for every caller) or private + /// (specific to the caller's credentials) using cacheScope. A missing or + /// cacheScope is treated as public. + /// + /// + /// When this callback is (the default), the source cannot tell callers apart, so all callers + /// share one cache and only public responses are accepted. A private response throws . + /// + /// + /// When this callback is set, each key gets its own cache, so cached skills are never shared between keys, + /// and private responses are accepted. Set it if the source is shared by several users or tenants, + /// or if the server returns private responses, even for a single user. + /// + /// + /// The callback runs on every skill discovery call and must return a non-empty key that matches the credentials + /// the MCP client sends. Use an opaque ID, not an access token. Do not share returned skills between callers. + /// Call so the provider calls this source, and therefore + /// this callback, every time. + /// + /// + public Func? CachePartitionKeySelector { get; set; } + + /// + /// Gets or sets the maximum number of cache partitions to keep, one per key returned by + /// . + /// + /// + /// When (the default), there is no limit, and a partition is kept for every key until the source is disposed. + /// Set a limit for services with many users. When the limit is reached, skills for a new key are still returned, + /// but are fetched from the server again on the next call. Must not be negative. + /// + public int? MaxCachedPartitions { get; set; } + + /// + /// Gets or sets how long a cache partition is kept after it was last used. + /// + /// + /// When (the default), unused partitions are kept until the source is disposed. + /// Set it to free memory used by inactive users. After a partition is removed, the next call for its key + /// fetches skills from the server again. Must be positive. + /// To control how long MCP responses are cached, use . + /// + public TimeSpan? PartitionSlidingExpiration { get; set; } + + /// + /// Gets or sets how long a cache partition is kept after it was created, even if it is still in use. + /// + /// + /// When (the default), partitions have no maximum lifetime. + /// After a partition is removed, the next call for its key fetches skills from the server again. Must be positive. + /// To control how long MCP responses are cached, use . + /// + public TimeSpan? PartitionAbsoluteExpiration { get; set; } + /// /// Gets or sets a callback that determines the cache duration for each skill-list page, skill metadata, and resource response. /// @@ -65,4 +126,22 @@ public sealed class AgentMcpSkillsSourceOptions /// There is no combined resource-size limit per skill. /// public long? ListedResourceMaxSizeBytes { get; set; } + + /// + /// Creates cache options from the partition retention settings. + /// + internal MemoryCacheOptions ToMemoryCacheOptions() => new() + { + SizeLimit = this.MaxCachedPartitions + }; + + /// + /// Creates entry options from the partition retention settings. + /// + internal MemoryCacheEntryOptions ToMemoryCacheEntryOptions() => new() + { + Size = 1, + SlidingExpiration = this.PartitionSlidingExpiration, + AbsoluteExpirationRelativeToNow = this.PartitionAbsoluteExpiration + }; } diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs index c7d0bc1ead4..fc7db6cade9 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs @@ -15,7 +15,7 @@ public static class AgentSkillsProviderBuilderMcpExtensions /// /// The builder to extend. /// An MCP client connected to a server that supports skills/list. - /// Optional TTL resolver and limits for listing pages, skill counts, and resource sizes. + /// Optional cache partitioning and retention settings, TTL resolver, and limits for listing pages, skill counts, and resource sizes. /// The builder for chaining calls. /// /// @@ -30,6 +30,8 @@ public static class AgentSkillsProviderBuilderMcpExtensions /// Configure to override TTLs for list pages, skill metadata, and resources. /// Call and disable other caches around /// this source so it can refresh the skill list when its cached data expires. + /// If the source is shared by several users or tenants, or the server returns private responses, + /// set . /// /// /// var provider = new AgentSkillsProviderBuilder() diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillCacheScopeConverter.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillCacheScopeConverter.cs new file mode 100644 index 00000000000..2a82a92dd32 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillCacheScopeConverter.cs @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Text.Json; +using System.Text.Json.Serialization; +using ModelContextProtocol.Protocol; + +namespace Microsoft.Agents.AI; + +/// +/// Converts skill cache scopes and rejects unrecognized values. +/// +internal sealed class McpSkillCacheScopeConverter : JsonConverter +{ + /// + public override CacheScope? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.String) + { + string? value = reader.GetString(); + if (string.Equals(value, "public", StringComparison.OrdinalIgnoreCase)) + { + return CacheScope.Public; + } + + if (string.Equals(value, "private", StringComparison.OrdinalIgnoreCase)) + { + return CacheScope.Private; + } + } + + throw new JsonException("The cacheScope must be 'public' or 'private'."); + } + + /// + public override void Write(Utf8JsonWriter writer, CacheScope? value, JsonSerializerOptions options) + { + if (value is null) + { + writer.WriteNullValue(); + return; + } + + writer.WriteStringValue(value switch + { + CacheScope.Public => "public", + CacheScope.Private => "private", + _ => throw new JsonException($"Unsupported {nameof(CacheScope)} value: {value}."), + }); + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs index 009a6d6e294..f3003c84208 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs @@ -65,6 +65,7 @@ internal sealed partial class McpSkillsCatalog : IDisposable private readonly int _maxSkillCount; private readonly TimeProvider _timeProvider; private readonly Func? _ttlResolver; + private readonly bool _hasIsolationKey; private readonly McpCache> _catalogCache; private readonly ConcurrentDictionary _skillsToRefresh = new(); private bool _disposed; @@ -76,9 +77,10 @@ internal sealed partial class McpSkillsCatalog : IDisposable /// Optional TTL resolver and limits for listing pages, skill counts, and resource sizes. /// Optional logger for skill validation diagnostics. /// Optional clock for checking expiration. + /// Whether this catalog belongs to a cache partition key. /// is . /// A configured limit is not positive. - public McpSkillsCatalog(McpClient client, AgentMcpSkillsSourceOptions? options = null, ILogger? logger = null, TimeProvider? timeProvider = null) + public McpSkillsCatalog(McpClient client, AgentMcpSkillsSourceOptions? options = null, ILogger? logger = null, TimeProvider? timeProvider = null, bool hasIsolationKey = false) { this._client = Throw.IfNull(client); this._logger = logger ?? NullLogger.Instance; @@ -87,6 +89,7 @@ public McpSkillsCatalog(McpClient client, AgentMcpSkillsSourceOptions? options = this._maxSkillCount = options?.ListedMaxSkillCount ?? DefaultMaxSkillCount; this._timeProvider = timeProvider ?? TimeProvider.System; this._ttlResolver = options?.TtlResolver; + this._hasIsolationKey = hasIsolationKey; ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxResourceSizeBytes, nameof(options.ListedResourceMaxSizeBytes)); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxPageCount, nameof(options.ListedMaxPageCount)); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxSkillCount, nameof(options.ListedMaxSkillCount)); @@ -145,6 +148,33 @@ public void QueueSkillRefresh(AgentMcpSkill skill) } } + /// + /// Treats missing or null scopes as public and rejects private responses without a cache partition key. + /// + /// The response's declared cache scope. + /// The MCP method producing the response. + /// The validated scope, or public when the scope is missing or null. + /// The response is private and this partition has no cache partition key. + public CacheScope ValidateAndNormalizeCacheScope(CacheScope? scope, string method) + { + // Treat missing or null scopes as public. + if (scope is null) + { + LogUnspecifiedCacheScope(this._logger, method); + scope = CacheScope.Public; + } + + // Private responses require an isolation key to prevent reuse across authorization contexts. + if (scope == CacheScope.Private && !this._hasIsolationKey) + { + throw new InvalidOperationException( + $"The {method} response has a private cacheScope, but no cache partition key was provided. " + + $"Configure {nameof(AgentMcpSkillsSourceOptions)}.{nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector)} to return a nonempty authorization-context key."); + } + + return scope.Value; + } + /// /// Fetches the complete list if it is missing or expired, or refreshes queued and expired skills in a fresh list. /// @@ -292,6 +322,8 @@ private async Task>> FetchSkillsAsync(Cancellati throw new InvalidOperationException($"The skills/get result must describe the requested skill '{skillUri}'."); } + result.CacheScope = this.ValidateAndNormalizeCacheScope(result.CacheScope, "skills/get"); + // Unlike listing responses, skills/get provides a TTL for this skill, // allowing it to expire and refresh independently of the catalog. return this.CreateSkill(result.Skill, McpCacheLifetime.Create(receivedTimestamp, result.TimeToLive, this._ttlResolver)); @@ -350,6 +382,7 @@ private McpCacheLifetime CombineLifetimes(List lifetimes) HashSet cursors = new(StringComparer.Ordinal); string? cursor = null; int pageCount = 0; + CacheScope? declaredScope = null; do { @@ -394,6 +427,15 @@ private McpCacheLifetime CombineLifetimes(List lifetimes) throw new InvalidOperationException($"A skills/list result must have resultType '{CompleteResultType}'. Received resultType '{result.ResultType}'."); } + result.CacheScope = this.ValidateAndNormalizeCacheScope(result.CacheScope, "skills/list"); + + if (declaredScope is { } previousScope && previousScope != result.CacheScope) + { + throw new InvalidOperationException("All skills/list pages must declare the same cacheScope."); + } + + declaredScope = result.CacheScope; + pageLifetimes.Add(McpCacheLifetime.Create(receivedTimestamp, result.TimeToLive, this._ttlResolver)); cursor = result.NextCursor; @@ -535,4 +577,10 @@ private void ValidateResourceManifestEntry([NotNull] McpSkillResourceEntry? reso /// [GeneratedRegex(@"\Asha256:[0-9a-f]{64}\z", RegexOptions.CultureInvariant)] private static partial Regex ResourceDigestRegex(); + + /// + /// Logs that a response has no cacheScope and is treated as public. + /// + [LoggerMessage(LogLevel.Debug, "The {Method} response has a missing or null cacheScope; treating it as public.")] + internal static partial void LogUnspecifiedCacheScope(ILogger logger, string method); } diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsGetResult.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsGetResult.cs index 3a6f8cf6611..514f6c29691 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsGetResult.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsGetResult.cs @@ -27,4 +27,11 @@ internal sealed class McpSkillsGetResult [JsonPropertyName("ttlMs")] [JsonConverter(typeof(TimeSpanMillisecondsConverter))] public TimeSpan? TimeToLive { get; init; } + + /// + /// Gets the declared cache scope, or null when the response's cacheScope field is missing or null. + /// + [JsonPropertyName("cacheScope")] + [JsonConverter(typeof(McpSkillCacheScopeConverter))] + public CacheScope? CacheScope { get; set; } } diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsListResult.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsListResult.cs index 32ca5ea312d..d9d466d1449 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsListResult.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsListResult.cs @@ -33,6 +33,13 @@ internal sealed class McpSkillsListResult [JsonPropertyName("ttlMs")] [JsonConverter(typeof(TimeSpanMillisecondsConverter))] public TimeSpan? TimeToLive { get; init; } + + /// + /// Gets the declared cache scope, or null when the response's cacheScope field is missing or null. + /// + [JsonPropertyName("cacheScope")] + [JsonConverter(typeof(McpSkillCacheScopeConverter))] + public CacheScope? CacheScope { get; set; } } /// diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs index 03ed9a1fd50..757c4b0c325 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs @@ -1206,8 +1206,13 @@ private static McpSkillsCatalog CreateCatalog(TimeProvider? timeProvider = null) }; /// - /// Creates an in-memory server using the test's resource-read handler. + /// Creates an in-memory server using the test's resource-read handler, defaulting scopes to public. /// private static InMemoryMcpServer CreateServer(Func> readHandler) => - new(builder => builder.WithReadResourceHandler((request, token) => readHandler(request.Params!.Uri, token))); + new(builder => builder.WithReadResourceHandler(async (request, token) => + { + ReadResourceResult result = await readHandler(request.Params!.Uri, token); + result.CacheScope ??= CacheScope.Public; + return result; + })); } diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSerializationTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSerializationTests.cs index a7a22a6ecc8..565a5c5e457 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSerializationTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSerializationTests.cs @@ -3,6 +3,7 @@ using System; using System.Text.Json; using System.Text.Json.Nodes; +using ModelContextProtocol.Protocol; namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; @@ -11,6 +12,78 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; /// public sealed class AgentMcpSkillsSerializationTests { + [Theory] + [InlineData(null, null)] + [InlineData("null", null)] + [InlineData("\"public\"", CacheScope.Public)] + [InlineData("\"PUBLIC\"", CacheScope.Public)] + [InlineData("\"private\"", CacheScope.Private)] + [InlineData("\"PRIVATE\"", CacheScope.Private)] + public void Deserialize_CacheScope_MatchesResourceSdkAndRoundTripsKnownScopes(string? json, CacheScope? expected) + { + // Arrange + JsonObject listing = new() + { + ["resultType"] = "complete", + ["skills"] = new JsonArray(), + }; + JsonObject get = new() + { + ["resultType"] = "complete", + ["skill"] = CreateEntry(), + }; + if (json is not null) + { + listing["cacheScope"] = JsonNode.Parse(json); + get["cacheScope"] = JsonNode.Parse(json); + } + + // Act + var listResult = listing.Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult); + var getResult = get.Deserialize(McpSkillsJsonContext.Default.McpSkillsGetResult); + var resourceResult = listing.Deserialize(); + + // Assert + Assert.NotNull(listResult); + Assert.NotNull(getResult); + Assert.NotNull(resourceResult); + Assert.Equal(expected, listResult.CacheScope); + Assert.Equal(expected, getResult.CacheScope); + Assert.Equal(expected, resourceResult.CacheScope); + Assert.Equal(expected, JsonSerializer.SerializeToNode(listResult, McpSkillsJsonContext.Default.McpSkillsListResult)! + .Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult)!.CacheScope); + Assert.Equal(expected, JsonSerializer.SerializeToNode(getResult, McpSkillsJsonContext.Default.McpSkillsGetResult)! + .Deserialize(McpSkillsJsonContext.Default.McpSkillsGetResult)!.CacheScope); + } + + [Theory] + [InlineData("\"future\"")] + [InlineData("\"\"")] + [InlineData("42")] + [InlineData("true")] + [InlineData("{\"nested\":[1,2]}")] + [InlineData("[{\"nested\":true}]")] + public void Deserialize_InvalidCacheScope_Throws(string json) + { + // Arrange + JsonObject listing = new() + { + ["resultType"] = "complete", + ["skills"] = new JsonArray(), + ["cacheScope"] = JsonNode.Parse(json), + }; + JsonObject get = new() + { + ["resultType"] = "complete", + ["skill"] = CreateEntry(), + ["cacheScope"] = JsonNode.Parse(json), + }; + + // Act & Assert + Assert.Throws(() => listing.Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult)); + Assert.Throws(() => get.Deserialize(McpSkillsJsonContext.Default.McpSkillsGetResult)); + } + [Theory] [InlineData("null", null)] [InlineData("0", 0L)] diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs new file mode 100644 index 00000000000..c41533c1fb3 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs @@ -0,0 +1,975 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json.Nodes; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.AI; +using Microsoft.Extensions.Logging; +using ModelContextProtocol.Protocol; +using Moq; + +namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; + +public sealed partial class AgentMcpSkillsTests +{ + [Theory] + [InlineData(nameof(AgentMcpSkillsSourceOptions.MaxCachedPartitions), -1)] + [InlineData(nameof(AgentMcpSkillsSourceOptions.PartitionSlidingExpiration), 0)] + [InlineData(nameof(AgentMcpSkillsSourceOptions.PartitionSlidingExpiration), -1)] + [InlineData(nameof(AgentMcpSkillsSourceOptions.PartitionAbsoluteExpiration), 0)] + [InlineData(nameof(AgentMcpSkillsSourceOptions.PartitionAbsoluteExpiration), -1)] + public void Constructor_InvalidPartitionRetention_Throws(string option, int value) + { + // Arrange + var client = CreateSkillsClient(); + var options = new AgentMcpSkillsSourceOptions + { + MaxCachedPartitions = option == nameof(AgentMcpSkillsSourceOptions.MaxCachedPartitions) ? value : null, + PartitionSlidingExpiration = option == nameof(AgentMcpSkillsSourceOptions.PartitionSlidingExpiration) ? TimeSpan.FromSeconds(value) : null, + PartitionAbsoluteExpiration = option == nameof(AgentMcpSkillsSourceOptions.PartitionAbsoluteExpiration) ? TimeSpan.FromSeconds(value) : null, + }; + + // Act & Assert + Assert.Throws(() => new AgentMcpSkillsSource(client.Object, options)); + } + + [Fact] + public async Task GetSkillsAsync_PartitionSlidingExpiration_ExtendsOnDiscoveryButNotResourceReadsAsync() + { + // Arrange + var clock = new ManualTimeProvider(); + int lists = 0; + var client = CreateTtlClient((request, _) => + { + if (request.Method == "skills/list") + { + lists++; + return Task.FromResult(TtlListResult(60000, Entry())); + } + + return Task.FromResult(ResourceReadResult(SkillUri, 60000, SkillContent)); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = _ => "A", + PartitionSlidingExpiration = TimeSpan.FromSeconds(10), + }, null, clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Act & Assert + clock.Advance(TimeSpan.FromSeconds(9)); + Assert.Same(first, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + clock.Advance(TimeSpan.FromSeconds(9)); + Assert.Equal(SkillContent, await first[0].GetContentAsync()); + clock.Advance(TimeSpan.FromSeconds(1)); + var replacement = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + Assert.NotSame(first, replacement); + Assert.NotSame(first[0], replacement[0]); + Assert.Same(replacement, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Equal(SkillContent, await first[0].GetContentAsync()); + Assert.Equal(2, lists); + client.Verify(c => c.DisposeAsync(), Times.Never); + } + + [Fact] + public async Task GetSkillsAsync_PartitionAbsoluteExpiration_ExpiresDespiteDiscoveryAsync() + { + // Arrange + var clock = new ManualTimeProvider(); + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(60000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = _ => "A", + PartitionSlidingExpiration = TimeSpan.FromSeconds(10), + PartitionAbsoluteExpiration = TimeSpan.FromSeconds(25), + }, null, clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Act & Assert + clock.Advance(TimeSpan.FromSeconds(9)); + Assert.Same(first, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + clock.Advance(TimeSpan.FromSeconds(9)); + Assert.Same(first, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + clock.Advance(TimeSpan.FromSeconds(7)); + Assert.NotSame(first, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Equal(2, lists); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_PartitionRetention_DoesNotChangeResponseTtlAsync(bool partitioned) + { + // Arrange + var clock = new ManualTimeProvider(); + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(1000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = partitioned ? _ => "A" : null, + PartitionSlidingExpiration = TimeSpan.FromSeconds(10), + PartitionAbsoluteExpiration = TimeSpan.FromSeconds(20), + }, null, clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Act + clock.Advance(TimeSpan.FromSeconds(1)); + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.NotSame(first, refreshed); + Assert.Equal(2, lists); + } + + [Fact] + public async Task GetSkillsAsync_PartitionRetention_DoesNotEvictDefaultPartitionAsync() + { + // Arrange + var clock = new ManualTimeProvider(); + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(60000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + MaxCachedPartitions = 1, + PartitionSlidingExpiration = TimeSpan.FromSeconds(1), + PartitionAbsoluteExpiration = TimeSpan.FromSeconds(1), + }, null, clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Act + clock.Advance(TimeSpan.FromSeconds(2)); + var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.Same(first, reused); + Assert.Equal(1, lists); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_PartitionEviction_PreservesInFlightDiscoveryAsync(bool capacity) + { + // Arrange + var clock = new ManualTimeProvider(); + string key = "A"; + int lists = 0; + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var client = CreateTtlClient((request, _) => + { + if (request.Method == "skills/list") + { + return Interlocked.Increment(ref lists) == 1 + ? release.Task + : Task.FromResult(TtlListResult(60000, Entry())); + } + + return Task.FromResult(ResourceReadResult(SkillUri, 60000, SkillContent)); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = _ => key, + MaxCachedPartitions = capacity ? 1 : null, + PartitionAbsoluteExpiration = capacity ? null : TimeSpan.FromSeconds(1), + }, null, clock); + Task> first = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Act + clock.Advance(TimeSpan.FromSeconds(1)); + key = capacity ? "B" : "A"; + try + { + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()).WaitAsync(TimeSpan.FromSeconds(10)); + + // Assert + Assert.False(first.IsCompleted); + Assert.Single(second); + Assert.Equal(SkillContent, await second[0].GetContentAsync()); + Assert.Equal(2, lists); + } + finally + { + release.TrySetResult(TtlListResult(60000, Entry())); + } + + var retained = await first.WaitAsync(TimeSpan.FromSeconds(10)); + Assert.Equal(SkillContent, await retained[0].GetContentAsync()); + client.Verify(c => c.DisposeAsync(), Times.Never); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task GetSkillsAsync_PrivateScopeWithoutPartitionKey_FailsAndRetriesWholeListingAsync(bool firstPagePrivate) + { + // Arrange + bool returnPrivate = true; + int calls = 0; + var client = CreateTtlClient((request, _) => + { + calls++; + bool firstPage = request.Params?["cursor"] is null; + var result = TtlListResult(60000, Entry()); + result["cacheScope"] = returnPrivate && (firstPagePrivate || !firstPage) ? "private" : "public"; + if (firstPage && !firstPagePrivate) + { + result["nextCursor"] = "last"; + } + + return Task.FromResult(result); + }); + using var source = new AgentMcpSkillsSource(client.Object); + + // Act & Assert + for (int attempt = 0; attempt < 2; attempt++) + { + var error = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Contains("skills/list", error.Message); + Assert.Contains("private", error.Message); + Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector), error.Message); + } + + Assert.Equal(firstPagePrivate ? 2 : 4, calls); + returnPrivate = false; + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + Assert.NotEmpty(skills); + Assert.Same(skills, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Equal(firstPagePrivate ? 3 : 6, calls); + } + + [Theory] + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public async Task GetSkillsAsync_PublicListingWithPrivateResourceWithoutPartitionKey_ThrowsWithoutCachingAsync(bool instructions, bool cachedPublic) + { + // Arrange + var clock = new ManualTimeProvider(); + bool returnPrivate = !cachedPublic; + int reads = 0; + int lists = 0; + var client = CreateTtlClient((request, _) => + { + if (request.Method == "skills/list") + { + lists++; + var listing = TtlListResult(60000, Entry()); + listing["cacheScope"] = "public"; + return Task.FromResult(listing); + } + + reads++; + string uri = request.Params!["uri"]!.GetValue(); + var result = ResourceReadResult(uri, 1000, uri == SkillUri ? SkillContent : "Checklist."); + result["cacheScope"] = returnPrivate ? "private" : "public"; + return Task.FromResult(result); + }); + using var source = new AgentMcpSkillsSource(client.Object, null, null, clock); + var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + if (cachedPublic) + { + await ReadSkillContentAsync(skill, instructions); + clock.Advance(TimeSpan.FromSeconds(1)); + returnPrivate = true; + } + + // Act & Assert + for (int attempt = 0; attempt < 2; attempt++) + { + var error = await Assert.ThrowsAsync(() => ReadSkillContentAsync(skill, instructions)); + Assert.Contains("resources/read", error.Message); + Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector), error.Message); + Assert.Same(skill, Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); + } + + returnPrivate = false; + await ReadSkillContentAsync(skill, instructions); + await ReadSkillContentAsync(skill, instructions); + Assert.Equal(cachedPublic ? 4 : 3, reads); + Assert.Equal(1, lists); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_PrivateMetadataScope_RequiresPartitionKeyAsync(bool partitioned) + { + // Arrange + bool returnPrivate = true; + int gets = 0; + var client = CreateTtlClient((request, _) => + { + JsonObject result; + switch (request.Method) + { + case "skills/list": + result = TtlListResult(60000, Entry()); + result["cacheScope"] = "public"; + break; + case "skills/get": + gets++; + result = GetSkillResult(Entry()); + result["cacheScope"] = returnPrivate ? "private" : "public"; + break; + default: + result = ResourceReadResult(SkillUri, 1000, SkillContent + "\nChanged."); + break; + } + + return Task.FromResult(result); + }); + using var source = new AgentMcpSkillsSource(client.Object, partitioned ? new() + { + CachePartitionKeySelector = _ => "authorization-context", + } : null); + var original = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + await Assert.ThrowsAsync(async () => await original.GetContentAsync()); + + // Act & Assert + if (!partitioned) + { + for (int attempt = 0; attempt < 2; attempt++) + { + var error = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Contains("skills/get", error.Message); + Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector), error.Message); + } + + returnPrivate = false; + } + + var refreshed = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.NotSame(original, refreshed); + Assert.Same(refreshed, Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); + Assert.Equal(partitioned ? 1 : 3, gets); + } + + [Fact] + public async Task GetSkillsAsync_ChangingSelector_DoesNotChangeRetainedCachePartitionAsync() + { + // Arrange + var options = new AgentMcpSkillsSourceOptions(); + var client = CreateTtlClient((request, _) => + { + var result = request.Method == "skills/list" + ? TtlListResult(60000, Entry()) + : ResourceReadResult(SkillUri, 60000, SkillContent); + result["cacheScope"] = request.Method == "skills/list" ? "public" : "private"; + return Task.FromResult(result); + }); + using var source = new AgentMcpSkillsSource(client.Object, options); + var unkeyed = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + options.CachePartitionKeySelector = _ => "authorization-context"; + var keyed = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + + // Act & Assert + await Assert.ThrowsAsync(async () => await unkeyed.GetContentAsync()); + Assert.Equal(SkillContent, await keyed.GetContentAsync()); + options.CachePartitionKeySelector = null; + Assert.Same(unkeyed, Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); + await Assert.ThrowsAsync(async () => await unkeyed.GetContentAsync()); + Assert.Equal(SkillContent, await keyed.GetContentAsync()); + } + + [Fact] + public async Task GetSkillsAsync_SharedClient_FetchesAndCachesContextSpecificContentAsync() + { + // Arrange + var authorizationKey = new AsyncLocal { Value = "A" }; + int lists = 0; + int reads = 0; + var client = CreateTtlClient((request, _) => + { + string content = SkillContent + "\n" + authorizationKey.Value; + if (request.Method == "skills/list") + { + lists++; + var result = TtlListResult(60000, Entry(content: content)); + result["cacheScope"] = "private"; + return Task.FromResult(result); + } + + reads++; + return Task.FromResult(ResourceReadResult(SkillUri, 60000, content)); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = _ => authorizationKey.Value!, + }); + + // Act + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + string firstContent = await first[0].GetContentAsync(); + authorizationKey.Value = "B"; + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + string secondContent = await second[0].GetContentAsync(); + authorizationKey.Value = "A"; + var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.Equal(SkillContent + "\nA", firstContent); + Assert.Equal(SkillContent + "\nB", secondContent); + Assert.Equal(firstContent, await reused[0].GetContentAsync()); + Assert.Same(first, reused); + Assert.Equal(2, lists); + Assert.Equal(2, reads); + } + + [Fact] + public async Task GetSkillsAsync_OptionsChangedAfterConstruction_NewPartitionUsesUpdatedOptionsAsync() + { + // Arrange + var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(60000, Entry()))); + var options = new AgentMcpSkillsSourceOptions { CachePartitionKeySelector = _ => "A" }; + using var source = new AgentMcpSkillsSource(client.Object, options); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + options.CachePartitionKeySelector = _ => "B"; + options.TtlResolver = _ => TimeSpan.Zero; + + // Act + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + options.CachePartitionKeySelector = _ => "A"; + var original = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.NotSame(first, second); + Assert.NotSame(second, reused); + Assert.Same(first, original); + } + + [Fact] + public async Task GetSkillsAsync_DisposedDuringKeySelection_DoesNotCreateNewPartitionAsync() + { + // Arrange + Action? duringSelection = null; + var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(60000, Entry()))); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = _ => + { + duringSelection?.Invoke(); + return "A"; + }, + }); + duringSelection = source.Dispose; + + // Act & Assert + await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Theory] + [InlineData("public", "private")] + [InlineData("private", "public")] + [InlineData("public", "public")] + [InlineData("private", "private")] + [InlineData(null, null)] + public async Task GetSkillsAsync_PartitionKeys_SeparateSkillsAndAllResourceCachesAsync(string? listScope, string? resourceScope) + { + // Arrange + string key = "authorization-A"; + int lists = 0; + int reads = 0; + var client = CreateTtlClient((request, _) => + { + if (request.Method == "skills/list") + { + lists++; + JsonObject listing = TtlListResult(60000, Entry()); + listing["cacheScope"] = listScope; + return Task.FromResult(listing); + } + + reads++; + string uri = request.Params!["uri"]!.GetValue(); + JsonObject result = ResourceReadResult(uri, 60000, uri == SkillUri ? SkillContent : "Checklist."); + result["cacheScope"] = resourceScope; + if (uri.EndsWith("icon.bin", StringComparison.Ordinal)) + { + result["contents"] = new JsonArray(new JsonObject + { + ["uri"] = uri, + ["blob"] = Convert.ToBase64String([1, 2, 3, 4]), + }); + } + + return Task.FromResult(result); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }); + + // Act + IList first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + await ReadSkillContentAsync(first[0], instructions: true); + await ReadSkillContentAsync(first[0], instructions: false); + var firstBinary = Assert.IsType(await first[0].GetResourceAsync("assets/icon.bin")); + Assert.Equal(new byte[] { 1, 2, 3, 4 }, Assert.IsType(await firstBinary.ReadAsync()).Data.ToArray()); + key = "authorization-B"; + IList second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + await ReadSkillContentAsync(second[0], instructions: true); + await ReadSkillContentAsync(second[0], instructions: false); + var secondBinary = Assert.IsType(await second[0].GetResourceAsync("assets/icon.bin")); + Assert.Equal(new byte[] { 1, 2, 3, 4 }, Assert.IsType(await secondBinary.ReadAsync()).Data.ToArray()); + key = "authorization-A"; + IList reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + await ReadSkillContentAsync(reused[0], instructions: true); + await ReadSkillContentAsync(reused[0], instructions: false); + await reused[0].GetResourceAsync("assets/icon.bin"); + + // Assert + Assert.Same(first, reused); + Assert.NotSame(first, second); + Assert.NotSame(first[0], second[0]); + Assert.Equal(2, lists); + Assert.Equal(6, reads); + } + + [Fact] + public async Task GetSkillsAsync_PartitionKeys_CompareOrdinallyAsync() + { + // Arrange + string key = "context"; + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(60000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }); + + // Act + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + key = "CONTEXT"; + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + key = "context"; + var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.NotSame(first, second); + Assert.Same(first, reused); + Assert.Equal(2, lists); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" \t")] + public async Task GetSkillsAsync_InvalidPartitionKey_ThrowsWithoutRequestsAsync(string? key) + { + // Arrange + var client = CreateSkillsClient(); + using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key! }); + + // Act & Assert + await Assert.ThrowsAnyAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task GetSkillsAsync_PartitionKeySelectorError_PropagatesWithoutRequestsAsync() + { + // Arrange + var error = new InvalidOperationException("Authorization context unavailable."); + var client = CreateSkillsClient(); + using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => throw error }); + + // Act + var actual = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + + // Assert + Assert.Same(error, actual); + client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_PartitionKeySelector_SkippedWithoutSupportOrWhenCanceledAsync(bool canceled) + { + // Arrange + var client = CreateSkillsClient(); + if (!canceled) + { + client.Object.ServerCapabilities.Extensions = null; + } + + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = _ => throw new InvalidOperationException("Must not run."), + }); + using var cts = new CancellationTokenSource(); + + // Act & Assert + if (canceled) + { + await cts.CancelAsync(); + await Assert.ThrowsAnyAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), cts.Token)); + } + else + { + Assert.Empty(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + } + } + + [Fact] + public async Task GetSkillsAsync_CachePartitions_ExpireIndependentlyAsync() + { + // Arrange + var clock = new ManualTimeProvider(); + string key = "A"; + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(1000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }, null, clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + clock.Advance(TimeSpan.FromMilliseconds(500)); + key = "B"; + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Act + clock.Advance(TimeSpan.FromMilliseconds(500)); + key = "A"; + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + key = "B"; + var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.NotSame(first, refreshed); + Assert.Same(second, reused); + Assert.Equal(3, lists); + } + + [Fact] + public async Task GetSkillsAsync_CachePartitions_KeepMismatchFallbackAndMetadataRefreshSeparateAsync() + { + // Arrange + var clock = new ManualTimeProvider(); + string key = "A"; + bool changed = false; + int gets = 0; + int reads = 0; + var client = CreateTtlClient((request, _) => + { + if (request.Method == "skills/list") + { + return Task.FromResult(TtlListResult(60000, Entry())); + } + + if (request.Method == "skills/get") + { + gets++; + return Task.FromResult(GetSkillResult(Entry(), 1000)); + } + + reads++; + return Task.FromResult(ResourceReadResult(SkillUri, 1000, changed ? SkillContent + "\nChanged." : SkillContent)); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }, null, clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + await first[0].GetContentAsync(); + key = "B"; + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + clock.Advance(TimeSpan.FromSeconds(1)); + changed = true; + key = "A"; + + // Act + Assert.Equal(SkillContent, await first[0].GetContentAsync()); + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + key = "B"; + var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.NotSame(first, refreshed); + Assert.Same(second, reused); + Assert.Equal(1, gets); + Assert.Equal(2, reads); + + // Act + clock.Advance(TimeSpan.FromSeconds(1)); + key = "A"; + Assert.Equal(SkillContent, await first[0].GetContentAsync()); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + key = "B"; + Assert.Same(second, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + await Assert.ThrowsAsync(async () => await second[0].GetContentAsync()); + + // Assert + Assert.Equal(2, gets); + Assert.Equal(3, reads); + } + + [Fact] + public async Task GetSkillsAsync_ConcurrentPartitionCreation_SharesPartitionPerKeyAsync() + { + // Arrange + var contextA = TestAgentSkillsSourceContextFactory.Create(); + var contextB = TestAgentSkillsSourceContextFactory.Create(); + var start = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + int lists = 0; + var client = CreateTtlClient((_, _) => + { + Interlocked.Increment(ref lists); + return Task.FromResult(TtlListResult(60000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = context => ReferenceEquals(context, contextA) ? "A" : "B", + }); + Task>[] requests = Enumerable.Range(0, 32).Select(index => Task.Run(async () => + { + await start.Task; + return await source.GetSkillsAsync(index % 2 == 0 ? contextA : contextB); + })).ToArray(); + + // Act + start.SetResult(); + IList[] results = await Task.WhenAll(requests).WaitAsync(TimeSpan.FromSeconds(10)); + + // Assert + Assert.Equal(2, lists); + Assert.NotSame(results[0], results[1]); + for (int i = 0; i < results.Length; i++) + { + Assert.Same(results[i % 2], results[i]); + } + } + + [Fact] + public async Task GetSkillsAsync_ConcurrentCachePartitions_DoNotBlockEachOtherAsync() + { + // Arrange + var contextA = TestAgentSkillsSourceContextFactory.Create(); + var contextB = TestAgentSkillsSourceContextFactory.Create(); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + int lists = 0; + var client = CreateTtlClient((_, _) => + Interlocked.Increment(ref lists) == 1 ? release.Task : Task.FromResult(TtlListResult(60000, Entry()))); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = context => ReferenceEquals(context, contextA) ? "A" : "B", + }); + + // Act + Task> first = source.GetSkillsAsync(contextA); + Task> sameKey = source.GetSkillsAsync(contextA); + try + { + var otherKey = await source.GetSkillsAsync(contextB).WaitAsync(TimeSpan.FromSeconds(10)); + + // Assert + Assert.Single(otherKey); + Assert.False(first.IsCompleted); + Assert.False(sameKey.IsCompleted); + Assert.Equal(2, lists); + } + finally + { + release.TrySetResult(TtlListResult(60000, Entry())); + } + + Assert.Same(await first, await sameKey); + Assert.Equal(2, lists); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_CachePartitionDisposal_PreservesRetainedSkillsAndClientAsync(bool inFlight) + { + // Arrange + string key = "A"; + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var client = CreateTtlClient((request, _) => request.Method == "skills/list" + ? key == "B" && inFlight ? release.Task : Task.FromResult(TtlListResult(60000, Entry())) + : Task.FromResult(ResourceReadResult(SkillUri, 60000, SkillContent))); + using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + key = "B"; + var second = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Act + source.Dispose(); + source.Dispose(); + release.TrySetResult(TtlListResult(60000, Entry())); + var retained = await second; + + // Assert + await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + key = "A"; + Assert.Equal(SkillContent, await first[0].GetContentAsync()); + key = "B"; + Assert.Equal(SkillContent, await retained[0].GetContentAsync()); + client.Verify(c => c.DisposeAsync(), Times.Never); + } + + [Theory] + [InlineData("public", "private", 3)] + [InlineData("private", "public", 2)] + [InlineData(null, "private", 3)] + [InlineData("private", null, 2)] + public async Task GetSkillsAsync_ConflictingPageScopes_FailsAndRetriesWholeListingAsync(string? firstScope, string? lastScope, int callsPerAttempt) + { + // Arrange + int calls = 0; + var client = CreateTtlClient((request, _) => + { + calls++; + string? cursor = request.Params?["cursor"]?.GetValue(); + var result = TtlListResult(60000); + string? scope = cursor is null ? firstScope : lastScope; + if (scope is not null) + { + result["cacheScope"] = scope; + } + if (cursor != "last") + { + result["nextCursor"] = cursor is null ? "missing-scope" : "last"; + if (cursor is not null) + { + result.Remove("cacheScope"); + } + } + + return Task.FromResult(result); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => "authorization-context" }); + + // Act & Assert + for (int attempt = 0; attempt < 2; attempt++) + { + var error = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Contains("cacheScope", error.Message); + } + + Assert.Equal(2 * callsPerAttempt, calls); + } + + [Theory] + [InlineData("public")] + public async Task GetSkillsAsync_PublicAndOmittedPageScopes_AcceptsListingAsync(string scope) + { + // Arrange + int calls = 0; + var client = CreateTtlClient((_, _) => + { + var result = TtlListResult(60000); + if (++calls != 2) + { + result["cacheScope"] = scope; + } + + if (calls < 3) + { + result["nextCursor"] = calls.ToString(System.Globalization.CultureInfo.InvariantCulture); + } + + return Task.FromResult(result); + }); + using var source = new AgentMcpSkillsSource(client.Object); + + // Act + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.Empty(skills); + Assert.Same(skills, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Equal(3, calls); + } + + [Theory] + [InlineData("skills/list", false)] + [InlineData("skills/list", true)] + [InlineData("skills/get", false)] + [InlineData("skills/get", true)] + [InlineData("resources/read", false)] + [InlineData("resources/read", true)] + public async Task GetSkillsAsync_UnspecifiedScope_DefaultsToPublicAndLogsDiagnosticsWithoutPartitionKeyAsync(string method, bool omitted) + { + // Arrange + const string PartitionKey = "do-not-log-this-key"; + var logger = new Mock(); + logger.Setup(l => l.IsEnabled(It.IsAny())).Returns(true); + var loggerFactory = new Mock(); + loggerFactory.Setup(f => f.CreateLogger(It.IsAny())).Returns(logger.Object); + var client = CreateTtlClient((request, _) => + { + JsonObject result = request.Method switch + { + "skills/list" => TtlListResult(60000, Entry()), + "skills/get" => GetSkillResult(Entry()), + _ => ResourceReadResult(SkillUri, 60000, SkillContent), + }; + if (request.Method == method) + { + if (omitted) + { + result.Remove("cacheScope"); + } + else + { + result["cacheScope"] = null; + } + } + else + { + result["cacheScope"] = "private"; + } + + return Task.FromResult(result); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CachePartitionKeySelector = _ => PartitionKey, + }, loggerFactory.Object); + + // Act + var skill = Assert.IsType(Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); + if (method == "skills/get") + { + client.Setup(c => c.SendRequestAsync(It.Is(r => r.Method == "resources/read"), It.IsAny())) + .Returns((JsonRpcRequest request, CancellationToken _) => Task.FromResult(new JsonRpcResponse + { + Id = request.Id, + Result = ResourceReadResult(SkillUri, 1000, SkillContent + "\nChanged."), + })); + await Assert.ThrowsAsync(async () => await skill.GetContentAsync()); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + } + else if (method == "resources/read") + { + Assert.Equal(SkillContent, await skill.GetContentAsync()); + } + + // Assert + logger.Verify(l => l.Log( + LogLevel.Debug, + It.IsAny(), + It.Is((state, _) => state.ToString() == + $"The {method} response has a missing or null cacheScope; treating it as public."), + It.IsAny(), + It.IsAny>()), Times.Once()); + Assert.DoesNotContain(logger.Invocations, invocation => invocation.Arguments.Any(argument => argument?.ToString()?.Contains(PartitionKey, StringComparison.Ordinal) == true)); + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs index b49b66a2ad4..5b35d5b12e2 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs @@ -1235,7 +1235,7 @@ public async Task GetSkillsAsync_DisposedCatalog_RetainedSkillRefetchesExpiredCo ["resultType"] = "complete", ["skill"] = entry, ["ttlMs"] = ttl, - ["cacheScope"] = "private", + ["cacheScope"] = "public", }; /// @@ -1247,7 +1247,7 @@ private static JsonObject ResourceReadResult(string uri, long? ttl, string text) { ["resultType"] = "complete", ["contents"] = new JsonArray(new JsonObject { ["uri"] = uri, ["text"] = text }), - ["cacheScope"] = "private", + ["cacheScope"] = "public", }; if (ttl is not null) { diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Ttl.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Ttl.cs index dbee4f36cc3..dc13db374db 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Ttl.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Ttl.cs @@ -786,6 +786,9 @@ private sealed class ManualTimeProvider(long timestampFrequency = TimeSpan.Ticks public override long TimestampFrequency => timestampFrequency; + public override DateTimeOffset GetUtcNow() => + DateTimeOffset.UnixEpoch + TimeSpan.FromSeconds((double)Interlocked.Read(ref this._timestamp) / this.TimestampFrequency); + public override long GetTimestamp() { long timestamp = Interlocked.Read(ref this._timestamp); diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs index 43ad5a975ca..fc6c589cb4d 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs @@ -1863,17 +1863,21 @@ private static JsonObject Resource(string uri) => uri.EndsWith("/assets/icon.bin }; /// - /// Creates an in-memory server with test handlers for listing skills, reading resources, and optionally refreshing a skill. + /// Creates an in-memory server with test handlers, defaulting resource scopes to public. /// #pragma warning disable MCPEXP002 // Custom extension methods require raw server request handlers. private static InMemoryMcpServer CreateServer( Func> listHandler, Func>? readHandler = null, Func>? getHandler = null) => - new(builder => builder.WithReadResourceHandler((request, token) => - readHandler is not null - ? readHandler(request.Params!.Uri, token) - : throw new McpProtocolException("Resource unavailable.", McpErrorCode.ResourceNotFound)), + new(builder => builder.WithReadResourceHandler(async (request, token) => + { + ReadResourceResult result = readHandler is not null + ? await readHandler(request.Params!.Uri, token) + : throw new McpProtocolException("Resource unavailable.", McpErrorCode.ResourceNotFound); + result.CacheScope ??= CacheScope.Public; + return result; + }), options => { options.Capabilities = new ServerCapabilities From c397cdac9b0a8bd9aac8b8195ab1ac5a7c67d32f Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Mon, 2 Nov 2026 19:01:48 +0000 Subject: [PATCH 02/12] .NET: Remove disposal from MCP skills catalog Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsSource.cs | 3 +- .../Skills/McpSkillsCatalog.cs | 24 ++---- .../Skills/AgentMcpSkillTests.ResourceTtl.cs | 4 +- .../Skills/AgentMcpSkillTests.cs | 12 +-- .../Skills/AgentMcpSkillsTests.Refresh.cs | 2 +- .../Skills/McpSkillsCatalogTests.cs | 78 +++---------------- 6 files changed, 25 insertions(+), 98 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index a6d97f1bdba..915666d287f 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -167,6 +167,8 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo ? Throw.IfNullOrWhitespace(selector(context)) : null; + ObjectDisposedException.ThrowIf(this._disposed, this); + McpSkillsCatalog catalog = key is null ? this._catalog : this.GetOrCreateCatalog(key); @@ -205,7 +207,6 @@ protected override void Dispose(bool disposing) if (disposing && !this._disposed) { this._disposed = true; - this._catalog.Dispose(); this._catalogs.Dispose(); } diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs index f3003c84208..eae451bcf9f 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs @@ -46,11 +46,10 @@ namespace Microsoft.Agents.AI; /// /// /// Only one caller can fetch or update the skill list at a time; other callers wait and then check the cache again. -/// No refreshes run in the background. Disposing the catalog clears its cached list and pending requests, -/// but does not dispose the MCP client or prevent previously returned skills from reading resources. +/// No refreshes run in the background. /// /// -internal sealed partial class McpSkillsCatalog : IDisposable +internal sealed partial class McpSkillsCatalog { private const string CatalogKey = "catalog"; private const string CompleteResultType = "complete"; @@ -68,7 +67,6 @@ internal sealed partial class McpSkillsCatalog : IDisposable private readonly bool _hasIsolationKey; private readonly McpCache> _catalogCache; private readonly ConcurrentDictionary _skillsToRefresh = new(); - private bool _disposed; /// /// Initializes a new instance of the class. @@ -103,7 +101,6 @@ public McpSkillsCatalog(McpClient client, AgentMcpSkillsSourceOptions? options = /// The shared mutable skill list for the cached or updated catalog. public async Task> GetSkillsAsync(CancellationToken cancellationToken) { - ObjectDisposedException.ThrowIf(this._disposed, this); cancellationToken.ThrowIfCancellationRequested(); IList skills = await this._catalogCache.GetOrRefreshAsync( @@ -118,31 +115,20 @@ public async Task> GetSkillsAsync(CancellationToken cancellati return skills; } - /// - public void Dispose() - { - if (!this._disposed) - { - this._disposed = true; - this._catalogCache.Clear(); - this._skillsToRefresh.Clear(); - } - } - /// /// Queues a skill from the cached list for refresh on the next call. /// /// /// Called after a resource's size or content hash differs from the values recorded for its owning skill. - /// Requests for skill objects no longer in the cached list, or made after disposal, are ignored. + /// Requests for skill objects no longer in the cached list are ignored. /// /// The skill object whose server-provided details need to be fetched again. public void QueueSkillRefresh(AgentMcpSkill skill) { _ = Throw.IfNull(skill); - // Ignore refresh requests after disposal or when the skill is no longer in the cache. - if (!this._disposed && this._catalogCache.Contains(CatalogKey, skills => skills.Contains(skill))) + // Ignore refresh requests when the skill is no longer in the cache. + if (this._catalogCache.Contains(CatalogKey, skills => skills.Contains(skill))) { this._skillsToRefresh.TryAdd(skill, 0); } diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs index c8c30f76d93..978cff33335 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs @@ -254,7 +254,7 @@ public async Task GetContentAsync_CanceledDuringFrontmatterValidation_DoesNotCac cts.Cancel(); } }); - using var catalog = CreateCatalog(clock); + var catalog = CreateCatalog(clock); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [Resource(SkillUri, Content)], client.Object, catalog, logger.Object, timeProvider: clock); if (cached) @@ -532,7 +532,7 @@ public async Task ReadResource_CanceledDuringMismatch_DoesNotReturnOrReplaceCach It.IsAny(), It.IsAny>())) .Callback(cts.Cancel); - using var catalog = CreateCatalog(clock); + var catalog = CreateCatalog(clock); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [Resource(SkillUri, SkillContent), Resource(SkillRootUri + "references/checklist.md", "Checklist.")], client.Object, catalog, logger.Object, timeProvider: clock); diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs index 757c4b0c325..9c9cb44bcd8 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs @@ -36,7 +36,7 @@ public async Task Constructor_PreservesProvidedFrontmatterAsync() var frontmatter = new AgentSkillFrontmatter("unit-converter", "Convert units.") { License = "MIT" }; // Act - using var catalog = CreateCatalog(); + var catalog = CreateCatalog(); var skill = new AgentMcpSkill(frontmatter, SkillUri, [Resource(SkillUri, SkillContent)], client, catalog); // Assert @@ -51,7 +51,7 @@ public async Task Constructor_MetadataLifetime_PreservesProvidedLifetimeOrDefaul // Arrange await using var server = CreateServer((uri, _) => ValueTask.FromResult(Text(uri, SkillContent))); await using var client = await server.CreateClientAsync(); - using var catalog = CreateCatalog(); + var catalog = CreateCatalog(); McpCacheLifetime? lifetime = provideLifetime ? new McpCacheLifetime(TimeProvider.System.GetTimestamp(), TimeSpan.FromSeconds(1)) : null; @@ -76,7 +76,7 @@ public async Task GetContentAsync_ResourceList_FindsInstructionsByExactUriAsync( return ValueTask.FromResult(Text(uri, uri == SkillUri ? SkillContent : "Nested instructions.")); }); await using var client = await server.CreateClientAsync(); - using var catalog = CreateCatalog(); + var catalog = CreateCatalog(); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [ Resource(NestedUri, "Nested instructions."), @@ -111,7 +111,7 @@ public async Task GetContentAsync_MissingInstructionResource_ThrowsWithoutReadin McpSkillResourceEntry[] resources = includeNestedInstructions ? [Resource(SkillRootUri + "nested/SKILL.md", SkillContent)] : []; - using var catalog = CreateCatalog(); + var catalog = CreateCatalog(); var skill = new AgentMcpSkill(frontmatter, SkillUri, resources, client, catalog); // Act @@ -464,7 +464,7 @@ public async Task GetContentAsync_OptionalFrontmatter_IsNotComparedOrAppliedAsyn await using var server = CreateServer((uri, _) => ValueTask.FromResult(Text(uri, Content))); await using var client = await server.CreateClientAsync(); var frontmatter = new AgentSkillFrontmatter("unit-converter", "Convert units.") { License = "MIT" }; - using var catalog = CreateCatalog(); + var catalog = CreateCatalog(); var skill = new AgentMcpSkill(frontmatter, SkillUri, [Resource(SkillUri, Content)], client, catalog); // Act @@ -902,7 +902,7 @@ public async Task GetContentAsync_Utf8Content_UsesByteCountAsync(bool useCharact await using var server = CreateServer((uri, _) => ValueTask.FromResult(Text(uri, Content))); await using var client = await server.CreateClientAsync(); - using var catalog = CreateCatalog(); + var catalog = CreateCatalog(); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [instructions], client, catalog); // Act & Assert diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs index 5b35d5b12e2..13f014a96c1 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs @@ -1198,7 +1198,7 @@ public async Task GetSkillsAsync_ListingRefresh_KeepsHeldManifestAndReturnedReso [Theory] [InlineData(true)] [InlineData(false)] - public async Task GetSkillsAsync_DisposedCatalog_RetainedSkillRefetchesExpiredContentAsync(bool instructions) + public async Task GetSkillsAsync_DisposedSource_RetainedSkillRefetchesExpiredContentAsync(bool instructions) { // Arrange var clock = new ManualTimeProvider(); diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs index 7a8a7567052..35de36687be 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Runtime.CompilerServices; using System.Text.Json.Nodes; using System.Threading; using System.Threading.Tasks; @@ -14,7 +13,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; /// -/// Tests skill-list caching, refresh coordination, and catalog disposal. +/// Tests skill-list caching and refresh coordination. /// public sealed class McpSkillsCatalogTests { @@ -32,7 +31,7 @@ public void Constructor_NullClient_ThrowsArgumentNullException() public void QueueSkillRefresh_NullSkill_ThrowsArgumentNullException() { // Arrange - using var catalog = new McpSkillsCatalog(new Mock().Object); + var catalog = new McpSkillsCatalog(new Mock().Object); // Act & Assert Assert.Throws("skill", () => catalog.QueueSkillRefresh(null!)); @@ -50,7 +49,7 @@ Task DiscoverAsync(JsonRpcRequest request, CancellationToken token) return Task.FromResult(CreateListResult(1000)); } - using var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); + var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); // Act var first = await catalog.GetSkillsAsync(CancellationToken.None); @@ -80,7 +79,7 @@ public async Task GetSkillsAsync_ModifiedReturnedList_SharesChangesWithCachedLis // Arrange var clock = new ManualTimeProvider(); int calls = 0; - using var catalog = new McpSkillsCatalog(CreateClient((_, _) => + var catalog = new McpSkillsCatalog(CreateClient((_, _) => { calls++; return Task.FromResult(CreateListResult(1000)); @@ -124,7 +123,7 @@ Task DiscoverAsync(JsonRpcRequest request, CancellationToken token) return Task.FromResult(result); } - using var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); + var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); var first = await catalog.GetSkillsAsync(CancellationToken.None); // Act @@ -147,7 +146,7 @@ public async Task GetSkillsAsync_ConcurrentCalls_ReuseOnlyFreshResultsAsync(long int calls = 0; Task DiscoverAsync(JsonRpcRequest request, CancellationToken token) => ++calls == 1 ? release.Task : Task.FromResult(CreateListResult(ttl)); - using var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); + var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); // Act Task>[] pending = Enumerable.Range(0, 8) @@ -180,7 +179,7 @@ public async Task GetSkillsAsync_FailedDiscovery_PreservesErrorAndReleasesWaiter int calls = 0; Task DiscoverAsync(JsonRpcRequest request, CancellationToken token) => ++calls == 1 ? release.Task : Task.FromResult(CreateListResult(1000)); - using var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); + var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); var active = catalog.GetSkillsAsync(CancellationToken.None); var waiting = catalog.GetSkillsAsync(CancellationToken.None); @@ -213,7 +212,7 @@ Task DiscoverAsync(JsonRpcRequest request, CancellationToken token) return Task.FromResult(CreateListResult(1000)); } - using var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); + var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); // Act & Assert await Assert.ThrowsAnyAsync(() => catalog.GetSkillsAsync(cts.Token)); @@ -237,7 +236,7 @@ Task DiscoverAsync(JsonRpcRequest request, CancellationToken token) return Task.FromResult(CreateListResult(1000)); } - using var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); + var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); if (cached) { @@ -252,65 +251,6 @@ Task DiscoverAsync(JsonRpcRequest request, CancellationToken token) Assert.Equal(cached ? 1 : 0, calls); } - [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task GetSkillsAsync_DisposedCatalog_ThrowsWithoutInvokingDiscoveryAsync(bool cached) - { - // Arrange - var clock = new ManualTimeProvider(); - int calls = 0; - Task DiscoverAsync(JsonRpcRequest request, CancellationToken token) - { - calls++; - return Task.FromResult(CreateListResult(1000)); - } - - using var catalog = new McpSkillsCatalog(CreateClient(DiscoverAsync), timeProvider: clock); - - if (cached) - { - await catalog.GetSkillsAsync(CancellationToken.None); - } - - // Act - catalog.Dispose(); - catalog.Dispose(); - - // Assert - await Assert.ThrowsAsync(() => catalog.GetSkillsAsync(CancellationToken.None)); - Assert.Equal(cached ? 1 : 0, calls); - } - - [Fact] - public async Task Dispose_RetainedSkill_DoesNotKeepOtherSkillsAliveAsync() - { - // Arrange - using var catalog = new McpSkillsCatalog(CreateClient((_, _) => - Task.FromResult(CreateListResult(3600000, "test-skill", "other-skill")))); - var (retained, other) = await PopulateCatalogAsync(catalog); - - // Act - catalog.Dispose(); - GC.Collect(); - GC.WaitForPendingFinalizers(); - GC.Collect(); - - // Assert - Assert.False(other.TryGetTarget(out _)); - GC.KeepAlive(retained); - - [MethodImpl(MethodImplOptions.NoInlining)] - static async Task<(AgentMcpSkill Retained, WeakReference Other)> PopulateCatalogAsync(McpSkillsCatalog catalog) - { - IList skills = await catalog.GetSkillsAsync(CancellationToken.None); - var retained = Assert.IsType(skills[0]); - var other = Assert.IsType(skills[1]); - catalog.QueueSkillRefresh(other); - return (retained, new(other)); - } - } - /// /// Creates a skill-list response with complete manifests. /// From 327fa11e00569fc77c100efb5f9fab19b0cdffcd Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 10:38:33 +0000 Subject: [PATCH 03/12] .NET: Decouple MCP skill scope validation from catalog instances Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkill.cs | 7 ++- .../Skills/McpSkillsCatalog.cs | 54 +++++++++---------- .../McpSkillsCatalogTests.CacheScope.cs | 46 ++++++++++++++++ .../Skills/McpSkillsCatalogTests.cs | 2 +- 4 files changed, 76 insertions(+), 33 deletions(-) create mode 100644 dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.CacheScope.cs diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs index 564b21ff09e..11bb005e90f 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs @@ -52,6 +52,7 @@ internal sealed partial class AgentMcpSkill : AgentSkill private readonly IReadOnlyList _resources; private readonly long _maxResourceSizeBytes; private readonly McpSkillsCatalog _skillsCatalog; + private readonly bool _hasIsolationKey; private readonly TimeProvider _timeProvider; private readonly Func? _ttlResolver; private readonly McpCache _resourceCache; @@ -69,8 +70,9 @@ internal sealed partial class AgentMcpSkill : AgentSkill /// Optional clock for resource expiration. /// Optional lifetime for skill metadata. Defaults to no independent expiration; the catalog lifetime controls refreshes. /// Optional callback to override the server's resource TTLs. + /// Whether the skill belongs to a cache partition key. /// is not positive. - public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyList resources, McpClient client, McpSkillsCatalog catalog, ILogger? logger = null, long maxResourceSizeBytes = DefaultMaxResourceSizeBytes, TimeProvider? timeProvider = null, McpCacheLifetime? metadataLifetime = null, Func? ttlResolver = null) + public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyList resources, McpClient client, McpSkillsCatalog catalog, ILogger? logger = null, long maxResourceSizeBytes = DefaultMaxResourceSizeBytes, TimeProvider? timeProvider = null, McpCacheLifetime? metadataLifetime = null, Func? ttlResolver = null, bool hasIsolationKey = false) { this.Frontmatter = Throw.IfNull(frontmatter); this.Uri = Throw.IfNullOrWhitespace(uri); @@ -79,6 +81,7 @@ public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyLis this._resources = Throw.IfNull(resources); this._client = Throw.IfNull(client); this._skillsCatalog = Throw.IfNull(catalog); + this._hasIsolationKey = hasIsolationKey; this._timeProvider = timeProvider ?? TimeProvider.System; this._ttlResolver = ttlResolver; this._resourceCache = new(this._timeProvider, StringComparer.Ordinal); @@ -197,7 +200,7 @@ private async ValueTask> FetchResourceContentAsy // Validation time consumes the TTL; network request time does not. long receivedTimestamp = this._timeProvider.GetTimestamp(); - response.CacheScope = this._skillsCatalog.ValidateAndNormalizeCacheScope(response.CacheScope, "resources/read"); + response.CacheScope = McpSkillsCatalog.ValidateAndNormalizeCacheScope(response.CacheScope, this._hasIsolationKey, this._logger, "resources/read"); ResourceContents content = this.GetValidatedResourceContent(resource, response); if (!this.TryVerifyResourceIntegrity(resource, content, out string? mismatch)) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs index eae451bcf9f..493fcb8afff 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs @@ -134,33 +134,6 @@ public void QueueSkillRefresh(AgentMcpSkill skill) } } - /// - /// Treats missing or null scopes as public and rejects private responses without a cache partition key. - /// - /// The response's declared cache scope. - /// The MCP method producing the response. - /// The validated scope, or public when the scope is missing or null. - /// The response is private and this partition has no cache partition key. - public CacheScope ValidateAndNormalizeCacheScope(CacheScope? scope, string method) - { - // Treat missing or null scopes as public. - if (scope is null) - { - LogUnspecifiedCacheScope(this._logger, method); - scope = CacheScope.Public; - } - - // Private responses require an isolation key to prevent reuse across authorization contexts. - if (scope == CacheScope.Private && !this._hasIsolationKey) - { - throw new InvalidOperationException( - $"The {method} response has a private cacheScope, but no cache partition key was provided. " + - $"Configure {nameof(AgentMcpSkillsSourceOptions)}.{nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector)} to return a nonempty authorization-context key."); - } - - return scope.Value; - } - /// /// Fetches the complete list if it is missing or expired, or refreshes queued and expired skills in a fresh list. /// @@ -308,7 +281,7 @@ private async Task>> FetchSkillsAsync(Cancellati throw new InvalidOperationException($"The skills/get result must describe the requested skill '{skillUri}'."); } - result.CacheScope = this.ValidateAndNormalizeCacheScope(result.CacheScope, "skills/get"); + result.CacheScope = ValidateAndNormalizeCacheScope(result.CacheScope, this._hasIsolationKey, this._logger, "skills/get"); // Unlike listing responses, skills/get provides a TTL for this skill, // allowing it to expire and refresh independently of the catalog. @@ -324,7 +297,7 @@ private AgentMcpSkill CreateSkill(McpSkillEntry entry, McpCacheLifetime metadata List resources = this.CreateSupportingResources(entry); - return new AgentMcpSkill(frontmatter, entry.Uri, resources, this._client, this, this._logger, this._maxResourceSizeBytes, this._timeProvider, metadataLifetime, this._ttlResolver); + return new AgentMcpSkill(frontmatter, entry.Uri, resources, this._client, this, this._logger, this._maxResourceSizeBytes, this._timeProvider, metadataLifetime, this._ttlResolver, this._hasIsolationKey); } /// @@ -413,7 +386,7 @@ private McpCacheLifetime CombineLifetimes(List lifetimes) throw new InvalidOperationException($"A skills/list result must have resultType '{CompleteResultType}'. Received resultType '{result.ResultType}'."); } - result.CacheScope = this.ValidateAndNormalizeCacheScope(result.CacheScope, "skills/list"); + result.CacheScope = ValidateAndNormalizeCacheScope(result.CacheScope, this._hasIsolationKey, this._logger, "skills/list"); if (declaredScope is { } previousScope && previousScope != result.CacheScope) { @@ -564,6 +537,27 @@ private void ValidateResourceManifestEntry([NotNull] McpSkillResourceEntry? reso [GeneratedRegex(@"\Asha256:[0-9a-f]{64}\z", RegexOptions.CultureInvariant)] private static partial Regex ResourceDigestRegex(); + /// + /// Treats missing or null scopes as public and rejects private responses without a cache partition key. + /// + internal static CacheScope ValidateAndNormalizeCacheScope(CacheScope? scope, bool hasIsolationKey, ILogger logger, string method) + { + if (scope is null) + { + LogUnspecifiedCacheScope(logger, method); + scope = CacheScope.Public; + } + + if (scope == CacheScope.Private && !hasIsolationKey) + { + throw new InvalidOperationException( + $"The {method} response has a private cacheScope, but no cache partition key was provided. " + + $"Configure {nameof(AgentMcpSkillsSourceOptions)}.{nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector)} to return a nonempty authorization-context key."); + } + + return scope.Value; + } + /// /// Logs that a response has no cacheScope and is treated as public. /// diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.CacheScope.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.CacheScope.cs new file mode 100644 index 00000000000..1784acafc62 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.CacheScope.cs @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using Microsoft.Extensions.Logging.Abstractions; +using ModelContextProtocol.Protocol; + +namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; + +public sealed partial class McpSkillsCatalogTests +{ + [Theory] + [InlineData(null, false, CacheScope.Public)] + [InlineData(null, true, CacheScope.Public)] + [InlineData(CacheScope.Public, false, CacheScope.Public)] + [InlineData(CacheScope.Public, true, CacheScope.Public)] + [InlineData(CacheScope.Private, true, CacheScope.Private)] + public void ValidateAndNormalizeCacheScope_AllowedScope_ReturnsNormalizedScope(CacheScope? scope, bool hasIsolationKey, CacheScope expected) + { + // Arrange + var logger = NullLogger.Instance; + + // Act + CacheScope actual = McpSkillsCatalog.ValidateAndNormalizeCacheScope(scope, hasIsolationKey, logger, "resources/read"); + + // Assert + Assert.Equal(expected, actual); + } + + [Theory] + [InlineData("skills/list")] + [InlineData("skills/get")] + [InlineData("resources/read")] + public void ValidateAndNormalizeCacheScope_PrivateWithoutPartitionKey_Throws(string method) + { + // Arrange + var logger = NullLogger.Instance; + + // Act + var error = Assert.Throws(() => + McpSkillsCatalog.ValidateAndNormalizeCacheScope(CacheScope.Private, false, logger, method)); + + // Assert + Assert.Contains(method, error.Message); + Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector), error.Message); + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs index 35de36687be..15069b08546 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs @@ -15,7 +15,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; /// /// Tests skill-list caching and refresh coordination. /// -public sealed class McpSkillsCatalogTests +public sealed partial class McpSkillsCatalogTests { [Fact] public void Constructor_NullClient_ThrowsArgumentNullException() From e202833a63fb00de74257bc1b18401cc69f95b8a Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 10:52:41 +0000 Subject: [PATCH 04/12] .NET: Replace MCP catalog refresh queue with per-skill flags Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkill.cs | 13 ++-- .../Skills/McpSkillsCatalog.cs | 70 ++++--------------- .../Skills/AgentMcpSkillTests.ResourceTtl.cs | 10 ++- .../Skills/AgentMcpSkillTests.cs | 26 ++----- .../Skills/AgentMcpSkillsTests.Refresh.cs | 61 ++++++++++++++-- .../Skills/McpSkillsCatalogTests.cs | 10 --- 6 files changed, 85 insertions(+), 105 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs index 11bb005e90f..aa82ee9fabc 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs @@ -51,11 +51,11 @@ internal sealed partial class AgentMcpSkill : AgentSkill private readonly string _skillRootUri; private readonly IReadOnlyList _resources; private readonly long _maxResourceSizeBytes; - private readonly McpSkillsCatalog _skillsCatalog; private readonly bool _hasIsolationKey; private readonly TimeProvider _timeProvider; private readonly Func? _ttlResolver; private readonly McpCache _resourceCache; + private volatile bool _needsRefresh; /// /// Initializes a new instance of the class. @@ -64,7 +64,6 @@ internal sealed partial class AgentMcpSkill : AgentSkill /// The resource URI of the skill's SKILL.md. /// The allowed resource URIs and their expected sizes and content hashes, including SKILL.md. /// The MCP client used to fetch SKILL.md and supporting resources. - /// The catalog to notify when a content mismatch requires updated skill details. /// Optional logger for validation diagnostics. /// The maximum allowed resource size in bytes. /// Optional clock for resource expiration. @@ -72,7 +71,7 @@ internal sealed partial class AgentMcpSkill : AgentSkill /// Optional callback to override the server's resource TTLs. /// Whether the skill belongs to a cache partition key. /// is not positive. - public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyList resources, McpClient client, McpSkillsCatalog catalog, ILogger? logger = null, long maxResourceSizeBytes = DefaultMaxResourceSizeBytes, TimeProvider? timeProvider = null, McpCacheLifetime? metadataLifetime = null, Func? ttlResolver = null, bool hasIsolationKey = false) + public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyList resources, McpClient client, ILogger? logger = null, long maxResourceSizeBytes = DefaultMaxResourceSizeBytes, TimeProvider? timeProvider = null, McpCacheLifetime? metadataLifetime = null, Func? ttlResolver = null, bool hasIsolationKey = false) { this.Frontmatter = Throw.IfNull(frontmatter); this.Uri = Throw.IfNullOrWhitespace(uri); @@ -80,7 +79,6 @@ public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyLis this._skillRootUri = this.Uri[..^"SKILL.md".Length]; this._resources = Throw.IfNull(resources); this._client = Throw.IfNull(client); - this._skillsCatalog = Throw.IfNull(catalog); this._hasIsolationKey = hasIsolationKey; this._timeProvider = timeProvider ?? TimeProvider.System; this._ttlResolver = ttlResolver; @@ -103,6 +101,11 @@ public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyLis /// internal McpCacheLifetime MetadataLifetime { get; } + /// + /// Gets whether a resource mismatch requires updated skill metadata. + /// + internal bool NeedsRefresh => this._needsRefresh; + /// /// /// Returns the text of SKILL.md, using cached content until it expires. @@ -208,7 +211,7 @@ private async ValueTask> FetchResourceContentAsy cancellationToken.ThrowIfCancellationRequested(); // The resource may have changed; request a new manifest without changing this skill's held manifest. - this._skillsCatalog.QueueSkillRefresh(this); + this._needsRefresh = true; LogResourceIntegrityMismatch(this._logger, resource.Uri, this.Uri, mismatch); cancellationToken.ThrowIfCancellationRequested(); diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs index 493fcb8afff..6fc2b14b1f8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -30,7 +29,7 @@ namespace Microsoft.Agents.AI; /// /// /// The list is cached until any page expires, then fetched again on the next call. -/// Queued refresh requests and individual metadata expiration trigger skills/get while the list is fresh. +/// Skills marked for refresh and individual metadata expiration trigger skills/get while the list is fresh. /// Full discovery takes priority when the list expires. /// /// @@ -42,7 +41,7 @@ namespace Microsoft.Agents.AI; /// /// /// Resource retrieval, verification, and content caching remain the responsibility of each skill object. -/// A size or digest mismatch queues a metadata refresh through . +/// A size or digest mismatch marks the skill for metadata refresh through . /// /// /// Only one caller can fetch or update the skill list at a time; other callers wait and then check the cache again. @@ -66,7 +65,6 @@ internal sealed partial class McpSkillsCatalog private readonly Func? _ttlResolver; private readonly bool _hasIsolationKey; private readonly McpCache> _catalogCache; - private readonly ConcurrentDictionary _skillsToRefresh = new(); /// /// Initializes a new instance of the class. @@ -103,35 +101,11 @@ public async Task> GetSkillsAsync(CancellationToken cancellati { cancellationToken.ThrowIfCancellationRequested(); - IList skills = await this._catalogCache.GetOrRefreshAsync( + return await this._catalogCache.GetOrRefreshAsync( CatalogKey, this.RefreshCatalogAsync, cancellationToken, - needsRefresh: cached => !this._skillsToRefresh.IsEmpty || - cached.OfType().Any(skill => !skill.MetadataLifetime.IsFresh(this._timeProvider))).ConfigureAwait(false); - - this.RemoveObsoleteRefreshRequests(); - - return skills; - } - - /// - /// Queues a skill from the cached list for refresh on the next call. - /// - /// - /// Called after a resource's size or content hash differs from the values recorded for its owning skill. - /// Requests for skill objects no longer in the cached list are ignored. - /// - /// The skill object whose server-provided details need to be fetched again. - public void QueueSkillRefresh(AgentMcpSkill skill) - { - _ = Throw.IfNull(skill); - - // Ignore refresh requests when the skill is no longer in the cache. - if (this._catalogCache.Contains(CatalogKey, skills => skills.Contains(skill))) - { - this._skillsToRefresh.TryAdd(skill, 0); - } + needsRefresh: cached => cached.OfType().Any(this.NeedsRefresh)).ConfigureAwait(false); } /// @@ -149,7 +123,7 @@ private async ValueTask>> RefreshCatalogAsync(Mc } /// - /// Queues expired skills and stores the updated list only if every queued refresh succeeds. + /// Refreshes marked and expired skills and stores the updated list only if every refresh succeeds. /// /// /// Replaces updated skills and removes missing ones in a new list, preserving the catalog's lifetime. @@ -157,24 +131,12 @@ private async ValueTask>> RefreshCatalogAsync(Mc /// private async Task>> RefreshSkillsAsync(McpCacheEntry> cached, CancellationToken cancellationToken) { - // Queue skills whose individual TTL has expired alongside those queued after a resource mismatch. - // Skills loaded through listing use Infinite and are refreshed with the catalog instead. - foreach (AgentMcpSkill skill in cached.Value.OfType()) - { - if (!skill.MetadataLifetime.IsFresh(this._timeProvider)) - { - this._skillsToRefresh.TryAdd(skill, 0); - } - } - List? updated = null; - foreach (AgentMcpSkill skill in this._skillsToRefresh.Keys) + foreach (AgentMcpSkill skill in cached.Value.OfType()) { - // Remove refresh requests for skill instances that are no longer in the cached catalog. - if (!cached.Value.Contains(skill)) + if (!this.NeedsRefresh(skill)) { - this._skillsToRefresh.TryRemove(skill, out _); continue; } @@ -204,24 +166,16 @@ private async Task>> RefreshSkillsAsync(McpCache cancellationToken.ThrowIfCancellationRequested(); - // Commit only after every refresh succeeds; failures leave the catalog and pending work intact. + // Commit only after every refresh succeeds. Old flags remain set for retries; replacements start unmarked. return new(updated, cached.Lifetime); } /// - /// Removes pending refresh requests for skill objects no longer in the published list. + /// Returns whether a skill was marked for refresh or its metadata expired. /// - private void RemoveObsoleteRefreshRequests() + private bool NeedsRefresh(AgentMcpSkill skill) { - // Keep requests for skills still in the list: they may have been queued during the refresh - // or just after the new catalog was stored. - foreach (AgentMcpSkill skill in this._skillsToRefresh.Keys) - { - if (!this._catalogCache.Contains(CatalogKey, skills => skills.Contains(skill))) - { - this._skillsToRefresh.TryRemove(skill, out _); - } - } + return skill.NeedsRefresh || !skill.MetadataLifetime.IsFresh(this._timeProvider); } /// @@ -297,7 +251,7 @@ private AgentMcpSkill CreateSkill(McpSkillEntry entry, McpCacheLifetime metadata List resources = this.CreateSupportingResources(entry); - return new AgentMcpSkill(frontmatter, entry.Uri, resources, this._client, this, this._logger, this._maxResourceSizeBytes, this._timeProvider, metadataLifetime, this._ttlResolver, this._hasIsolationKey); + return new AgentMcpSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes, this._timeProvider, metadataLifetime, this._ttlResolver, this._hasIsolationKey); } /// diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs index 978cff33335..21a118f7572 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs @@ -197,7 +197,7 @@ public async Task GetContentAsync_ValidationTime_ConsumesResourceTtlAsync(int va It.IsAny>())) .Callback(() => clock.Advance(TimeSpan.FromMilliseconds(validationMilliseconds))); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, - [Resource(SkillUri, Content)], client, CreateCatalog(clock), logger.Object, timeProvider: clock); + [Resource(SkillUri, Content)], client, logger.Object, timeProvider: clock); // Act Assert.Equal(Content, await skill.GetContentAsync()); @@ -254,9 +254,8 @@ public async Task GetContentAsync_CanceledDuringFrontmatterValidation_DoesNotCac cts.Cancel(); } }); - var catalog = CreateCatalog(clock); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, - [Resource(SkillUri, Content)], client.Object, catalog, logger.Object, timeProvider: clock); + [Resource(SkillUri, Content)], client.Object, logger.Object, timeProvider: clock); if (cached) { Assert.Equal(Content, await skill.GetContentAsync()); @@ -376,7 +375,7 @@ public async Task ReadResource_ExpiredTtl_ReturnsCachedContentOnMismatchWithoutR logger.Setup(l => l.IsEnabled(It.IsAny())).Returns(true); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [Resource(SkillUri, SkillContent), Resource(SkillRootUri + "references/checklist.md", "Checklist.")], - client, CreateCatalog(clock), logger.Object, timeProvider: clock); + client, logger.Object, timeProvider: clock); await ReadSnapshotAsync(skill, instructions); clock.Advance(TimeSpan.FromMilliseconds(Math.Max(ttl.GetValueOrDefault(), 0))); @@ -532,10 +531,9 @@ public async Task ReadResource_CanceledDuringMismatch_DoesNotReturnOrReplaceCach It.IsAny(), It.IsAny>())) .Callback(cts.Cancel); - var catalog = CreateCatalog(clock); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [Resource(SkillUri, SkillContent), Resource(SkillRootUri + "references/checklist.md", "Checklist.")], - client.Object, catalog, logger.Object, timeProvider: clock); + client.Object, logger.Object, timeProvider: clock); if (cached) { await ReadSnapshotAsync(skill, instructions); diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs index 9c9cb44bcd8..c248f74cfa4 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs @@ -36,8 +36,7 @@ public async Task Constructor_PreservesProvidedFrontmatterAsync() var frontmatter = new AgentSkillFrontmatter("unit-converter", "Convert units.") { License = "MIT" }; // Act - var catalog = CreateCatalog(); - var skill = new AgentMcpSkill(frontmatter, SkillUri, [Resource(SkillUri, SkillContent)], client, catalog); + var skill = new AgentMcpSkill(frontmatter, SkillUri, [Resource(SkillUri, SkillContent)], client); // Assert Assert.Same(frontmatter, skill.Frontmatter); @@ -51,14 +50,13 @@ public async Task Constructor_MetadataLifetime_PreservesProvidedLifetimeOrDefaul // Arrange await using var server = CreateServer((uri, _) => ValueTask.FromResult(Text(uri, SkillContent))); await using var client = await server.CreateClientAsync(); - var catalog = CreateCatalog(); McpCacheLifetime? lifetime = provideLifetime ? new McpCacheLifetime(TimeProvider.System.GetTimestamp(), TimeSpan.FromSeconds(1)) : null; // Act var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, - [Resource(SkillUri, SkillContent)], client, catalog, metadataLifetime: lifetime); + [Resource(SkillUri, SkillContent)], client, metadataLifetime: lifetime); // Assert Assert.Same(lifetime ?? McpCacheLifetime.Infinite, skill.MetadataLifetime); @@ -76,12 +74,11 @@ public async Task GetContentAsync_ResourceList_FindsInstructionsByExactUriAsync( return ValueTask.FromResult(Text(uri, uri == SkillUri ? SkillContent : "Nested instructions.")); }); await using var client = await server.CreateClientAsync(); - var catalog = CreateCatalog(); var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [ Resource(NestedUri, "Nested instructions."), Resource(SkillUri, SkillContent), - ], client, catalog); + ], client); // Act string instructions = await skill.GetContentAsync(); @@ -111,8 +108,7 @@ public async Task GetContentAsync_MissingInstructionResource_ThrowsWithoutReadin McpSkillResourceEntry[] resources = includeNestedInstructions ? [Resource(SkillRootUri + "nested/SKILL.md", SkillContent)] : []; - var catalog = CreateCatalog(); - var skill = new AgentMcpSkill(frontmatter, SkillUri, resources, client, catalog); + var skill = new AgentMcpSkill(frontmatter, SkillUri, resources, client); // Act var exception = await Assert.ThrowsAsync(async () => await skill.GetContentAsync()); @@ -464,8 +460,7 @@ public async Task GetContentAsync_OptionalFrontmatter_IsNotComparedOrAppliedAsyn await using var server = CreateServer((uri, _) => ValueTask.FromResult(Text(uri, Content))); await using var client = await server.CreateClientAsync(); var frontmatter = new AgentSkillFrontmatter("unit-converter", "Convert units.") { License = "MIT" }; - var catalog = CreateCatalog(); - var skill = new AgentMcpSkill(frontmatter, SkillUri, [Resource(SkillUri, Content)], client, catalog); + var skill = new AgentMcpSkill(frontmatter, SkillUri, [Resource(SkillUri, Content)], client); // Act string verified = await skill.GetContentAsync(); @@ -902,8 +897,7 @@ public async Task GetContentAsync_Utf8Content_UsesByteCountAsync(bool useCharact await using var server = CreateServer((uri, _) => ValueTask.FromResult(Text(uri, Content))); await using var client = await server.CreateClientAsync(); - var catalog = CreateCatalog(); - var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [instructions], client, catalog); + var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [instructions], client); // Act & Assert if (useCharacterCount) @@ -1172,13 +1166,7 @@ .. resources ?? Resource(root + "assets/icon.bin", new byte[] { 1, 2, 3, 4 }), Resource(root + "empty.txt", ""), ], - ], client, CreateCatalog(timeProvider), maxResourceSizeBytes: maxResourceSizeBytes, timeProvider: timeProvider); - - /// - /// Creates a catalog that fails on unexpected listing or skill-refresh requests. - /// - private static McpSkillsCatalog CreateCatalog(TimeProvider? timeProvider = null) => - new(new Mock(MockBehavior.Strict).Object, timeProvider: timeProvider); + ], client, maxResourceSizeBytes: maxResourceSizeBytes, timeProvider: timeProvider); /// /// Creates a resource entry with the supplied text's expected size and content hash. diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs index 13f014a96c1..647503ffebc 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Refresh.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; using System.Text.Json; using System.Text.Json.Nodes; @@ -511,6 +512,7 @@ public async Task GetSkillsAsync_VerificationFailure_QueuesOwningSkillRefreshAsy }); using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); IList original = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); + Assert.All(original, skill => Assert.False(Assert.IsType(skill).NeedsRefresh)); if (cached) { await ReadSkillContentAsync(original[0], instructions); @@ -533,6 +535,8 @@ public async Task GetSkillsAsync_VerificationFailure_QueuesOwningSkillRefreshAsy } Assert.DoesNotContain("skills/get", methods); + Assert.True(Assert.IsType(original[0]).NeedsRefresh); + Assert.False(Assert.IsType(original[1]).NeedsRefresh); // Act IList refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); @@ -541,6 +545,7 @@ public async Task GetSkillsAsync_VerificationFailure_QueuesOwningSkillRefreshAsy Assert.NotSame(original, refreshed); Assert.NotSame(original[0], refreshed[0]); Assert.Same(original[1], refreshed[1]); + Assert.False(Assert.IsType(refreshed[0]).NeedsRefresh); Assert.Single(methods, method => method == "skills/list"); Assert.Single(methods, method => method == "skills/get"); Assert.Equal(2, methods.FindAll(method => method == "resources/read").Count); @@ -1033,8 +1038,10 @@ public async Task GetSkillsAsync_ConcurrentVerificationRefreshes_SharePublishedC Assert.NotSame(original, Assert.Single(results[0])); } - [Fact] - public async Task GetSkillsAsync_VerificationFailureDuringRefresh_RemainsPendingForOtherSkillAsync() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_VerificationFailureDuringRefresh_RemainsPendingForOtherSkillAsync(bool alreadyChecked) { // Arrange var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -1054,21 +1061,23 @@ public async Task GetSkillsAsync_VerificationFailureDuringRefresh_RemainsPending Assert.Equal("skills/get", request.Method); gets.Add(uri); - return gets.Count == 1 ? release.Task : Task.FromResult(GetSkillResult(Entry("skill://other/unit-converter/"))); + return gets.Count == 1 ? release.Task : Task.FromResult(GetSkillResult(Entry(uri[..^"SKILL.md".Length]))); }); using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: new ManualTimeProvider()); IList original = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); - await Assert.ThrowsAsync(async () => await original[0].GetContentAsync()); + int first = alreadyChecked ? 1 : 0; + int other = 1 - first; + await Assert.ThrowsAsync(async () => await original[first].GetContentAsync()); // Act Task> firstRefresh = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); - await Assert.ThrowsAsync(async () => await original[1].GetContentAsync()); - release.SetResult(GetSkillResult(Entry())); + await Assert.ThrowsAsync(async () => await original[other].GetContentAsync()); + release.SetResult(GetSkillResult(Entry(alreadyChecked ? "skill://other/unit-converter/" : "skill://unit-converter/"))); await firstRefresh.WaitAsync(TimeSpan.FromSeconds(10)); IList refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert - Assert.Equal([SkillUri, "skill://other/unit-converter/SKILL.md"], gets); + Assert.Equal(alreadyChecked ? ["skill://other/unit-converter/SKILL.md", SkillUri] : [SkillUri, "skill://other/unit-converter/SKILL.md"], gets); Assert.NotSame(original[0], refreshed[0]); Assert.NotSame(original[1], refreshed[1]); } @@ -1227,6 +1236,44 @@ public async Task GetSkillsAsync_DisposedSource_RetainedSkillRefetchesExpiredCon Assert.Equal(["skills/list", "resources/read", "resources/read"], methods); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_RetainedSkill_DoesNotKeepCatalogOrSiblingAliveAsync(bool partitioned) + { + // Arrange + var (retained, catalog, sibling) = await CreateRetainedSkillAsync(partitioned); + + // Act + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + // Assert + Assert.False(catalog.TryGetTarget(out _)); + Assert.False(sibling.TryGetTarget(out _)); + Assert.Equal(SkillContent, await retained.GetContentAsync()); + + [MethodImpl(MethodImplOptions.NoInlining)] + static async Task<(AgentSkill Retained, WeakReference Catalog, WeakReference Sibling)> CreateRetainedSkillAsync(bool partitioned) + { + var client = CreateTtlClient((request, _) => + { + JsonObject result = request.Method switch + { + "skills/list" => TtlListResult(60000, Entry(), Entry("skill://other/unit-converter/")), + "resources/read" => ResourceReadResult(request.Params!["uri"]!.GetValue(), 1000, SkillContent), + _ => throw new InvalidOperationException(request.Method), + }; + result["cacheScope"] = partitioned ? "private" : "public"; + return Task.FromResult(result); + }); + var catalog = new McpSkillsCatalog(client.Object, hasIsolationKey: partitioned); + IList skills = await catalog.GetSkillsAsync(CancellationToken.None); + return (skills[0], new(catalog), new(skills[1])); + } + } + /// /// Creates a skills/get response for the supplied skill. /// diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs index 15069b08546..c45476b9f6d 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.cs @@ -27,16 +27,6 @@ public void Constructor_NullClient_ThrowsArgumentNullException() Assert.Throws("client", () => new McpSkillsCatalog(null!, timeProvider: clock)); } - [Fact] - public void QueueSkillRefresh_NullSkill_ThrowsArgumentNullException() - { - // Arrange - var catalog = new McpSkillsCatalog(new Mock().Object); - - // Act & Assert - Assert.Throws("skill", () => catalog.QueueSkillRefresh(null!)); - } - [Fact] public async Task GetSkillsAsync_PositiveTtl_ReusesListUntilExactExpirationAsync() { From 5f7c7196cba30af8c0a630838e292d25467318d0 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 11:08:19 +0000 Subject: [PATCH 05/12] .NET: Align MCP cache isolation key selector naming Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Agent_Step06_McpBasedSkills/README.md | 4 +- .../Skills/AgentMcpSkillsSource.cs | 6 +-- .../Skills/AgentMcpSkillsSourceOptions.cs | 4 +- ...AgentSkillsProviderBuilderMcpExtensions.cs | 2 +- .../Skills/McpSkillsCatalog.cs | 2 +- .../Skills/AgentMcpSkillsTests.CacheScope.cs | 54 +++++++++---------- .../McpSkillsCatalogTests.CacheScope.cs | 2 +- 7 files changed, 37 insertions(+), 37 deletions(-) diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md index 6e1d5914d53..8ec708b868d 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md @@ -91,14 +91,14 @@ By default, it doesn't: all callers share one cache, so the source accepts only `InvalidOperationException` for a private one. Each response is checked on its own, so a skill from a public list can still fail if its `SKILL.md` or another file is returned as private. -To accept private responses, set `CachePartitionKeySelector` to return a key for the current caller, such as +To accept private responses, set `CacheIsolationKeySelector` to return a key for the current caller, such as a user or tenant ID. Each key gets its own cache partition, so cached skills are never shared between keys. Set it whenever the server can return private responses, even if your app has only one user: ```csharp var options = new AgentMcpSkillsSourceOptions { - CachePartitionKeySelector = context => GetApplicationAuthorizationKey(context), + CacheIsolationKeySelector = context => GetApplicationAuthorizationKey(context), MaxCachedPartitions = 1000, PartitionSlidingExpiration = TimeSpan.FromMinutes(30), PartitionAbsoluteExpiration = TimeSpan.FromHours(8), diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index 915666d287f..9cf2a10935b 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -35,7 +35,7 @@ namespace Microsoft.Agents.AI; /// /// /// By default, all callers share one cache. If this source is shared by several users or tenants, -/// set so each of them gets its own cache partition. +/// set so each of them gets its own cache partition. /// /// /// According to the MCP specification, each skills/list, skills/get, and resources/read response @@ -43,7 +43,7 @@ namespace Microsoft.Agents.AI; /// /// Missing or : treated as public. /// public: always accepted. -/// private: accepted only when is set; +/// private: accepted only when is set; /// otherwise, is thrown. /// /// Each response is checked on its own. For example, a skill from a public list still fails if its @@ -163,7 +163,7 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo return []; } - string? key = this._options?.CachePartitionKeySelector is { } selector + string? key = this._options?.CacheIsolationKeySelector is { } selector ? Throw.IfNullOrWhitespace(selector(context)) : null; diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs index a78fadeaf78..57fae4465b3 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -44,11 +44,11 @@ public sealed class AgentMcpSkillsSourceOptions /// this callback, every time. /// /// - public Func? CachePartitionKeySelector { get; set; } + public Func? CacheIsolationKeySelector { get; set; } /// /// Gets or sets the maximum number of cache partitions to keep, one per key returned by - /// . + /// . /// /// /// When (the default), there is no limit, and a partition is kept for every key until the source is disposed. diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs index fc7db6cade9..04eecfe53e0 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs @@ -31,7 +31,7 @@ public static class AgentSkillsProviderBuilderMcpExtensions /// Call and disable other caches around /// this source so it can refresh the skill list when its cached data expires. /// If the source is shared by several users or tenants, or the server returns private responses, - /// set . + /// set . /// /// /// var provider = new AgentSkillsProviderBuilder() diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs index 6fc2b14b1f8..4cbc4290a12 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCatalog.cs @@ -506,7 +506,7 @@ internal static CacheScope ValidateAndNormalizeCacheScope(CacheScope? scope, boo { throw new InvalidOperationException( $"The {method} response has a private cacheScope, but no cache partition key was provided. " + - $"Configure {nameof(AgentMcpSkillsSourceOptions)}.{nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector)} to return a nonempty authorization-context key."); + $"Configure {nameof(AgentMcpSkillsSourceOptions)}.{nameof(AgentMcpSkillsSourceOptions.CacheIsolationKeySelector)} to return a nonempty authorization-context key."); } return scope.Value; diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs index c41533c1fb3..0ab2a7ea4b6 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs @@ -54,7 +54,7 @@ public async Task GetSkillsAsync_PartitionSlidingExpiration_ExtendsOnDiscoveryBu }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = _ => "A", + CacheIsolationKeySelector = _ => "A", PartitionSlidingExpiration = TimeSpan.FromSeconds(10), }, null, clock); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); @@ -88,7 +88,7 @@ public async Task GetSkillsAsync_PartitionAbsoluteExpiration_ExpiresDespiteDisco }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = _ => "A", + CacheIsolationKeySelector = _ => "A", PartitionSlidingExpiration = TimeSpan.FromSeconds(10), PartitionAbsoluteExpiration = TimeSpan.FromSeconds(25), }, null, clock); @@ -119,7 +119,7 @@ public async Task GetSkillsAsync_PartitionRetention_DoesNotChangeResponseTtlAsyn }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = partitioned ? _ => "A" : null, + CacheIsolationKeySelector = partitioned ? _ => "A" : null, PartitionSlidingExpiration = TimeSpan.FromSeconds(10), PartitionAbsoluteExpiration = TimeSpan.FromSeconds(20), }, null, clock); @@ -185,7 +185,7 @@ public async Task GetSkillsAsync_PartitionEviction_PreservesInFlightDiscoveryAsy }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = _ => key, + CacheIsolationKeySelector = _ => key, MaxCachedPartitions = capacity ? 1 : null, PartitionAbsoluteExpiration = capacity ? null : TimeSpan.FromSeconds(1), }, null, clock); @@ -243,7 +243,7 @@ public async Task GetSkillsAsync_PrivateScopeWithoutPartitionKey_FailsAndRetries var error = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); Assert.Contains("skills/list", error.Message); Assert.Contains("private", error.Message); - Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector), error.Message); + Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CacheIsolationKeySelector), error.Message); } Assert.Equal(firstPagePrivate ? 2 : 4, calls); @@ -296,7 +296,7 @@ public async Task GetSkillsAsync_PublicListingWithPrivateResourceWithoutPartitio { var error = await Assert.ThrowsAsync(() => ReadSkillContentAsync(skill, instructions)); Assert.Contains("resources/read", error.Message); - Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector), error.Message); + Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CacheIsolationKeySelector), error.Message); Assert.Same(skill, Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); } @@ -338,7 +338,7 @@ public async Task GetSkillsAsync_PrivateMetadataScope_RequiresPartitionKeyAsync( }); using var source = new AgentMcpSkillsSource(client.Object, partitioned ? new() { - CachePartitionKeySelector = _ => "authorization-context", + CacheIsolationKeySelector = _ => "authorization-context", } : null); var original = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); await Assert.ThrowsAsync(async () => await original.GetContentAsync()); @@ -350,7 +350,7 @@ public async Task GetSkillsAsync_PrivateMetadataScope_RequiresPartitionKeyAsync( { var error = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); Assert.Contains("skills/get", error.Message); - Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector), error.Message); + Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CacheIsolationKeySelector), error.Message); } returnPrivate = false; @@ -377,13 +377,13 @@ public async Task GetSkillsAsync_ChangingSelector_DoesNotChangeRetainedCachePart }); using var source = new AgentMcpSkillsSource(client.Object, options); var unkeyed = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - options.CachePartitionKeySelector = _ => "authorization-context"; + options.CacheIsolationKeySelector = _ => "authorization-context"; var keyed = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); // Act & Assert await Assert.ThrowsAsync(async () => await unkeyed.GetContentAsync()); Assert.Equal(SkillContent, await keyed.GetContentAsync()); - options.CachePartitionKeySelector = null; + options.CacheIsolationKeySelector = null; Assert.Same(unkeyed, Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); await Assert.ThrowsAsync(async () => await unkeyed.GetContentAsync()); Assert.Equal(SkillContent, await keyed.GetContentAsync()); @@ -412,7 +412,7 @@ public async Task GetSkillsAsync_SharedClient_FetchesAndCachesContextSpecificCon }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = _ => authorizationKey.Value!, + CacheIsolationKeySelector = _ => authorizationKey.Value!, }); // Act @@ -438,16 +438,16 @@ public async Task GetSkillsAsync_OptionsChangedAfterConstruction_NewPartitionUse { // Arrange var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(60000, Entry()))); - var options = new AgentMcpSkillsSourceOptions { CachePartitionKeySelector = _ => "A" }; + var options = new AgentMcpSkillsSourceOptions { CacheIsolationKeySelector = _ => "A" }; using var source = new AgentMcpSkillsSource(client.Object, options); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - options.CachePartitionKeySelector = _ => "B"; + options.CacheIsolationKeySelector = _ => "B"; options.TtlResolver = _ => TimeSpan.Zero; // Act var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - options.CachePartitionKeySelector = _ => "A"; + options.CacheIsolationKeySelector = _ => "A"; var original = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); // Assert @@ -464,7 +464,7 @@ public async Task GetSkillsAsync_DisposedDuringKeySelection_DoesNotCreateNewPart var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(60000, Entry()))); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = _ => + CacheIsolationKeySelector = _ => { duringSelection?.Invoke(); return "A"; @@ -514,7 +514,7 @@ public async Task GetSkillsAsync_PartitionKeys_SeparateSkillsAndAllResourceCache return Task.FromResult(result); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }); // Act IList first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); @@ -553,7 +553,7 @@ public async Task GetSkillsAsync_PartitionKeys_CompareOrdinallyAsync() lists++; return Task.FromResult(TtlListResult(60000, Entry())); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }); // Act var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); @@ -576,7 +576,7 @@ public async Task GetSkillsAsync_InvalidPartitionKey_ThrowsWithoutRequestsAsync( { // Arrange var client = CreateSkillsClient(); - using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key! }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key! }); // Act & Assert await Assert.ThrowsAnyAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); @@ -589,7 +589,7 @@ public async Task GetSkillsAsync_PartitionKeySelectorError_PropagatesWithoutRequ // Arrange var error = new InvalidOperationException("Authorization context unavailable."); var client = CreateSkillsClient(); - using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => throw error }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => throw error }); // Act var actual = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); @@ -613,7 +613,7 @@ public async Task GetSkillsAsync_PartitionKeySelector_SkippedWithoutSupportOrWhe using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = _ => throw new InvalidOperationException("Must not run."), + CacheIsolationKeySelector = _ => throw new InvalidOperationException("Must not run."), }); using var cts = new CancellationTokenSource(); @@ -641,7 +641,7 @@ public async Task GetSkillsAsync_CachePartitions_ExpireIndependentlyAsync() lists++; return Task.FromResult(TtlListResult(1000, Entry())); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }, null, clock); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }, null, clock); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); clock.Advance(TimeSpan.FromMilliseconds(500)); key = "B"; @@ -685,7 +685,7 @@ public async Task GetSkillsAsync_CachePartitions_KeepMismatchFallbackAndMetadata reads++; return Task.FromResult(ResourceReadResult(SkillUri, 1000, changed ? SkillContent + "\nChanged." : SkillContent)); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }, null, clock); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }, null, clock); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); await first[0].GetContentAsync(); key = "B"; @@ -735,7 +735,7 @@ public async Task GetSkillsAsync_ConcurrentPartitionCreation_SharesPartitionPerK }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = context => ReferenceEquals(context, contextA) ? "A" : "B", + CacheIsolationKeySelector = context => ReferenceEquals(context, contextA) ? "A" : "B", }); Task>[] requests = Enumerable.Range(0, 32).Select(index => Task.Run(async () => { @@ -768,7 +768,7 @@ public async Task GetSkillsAsync_ConcurrentCachePartitions_DoNotBlockEachOtherAs Interlocked.Increment(ref lists) == 1 ? release.Task : Task.FromResult(TtlListResult(60000, Entry()))); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = context => ReferenceEquals(context, contextA) ? "A" : "B", + CacheIsolationKeySelector = context => ReferenceEquals(context, contextA) ? "A" : "B", }); // Act @@ -804,7 +804,7 @@ public async Task GetSkillsAsync_CachePartitionDisposal_PreservesRetainedSkillsA var client = CreateTtlClient((request, _) => request.Method == "skills/list" ? key == "B" && inFlight ? release.Task : Task.FromResult(TtlListResult(60000, Entry())) : Task.FromResult(ResourceReadResult(SkillUri, 60000, SkillContent))); - using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => key }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); key = "B"; var second = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); @@ -854,7 +854,7 @@ public async Task GetSkillsAsync_ConflictingPageScopes_FailsAndRetriesWholeListi return Task.FromResult(result); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CachePartitionKeySelector = _ => "authorization-context" }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => "authorization-context" }); // Act & Assert for (int attempt = 0; attempt < 2; attempt++) @@ -941,7 +941,7 @@ public async Task GetSkillsAsync_UnspecifiedScope_DefaultsToPublicAndLogsDiagnos }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CachePartitionKeySelector = _ => PartitionKey, + CacheIsolationKeySelector = _ => PartitionKey, }, loggerFactory.Object); // Act diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.CacheScope.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.CacheScope.cs index 1784acafc62..6d394fac5b7 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.CacheScope.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCatalogTests.CacheScope.cs @@ -41,6 +41,6 @@ public void ValidateAndNormalizeCacheScope_PrivateWithoutPartitionKey_Throws(str // Assert Assert.Contains(method, error.Message); - Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CachePartitionKeySelector), error.Message); + Assert.Contains(nameof(AgentMcpSkillsSourceOptions.CacheIsolationKeySelector), error.Message); } } From 453e49946fdfe96c07267a534667f2324d83daba Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 11:14:31 +0000 Subject: [PATCH 06/12] .NET: Limit MCP skill cache partitions to 100 by default Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Agent_Step06_McpBasedSkills/README.md | 6 +-- .../Skills/AgentMcpSkillsSourceOptions.cs | 8 +-- .../Skills/AgentMcpSkillsTests.CacheScope.cs | 50 +++++++++++++++++++ 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md index 8ec708b868d..bcef2202d50 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md @@ -123,10 +123,10 @@ The source only separates caches; it does not handle authentication. Your app is `DisableCaching()` turns off the provider's own cache, so the source, and therefore the key selector, runs on every call. -By default, a partition is kept for every key until the source is disposed. In services with many users, -use the other options to limit memory: +By default, the source keeps up to 100 cache partitions. Adjust the limit or configure expiration +to control how many partitions are kept and for how long: -- `MaxCachedPartitions` - the maximum number of partitions to keep. +- `MaxCachedPartitions` - the maximum number of partitions to keep. Defaults to 100; set to `null` for no partition-count limit. - `PartitionSlidingExpiration` - removes a partition that has not been used for this long. - `PartitionAbsoluteExpiration` - removes a partition this long after it was created, even if it is still in use. diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs index 57fae4465b3..a491e3faa13 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -51,17 +51,17 @@ public sealed class AgentMcpSkillsSourceOptions /// . /// /// - /// When (the default), there is no limit, and a partition is kept for every key until the source is disposed. - /// Set a limit for services with many users. When the limit is reached, skills for a new key are still returned, + /// Defaults to 100. Set to for no partition-count limit. + /// When the limit is reached, skills for a new key are still returned, /// but are fetched from the server again on the next call. Must not be negative. /// - public int? MaxCachedPartitions { get; set; } + public int? MaxCachedPartitions { get; set; } = 100; /// /// Gets or sets how long a cache partition is kept after it was last used. /// /// - /// When (the default), unused partitions are kept until the source is disposed. + /// When (the default), unused partitions do not expire but can still be removed by the partition-count limit. /// Set it to free memory used by inactive users. After a partition is removed, the next call for its key /// fetches skills from the server again. Must be positive. /// To control how long MCP responses are cached, use . diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs index 0ab2a7ea4b6..8053849c46b 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs @@ -15,6 +15,56 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; public sealed partial class AgentMcpSkillsTests { + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_PartitionLimit_DefaultsTo100AndNullAllowsUnlimitedAsync(bool unlimited) + { + // Arrange + string key = ""; + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(60000, Entry())); + }); + var options = new AgentMcpSkillsSourceOptions + { + CacheIsolationKeySelector = _ => key, + }; + if (unlimited) + { + options.MaxCachedPartitions = null; + } + + Assert.Equal(unlimited ? null : (int?)100, options.MaxCachedPartitions); + Assert.Equal(unlimited ? null : (long?)100, options.ToMemoryCacheOptions().SizeLimit); + using var source = new AgentMcpSkillsSource(client.Object, options, null, new ManualTimeProvider()); + + // Act & Assert + for (int i = 0; i < 100; i++) + { + key = $"user-{i}"; + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + Assert.Same(skills, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + } + + Assert.Equal(100, lists); + key = "user-100"; + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + if (unlimited) + { + Assert.Same(first, second); + } + else + { + Assert.NotSame(first, second); + } + + Assert.Equal(unlimited ? 101 : 102, lists); + } + [Theory] [InlineData(nameof(AgentMcpSkillsSourceOptions.MaxCachedPartitions), -1)] [InlineData(nameof(AgentMcpSkillsSourceOptions.PartitionSlidingExpiration), 0)] From 739fb1fc878147d6506cec72ca966d1f9cf9aa37 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 11:21:55 +0000 Subject: [PATCH 07/12] .NET: Fix MCP skills sample response metadata placement Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../AgentSkills/Agent_Step06_McpBasedSkills/Program.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/Program.cs b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/Program.cs index 96c390a3755..c2e0e788c43 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/Program.cs +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/Program.cs @@ -169,6 +169,8 @@ 3. Present the converted value clearly with both units private static readonly string s_skillsListJson = $$""" { "resultType": "complete", + "ttlMs": 300000, + "cacheScope": "public", "skills": [ { "uri": "{{SkillUri}}", @@ -187,9 +189,7 @@ 3. Present the converted value clearly with both units "digest": "{{ComputeDigest(ConversionTable)}}", "size": {{Encoding.UTF8.GetByteCount(ConversionTable)}} } - ], - "ttlMs": 300000, - "cacheScope": "public" + ] } ] } From 0caa9ef12b75d376b01e5331b9cae497434165d6 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 11:26:26 +0000 Subject: [PATCH 08/12] .NET: Remove unused import from MCP skill tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs index c248f74cfa4..27eeaee2324 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs @@ -14,7 +14,6 @@ using ModelContextProtocol; using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; -using Moq; namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; From 051c4562b2d5cfabf2224dfafa1287b6e8392ecc Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 11:38:12 +0000 Subject: [PATCH 09/12] .NET: Remove redundant casts from MCP cache limit tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsTests.CacheScope.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs index 8053849c46b..7c0bceb667f 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs @@ -37,8 +37,8 @@ public async Task GetSkillsAsync_PartitionLimit_DefaultsTo100AndNullAllowsUnlimi options.MaxCachedPartitions = null; } - Assert.Equal(unlimited ? null : (int?)100, options.MaxCachedPartitions); - Assert.Equal(unlimited ? null : (long?)100, options.ToMemoryCacheOptions().SizeLimit); + Assert.Equal(unlimited ? null : 100, options.MaxCachedPartitions); + Assert.Equal(unlimited ? null : 100, options.ToMemoryCacheOptions().SizeLimit); using var source = new AgentMcpSkillsSource(client.Object, options, null, new ManualTimeProvider()); // Act & Assert From 024dd9142bc8d4f1be0cec1ceeedf202825a6688 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 15:01:14 +0000 Subject: [PATCH 10/12] .NET: Clarify MCP skills cache layers Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../AgentSkills/Agent_Step06_McpBasedSkills/README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md index bcef2202d50..c343647dd55 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md @@ -110,6 +110,13 @@ using var provider = new AgentSkillsProviderBuilder() .Build(); ``` +The builder adds a separate cache for the resolved skill list. `CacheIsolationKeySelector` above +configures only the MCP source's internal caches, not the builder's cache. By default, the builder's +cache can return one caller's skills to another without invoking the MCP source or its key selector. +`DisableCaching()` removes only that outer cache; the MCP source still caches skills and resources +within each partition. Every call reaches the source so it can select the caller's partition and +check MCP response expiration. + `GetApplicationAuthorizationKey` is your own helper, not a framework API. It should return a non-empty ID for the caller whose credentials the MCP client sends, for example a user or tenant ID. Use an opaque ID, not an access token. @@ -120,9 +127,6 @@ The source only separates caches; it does not handle authentication. Your app is - keeping the same caller's credentials when a returned skill later reads `SKILL.md` or other files; - not sharing returned skills between callers. -`DisableCaching()` turns off the provider's own cache, so the source, and therefore the key selector, -runs on every call. - By default, the source keeps up to 100 cache partitions. Adjust the limit or configure expiration to control how many partitions are kept and for how long: From 8dbd01a42125b451cf3a804ca720e80330710f56 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 15:07:26 +0000 Subject: [PATCH 11/12] .NET: Support asynchronous MCP cache isolation key selection Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Agent_Step06_McpBasedSkills/README.md | 12 +- .../Skills/AgentMcpSkillsSource.cs | 3 +- .../Skills/AgentMcpSkillsSourceOptions.cs | 9 +- .../Skills/AgentMcpSkillsTests.CacheScope.cs | 157 +++++++++++++++--- 4 files changed, 150 insertions(+), 31 deletions(-) diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md index c343647dd55..f4e8aa47f81 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md @@ -98,7 +98,7 @@ Set it whenever the server can return private responses, even if your app has on ```csharp var options = new AgentMcpSkillsSourceOptions { - CacheIsolationKeySelector = context => GetApplicationAuthorizationKey(context), + CacheIsolationKeySelector = (context, _) => new ValueTask(GetApplicationAuthorizationKey(context)), MaxCachedPartitions = 1000, PartitionSlidingExpiration = TimeSpan.FromMinutes(30), PartitionAbsoluteExpiration = TimeSpan.FromHours(8), @@ -121,6 +121,16 @@ check MCP response expiration. for the caller whose credentials the MCP client sends, for example a user or tenant ID. Use an opaque ID, not an access token. +If resolving the key requires asynchronous work, the selector can await it and use the supplied +cancellation token: + +```csharp +options.CacheIsolationKeySelector = async (context, cancellationToken) => + await ResolveAuthorizationKeyAsync(context, cancellationToken); +``` + +`ResolveAuthorizationKeyAsync` is an application helper, just like `GetApplicationAuthorizationKey`. + The source only separates caches; it does not handle authentication. Your app is responsible for: - configuring the MCP client to send the current caller's credentials; diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index 9cf2a10935b..e746e7f40d7 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -164,9 +164,10 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo } string? key = this._options?.CacheIsolationKeySelector is { } selector - ? Throw.IfNullOrWhitespace(selector(context)) + ? Throw.IfNullOrWhitespace(await selector(context, cancellationToken).ConfigureAwait(false)) : null; + cancellationToken.ThrowIfCancellationRequested(); ObjectDisposedException.ThrowIf(this._disposed, this); McpSkillsCatalog catalog = key is null diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs index a491e3faa13..0323062174d 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft. All rights reserved. using System; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.Caching.Memory; namespace Microsoft.Agents.AI; @@ -19,7 +21,7 @@ namespace Microsoft.Agents.AI; public sealed class AgentMcpSkillsSourceOptions { /// - /// Gets or sets a callback that returns a key identifying the current caller, such as a user or tenant ID. + /// Gets or sets an asynchronous callback that returns a key identifying the current caller, such as a user or tenant ID. /// The source keeps a separate skill cache for each key. /// /// @@ -38,13 +40,14 @@ public sealed class AgentMcpSkillsSourceOptions /// or if the server returns private responses, even for a single user. /// /// - /// The callback runs on every skill discovery call and must return a non-empty key that matches the credentials + /// The callback receives the source context and the discovery call's cancellation token. + /// It runs on every skill discovery call and must return a non-empty key that matches the credentials /// the MCP client sends. Use an opaque ID, not an access token. Do not share returned skills between callers. /// Call so the provider calls this source, and therefore /// this callback, every time. /// /// - public Func? CacheIsolationKeySelector { get; set; } + public Func>? CacheIsolationKeySelector { get; set; } /// /// Gets or sets the maximum number of cache partitions to keep, one per key returned by diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs index 7c0bceb667f..d55e4bd1b17 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs @@ -30,7 +30,7 @@ public async Task GetSkillsAsync_PartitionLimit_DefaultsTo100AndNullAllowsUnlimi }); var options = new AgentMcpSkillsSourceOptions { - CacheIsolationKeySelector = _ => key, + CacheIsolationKeySelector = (_, _) => new(key), }; if (unlimited) { @@ -104,7 +104,7 @@ public async Task GetSkillsAsync_PartitionSlidingExpiration_ExtendsOnDiscoveryBu }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = _ => "A", + CacheIsolationKeySelector = (_, _) => new("A"), PartitionSlidingExpiration = TimeSpan.FromSeconds(10), }, null, clock); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); @@ -138,7 +138,7 @@ public async Task GetSkillsAsync_PartitionAbsoluteExpiration_ExpiresDespiteDisco }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = _ => "A", + CacheIsolationKeySelector = (_, _) => new("A"), PartitionSlidingExpiration = TimeSpan.FromSeconds(10), PartitionAbsoluteExpiration = TimeSpan.FromSeconds(25), }, null, clock); @@ -169,7 +169,7 @@ public async Task GetSkillsAsync_PartitionRetention_DoesNotChangeResponseTtlAsyn }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = partitioned ? _ => "A" : null, + CacheIsolationKeySelector = partitioned ? (_, _) => new("A") : null, PartitionSlidingExpiration = TimeSpan.FromSeconds(10), PartitionAbsoluteExpiration = TimeSpan.FromSeconds(20), }, null, clock); @@ -235,7 +235,7 @@ public async Task GetSkillsAsync_PartitionEviction_PreservesInFlightDiscoveryAsy }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = _ => key, + CacheIsolationKeySelector = (_, _) => new(key), MaxCachedPartitions = capacity ? 1 : null, PartitionAbsoluteExpiration = capacity ? null : TimeSpan.FromSeconds(1), }, null, clock); @@ -388,7 +388,7 @@ public async Task GetSkillsAsync_PrivateMetadataScope_RequiresPartitionKeyAsync( }); using var source = new AgentMcpSkillsSource(client.Object, partitioned ? new() { - CacheIsolationKeySelector = _ => "authorization-context", + CacheIsolationKeySelector = (_, _) => new("authorization-context"), } : null); var original = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); await Assert.ThrowsAsync(async () => await original.GetContentAsync()); @@ -427,7 +427,7 @@ public async Task GetSkillsAsync_ChangingSelector_DoesNotChangeRetainedCachePart }); using var source = new AgentMcpSkillsSource(client.Object, options); var unkeyed = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - options.CacheIsolationKeySelector = _ => "authorization-context"; + options.CacheIsolationKeySelector = (_, _) => new("authorization-context"); var keyed = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); // Act & Assert @@ -462,7 +462,7 @@ public async Task GetSkillsAsync_SharedClient_FetchesAndCachesContextSpecificCon }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = _ => authorizationKey.Value!, + CacheIsolationKeySelector = (_, _) => new(authorizationKey.Value!), }); // Act @@ -488,16 +488,16 @@ public async Task GetSkillsAsync_OptionsChangedAfterConstruction_NewPartitionUse { // Arrange var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(60000, Entry()))); - var options = new AgentMcpSkillsSourceOptions { CacheIsolationKeySelector = _ => "A" }; + var options = new AgentMcpSkillsSourceOptions { CacheIsolationKeySelector = (_, _) => new("A") }; using var source = new AgentMcpSkillsSource(client.Object, options); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - options.CacheIsolationKeySelector = _ => "B"; + options.CacheIsolationKeySelector = (_, _) => new("B"); options.TtlResolver = _ => TimeSpan.Zero; // Act var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - options.CacheIsolationKeySelector = _ => "A"; + options.CacheIsolationKeySelector = (_, _) => new("A"); var original = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); // Assert @@ -514,10 +514,10 @@ public async Task GetSkillsAsync_DisposedDuringKeySelection_DoesNotCreateNewPart var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(60000, Entry()))); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = _ => + CacheIsolationKeySelector = (_, _) => { duringSelection?.Invoke(); - return "A"; + return new("A"); }, }); duringSelection = source.Dispose; @@ -564,7 +564,7 @@ public async Task GetSkillsAsync_PartitionKeys_SeparateSkillsAndAllResourceCache return Task.FromResult(result); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = (_, _) => new(key) }); // Act IList first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); @@ -603,7 +603,7 @@ public async Task GetSkillsAsync_PartitionKeys_CompareOrdinallyAsync() lists++; return Task.FromResult(TtlListResult(60000, Entry())); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = (_, _) => new(key) }); // Act var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); @@ -626,7 +626,7 @@ public async Task GetSkillsAsync_InvalidPartitionKey_ThrowsWithoutRequestsAsync( { // Arrange var client = CreateSkillsClient(); - using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key! }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = (_, _) => new(key!) }); // Act & Assert await Assert.ThrowsAnyAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); @@ -634,12 +634,117 @@ public async Task GetSkillsAsync_InvalidPartitionKey_ThrowsWithoutRequestsAsync( } [Fact] - public async Task GetSkillsAsync_PartitionKeySelectorError_PropagatesWithoutRequestsAsync() + public async Task GetSkillsAsync_AsyncPartitionKeySelector_AwaitsKeyAndReusesPartitionAsync() + { + // Arrange + var context = TestAgentSkillsSourceContextFactory.Create(); + using var cts = new CancellationTokenSource(); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + int selections = 0; + int lists = 0; + var client = CreateTtlClient((_, _) => + { + Interlocked.Increment(ref lists); + return Task.FromResult(TtlListResult(60000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CacheIsolationKeySelector = async (actualContext, cancellationToken) => + { + Assert.Same(context, actualContext); + Assert.Equal(cts.Token, cancellationToken); + Interlocked.Increment(ref selections); + return await release.Task.WaitAsync(cancellationToken); + }, + }); + + // Act + var first = source.GetSkillsAsync(context, cts.Token); + var second = source.GetSkillsAsync(context, cts.Token); + + // Assert + Assert.False(first.IsCompleted); + Assert.False(second.IsCompleted); + client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Never); + + // Act + release.SetResult("A"); + var results = await Task.WhenAll(first, second).WaitAsync(TimeSpan.FromSeconds(10)); + var reused = await source.GetSkillsAsync(context, cts.Token); + + // Assert + Assert.Single(reused); + Assert.Same(results[0], results[1]); + Assert.Same(results[0], reused); + Assert.Equal(3, selections); + Assert.Equal(1, lists); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_CanceledDuringAsyncKeySelection_ThrowsWithoutRequestsAsync(bool selectorObservesCancellation) + { + // Arrange + using var cts = new CancellationTokenSource(); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var client = CreateSkillsClient(); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CacheIsolationKeySelector = (_, cancellationToken) => + { + Assert.Equal(cts.Token, cancellationToken); + return new(selectorObservesCancellation ? release.Task.WaitAsync(cancellationToken) : release.Task); + }, + }); + + // Act + var discovery = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), cts.Token); + Assert.False(discovery.IsCompleted); + await cts.CancelAsync(); + release.SetResult("A"); + + // Assert + var error = await Assert.ThrowsAnyAsync( + () => discovery.WaitAsync(TimeSpan.FromSeconds(10))); + Assert.Equal(cts.Token, error.CancellationToken); + client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task GetSkillsAsync_DisposedDuringAsyncKeySelection_ThrowsWithoutRequestsAsync() + { + // Arrange + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var client = CreateSkillsClient(); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CacheIsolationKeySelector = (_, _) => new(release.Task), + }); + + // Act + var discovery = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + Assert.False(discovery.IsCompleted); + source.Dispose(); + release.SetResult("A"); + + // Assert + await Assert.ThrowsAsync(() => discovery.WaitAsync(TimeSpan.FromSeconds(10))); + client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_PartitionKeySelectorError_PropagatesWithoutRequestsAsync(bool asynchronous) { // Arrange var error = new InvalidOperationException("Authorization context unavailable."); var client = CreateSkillsClient(); - using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => throw error }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CacheIsolationKeySelector = (_, _) => asynchronous ? new(Task.FromException(error)) : throw error, + }); // Act var actual = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); @@ -663,7 +768,7 @@ public async Task GetSkillsAsync_PartitionKeySelector_SkippedWithoutSupportOrWhe using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = _ => throw new InvalidOperationException("Must not run."), + CacheIsolationKeySelector = (_, _) => throw new InvalidOperationException("Must not run."), }); using var cts = new CancellationTokenSource(); @@ -691,7 +796,7 @@ public async Task GetSkillsAsync_CachePartitions_ExpireIndependentlyAsync() lists++; return Task.FromResult(TtlListResult(1000, Entry())); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }, null, clock); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = (_, _) => new(key) }, null, clock); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); clock.Advance(TimeSpan.FromMilliseconds(500)); key = "B"; @@ -735,7 +840,7 @@ public async Task GetSkillsAsync_CachePartitions_KeepMismatchFallbackAndMetadata reads++; return Task.FromResult(ResourceReadResult(SkillUri, 1000, changed ? SkillContent + "\nChanged." : SkillContent)); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }, null, clock); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = (_, _) => new(key) }, null, clock); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); await first[0].GetContentAsync(); key = "B"; @@ -785,7 +890,7 @@ public async Task GetSkillsAsync_ConcurrentPartitionCreation_SharesPartitionPerK }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = context => ReferenceEquals(context, contextA) ? "A" : "B", + CacheIsolationKeySelector = (context, _) => new(ReferenceEquals(context, contextA) ? "A" : "B"), }); Task>[] requests = Enumerable.Range(0, 32).Select(index => Task.Run(async () => { @@ -818,7 +923,7 @@ public async Task GetSkillsAsync_ConcurrentCachePartitions_DoNotBlockEachOtherAs Interlocked.Increment(ref lists) == 1 ? release.Task : Task.FromResult(TtlListResult(60000, Entry()))); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = context => ReferenceEquals(context, contextA) ? "A" : "B", + CacheIsolationKeySelector = (context, _) => new(ReferenceEquals(context, contextA) ? "A" : "B"), }); // Act @@ -854,7 +959,7 @@ public async Task GetSkillsAsync_CachePartitionDisposal_PreservesRetainedSkillsA var client = CreateTtlClient((request, _) => request.Method == "skills/list" ? key == "B" && inFlight ? release.Task : Task.FromResult(TtlListResult(60000, Entry())) : Task.FromResult(ResourceReadResult(SkillUri, 60000, SkillContent))); - using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => key }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = (_, _) => new(key) }); var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); key = "B"; var second = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); @@ -904,7 +1009,7 @@ public async Task GetSkillsAsync_ConflictingPageScopes_FailsAndRetriesWholeListi return Task.FromResult(result); }); - using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = _ => "authorization-context" }); + using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = (_, _) => new("authorization-context") }); // Act & Assert for (int attempt = 0; attempt < 2; attempt++) @@ -991,7 +1096,7 @@ public async Task GetSkillsAsync_UnspecifiedScope_DefaultsToPublicAndLogsDiagnos }); using var source = new AgentMcpSkillsSource(client.Object, new() { - CacheIsolationKeySelector = _ => PartitionKey, + CacheIsolationKeySelector = (_, _) => new(PartitionKey), }, loggerFactory.Object); // Act From 8c25758817a72132f1b7c4d8fd7b86033fb2646b Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 3 Nov 2026 15:37:51 +0000 Subject: [PATCH 12/12] .NET: Compact MCP partition cache before adding new keys Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Agent_Step06_McpBasedSkills/README.md | 2 +- .../Skills/AgentMcpSkillsSource.cs | 17 ++- .../Skills/AgentMcpSkillsSourceOptions.cs | 4 +- .../Skills/AgentMcpSkillsTests.CacheScope.cs | 130 +++++++++++++++++- 4 files changed, 143 insertions(+), 10 deletions(-) diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md index f4e8aa47f81..bf6733f96c3 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step06_McpBasedSkills/README.md @@ -140,7 +140,7 @@ The source only separates caches; it does not handle authentication. Your app is By default, the source keeps up to 100 cache partitions. Adjust the limit or configure expiration to control how many partitions are kept and for how long: -- `MaxCachedPartitions` - the maximum number of partitions to keep. Defaults to 100; set to `null` for no partition-count limit. +- `MaxCachedPartitions` - the maximum number of partitions to keep. Defaults to 100; set to `null` for no partition-count limit or `0` to disable partition caching. When full, expired and least recently used partitions are removed to make room for a new key. - `PartitionSlidingExpiration` - removes a partition that has not been used for this long. - `PartitionAbsoluteExpiration` - removes a partition this long after it was created, even if it is still in use. diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index e746e7f40d7..19dfa391674 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -114,6 +114,7 @@ public sealed partial class AgentMcpSkillsSource : AgentSkillsSource private readonly AgentMcpSkillsSourceOptions? _options; private readonly TimeProvider _timeProvider; private readonly MemoryCache _catalogs; + private readonly int? _maxCachedPartitions; private readonly MemoryCacheEntryOptions? _catalogEntryOptions; private bool _disposed; @@ -144,6 +145,7 @@ internal AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? opt cacheOptions.Clock = new CatalogClock(this._timeProvider); this._catalogs = new MemoryCache(cacheOptions); + this._maxCachedPartitions = options?.MaxCachedPartitions; this._catalogEntryOptions = options?.ToMemoryCacheEntryOptions(); } @@ -187,9 +189,22 @@ private McpSkillsCatalog GetOrCreateCatalog(string key) return catalog!; } - // MemoryCache.GetOrCreate may run the factory concurrently; the lock ensures one partition per key. + // Serialize partition creation and compaction so a new partition has room in the cache. lock (this._catalogs) { + if (this._catalogs.TryGetValue(key, out catalog)) + { + return catalog!; + } + + int count = this._catalogs.Count; + if (this._maxCachedPartitions is > 0 && count >= this._maxCachedPartitions) + { + // Target 5% (5 of 100 entries) to avoid compacting for every new key. + // Since Compact rounds down, 1.0 / count ensures a target of at least one entry for small caches. + this._catalogs.Compact(Math.Max(0.05, 1.0 / count)); + } + return this._catalogs.GetOrCreate( key, _ => new McpSkillsCatalog(this._client, this._options, this._logger, this._timeProvider, hasIsolationKey: true), diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs index 0323062174d..3c3a81ccfba 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -55,8 +55,8 @@ public sealed class AgentMcpSkillsSourceOptions /// /// /// Defaults to 100. Set to for no partition-count limit. - /// When the limit is reached, skills for a new key are still returned, - /// but are fetched from the server again on the next call. Must not be negative. + /// When the limit is reached, expired and least recently used partitions are removed to make room for a new key. + /// Set to 0 to disable partition caching. Must not be negative. /// public int? MaxCachedPartitions { get; set; } = 100; diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs index d55e4bd1b17..c842d87eac4 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs @@ -53,16 +53,131 @@ public async Task GetSkillsAsync_PartitionLimit_DefaultsTo100AndNullAllowsUnlimi key = "user-100"; var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + Assert.Same(first, second); + Assert.Equal(101, lists); if (unlimited) { - Assert.Same(first, second); + for (int i = 0; i < 100; i++) + { + key = $"user-{i}"; + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + } + + Assert.Equal(101, lists); } - else + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(19)] + [InlineData(20)] + [InlineData(100)] + public async Task GetSkillsAsync_FullPartitionCache_EvictsLeastRecentlyUsedAndCachesNewKeyAsync(int limit) + { + // Arrange + var clock = new ManualTimeProvider(); + string key = ""; + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(60000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CacheIsolationKeySelector = (_, _) => new(key), + MaxCachedPartitions = limit, + }, null, clock); + var partitions = new List>(); + for (int i = 0; i < limit; i++) { - Assert.NotSame(first, second); + key = $"user-{i}"; + partitions.Add(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + clock.Advance(TimeSpan.FromMilliseconds(1)); } - Assert.Equal(unlimited ? 101 : 102, lists); + key = $"user-{limit - 1}"; + Assert.Same(partitions[limit - 1], await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + clock.Advance(TimeSpan.FromMilliseconds(1)); + + // Act + key = "new-user"; + var added = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.Same(added, reused); + Assert.Equal(limit + 1, lists); + if (limit > 1) + { + key = $"user-{limit - 1}"; + Assert.Same(partitions[limit - 1], await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Equal(limit + 1, lists); + } + + key = "user-0"; + Assert.NotSame(partitions[0], await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Equal(limit + 2, lists); + } + + [Fact] + public async Task GetSkillsAsync_ZeroPartitionLimit_DoesNotCachePartitionsAsync() + { + // Arrange + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(60000, Entry())); + }); + using var source = new AgentMcpSkillsSource(client.Object, new() + { + CacheIsolationKeySelector = (_, _) => new("A"), + MaxCachedPartitions = 0, + }); + + // Act + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.NotSame(first, second); + Assert.Equal(2, lists); + } + + [Theory] + [InlineData(null)] + [InlineData(0)] + [InlineData(100)] + public async Task GetSkillsAsync_PartitionLimitChangedAfterConstruction_UsesOriginalLimitAsync(int? updatedLimit) + { + // Arrange + string key = "A"; + int lists = 0; + var client = CreateTtlClient((_, _) => + { + lists++; + return Task.FromResult(TtlListResult(60000, Entry())); + }); + var options = new AgentMcpSkillsSourceOptions + { + CacheIsolationKeySelector = (_, _) => new(key), + MaxCachedPartitions = 1, + }; + using var source = new AgentMcpSkillsSource(client.Object, options); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + options.MaxCachedPartitions = updatedLimit; + + // Act + key = "B"; + var added = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + var reused = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.Same(added, reused); + Assert.Equal(2, lists); } [Theory] @@ -875,8 +990,10 @@ public async Task GetSkillsAsync_CachePartitions_KeepMismatchFallbackAndMetadata Assert.Equal(3, reads); } - [Fact] - public async Task GetSkillsAsync_ConcurrentPartitionCreation_SharesPartitionPerKeyAsync() + [Theory] + [InlineData(null)] + [InlineData(2)] + public async Task GetSkillsAsync_ConcurrentPartitionCreation_SharesPartitionPerKeyAsync(int? limit) { // Arrange var contextA = TestAgentSkillsSourceContextFactory.Create(); @@ -891,6 +1008,7 @@ public async Task GetSkillsAsync_ConcurrentPartitionCreation_SharesPartitionPerK using var source = new AgentMcpSkillsSource(client.Object, new() { CacheIsolationKeySelector = (context, _) => new(ReferenceEquals(context, contextA) ? "A" : "B"), + MaxCachedPartitions = limit, }); Task>[] requests = Enumerable.Range(0, 32).Select(index => Task.Run(async () => {