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..c2e0e788c43 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 --- @@ -168,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}}", @@ -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..bf6733f96c3 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,81 @@ 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 `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 +{ + CacheIsolationKeySelector = (context, _) => new ValueTask(GetApplicationAuthorizationKey(context)), + MaxCachedPartitions = 1000, + PartitionSlidingExpiration = TimeSpan.FromMinutes(30), + PartitionAbsoluteExpiration = TimeSpan.FromHours(8), +}; + +using var provider = new AgentSkillsProviderBuilder() + .UseMcpSkills(sharedMcpClient, options) + .DisableCaching() + .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. + +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; +- keeping the same caller's credentials when a returned skill later reads `SKILL.md` or other files; +- not sharing returned skills between callers. + +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 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. + +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..aa82ee9fabc 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs @@ -51,10 +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. @@ -63,14 +64,14 @@ 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. /// 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, 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); @@ -78,7 +79,7 @@ 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; this._resourceCache = new(this._timeProvider, StringComparer.Ordinal); @@ -100,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. @@ -107,7 +113,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 +142,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,13 +203,15 @@ private async ValueTask> FetchResourceContentAsy // Validation time consumes the TTL; network request time does not. long receivedTimestamp = this._timeProvider.GetTimestamp(); + 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)) { 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/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index c2eb9274cec..19dfa391674 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,18 @@ 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 int? _maxCachedPartitions; + 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 +136,18 @@ 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._maxCachedPartitions = options?.MaxCachedPartitions; + + this._catalogEntryOptions = options?.ToMemoryCacheEntryOptions(); } /// @@ -130,19 +165,65 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo return []; } - IList skills = await this._catalog.GetSkillsAsync(cancellationToken).ConfigureAwait(false); + string? key = this._options?.CacheIsolationKeySelector is { } selector + ? Throw.IfNullOrWhitespace(await selector(context, cancellationToken).ConfigureAwait(false)) + : null; + + cancellationToken.ThrowIfCancellationRequested(); + ObjectDisposedException.ThrowIf(this._disposed, this); + + 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!; + } + + // 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), + 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._catalog.Dispose(); + this._disposed = true; + 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..3c3a81ccfba 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft. All rights reserved. using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Caching.Memory; namespace Microsoft.Agents.AI; @@ -17,6 +20,67 @@ namespace Microsoft.Agents.AI; /// public sealed class AgentMcpSkillsSourceOptions { + /// + /// 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. + /// + /// + /// + /// 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 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; } + + /// + /// Gets or sets the maximum number of cache partitions to keep, one per key returned by + /// . + /// + /// + /// Defaults to 100. Set to for no partition-count limit. + /// 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; + + /// + /// Gets or sets how long a cache partition is kept after it was last used. + /// + /// + /// 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 . + /// + 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 +129,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..04eecfe53e0 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..4cbc4290a12 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,15 +41,14 @@ 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. -/// 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"; @@ -65,9 +63,8 @@ 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; /// /// Initializes a new instance of the class. @@ -76,9 +73,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 +85,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)); @@ -100,49 +99,13 @@ 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( + 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; - } - - /// - 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. - /// - /// 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))) - { - this._skillsToRefresh.TryAdd(skill, 0); - } + needsRefresh: cached => cached.OfType().Any(this.NeedsRefresh)).ConfigureAwait(false); } /// @@ -160,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. @@ -168,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; } @@ -215,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); } /// @@ -292,6 +235,8 @@ private async Task>> FetchSkillsAsync(Cancellati throw new InvalidOperationException($"The skills/get result must describe the requested skill '{skillUri}'."); } + 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. return this.CreateSkill(result.Skill, McpCacheLifetime.Create(receivedTimestamp, result.TimeToLive, this._ttlResolver)); @@ -306,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); + return new AgentMcpSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes, this._timeProvider, metadataLifetime, this._ttlResolver, this._hasIsolationKey); } /// @@ -350,6 +295,7 @@ private McpCacheLifetime CombineLifetimes(List lifetimes) HashSet cursors = new(StringComparer.Ordinal); string? cursor = null; int pageCount = 0; + CacheScope? declaredScope = null; do { @@ -394,6 +340,15 @@ private McpCacheLifetime CombineLifetimes(List lifetimes) throw new InvalidOperationException($"A skills/list result must have resultType '{CompleteResultType}'. Received resultType '{result.ResultType}'."); } + result.CacheScope = ValidateAndNormalizeCacheScope(result.CacheScope, this._hasIsolationKey, this._logger, "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 +490,31 @@ 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.CacheIsolationKeySelector)} to return a nonempty authorization-context key."); + } + + return scope.Value; + } + + /// + /// 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.ResourceTtl.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.ResourceTtl.cs index c8c30f76d93..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(); } }); - using 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); - using 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 03ed9a1fd50..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; @@ -36,8 +35,7 @@ public async Task Constructor_PreservesProvidedFrontmatterAsync() var frontmatter = new AgentSkillFrontmatter("unit-converter", "Convert units.") { License = "MIT" }; // Act - using 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 +49,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(); - using 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 +73,11 @@ 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 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 +107,7 @@ public async Task GetContentAsync_MissingInstructionResource_ThrowsWithoutReadin McpSkillResourceEntry[] resources = includeNestedInstructions ? [Resource(SkillRootUri + "nested/SKILL.md", SkillContent)] : []; - using 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 +459,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 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 +896,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 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 +1165,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. @@ -1206,8 +1193,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..c842d87eac4 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.CacheScope.cs @@ -0,0 +1,1248 @@ +// 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(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 = (_, _) => new(key), + }; + if (unlimited) + { + options.MaxCachedPartitions = null; + } + + 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 + 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()); + Assert.Same(first, second); + Assert.Equal(101, lists); + if (unlimited) + { + for (int i = 0; i < 100; i++) + { + key = $"user-{i}"; + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + } + + Assert.Equal(101, lists); + } + } + + [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++) + { + key = $"user-{i}"; + partitions.Add(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + clock.Advance(TimeSpan.FromMilliseconds(1)); + } + + 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] + [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() + { + CacheIsolationKeySelector = (_, _) => new("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() + { + CacheIsolationKeySelector = (_, _) => new("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() + { + CacheIsolationKeySelector = partitioned ? (_, _) => new("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() + { + CacheIsolationKeySelector = (_, _) => new(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.CacheIsolationKeySelector), 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.CacheIsolationKeySelector), 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() + { + CacheIsolationKeySelector = (_, _) => new("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.CacheIsolationKeySelector), 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.CacheIsolationKeySelector = (_, _) => new("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.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()); + } + + [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() + { + CacheIsolationKeySelector = (_, _) => new(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 { CacheIsolationKeySelector = (_, _) => new("A") }; + using var source = new AgentMcpSkillsSource(client.Object, options); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + 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 = (_, _) => new("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() + { + CacheIsolationKeySelector = (_, _) => + { + duringSelection?.Invoke(); + return new("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() { CacheIsolationKeySelector = (_, _) => new(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() { CacheIsolationKeySelector = (_, _) => new(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() { CacheIsolationKeySelector = (_, _) => new(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_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 = (_, _) => asynchronous ? new(Task.FromException(error)) : 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() + { + CacheIsolationKeySelector = (_, _) => 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() { CacheIsolationKeySelector = (_, _) => new(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() { CacheIsolationKeySelector = (_, _) => new(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); + } + + [Theory] + [InlineData(null)] + [InlineData(2)] + public async Task GetSkillsAsync_ConcurrentPartitionCreation_SharesPartitionPerKeyAsync(int? limit) + { + // 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() + { + CacheIsolationKeySelector = (context, _) => new(ReferenceEquals(context, contextA) ? "A" : "B"), + MaxCachedPartitions = limit, + }); + 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() + { + CacheIsolationKeySelector = (context, _) => new(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() { CacheIsolationKeySelector = (_, _) => new(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() { CacheIsolationKeySelector = (_, _) => new("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() + { + CacheIsolationKeySelector = (_, _) => new(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..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]); } @@ -1198,7 +1207,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(); @@ -1227,6 +1236,44 @@ public async Task GetSkillsAsync_DisposedCatalog_RetainedSkillRefetchesExpiredCo 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. /// @@ -1235,7 +1282,7 @@ public async Task GetSkillsAsync_DisposedCatalog_RetainedSkillRefetchesExpiredCo ["resultType"] = "complete", ["skill"] = entry, ["ttlMs"] = ttl, - ["cacheScope"] = "private", + ["cacheScope"] = "public", }; /// @@ -1247,7 +1294,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 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..6d394fac5b7 --- /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.CacheIsolationKeySelector), 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 7a8a7567052..c45476b9f6d 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,9 +13,9 @@ 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 +public sealed partial class McpSkillsCatalogTests { [Fact] public void Constructor_NullClient_ThrowsArgumentNullException() @@ -28,16 +27,6 @@ public void Constructor_NullClient_ThrowsArgumentNullException() Assert.Throws("client", () => new McpSkillsCatalog(null!, timeProvider: clock)); } - [Fact] - public void QueueSkillRefresh_NullSkill_ThrowsArgumentNullException() - { - // Arrange - using var catalog = new McpSkillsCatalog(new Mock().Object); - - // Act & Assert - Assert.Throws("skill", () => catalog.QueueSkillRefresh(null!)); - } - [Fact] public async Task GetSkillsAsync_PositiveTtl_ReusesListUntilExactExpirationAsync() { @@ -50,7 +39,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 +69,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 +113,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 +136,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 +169,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 +202,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 +226,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 +241,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. ///