From dc7c9192614800ed4333ec40ff78542b78432a41 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 23 Oct 2026 16:32:43 +0100 Subject: [PATCH 1/6] Remove legacy MCP skill index loading Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkill.cs | 195 --- .../Skills/AgentMcpSkillResource.cs | 59 - .../Skills/AgentMcpSkillsSource.cs | 175 +-- .../Skills/AgentMcpSkillsSourceOptions.cs | 79 - ...AgentSkillsProviderBuilderMcpExtensions.cs | 12 +- .../Loaders/AgentMcpSkillArchiveExtractor.cs | 215 --- .../Skills/Loaders/ArchiveEntryLoader.cs | 462 ------ .../Skills/Loaders/ArchiveFormat.cs | 15 - .../Skills/Loaders/IMcpSkillEntryLoader.cs | 36 - .../Skills/Loaders/SkillMdEntryLoader.cs | 80 - .../Skills/McpJsonContext.cs | 14 - .../Skills/McpSkillIndex.cs | 97 -- .../AgentMcpSkillsDiscoverySelectionTests.cs | 6 +- .../AgentMcpSkillsSourceArchiveTests.cs | 1341 ----------------- .../Skills/AgentMcpSkillsSourceTests.cs | 663 -------- .../Listed/AgentMcpListedSkillsTests.cs | 87 +- 16 files changed, 64 insertions(+), 3472 deletions(-) delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillResource.cs delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/IMcpSkillEntryLoader.cs delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/SkillMdEntryLoader.cs delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpJsonContext.cs delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillIndex.cs delete mode 100644 dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs delete mode 100644 dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceTests.cs diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs deleted file mode 100644 index d5453e7d9be..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs +++ /dev/null @@ -1,195 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.Logging; -using Microsoft.Shared.Diagnostics; -using ModelContextProtocol.Client; -using ModelContextProtocol.Protocol; - -namespace Microsoft.Agents.AI; - -/// -/// An discovered from an MCP server exposing the Agent Skills convention. -/// -/// -/// -/// The skill is constructed from skill://index.json discovery metadata only; -/// fetches the full SKILL.md content from the MCP server on demand via resources/read. -/// -/// -/// Per SEP-2640, resources referenced inside SKILL.md are fetched on demand via the originating MCP -/// server: resolves a relative resource name against the -/// skill's root URI, issues a resources/read request, and returns an -/// with pre-fetched content. -/// -/// -internal sealed partial class AgentMcpSkill : AgentSkill -{ - private const string SkillMdSuffix = "SKILL.md"; - private const int MaxResourceNameDecodingDepth = 32; - - private readonly McpClient _client; - private readonly ILogger _logger; - private readonly string _skillMdUri; - private readonly string _skillRootUri; - private string? _content; - - /// - /// Initializes a new instance of the class. - /// - /// The parsed frontmatter metadata for this skill. - /// - /// The full MCP resource URI of the SKILL.md resource (e.g. skill://unit-converter/SKILL.md). - /// Used by to fetch the skill content on demand. The skill's root URI - /// (used to resolve sibling resources) is derived by stripping the trailing SKILL.md segment. - /// - /// The MCP client used to fetch resources on demand. - /// The logger used to report rejected resource names. - public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string skillMdUri, McpClient client, ILogger logger) - { - this.Frontmatter = Throw.IfNull(frontmatter); - this._skillMdUri = Throw.IfNullOrWhitespace(skillMdUri); - this._skillRootUri = ComputeSkillRootUri(skillMdUri); - this._client = Throw.IfNull(client); - this._logger = Throw.IfNull(logger); - } - - /// - public override AgentSkillFrontmatter Frontmatter { get; } - - /// - /// - /// Fetches the SKILL.md content from the MCP server via resources/read on the first call - /// and caches the result. - /// - public override async ValueTask GetContentAsync(CancellationToken cancellationToken = default) - { - if (this._content is not null) - { - return this._content; - } - -#pragma warning disable CA2234 // Pass system uri objects instead of strings - ReadResourceResult result = await this._client.ReadResourceAsync(this._skillMdUri, cancellationToken: cancellationToken).ConfigureAwait(false); -#pragma warning restore CA2234 // Pass system uri objects instead of strings - - string text = string.Join("\n", result.Contents.OfType().Select(c => c.Text)); - - if (text.Length == 0) - { - throw new InvalidOperationException($"The MCP server returned no text content for SKILL.md resource '{this._skillMdUri}'."); - } - - return this._content = text; - } - - /// - /// - /// Resolves as a relative path against the skill's root URI, issues a - /// resources/read request to the MCP server, and returns an - /// with the pre-fetched content. Absolute paths, parent traversal (including percent-encoded forms), - /// embedded URI schemes, and control characters are rejected before sending a request. - /// Resource names requiring more than 32 percent-decoding passes are also rejected. - /// Returns when the name is empty or unsafe, the server - /// returns no content, or the resource does not exist on the server. - /// - public override async ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) - { - if (string.IsNullOrWhiteSpace(name)) - { - return null; - } - - // Treat backslashes as separators, e.g. "..\x" is checked and requested as "../x". - string normalized = name.Replace('\\', '/'); - if (!IsResourceNameSafe(normalized)) - { - LogUnsafeResourceName(this._logger); - return null; - } - - string uri = this._skillRootUri + normalized; - - ReadResourceResult result; - try - { -#pragma warning disable CA2234 // Pass system uri objects instead of strings - result = await this._client.ReadResourceAsync(uri, cancellationToken: cancellationToken).ConfigureAwait(false); -#pragma warning restore CA2234 // Pass system uri objects instead of strings - } - catch (Exception ex) when (ex is not OperationCanceledException) - { - return null; - } - - return new AgentMcpSkillResource(name: name, result: result); - } - - private static bool IsResourceNameSafe(string normalized) - { - // Split at the first literal "?"/"#" before decoding, so "a%3f/%2e%2e/x" stays one path while - // "a/b.md?q=/../x" leaves "/../x" in the query; then decode each part once. - string[] parts = normalized.Split(['?', '#'], 2); - string? path = FullyUnescape(parts[0]); - string? suffix = FullyUnescape(parts.Length > 1 ? parts[1] : string.Empty); - - // Excessive encoding depth, e.g. a name requiring more than 32 decoding passes. - return path is not null && suffix is not null - // Absolute path, e.g. "/etc/passwd" or "%2fetc/passwd". - && !path.StartsWith('/') - // Embedded URI, e.g. "http://example.com/other" or "%68ttp%3a%2f%2fexample.com". - && !path.Contains("://", StringComparison.Ordinal) - // Parent traversal, e.g. "../x", "%2e%2e/x", "%252e%252e/x", "a%3f/../../x", or ".. " (URI parsers can trim trailing spaces). - && !path.Split(['/', '?', '#']).Any(segment => segment.TrimEnd(' ') == "..") - // Control characters anywhere, e.g. "a/\0/b.md", ".\t./x", or ".%09./x". - && !(path + suffix).Any(char.IsControl); - } - - /// - /// Percent-decodes until stable, normalizing backslashes to forward slashes. - /// Decoding never removes a literal ., /, :, or control character, so checking - /// the final form also covers every nested encoding layer (e.g. %252e). - /// - /// The decoded value, or if the decoding depth exceeds the limit. - private static string? FullyUnescape(string value) - { - // The final pass only checks that decoding has stabilized. - for (int depth = 0; depth <= MaxResourceNameDecodingDepth; depth++) - { - string decoded = Uri.UnescapeDataString(value).Replace('\\', '/'); - if (decoded == value) - { - return value; - } - - value = decoded; - } - - return null; - } - - [LoggerMessage(LogLevel.Debug, "Rejecting MCP skill resource name with unsafe path components.")] - private static partial void LogUnsafeResourceName(ILogger logger); - - /// - /// Strips the trailing SKILL.md from the URI to produce the skill's root directory URI. - /// If the URI doesn't end with SKILL.md, ensures it ends with a trailing slash. - /// - private static string ComputeSkillRootUri(string skillMdUri) - { - if (skillMdUri.EndsWith(SkillMdSuffix, StringComparison.Ordinal)) - { - return skillMdUri.Substring(0, skillMdUri.Length - SkillMdSuffix.Length); - } - - if (skillMdUri.EndsWith("/", StringComparison.Ordinal)) - { - return skillMdUri; - } - - return skillMdUri + "/"; - } -} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillResource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillResource.cs deleted file mode 100644 index a6fcb3e3d2a..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillResource.cs +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.AI; -using Microsoft.Shared.Diagnostics; -using ModelContextProtocol; -using ModelContextProtocol.Protocol; - -namespace Microsoft.Agents.AI; - -/// -/// An backed by content fetched from an MCP server. -/// -/// -/// The is fetched eagerly by -/// at construction time; extracts the content from the result. -/// -internal sealed class AgentMcpSkillResource : AgentSkillResource -{ - private readonly ReadResourceResult _result; - - /// - /// Initializes a new instance of the class with a pre-fetched result. - /// - /// The resource name (e.g. a relative path or identifier). - /// The result returned by the MCP server's resources/read request. - /// An optional description of the resource. - public AgentMcpSkillResource(string name, ReadResourceResult result, string? description = null) - : base(Throw.IfNullOrWhitespace(name), description) - { - this._result = Throw.IfNull(result); - } - - /// - /// - /// A when the resource contains binary content, a when - /// it contains text, or when the server returned no content blocks. - /// - public override Task ReadAsync(IServiceProvider? serviceProvider = null, CancellationToken cancellationToken = default) - { - BlobResourceContents? blob = this._result.Contents.OfType().FirstOrDefault(); - if (blob is not null) - { - return Task.FromResult(blob.ToAIContent()); - } - - string text = string.Join("\n", this._result.Contents.OfType().Select(c => c.Text)); - - if (text.Length == 0) - { - return Task.FromResult(null); - } - - return Task.FromResult(text); - } -} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index 531fea91fe6..4b6ac041bb7 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -1,18 +1,13 @@ // Copyright (c) Microsoft. All rights reserved. -using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Shared.Diagnostics; -using ModelContextProtocol; using ModelContextProtocol.Client; -using ModelContextProtocol.Protocol; namespace Microsoft.Agents.AI; @@ -22,7 +17,7 @@ namespace Microsoft.Agents.AI; /// /// /// Uses skills/list when the server declares the io.modelcontextprotocol/skills extension. -/// Otherwise, uses skill://index.json for backward compatibility. +/// Returns an empty skill list without sending a discovery request when the extension is not declared. /// /// /// Listed skill discovery is based on the Skills extension specification incorporating the @@ -34,7 +29,6 @@ namespace Microsoft.Agents.AI; /// Listed skills provide a name, description, and list of resources. Instructions and supporting resources /// are fetched from the MCP server when requested. Only supporting resources listed by the server are available. /// Each listed resource read must return exactly one text or binary content block; SKILL.md must contain text. -/// Discovery through skills/list leaves existing archive directories untouched. /// Page counts, total skill counts, and individual resource sizes /// are bounded by . Discovery follows continuation cursors /// until exhausted and fails on repeated cursors, invalid pages, or exceeded limits instead of returning a partial catalog. @@ -48,61 +42,21 @@ namespace Microsoft.Agents.AI; /// including the builder's cache by calling . /// /// -/// skill://index.json is used only when the server does not declare the Skills extension. -/// It lists skill-md entries pointing to individual SKILL.md resources and archive -/// entries pointing to archives containing skill files. Entries with unsupported types are skipped. -/// An absent, unreadable, empty, or invalid index produces an empty skill list. -/// Index-based discovery and archive-distributed skills are compatibility features and will be deprecated -/// in a future release. -/// -/// -/// Archive entries may supply a digest in the form sha256: followed by 64 lowercase -/// hexadecimal characters. It is verified against the decoded archive bytes before extraction. -/// Malformed, unsupported, or mismatched digest strings are logged as warnings and the affected archives -/// are skipped; omitted or null digests remain supported. Discovery returns the remaining skills, -/// so a successful cache refresh replaces its list without rejected archives. This verification -/// does not apply to lazily fetched skill-md entries or their supporting resources. -/// -/// -/// Thread safety and archive reconciliation. During skill://index.json discovery, every call to -/// reconciles the archive skills directory: it extracts newly advertised -/// skills, re-extracts existing ones, and prunes those the server no longer advertises. Because that -/// work mutates files (and internal state) that concurrent calls would also touch, running it from -/// multiple threads at once could corrupt the directory or surface partially-extracted skills. To -/// prevent this, the archive reconciliation is guarded by a per-instance lock: only one call performs -/// it at a time, and other concurrent callers wait and then run sequentially. -/// -/// -/// This keeps the directory consistent but means concurrent (and repeated) calls do not share work — -/// each one re-contacts the MCP server and re-reconciles. To avoid that redundant work, place a caching -/// layer in front of this source (for example, via , which adds -/// one by default): -/// -/// When the server's skills do not change at runtime, cache with no isolation key -/// (the default ), so a single -/// fetch and reconciliation is shared by all callers for the lifetime of the cache. -/// When the server's skills can change, additionally set -/// so the cache periodically re-fetches -/// and reconciles instead of doing so on every call. -/// -/// -/// /// Supporting resource names are checked for absolute paths, parent traversal (including -/// percent-encoded forms), and control characters before an MCP request is sent. Index URLs +/// percent-encoded forms), and control characters before an MCP request is sent. Resource URIs /// retain their MCP resource schemes: they are sent to the connected server, not opened locally. /// This client-side guard does not replace server-side authorization. /// /// /// Security considerations: This source discovers and loads skills — including full -/// skill instructions and, for archive-type entries, files extracted to local disk — from a +/// skill instructions and supporting resources — from a /// remote MCP server that the caller connects to explicitly (via UseMcpSkills); it is never /// enabled by default. A compromised, malicious, or simply untrustworthy MCP server can return /// adversarial skill content designed to manipulate the agent through indirect prompt injection, or /// instructions/scripts designed to exfiltrate data once loaded and, for script-capable skills, -/// executed. Only connect this source to MCP servers you trust, and review archive extraction limits () -/// to bound the impact of a malicious server. A matching digest proves consistency with the index, -/// not trustworthiness: a server controlling both the index and archive can replace both or omit -/// the digest. +/// executed. Only connect this source to MCP servers you trust, and review discovery and resource limits () +/// to bound the impact of a malicious server. A matching digest proves consistency with the resource manifest, +/// not trustworthiness: a server controlling both the manifest and resource can replace both. /// /// /// This source does not impose a byte limit on individual or combined skills/list responses. @@ -115,23 +69,17 @@ namespace Microsoft.Agents.AI; /// public sealed partial class AgentMcpSkillsSource : AgentSkillsSource { - /// - /// Discovery document URI for the index compatibility path. - /// - private const string IndexUri = "skill://index.json"; - private const string SkillsExtensionName = "io.modelcontextprotocol/skills"; [SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "The MCP client is supplied and owned by the caller, who is responsible for disposing it.")] private readonly McpClient _client; private readonly ILogger _logger; - private readonly Dictionary _loaders; private readonly McpListedSkillsLoader _listedSkillsLoader; /// /// Initializes a new instance of the class. /// - /// An MCP client connected to a server that supports skills/list or skill://index.json. The caller retains ownership of the client and is responsible for disposing it. + /// 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 options that control skill loading. /// Optional logger factory. public AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? options = null, ILoggerFactory? loggerFactory = null) @@ -140,14 +88,6 @@ public AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? optio loggerFactory ??= NullLoggerFactory.Instance; this._logger = loggerFactory.CreateLogger(); this._listedSkillsLoader = new McpListedSkillsLoader(this._client, this._logger, options); - - IMcpSkillEntryLoader[] loaders = - [ - new SkillMdEntryLoader(this._client, loggerFactory), - new ArchiveEntryLoader(this._client, options, loggerFactory), - ]; - - this._loaders = loaders.ToDictionary(l => l.EntryType, StringComparer.OrdinalIgnoreCase); } /// @@ -157,46 +97,15 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo FeatureUsage.MarkUsed((int)Mcp.FeatureIndex.CoreMcpSkillsSource); #pragma warning restore MAAI001 - // skills/list discovery must bypass index and archive loaders. - if (this._client.ServerCapabilities.Extensions?.ContainsKey(SkillsExtensionName) == true) - { - IList listedSkills = await this._listedSkillsLoader.DiscoverAsync(cancellationToken).ConfigureAwait(false); - - LogSkillsLoadedTotal(this._logger, listedSkills.Count); - - return listedSkills; - } - - McpSkillIndex? index = await this.TryReadIndexAsync(cancellationToken).ConfigureAwait(false); + cancellationToken.ThrowIfCancellationRequested(); - // Group entries by type and set aside those a registered loader can handle; entries of any - // other type are unsupported and logged. - var entriesByType = new Dictionary>(StringComparer.OrdinalIgnoreCase); - - foreach (var group in (index?.Skills ?? []).GroupBy(e => e.Type ?? string.Empty, StringComparer.OrdinalIgnoreCase)) + if (this._client.ServerCapabilities.Extensions?.ContainsKey(SkillsExtensionName) != true) { - if (this._loaders.ContainsKey(group.Key)) - { - entriesByType[group.Key] = group.ToList(); - } - else - { - foreach (var entry in group) - { - LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", $"unsupported type '{entry.Type ?? "(none)"}'"); - } - } + LogSkillsExtensionAbsent(this._logger); + return []; } - // Invoke every registered loader, even when the server advertises no entries of its type, so - // each type's lifecycle still runs (e.g. the archive loader prunes leftover directories). - var skills = new List(); - - foreach (var loader in this._loaders.Values) - { - var entries = entriesByType.TryGetValue(loader.EntryType, out List? matched) ? matched : []; - skills.AddRange(await loader.LoadAsync(entries, context, cancellationToken).ConfigureAwait(false)); - } + IList skills = await this._listedSkillsLoader.DiscoverAsync(cancellationToken).ConfigureAwait(false); LogSkillsLoadedTotal(this._logger, skills.Count); @@ -209,70 +118,14 @@ protected override void Dispose(bool disposing) if (disposing) { this._listedSkillsLoader.Dispose(); - - foreach (var loader in this._loaders.Values) - { - (loader as IDisposable)?.Dispose(); - } } base.Dispose(disposing); } - private async Task TryReadIndexAsync(CancellationToken cancellationToken) - { - ReadResourceResult result; - - try - { -#pragma warning disable CA2234 // Pass system uri objects instead of strings - result = await this._client.ReadResourceAsync(IndexUri, cancellationToken: cancellationToken).ConfigureAwait(false); -#pragma warning restore CA2234 // Pass system uri objects instead of strings - } - catch (McpException ex) when (ex is McpProtocolException pex && pex.ErrorCode == McpErrorCode.ResourceNotFound) - { - LogIndexAbsent(this._logger, ex.Message); - return null; - } - catch (McpException ex) - { - LogIndexReadFailed(this._logger, ex); - return null; - } - - string? indexText = result.Contents.OfType().FirstOrDefault()?.Text; - if (string.IsNullOrWhiteSpace(indexText)) - { - LogIndexEmpty(this._logger); - return null; - } - - try - { - return JsonSerializer.Deserialize(indexText, McpJsonContext.Default.McpSkillIndex); - } - catch (JsonException ex) - { - LogIndexParseFailed(this._logger, ex); - return null; - } - } - [LoggerMessage(LogLevel.Information, "Successfully loaded {Count} skills from MCP server")] private static partial void LogSkillsLoadedTotal(ILogger logger, int count); - [LoggerMessage(LogLevel.Debug, "No skill://index.json resource available on MCP server: {Reason}")] - private static partial void LogIndexAbsent(ILogger logger, string reason); - - [LoggerMessage(LogLevel.Warning, "Failed to read skill://index.json from MCP server.")] - private static partial void LogIndexReadFailed(ILogger logger, Exception exception); - - [LoggerMessage(LogLevel.Debug, "skill://index.json on MCP server returned empty/non-text contents")] - private static partial void LogIndexEmpty(ILogger logger); - - [LoggerMessage(LogLevel.Warning, "Failed to parse skill://index.json JSON document.")] - private static partial void LogIndexParseFailed(ILogger logger, Exception exception); - - [LoggerMessage(LogLevel.Debug, "Skipping skill index entry '{SkillName}': {Reason}")] - private static partial void LogIndexEntrySkipped(ILogger logger, string skillName, string reason); + [LoggerMessage(LogLevel.Debug, "No io.modelcontextprotocol/skills extension declared by MCP server; skill discovery is skipped.")] + private static partial void LogSkillsExtensionAbsent(ILogger logger); } diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs index 29c4913a9c1..a18e06cbb52 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -1,7 +1,5 @@ // Copyright (c) Microsoft. All rights reserved. -using System.Collections.Generic; - namespace Microsoft.Agents.AI; /// @@ -14,11 +12,6 @@ namespace Microsoft.Agents.AI; /// Application developers are responsible for evaluating server trustworthiness and configuring appropriate /// input-size and resource limits through their chosen MCP client, transport, or hosting environment. /// See for security considerations. -/// Security consideration: The archive limits (, -/// , ) bound how much a -/// single MCP server response can extract to local disk. Since the server supplying these archives is -/// an external, potentially untrusted system, keep these limits conservative unless you have vetted the -/// server and legitimately need to raise them. /// public sealed class AgentMcpSkillsSourceOptions { @@ -53,76 +46,4 @@ public sealed class AgentMcpSkillsSourceOptions /// There is no combined resource-size limit per skill. /// public long? ListedResourceMaxSizeBytes { get; set; } - - /// - /// Gets or sets the base directory that archive-type skills are extracted to and served from. - /// - /// - /// Archives are extracted beneath this directory as {ArchiveSkillsDirectory}/{skill-name}/. - /// When , the source extracts to a per-instance unique location of - /// {currentDirectory}/{guid}/{skill-name}/, where the GUID is generated once per - /// instance so that multiple sources never overwrite one - /// another. Set this to a fixed value to get a predictable, reusable extraction location. - /// When set, each source must use its own unique directory: the source treats the directory as - /// exclusively its own and, on every index-based discovery, prunes any sub-directory that the MCP - /// server no longer advertises or whose index entry is not actionable (e.g., missing a required field). - /// Pointing two sources at the same directory would therefore cause them to - /// delete each other's extracted skills. - /// Discovery through skills/list does not extract archives or reconcile this directory; - /// previously extracted files are left untouched. - /// - public string? ArchiveSkillsDirectory { get; set; } - - /// - /// Gets or sets the allowed file extensions for resources discovered in extracted archive-type skills. - /// - /// - /// When , defaults to .md, .json, .yaml, .yml, - /// .csv, .xml, and .txt. - /// - public IEnumerable? ArchiveResourceExtensions { get; set; } - - /// - /// Gets or sets the maximum depth to search for resource files within each extracted archive-type - /// skill directory. A value of 1 searches only the skill root directory. A value of 2 - /// searches the root and one level of subdirectories. - /// - /// - /// When , the source uses the default depth of 2. - /// - public int? ArchiveResourceSearchDepth { get; set; } - - /// - /// Gets or sets the maximum number of files that may be extracted from a single archive-type skill. - /// - /// - /// Guards against excessive-file-count denial-of-service archives. When , the - /// source uses a default of 20, sized for a typical well-formed skill (a handful of files). - /// Raise this for archive-type skills that legitimately bundle many files. An archive that exceeds - /// the limit is skipped. - /// - public int? ArchiveMaxFileCount { get; set; } - - /// - /// Gets or sets the maximum size, in bytes, of a downloaded archive-type skill resource. - /// - /// - /// Guards against archive resources that are too large to materialize safely. When - /// , the source uses a default of 1 MB, sized for a typical - /// well-formed skill archive. Raise this for archive-type skills that legitimately require - /// larger archive payloads. An archive that exceeds the limit is skipped. - /// - public long? ArchiveMaxSizeBytes { get; set; } - - /// - /// Gets or sets the maximum total uncompressed size, in bytes, of all files extracted from a single - /// archive-type skill. - /// - /// - /// Guards against decompression-bomb archives. When , the source uses a default - /// of 1 MB, sized for a typical well-formed skill (well under ~1 MB). Raise this for - /// archive-type skills that legitimately bundle larger content. An archive that exceeds the limit is - /// skipped. - /// - public long? ArchiveMaxUncompressedSizeBytes { get; set; } } diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs index e76082eda80..602ccc677aa 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs @@ -14,14 +14,14 @@ public static class AgentSkillsProviderBuilderMcpExtensions /// Adds a skill source that discovers skills served over MCP via the supplied . /// /// The builder to extend. - /// An MCP client connected to a server that supports skills/list or skill://index.json. + /// An MCP client connected to a server that supports skills/list. /// Optional options that control skill loading. /// The builder instance for chaining. /// /// /// Supports skills discovered through skills/list when the server declares the - /// io.modelcontextprotocol/skills extension. Otherwise, discovers skills through - /// skill://index.json. + /// io.modelcontextprotocol/skills extension. Otherwise, returns an empty skill list + /// without sending a discovery request. /// Listed skills fetch instructions and supporting resources when requested. /// Page counts, total skill counts, and individual resource sizes /// are bounded by . @@ -63,12 +63,8 @@ public static class AgentSkillsProviderBuilderMcpExtensions /// using var provider = new AgentSkillsProvider(source, ownsSource: true); /// /// - /// Index-based discovery and archive-distributed skills are compatibility features and will be deprecated - /// in a future release. - /// - /// /// Security considerations: Calling this method is an explicit opt-in to loading - /// skills — including instructions and, for archive-type entries, extracted files — from the MCP + /// skills — including instructions and supporting resources — from the MCP /// server that is connected to. External skill sources may introduce /// adversarial or compromised skills designed to influence the agent via indirect prompt injection /// or to exfiltrate data through instructions or scripts the agent is induced to run. Only connect diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs deleted file mode 100644 index 2f4c3105132..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs +++ /dev/null @@ -1,215 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Buffers; -using System.IO; -using System.IO.Compression; - -namespace Microsoft.Agents.AI; - -/// -/// Unpacks skill archives downloaded from an MCP server into a local directory. -/// -/// -/// Supports ZIP payloads. Extraction is guarded against path-traversal ("zip-slip") attacks and -/// bounded by a maximum file count and total uncompressed size to mitigate decompression-bomb attacks. -/// -internal static class AgentMcpSkillArchiveExtractor -{ - /// - /// The default maximum number of files that may be extracted from a single archive, sized for a - /// typical well-formed skill (a handful of files) with headroom over the observed average. - /// - internal const int DefaultMaxFileCount = 20; - - /// - /// The default maximum total uncompressed size, in bytes, of all files extracted from a single - /// archive, sized for a typical well-formed skill (well under ~1 MB). - /// - internal const long DefaultMaxUncompressedSizeBytes = 1L * 1024 * 1024; - - private const int CopyBufferSize = 81920; - - /// - /// Determines the archive format from the advertised media type, the source URL, and the leading - /// bytes of the payload (magic-number sniffing takes precedence as it is the most reliable). - /// - /// The archive bytes. - /// The advertised MIME type, if any. - /// The resource URL the archive was read from, if any. - /// The detected , or . - internal static ArchiveFormat DetectFormat(byte[] bytes, string? mediaType, string? url) - { - // Magic-number sniffing is the most reliable signal. - // Reject gzip by signature before considering potentially incorrect MIME type or URL hints. - if (bytes.Length >= 2 && bytes[0] == 0x1F && bytes[1] == 0x8B) - { - return ArchiveFormat.Unknown; - } - - if (bytes.Length >= 4 && bytes[0] == 0x50 && bytes[1] == 0x4B && - (bytes[2] == 0x03 || bytes[2] == 0x05 || bytes[2] == 0x07)) - { - return ArchiveFormat.Zip; - } - - string media = mediaType?.Trim() ?? string.Empty; - if (string.Equals(media, "application/zip", StringComparison.OrdinalIgnoreCase) || - string.Equals(media, "application/x-zip-compressed", StringComparison.OrdinalIgnoreCase)) - { - return ArchiveFormat.Zip; - } - - string u = url ?? string.Empty; - if (u.EndsWith(".zip", StringComparison.OrdinalIgnoreCase)) - { - return ArchiveFormat.Zip; - } - - return ArchiveFormat.Unknown; - } - - /// - /// Extracts the supplied archive into . - /// - /// The archive bytes. - /// The archive container format. - /// The directory the archive is unpacked into. Created if missing. - /// The maximum number of files that may be extracted from the archive. - /// The maximum total uncompressed size, in bytes, of all extracted files. - /// The format is not . - /// The archive exceeds one of the supplied limits. - internal static void Extract( - byte[] bytes, - ArchiveFormat format, - string targetDirectory, - int? maxFileCount = null, - long? maxUncompressedSizeBytes = null) - { - maxFileCount ??= DefaultMaxFileCount; - maxUncompressedSizeBytes ??= DefaultMaxUncompressedSizeBytes; - - if (format != ArchiveFormat.Zip) - { - throw new NotSupportedException($"Unsupported skill archive format '{format}'. Use ZIP instead."); - } - - Directory.CreateDirectory(targetDirectory); - string fullTarget = Path.GetFullPath(targetDirectory); - - using var source = new MemoryStream(bytes, writable: false); - ExtractZip(source, fullTarget, maxFileCount.Value, maxUncompressedSizeBytes.Value); - } - - private static void ExtractZip(Stream source, string fullTarget, int maxFileCount, long maxUncompressedSizeBytes) - { - using var archive = new ZipArchive(source, ZipArchiveMode.Read, leaveOpen: true); - - long remainingBytes = maxUncompressedSizeBytes; - int fileCount = 0; - - foreach (ZipArchiveEntry entry in archive.Entries) - { - // Directory entries have an empty Name. - if (entry.Name.Length == 0) - { - continue; - } - - if (++fileCount > maxFileCount) - { - throw new InvalidDataException($"Skill archive exceeds the maximum allowed file count ({maxFileCount})."); - } - - string? destination = ResolveDestination(fullTarget, entry.FullName); // CodeQL [SM02729] ResolveDestination rejects paths outside fullTarget. - if (destination is null) - { - continue; - } - - Directory.CreateDirectory(Path.GetDirectoryName(destination)!); - - using Stream entryStream = entry.Open(); - using FileStream output = File.Create(destination); - CopyWithLimit(entryStream, output, ref remainingBytes); - } - } - - /// - /// Copies to while decrementing a shared - /// uncompressed-byte budget, throwing once it is exhausted. This is the authoritative defense against - /// decompression bombs because it counts bytes actually produced by the decompressor rather than - /// trusting archive metadata which may be inaccurate. Peak memory is bounded to a single buffer. - /// - /// The remaining byte budget is exceeded. - private static void CopyWithLimit(Stream source, Stream destination, ref long remainingBytes) - { - byte[] buffer = ArrayPool.Shared.Rent(CopyBufferSize); - try - { - int read; - while ((read = source.Read(buffer, 0, buffer.Length)) > 0) - { - remainingBytes -= read; - if (remainingBytes < 0) - { - throw new InvalidDataException("Skill archive exceeds the maximum allowed uncompressed size."); - } - - destination.Write(buffer, 0, read); - } - } - finally - { - ArrayPool.Shared.Return(buffer); - } - } - - /// - /// Resolves an archive entry path to an absolute destination beneath , - /// or when the entry would escape the target directory (zip-slip). - /// - private static string? ResolveDestination(string fullTarget, string entryPath) - { - if (string.IsNullOrWhiteSpace(entryPath)) - { - return null; - } - - // Normalize separators and reject absolute / rooted entry paths outright. - string normalized = entryPath.Replace('\\', '/').TrimStart('/'); - if (normalized.Length == 0 || Path.IsPathRooted(normalized)) - { - return null; - } - - string destination = Path.GetFullPath(Path.Combine(fullTarget, normalized)); - if (!IsPathContainedIn(fullTarget, destination)) - { - return null; - } - - return destination; - } - - /// - /// Returns when resolves to a location - /// beneath . - /// - internal static bool IsPathContainedIn(string parentDirectory, string candidatePath) - { - string fullParent = Path.GetFullPath(parentDirectory); - string fullCandidate = Path.GetFullPath(candidatePath); - - // Append a trailing separator so the containment check doesn't false-match sibling - // directories. e.g. "/skills/myskill" matches "/skills/myskill-evil/", but - // "/skills/myskill/" does not. - string prefix = fullParent.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal) - ? fullParent - : fullParent + Path.DirectorySeparatorChar; - - return fullCandidate.StartsWith( - prefix, - OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); - } -} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs deleted file mode 100644 index 9b4791747f8..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs +++ /dev/null @@ -1,462 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Security.Cryptography; -using System.Text.RegularExpressions; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.Logging; -using ModelContextProtocol; -using ModelContextProtocol.Client; -using ModelContextProtocol.Protocol; - -namespace Microsoft.Agents.AI; - -/// -/// Loads archive index entries: each entry's url points to a single archive resource -/// in ZIP format whose content unpacks into the skill's namespace. Archives are downloaded, -/// extracted to a local directory, and the resulting files are discovered via an internal -/// that this loader proxies to. -/// -/// -/// Because MCP-delivered skills are treated strictly as instructor-format text, scripts bundled -/// inside an archive are surfaced as readable resources only; they are never discovered as -/// executable scripts. Supplied SHA-256 digests are verified before extraction; archives without -/// a digest remain supported. -/// -internal sealed partial class ArchiveEntryLoader : IMcpSkillEntryLoader, IDisposable -{ - /// - /// The default maximum size, in bytes, of a downloaded archive resource. - /// - internal const long DefaultMaxArchiveSizeBytes = 1L * 1024 * 1024; - - private readonly McpClient _client; - private readonly AgentMcpSkillsSourceOptions? _options; - private readonly ILoggerFactory _loggerFactory; - private readonly ILogger _logger; - - // Serializes the reconcile -> extract -> read sequence so concurrent loads never mutate the - // shared on-disk directory (or the _archiveSkillsDirectory field) at the same time. - private readonly SemaphoreSlim _reconcileGate = new(1, 1); - - private string? _archiveSkillsDirectory; - - public ArchiveEntryLoader(McpClient client, AgentMcpSkillsSourceOptions? options, ILoggerFactory loggerFactory) - { - this._client = client; - this._options = options; - this._loggerFactory = loggerFactory; - this._logger = loggerFactory.CreateLogger(); - } - - /// - public string EntryType => "archive"; - - /// - public async Task> LoadAsync(IReadOnlyList entries, AgentSkillsSourceContext context, CancellationToken cancellationToken) - { - // Filter out entries that are missing required fields or have invalid names. - var archiveEntries = this.FilterValidEntries(entries); - - // The reconcile -> extract -> read sequence mutates a shared on-disk directory and the - // _archiveSkillsDirectory field, so it must run as a single critical section. Concurrent - // callers wait here and execute one at a time, keeping the directory consistent. - await this._reconcileGate.WaitAsync(cancellationToken).ConfigureAwait(false); - try - { - // Determine the target directory from prior state or caller-supplied options. - var archiveSkillsDirectory = this._archiveSkillsDirectory ?? this._options?.ArchiveSkillsDirectory; - - // Reconcile on-disk state with the current set of advertised skills. - this.ReconcileArchiveSkillDirectories(archiveSkillsDirectory, archiveEntries); - - if (archiveEntries.Count == 0) - { - return []; - } - - // Resolve or generate the skills directory and ensure it exists on disk. - this._archiveSkillsDirectory = archiveSkillsDirectory ?? Path.Combine(Directory.GetCurrentDirectory(), Guid.NewGuid().ToString("N")); - Directory.CreateDirectory(this._archiveSkillsDirectory); - - // Download and extract each archive entry into its own subdirectory. - var skillDirectories = new List(archiveEntries.Count); - - foreach (var entry in archiveEntries) - { - skillDirectories.AddRange(await this.TryDownloadAndExtractSkillAsync(entry, this._archiveSkillsDirectory, cancellationToken).ConfigureAwait(false)); - } - - // Delegate discovery of extracted content to a file-based skills source. - AgentFileSkillsSource fileSource = this.CreateFileSkillsSource(skillDirectories); - - return await fileSource.GetSkillsAsync(context, cancellationToken).ConfigureAwait(false); - } - finally - { - this._reconcileGate.Release(); - } - } - - /// - /// Releases the resources used by this loader. - /// - public void Dispose() - { - this._reconcileGate.Dispose(); - } - - /// - /// Reconciles the on-disk archive skill directories with the current set of advertised entries. - /// When no entries are advertised, all existing directories are removed. Otherwise, only stale - /// directories (not in the current advertised set) are pruned. - /// - private void ReconcileArchiveSkillDirectories(string? archiveSkillsDirectory, List archiveEntries) - { - // Nothing to reconcile if the directory hasn't been established yet. - if (string.IsNullOrEmpty(archiveSkillsDirectory) || !Directory.Exists(archiveSkillsDirectory)) - { - return; - } - - // Server advertises no archive skills - remove all previously extracted directories. - if (archiveEntries.Count == 0) - { - this.ClearSkillsDirectory(archiveSkillsDirectory); - return; - } - - // Compare the advertised set against what's on disk; prune any stale leftovers. - var advertisedSkillNames = archiveEntries - .Select(e => e.Name!) - .ToHashSet(OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); - - foreach (string directory in Directory.EnumerateDirectories(archiveSkillsDirectory)) - { - string name = Path.GetFileName(directory); - if (advertisedSkillNames.Contains(name)) - { - continue; - } - - try - { - Directory.Delete(directory, recursive: true); - LogArchiveSkillPruned(this._logger, name, SanitizePathForLog(directory)); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - LogArchiveSkillPruneFailed(this._logger, name, ex); - } - } - } - - /// - /// Removes all subdirectories of the specified directory. Failures on individual directories - /// are swallowed so that a single locked directory does not prevent cleanup of the others. - /// - private void ClearSkillsDirectory(string archiveSkillsDirectory) - { - foreach (var dir in Directory.GetDirectories(archiveSkillsDirectory)) - { - try - { - Directory.Delete(dir, true); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - LogArchiveSkillPruneFailed(this._logger, Path.GetFileName(dir), ex); - } - } - } - - /// - /// Creates the internal that discovers extracted archive - /// content. Scripts are never treated as executable; the allowed script extensions list is left - /// empty so that script files are not discovered as runnable scripts. - /// - private AgentFileSkillsSource CreateFileSkillsSource(IReadOnlyList archiveSkillDirectories) - { - var fileOptions = new AgentFileSkillsSourceOptions - { - AllowedScriptExtensions = [], - AllowedResourceExtensions = this._options?.ArchiveResourceExtensions, - SearchDepth = this._options?.ArchiveResourceSearchDepth, - }; - - return new AgentFileSkillsSource( - archiveSkillDirectories, - scriptRunner: null, - options: fileOptions, - loggerFactory: this._loggerFactory); - } - - /// - /// Downloads the archive for a single skill entry, detects its format, and extracts it into a - /// subdirectory of . Entries that cannot be materialized - /// (path escape, download failure, unsupported format, or extraction error) are skipped and logged; - /// partial content is cleaned up on failure. - /// - private async Task> TryDownloadAndExtractSkillAsync(McpSkillIndexEntry entry, string archiveSkillsDirectory, CancellationToken cancellationToken) - { - string skillDirectory = Path.Combine(archiveSkillsDirectory, entry.Name!); - - // Defense-in-depth: the skill directory is derived from the server-supplied entry name. - // Even though invalid names are filtered upstream, verify the resolved path cannot escape - // the archive skills directory before any file system operation is performed. - if (!AgentMcpSkillArchiveExtractor.IsPathContainedIn(archiveSkillsDirectory, skillDirectory)) - { - LogIndexEntrySkipped(this._logger, entry.Name!, "resolved skill directory escapes the archive skills directory"); - return []; - } - - // Clean the target directory so that content is always freshly extracted. - if (!this.TryDeleteSkillDirectory(entry.Name!, skillDirectory)) - { - return []; - } - - (byte[]? bytes, string? mimeType) = await this.DownloadSkillBytesAsync(entry, cancellationToken).ConfigureAwait(false); - if (bytes is null) - { - return []; - } - - var format = AgentMcpSkillArchiveExtractor.DetectFormat(bytes, mimeType, entry.Url); - if (format == ArchiveFormat.Unknown) - { - LogIndexEntrySkipped(this._logger, entry.Name!, $"unsupported archive media type '{mimeType ?? "(none)"}'"); - return []; - } - - try - { - AgentMcpSkillArchiveExtractor.Extract( - bytes, - format, - skillDirectory, - this._options?.ArchiveMaxFileCount, - this._options?.ArchiveMaxUncompressedSizeBytes); - } - catch (Exception ex) - { - LogArchiveExtractFailed(this._logger, entry.Name!, ex); - - // Remove any partially-extracted content so it is not later treated as a valid, - // already-materialized skill by the reuse check above. - _ = this.TryDeleteSkillDirectory(entry.Name!, skillDirectory); - return []; - } - - LogArchiveExtracted(this._logger, entry.Name!, SanitizePathForLog(skillDirectory)); - - return [skillDirectory]; - } - - /// - /// Downloads and decodes the binary content of a skill's archive resource. Returns - /// bytes when the resource cannot be read, contains no binary content, is empty, exceeds the - /// size limit, or has an invalid or mismatched digest. - /// - private async Task<(byte[]? Bytes, string? MimeType)> DownloadSkillBytesAsync(McpSkillIndexEntry entry, CancellationToken cancellationToken) - { - ReadResourceResult result; - - try - { -#pragma warning disable CA2234 // Pass system uri objects instead of strings - result = await this._client.ReadResourceAsync(entry.Url!, cancellationToken: cancellationToken).ConfigureAwait(false); -#pragma warning restore CA2234 // Pass system uri objects instead of strings - } - catch (McpException ex) - { - LogArchiveReadFailed(this._logger, entry.Name!, ex); - return (null, null); - } - - BlobResourceContents? blobContent = result.Contents.OfType().FirstOrDefault(); - if (blobContent is null) - { - LogIndexEntrySkipped(this._logger, entry.Name!, "archive resource returned no binary content"); - return (null, null); - } - - long maxArchiveSizeBytes = this._options?.ArchiveMaxSizeBytes ?? DefaultMaxArchiveSizeBytes; - - byte[] bytes; - try - { - if (blobContent.DecodedData.Length > maxArchiveSizeBytes) - { - LogIndexEntrySkipped(this._logger, entry.Name!, $"archive resource exceeds the maximum allowed size ({maxArchiveSizeBytes} bytes)"); - return (null, null); - } - - bytes = blobContent.DecodedData.ToArray(); - } - catch (FormatException ex) - { - LogArchiveDecodeFailed(this._logger, entry.Name!, ex); - return (null, null); - } - - if (bytes.Length == 0) - { - LogIndexEntrySkipped(this._logger, entry.Name!, "archive resource returned empty content"); - return (null, null); - } - - if (entry.Digest is not null && !this.VerifyDigest(entry.Name!, entry.Digest, bytes)) - { - return (null, null); - } - - return (bytes, blobContent.MimeType); - } - - /// - /// Verifies a supplied digest against decoded archive bytes before extraction. - /// - private bool VerifyDigest(string skillName, string digest, byte[] bytes) - { - if (!ArchiveDigestRegex().IsMatch(digest)) - { - LogArchiveDigestVerificationFailed(this._logger, skillName, "digest must be 'sha256:' followed by 64 lowercase hexadecimal characters"); - return false; - } - - string actualDigest = Convert.ToHexString(SHA256.HashData(bytes)); - if (!actualDigest.AsSpan().Equals(digest.AsSpan("sha256:".Length), StringComparison.OrdinalIgnoreCase)) - { - LogArchiveDigestVerificationFailed(this._logger, skillName, "digest does not match downloaded content"); - return false; - } - - return true; - } - - [GeneratedRegex(@"\Asha256:[0-9a-f]{64}\z", RegexOptions.CultureInvariant)] - private static partial Regex ArchiveDigestRegex(); - - /// - /// Filters archive entries to those that are valid for materialization. Entries with missing or - /// invalid names (not usable as directory names) or missing URLs are excluded and logged. - /// - private List FilterValidEntries(IReadOnlyList entries) - { - var invalidFileNameChars = Path.GetInvalidFileNameChars(); - - var valid = new List(entries.Count); - - foreach (var entry in entries) - { - if (string.IsNullOrWhiteSpace(entry.Name)) - { - LogIndexEntrySkipped(this._logger, "(unnamed)", "archive entry missing required 'name' field"); - continue; - } - - if (!IsValidDirectoryName(entry.Name!, invalidFileNameChars)) - { - LogIndexEntrySkipped(this._logger, entry.Name!, "name contains invalid characters for a directory name"); - continue; - } - - if (string.IsNullOrWhiteSpace(entry.Url)) - { - LogIndexEntrySkipped(this._logger, entry.Name!, "missing required 'url' field"); - continue; - } - - valid.Add(entry); - } - - return valid; - - static bool IsValidDirectoryName(string value, char[] invalidChars) - { - foreach (char c in value) - { - if (Array.IndexOf(invalidChars, c) >= 0 || c == Path.DirectorySeparatorChar || c == Path.AltDirectorySeparatorChar) - { - return false; - } - } - - // Names consisting solely of dots or whitespace are invalid directory names. - string trimmed = value.Trim().Trim('.'); - return trimmed.Length > 0; - } - } - - /// - /// Replaces control characters in a file-system path with ? so the path is safe to include - /// in log messages without risking terminal-escape injection. - /// - private static string SanitizePathForLog(string path) - { - char[]? chars = null; - for (int i = 0; i < path.Length; i++) - { - if (char.IsControl(path[i])) - { - chars ??= path.ToCharArray(); - chars[i] = '?'; - } - } - - return chars is null ? path : new string(chars); - } - - /// - /// Attempts to recursively delete a skill directory. - /// - private bool TryDeleteSkillDirectory(string skillName, string directory) - { - try - { - if (Directory.Exists(directory)) - { - Directory.Delete(directory, recursive: true); - } - - return true; - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - LogArchiveSkillCleanupFailed(this._logger, skillName, ex); - return false; - } - } - - [LoggerMessage(LogLevel.Debug, "Skipping skill index entry '{SkillName}': {Reason}")] - private static partial void LogIndexEntrySkipped(ILogger logger, string skillName, string reason); - - [LoggerMessage(LogLevel.Debug, "Extracted archive skill '{SkillName}' to {TargetDirectory}")] - private static partial void LogArchiveExtracted(ILogger logger, string skillName, string targetDirectory); - - [LoggerMessage(LogLevel.Debug, "Pruned stale archive skill '{SkillName}' at {TargetDirectory}")] - private static partial void LogArchiveSkillPruned(ILogger logger, string skillName, string targetDirectory); - - [LoggerMessage(LogLevel.Warning, "Failed to prune stale archive skill '{SkillName}'.")] - private static partial void LogArchiveSkillPruneFailed(ILogger logger, string skillName, Exception exception); - - [LoggerMessage(LogLevel.Warning, "Failed to clean existing archive skill directory for '{SkillName}'.")] - private static partial void LogArchiveSkillCleanupFailed(ILogger logger, string skillName, Exception exception); - - [LoggerMessage(LogLevel.Warning, "Failed to read archive resource for skill '{SkillName}'.")] - private static partial void LogArchiveReadFailed(ILogger logger, string skillName, Exception exception); - - [LoggerMessage(LogLevel.Warning, "Failed to decode archive resource for skill '{SkillName}'.")] - private static partial void LogArchiveDecodeFailed(ILogger logger, string skillName, Exception exception); - - [LoggerMessage(LogLevel.Warning, "Skipping archive skill '{SkillName}': {Reason}")] - private static partial void LogArchiveDigestVerificationFailed(ILogger logger, string skillName, string reason); - - [LoggerMessage(LogLevel.Warning, "Failed to extract archive for skill '{SkillName}'.")] - private static partial void LogArchiveExtractFailed(ILogger logger, string skillName, Exception exception); -} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs deleted file mode 100644 index 99a510b3051..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -namespace Microsoft.Agents.AI; - -/// -/// The archive container formats supported by . -/// -internal enum ArchiveFormat -{ - /// The format could not be determined. - Unknown, - - /// A ZIP archive. - Zip, -} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/IMcpSkillEntryLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/IMcpSkillEntryLoader.cs deleted file mode 100644 index 9971d0f100c..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/IMcpSkillEntryLoader.cs +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; - -namespace Microsoft.Agents.AI; - -/// -/// Loads instances from skill://index.json entries of a single -/// distribution (e.g. skill-md or archive). -/// -/// -/// Implementations receive all index entries of their type as a batch so that types whose -/// materialization spans the whole set (e.g. archive, which reconciles and prunes a shared -/// on-disk directory) can reason about every advertised entry at once. The batch may be empty: every -/// registered loader is invoked even when the server advertises no entries of its type, so that -/// type-wide lifecycle work (such as pruning leftover archive directories) still runs. -/// -internal interface IMcpSkillEntryLoader -{ - /// - /// Gets the index entry type this loader handles (matched case-insensitively). - /// - string EntryType { get; } - - /// - /// Loads the skills for every supplied entry. Entries that are invalid or fail to load are - /// skipped (and logged) rather than throwing; the returned list contains only the skills that - /// loaded successfully. - /// - /// The index entries of this loader's . May be empty. - /// Contextual information about the agent and session requesting skills. - /// A token to cancel the operation. - Task> LoadAsync(IReadOnlyList entries, AgentSkillsSourceContext context, CancellationToken cancellationToken); -} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/SkillMdEntryLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/SkillMdEntryLoader.cs deleted file mode 100644 index 7cb7c7a696e..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/SkillMdEntryLoader.cs +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.Logging; -using ModelContextProtocol.Client; - -namespace Microsoft.Agents.AI; - -/// -/// Loads skill-md index entries: each entry's SKILL.md and sibling resources are -/// fetched on demand from the MCP server via an . -/// -internal sealed partial class SkillMdEntryLoader : IMcpSkillEntryLoader -{ - private readonly McpClient _client; - private readonly ILogger _logger; - - public SkillMdEntryLoader(McpClient client, ILoggerFactory loggerFactory) - { - this._client = client; - this._logger = loggerFactory.CreateLogger(); - } - - /// - public string EntryType => "skill-md"; - - /// - public Task> LoadAsync(IReadOnlyList entries, AgentSkillsSourceContext context, CancellationToken cancellationToken) - { - var skills = new List(); - - foreach (var entry in entries) - { - if (this.TryLoadSkillMdEntry(entry, out AgentSkill? skill)) - { - skills.Add(skill); - } - } - - return Task.FromResult>(skills); - } - - private bool TryLoadSkillMdEntry(McpSkillIndexEntry entry, [NotNullWhen(true)] out AgentSkill? skill) - { - skill = null; - - if (string.IsNullOrWhiteSpace(entry.Url)) - { - LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", "missing required 'url' field"); - return false; - } - - AgentSkillFrontmatter frontmatter; - try - { - frontmatter = new AgentSkillFrontmatter(entry.Name!, entry.Description!); - } - catch (ArgumentException ex) - { - LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", $"invalid metadata: {ex.Message}"); - return false; - } - - skill = new AgentMcpSkill(frontmatter, entry.Url!, this._client, this._logger); - - LogSkillLoaded(this._logger, frontmatter.Name); - - return true; - } - - [LoggerMessage(LogLevel.Information, "Loaded MCP skill: {SkillName}")] - private static partial void LogSkillLoaded(ILogger logger, string skillName); - - [LoggerMessage(LogLevel.Debug, "Skipping skill index entry '{SkillName}': {Reason}")] - private static partial void LogIndexEntrySkipped(ILogger logger, string skillName, string reason); -} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpJsonContext.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpJsonContext.cs deleted file mode 100644 index fa4b94966c6..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpJsonContext.cs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System.Text.Json; -using System.Text.Json.Serialization; - -namespace Microsoft.Agents.AI; - -/// -/// Source-generated JSON context for MCP-skills well-known DTOs. -/// -[JsonSourceGenerationOptions(JsonSerializerDefaults.Web, AllowTrailingCommas = true, ReadCommentHandling = JsonCommentHandling.Skip)] -[JsonSerializable(typeof(McpSkillIndex))] -[JsonSerializable(typeof(McpSkillIndexEntry))] -internal sealed partial class McpJsonContext : JsonSerializerContext; diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillIndex.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillIndex.cs deleted file mode 100644 index 9d032bba902..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillIndex.cs +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System.Collections.Generic; -using System.Text.Json.Serialization; - -namespace Microsoft.Agents.AI; - -/// -/// DTO for the skill discovery index document served at skill://index.json. -/// -/// -/// -/// Schema reference: -/// (Agent Skills Discovery v0.2.0), as used by the legacy SEP-2640 MCP index binding. -/// The url field contains a full MCP resource URI. Unlike the base schema, the -/// digest field is optional; supplied archive digests are verified in addition to -/// relying on the authenticated MCP transport. -/// -/// -/// All properties are nullable so that deserialization succeeds even when the server-side index -/// is incomplete or malformed; callers MUST validate required fields before use. -/// -/// -internal sealed class McpSkillIndex -{ - /// - /// Gets or sets the opaque schema identifier URI. Required by the base schema; clients SHOULD - /// match this against known schema URIs (e.g. - /// https://schemas.agentskills.io/discovery/0.2.0/schema.json) before processing the index. - /// - [JsonPropertyName("$schema")] - public string? Schema { get; set; } - - /// - /// Gets or sets the array of skill entries. Required by the schema; an empty or missing - /// skills array means the index advertises no skills. - /// - [JsonPropertyName("skills")] - public List? Skills { get; set; } -} - -/// -/// A single entry in the skill discovery index. -/// -/// -/// Field requirements per the v0.2.0 schema and the SEP-2640 binding: -/// -/// type, description, and url are REQUIRED. -/// name is REQUIRED for skill-md and archive entries; OMITTED for mcp-resource-template. -/// digest is optional under this MCP index binding; supplied archive digests are verified before extraction. -/// -/// All properties are nullable to keep deserialization lenient; callers validate required fields before use. -/// -internal sealed class McpSkillIndexEntry -{ - /// - /// Gets or sets the skill name (1-64 chars, lowercase alphanumeric and hyphens; no leading, - /// trailing, or consecutive hyphens). Required for skill-md and archive entries; - /// omitted for mcp-resource-template. - /// - [JsonPropertyName("name")] - public string? Name { get; set; } - - /// - /// Gets or sets the entry distribution type. Required. Schema-defined values are - /// skill-md and archive; the SEP-2640 MCP binding additionally defines - /// mcp-resource-template. - /// - [JsonPropertyName("type")] - public string? Type { get; set; } - - /// - /// Gets or sets the skill description (max 1024 chars per the Agent Skills specification). - /// Required. For skill-md entries, SHOULD match the description in the skill's - /// SKILL.md frontmatter. - /// - [JsonPropertyName("description")] - public string? Description { get; set; } - - /// - /// Gets or sets the artifact URL. Required. For skill-md, points at the - /// SKILL.md resource. For archive, points at the archive file. For - /// mcp-resource-template, an RFC 6570 URI template that resolves to a SKILL.md - /// resource URI. - /// - [JsonPropertyName("url")] - public string? Url { get; set; } - - /// - /// Gets or sets the SHA-256 digest of the artifact bytes, formatted as sha256: followed - /// by 64 lowercase hexadecimal characters. Required by the base v0.2.0 schema but optional - /// under this MCP index binding. Supplied archive digests are verified before extraction; - /// verification does not apply to skill-md entries. - /// - [JsonPropertyName("digest")] - public string? Digest { get; set; } -} diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsDiscoverySelectionTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsDiscoverySelectionTests.cs index bc2e54aea55..66fce6f2288 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsDiscoverySelectionTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsDiscoverySelectionTests.cs @@ -10,7 +10,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; /// -/// Tests selection between skills/list and skill://index.json based on the server's Skills extension. +/// Tests that skill discovery requires the server's Skills extension. /// public sealed class AgentMcpSkillsDiscoverySelectionTests { @@ -20,7 +20,7 @@ public sealed class AgentMcpSkillsDiscoverySelectionTests [InlineData("""{"extensions":{}}""")] [InlineData("""{"extensions":{"example.test/other":{}}}""")] [InlineData("""{"extensions":{"IO.MODELCONTEXTPROTOCOL/SKILLS":{}}}""")] - public async Task GetSkillsAsync_AbsentSkillsKey_ReadsIndexAsync(string capabilitiesJson) + public async Task GetSkillsAsync_AbsentSkillsKey_ReturnsEmptyWithoutRequestsAsync(string capabilitiesJson) { // Arrange List requests = []; @@ -34,7 +34,7 @@ public async Task GetSkillsAsync_AbsentSkillsKey_ReadsIndexAsync(string capabili // Assert Assert.Empty(skills); - Assert.Equal("skill://index.json", Assert.Single(requests)); + Assert.Empty(requests); } [Theory] diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs deleted file mode 100644 index f9754c34960..00000000000 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs +++ /dev/null @@ -1,1341 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.IO; -using System.IO.Compression; -using System.Linq; -using System.Security.Cryptography; -using System.Text; -using System.Text.Json.Nodes; -using System.Threading.Tasks; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using ModelContextProtocol.Protocol; -using ModelContextProtocol.Server; -using Moq; - -namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; - -/// -/// Unit tests for archive-distributed skill handling in . -/// -public sealed class AgentMcpSkillsSourceArchiveTests : IDisposable -{ - private const string ArchivedSkillMd = """ - --- - name: archived-skill - description: A skill delivered as an archive. - --- - # Archived Skill - - Body from the archive. - """; - - private const string StaleSkillMd = """ - --- - name: stale-skill - description: Left over from a previous session. - --- - Old content. - """; - - private const string SkillAMd = """ - --- - name: skill-a - description: Skill A. - --- - Content A. - """; - - private const string SkillBMd = """ - --- - name: skill-b - description: Skill B. - --- - Content B. - """; - - private readonly string _extractionRoot = - Path.Combine(Path.GetTempPath(), "af-mcp-archive-tests", Guid.NewGuid().ToString("N")); - - private const int ManyFileArchiveFileCount = 60; - - [Theory] - [InlineData("description: First\ndescription: >-\n Second", "\n")] - [InlineData("description: |-\n First\nDescription: Second", "\n")] - [InlineData("description: Valid\nmetadata:\n author: First\nmetadata:\n author: Second", "\n")] - [InlineData("description: Valid\nAllowed-Tools: read", "\n")] - [InlineData("description: Valid\nallowed-tools: read\n\"allowed-tools\": other", "\n")] - [InlineData("description: Valid\nallowed-tools: read\n\"allowed-tools\": other", "\r\n")] - [InlineData("description: Valid\n'allowed-tools': other\nallowed-tools: read", "\n")] - [InlineData("description: Valid\n'allowed-tools': other\nallowed-tools: read", "\r\n")] - [InlineData("description: Valid\n\"description\": Other text", "\n")] - [InlineData("'description': Other text\ndescription: Valid", "\r\n")] - [InlineData("description: Valid\n\"Allowed-Tools\": other", "\n")] - [InlineData("description: Valid\nmetadata:\n author: First\n\"metadata\":\n author: Second", "\r\n")] - [InlineData("description: Valid\nlicense: MIT\n'license':", "\n")] - [InlineData("description: Valid\ncompatibility: Any runtime\n\"compatibility\": Other runtime", "\r\n")] - [InlineData("description: Valid\n\"name\": archived-skill", "\n")] - public async Task GetSkillsAsync_AmbiguousArchiveFrontmatter_SkipsSkillAsync(string fields, string newline) - { - // Arrange - string content = $"---\nname: archived-skill\n{fields}\n---\nBody."; - byte[] archive = BuildZip(("SKILL.md", content.Replace("\n", newline))); - await using var server = CreateArchiveServer( - ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"), - new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Theory] - [InlineData("\n")] - [InlineData("\r\n")] - public async Task GetSkillsAsync_ArchiveValueOnIndentedNextLine_PreservedAsync(string newline) - { - // Arrange - const string Content = "---\nname: archived-skill\ndescription:\n 'Read files'\nlicense: MIT\n---\nBody."; - byte[] archive = BuildZip(("SKILL.md", Content.Replace("\n", newline))); - await using var server = CreateArchiveServer( - ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"), - new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - var frontmatter = Assert.Single(skills).Frontmatter; - Assert.Equal("Read files", frontmatter.Description); - Assert.Equal("MIT", frontmatter.License); - } - - [Theory] - [InlineData("license", "\n")] - [InlineData("license", "\r\n")] - [InlineData("compatibility", "\n")] - [InlineData("compatibility", "\r\n")] - [InlineData("allowed-tools", "\n")] - [InlineData("allowed-tools", "\r\n")] - public async Task GetSkillsAsync_ArchiveEmptyOptionalScalar_RemainsNullAsync(string field, string newline) - { - // Arrange - string content = $"---\nname: archived-skill\ndescription: Read files\n{field}: \t\n---\nBody."; - byte[] archive = BuildZip(("SKILL.md", content.Replace("\n", newline))); - await using var server = CreateArchiveServer( - ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"), - new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - var frontmatter = Assert.Single(skills).Frontmatter; - Assert.Equal("Read files", frontmatter.Description); - Assert.Null(frontmatter.License); - Assert.Null(frontmatter.Compatibility); - Assert.Null(frontmatter.AllowedTools); - } - - [Theory] - [InlineData("author", "\n", "")] - [InlineData("author", "\r\n", "")] - [InlineData("Author", "\n", "")] - [InlineData("Author", "\r\n", "")] - [InlineData("author", "\n", "'")] - [InlineData("author", "\r\n", "'")] - [InlineData("Author", "\n", "'")] - [InlineData("Author", "\r\n", "'")] - [InlineData("author", "\n", "\"")] - [InlineData("author", "\r\n", "\"")] - [InlineData("Author", "\n", "\"")] - [InlineData("Author", "\r\n", "\"")] - public async Task GetSkillsAsync_ArchiveDuplicateMetadata_KeepsFirstValueAndLogsWarningAsync( - string secondKey, string newline, string quote) - { - // Arrange - string content = - $"---\n{quote}name{quote}: archived-skill\n{quote}description{quote}: >-\n Read\n files\n" + - $"{quote}compatibility{quote}:\n Any runtime\n{quote}allowed-tools{quote}: 'read'\n{quote}metadata{quote}:\n" + - $" author:\n 'First'\n {secondKey}: Second\n author: Third\n version: '1.0'\n" + - $"{quote}license{quote}: |-\n MIT\n License\n---\nBody."; - byte[] archive = BuildZip(("SKILL.md", content.Replace("\n", newline))); - await using var server = CreateArchiveServer( - ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"), - new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - 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 source = new AgentMcpSkillsSource( - client, new() { ArchiveSkillsDirectory = this._extractionRoot }, loggerFactory.Object); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - var skill = Assert.Single(skills); - Assert.NotNull(skill.Frontmatter.Metadata); - Assert.Equal(2, skill.Frontmatter.Metadata.Count); - Assert.Equal("First", skill.Frontmatter.Metadata["author"]); - Assert.Equal("First", skill.Frontmatter.Metadata["Author"]); - Assert.Equal("1.0", skill.Frontmatter.Metadata["version"]); - Assert.Equal("Read files", skill.Frontmatter.Description); - Assert.Equal("MIT\nLicense", skill.Frontmatter.License); - Assert.Equal("Any runtime", skill.Frontmatter.Compatibility); - Assert.Equal("read", skill.Frontmatter.AllowedTools); - Assert.Contains("Body.", await skill.GetContentAsync()); - logger.Verify( - l => l.Log( - LogLevel.Warning, - It.IsAny(), - It.Is((state, _) => - state.ToString()!.Contains("duplicate metadata key", StringComparison.Ordinal) && - state.ToString()!.Contains("keeping the first value", StringComparison.Ordinal)), - It.IsAny(), - It.IsAny>()), - Times.Exactly(2)); - } - - [Theory] - [InlineData("First", "First", "\n")] - [InlineData("First", "First", "\r\n")] - [InlineData("\n 'First'", "First", "\n")] - [InlineData("\n 'First'", "First", "\r\n")] - [InlineData("|-\n First\n Second", "|-", "\n")] - [InlineData("|-\n First\n Second", "|-", "\r\n")] - [InlineData(">-\n First\n Second", ">-", "\n")] - [InlineData(">-\n First\n Second", ">-", "\r\n")] - public async Task GetSkillsAsync_ArchiveExistingMetadataScalarFormats_PreservedAsync(string value, string expected, string newline) - { - // Arrange - string content = "---\nname: archived-skill\ndescription: Read files\n" + - $"metadata:\n author: {value}\n version: '1.0'\n---\nBody."; - byte[] archive = BuildZip(("SKILL.md", content.Replace("\n", newline))); - await using var server = CreateArchiveServer( - ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"), - new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - preserve the existing metadata scalar representation. - var skill = Assert.Single(skills); - Assert.NotNull(skill.Frontmatter.Metadata); - Assert.Equal(2, skill.Frontmatter.Metadata.Count); - Assert.Equal(expected, skill.Frontmatter.Metadata["author"]); - Assert.Equal("1.0", skill.Frontmatter.Metadata["version"]); - Assert.Contains("Body.", await skill.GetContentAsync()); - } - - /// - /// Malformed, unsupported, and mismatched archive digests that must be rejected. - /// - public static TheoryData RejectedDigests => new() - { - "", - " ", - new string('0', 64), - "sha256:" + new string('a', 63), - "sha256:" + new string('a', 65), - "sha256:" + new string('g', 64), - "sha256:" + new string('A', 64), - "SHA256:" + new string('a', 64), - " sha256:" + new string('a', 64), - "sha256:" + new string('a', 64) + "\n", - "sha512:" + new string('a', 128), - "sha256:" + new string('0', 64), - }; - - [Theory] - [InlineData("omitted")] - [InlineData("null")] - [InlineData("matching")] - public async Task GetSkillsAsync_ArchiveDigest_LoadsValidArchiveAsync(string digestMode) - { - // Arrange - byte[] archive = BuildZip(("SKILL.md", ArchivedSkillMd), ("reference.md", "Verified resource.")); - string index = digestMode == "omitted" - ? ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip") - : ArchiveIndexWithDigest(digestMode == "matching" ? ArchiveDigest(archive) : null); - await using var server = CreateArchiveServer(index, new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - var skill = Assert.Single(skills); - Assert.Contains("Body from the archive.", await skill.GetContentAsync()); - var resource = await skill.GetResourceAsync("reference.md"); - Assert.NotNull(resource); - Assert.Equal("Verified resource.", await resource.ReadAsync()); - } - - [Theory] - [MemberData(nameof(RejectedDigests))] - public async Task GetSkillsAsync_RejectedDigest_SkipsBeforeExtractionAndLogsWarningAsync(string digest) - { - // Arrange - byte[] archive = BuildZip(("SKILL.md", ArchivedSkillMd)); - string index = ArchiveIndexWithDigest(digest); - await using var server = CreateArchiveServer(index, new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - 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 source = new AgentMcpSkillsSource( - client, new() { ArchiveSkillsDirectory = this._extractionRoot }, loggerFactory.Object); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - Assert.False(Directory.Exists(Path.Combine(this._extractionRoot, "archived-skill"))); - logger.Verify( - l => l.Log( - LogLevel.Warning, - It.IsAny(), - It.Is((state, _) => - state.ToString()!.Contains("Skipping archive skill 'archived-skill': digest", StringComparison.Ordinal) && - !state.ToString()!.Contains("skill://", StringComparison.Ordinal)), - It.IsAny(), - It.IsAny>()), - Times.Once); - } - - [Theory] - [InlineData("original")] - [InlineData("base64")] - [InlineData("skill-md")] - public async Task GetSkillsAsync_DigestOfDifferentBytes_SkipsArchiveAsync(string digestSource) - { - // Arrange - byte[] original = BuildZip(("SKILL.md", ArchivedSkillMd)); - string tamperedContent = ArchivedSkillMd.Replace("Body from the archive.", "Substituted instructions.", StringComparison.Ordinal); - byte[] archive = BuildZip(("SKILL.md", tamperedContent)); - byte[] digestBytes = digestSource switch - { - "original" => original, - "base64" => Encoding.UTF8.GetBytes(Convert.ToBase64String(archive)), - _ => Encoding.UTF8.GetBytes(tamperedContent), - }; - await using var server = CreateArchiveServer( - ArchiveIndexWithDigest(ArchiveDigest(digestBytes)), - new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - Assert.False(Directory.Exists(Path.Combine(this._extractionRoot, "archived-skill"))); - } - - [Theory] - [InlineData("")] - [InlineData("sha256:0000000000000000000000000000000000000000000000000000000000000000")] - public async Task GetSkillsAsync_RejectedDigest_KeepsValidSiblingAsync(string digest) - { - // Arrange - byte[] rejectedArchive = BuildZip(("SKILL.md", ArchivedSkillMd)); - byte[] validArchive = BuildZip(("SKILL.md", SkillAMd)); - JsonNode index = JsonNode.Parse(ArchiveIndexWithDigest(digest))!; - index["skills"]!.AsArray().Add(new JsonObject - { - ["name"] = "skill-a", - ["type"] = "archive", - ["description"] = "Skill A.", - ["url"] = "skill://archives/skill-a.zip", - ["digest"] = ArchiveDigest(validArchive), - }); - await using var server = CreateArchiveServer(index.ToJsonString(), new Dictionary - { - ["archived-skill"] = rejectedArchive, - ["skill-a"] = validArchive, - }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Equal("skill-a", Assert.Single(skills).Frontmatter.Name); - Assert.False(Directory.Exists(Path.Combine(this._extractionRoot, "archived-skill"))); - Assert.True(File.Exists(Path.Combine(this._extractionRoot, "skill-a", "SKILL.md"))); - } - - [Theory] - [InlineData("123")] - [InlineData("false")] - [InlineData("{}")] - [InlineData("[]")] - public async Task GetSkillsAsync_NonStringDigest_RejectsIndexAsync(string digestJson) - { - // Arrange - JsonNode index = JsonNode.Parse(ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"))!; - index["skills"]![0]!["digest"] = JsonNode.Parse(digestJson); - await using var server = CreateArchiveServer( - index.ToJsonString(), - new Dictionary { ["archived-skill"] = BuildZip(("SKILL.md", ArchivedSkillMd)) }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - Assert.False(Directory.Exists(this._extractionRoot)); - } - - [Theory] - [InlineData("invalid-zip")] - [InlineData("gzip")] - [InlineData("file-count")] - [InlineData("uncompressed-size")] - [InlineData("download-size")] - public async Task GetSkillsAsync_MatchingDigest_PreservesArchiveGuardsAsync(string failure) - { - // Arrange - byte[] archive = failure switch - { - "invalid-zip" => [0x50, 0x4B, 0x03, 0x04], - "gzip" => [0x1F, 0x8B], - _ => BuildZip(("SKILL.md", ArchivedSkillMd), ("reference.md", "Reference.")), - }; - await using var server = CreateArchiveServer( - ArchiveIndexWithDigest(ArchiveDigest(archive)), - new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() - { - ArchiveSkillsDirectory = this._extractionRoot, - ArchiveMaxFileCount = failure == "file-count" ? 1 : 20, - ArchiveMaxSizeBytes = failure == "download-size" ? 1 : null, - ArchiveMaxUncompressedSizeBytes = failure == "uncompressed-size" ? 1 : null, - }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - Assert.False(Directory.Exists(Path.Combine(this._extractionRoot, "archived-skill"))); - } - - [Fact] - public async Task GetSkillsAsync_MatchingDigest_SkipsTraversalEntryAsync() - { - // Arrange - byte[] archive = BuildZip(("SKILL.md", ArchivedSkillMd), ("../escape.md", "Outside skill.")); - await using var server = CreateArchiveServer( - ArchiveIndexWithDigest(ArchiveDigest(archive)), - new Dictionary { ["archived-skill"] = archive }); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Single(skills); - Assert.True(File.Exists(Path.Combine(this._extractionRoot, "archived-skill", "SKILL.md"))); - Assert.False(File.Exists(Path.Combine(this._extractionRoot, "escape.md"))); - } - - [Fact] - public async Task GetSkillsAsync_DigestMismatchOnRefresh_RemovesPreviousExtractionAsync() - { - // Arrange - byte[] original = BuildZip(("SKILL.md", ArchivedSkillMd)); - var archives = new Dictionary { ["archived-skill"] = original }; - await using var server = CreateArchiveServer(ArchiveIndexWithDigest(ArchiveDigest(original)), archives); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - var context = TestAgentSkillsSourceContextFactory.Create(); - Assert.Single(await source.GetSkillsAsync(context)); - Assert.True(File.Exists(Path.Combine(this._extractionRoot, "archived-skill", "SKILL.md"))); - archives["archived-skill"] = BuildZip(("SKILL.md", ArchivedSkillMd.Replace( - "Body from the archive.", "Substituted instructions.", StringComparison.Ordinal))); - - // Act - var skills = await source.GetSkillsAsync(context); - - // Assert - Assert.Empty(skills); - Assert.False(Directory.Exists(Path.Combine(this._extractionRoot, "archived-skill"))); - archives["archived-skill"] = original; - Assert.Single(await source.GetSkillsAsync(context)); - } - - [Fact] - public async Task GetSkillsAsync_ZipArchive_DiscoversSkillAsync() - { - // Arrange - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - var skill = Assert.Single(skills); - Assert.Equal("archived-skill", skill.Frontmatter.Name); - Assert.Equal("A skill delivered as an archive.", skill.Frontmatter.Description); - Assert.Contains("Body from the archive.", await skill.GetContentAsync()); - } - - [Fact] - public async Task GetSkillsAsync_TarGzArchive_SkipsSkillAsync() - { - // Arrange - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public void DetectFormat_TarSignals_ReturnUnknown() - { - // Arrange / Act / Assert - TAR signals never become a supported archive format, - // including when weaker metadata claims that gzip data is ZIP. - Assert.Equal( - ArchiveFormat.Unknown, - AgentMcpSkillArchiveExtractor.DetectFormat([0x1F, 0x8B], "application/zip", "skill://archive.zip")); - Assert.Equal( - ArchiveFormat.Unknown, - AgentMcpSkillArchiveExtractor.DetectFormat([], "application/x-tar", null)); - Assert.Equal( - ArchiveFormat.Unknown, - AgentMcpSkillArchiveExtractor.DetectFormat([], null, "skill://archive.tar")); - Assert.Equal( - ArchiveFormat.Unknown, - AgentMcpSkillArchiveExtractor.DetectFormat([], null, "skill://archive.tgz")); - } - - [Fact] - public async Task GetSkillsAsync_ArchiveWithScript_SurfacesScriptAsReadableResourceAsync() - { - // Arrange - archive bundles a script file alongside SKILL.md. Over MCP, scripts must never be - // executable; they are surfaced as readable resources only when explicitly included via options. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions - { - ArchiveSkillsDirectory = this._extractionRoot, - ArchiveResourceExtensions = [".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt", ".py"], - }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - var resource = await skill.GetResourceAsync("scripts/run.py"); - - // Assert - the .py file is readable as a resource (not an executable script). - Assert.NotNull(resource); - var content = await resource!.ReadAsync(); - Assert.Equal("print('hello')", content); - } - - [Fact] - public async Task GetSkillsAsync_TwoSourcesWithSeparateDirectories_DoNotCollideAsync() - { - // Arrange - two servers publish an archive skill with the SAME name but different content. - // Pointing each source at its own directory keeps their extracted content separate. - await using var serverA = new InMemoryMcpServer(builder => builder.WithResources()); - await using var clientA = await serverA.CreateClientAsync(); - await using var serverB = new InMemoryMcpServer(builder => builder.WithResources()); - await using var clientB = await serverB.CreateClientAsync(); - - var sourceA = new AgentMcpSkillsSource(clientA, new() { ArchiveSkillsDirectory = Path.Combine(this._extractionRoot, "a") }); - var sourceB = new AgentMcpSkillsSource(clientB, new() { ArchiveSkillsDirectory = Path.Combine(this._extractionRoot, "b") }); - - // Act - var skillA = Assert.Single(await sourceA.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - var skillB = Assert.Single(await sourceB.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - - // Assert - each source kept its own content despite the shared skill name. - Assert.Equal("shared-skill", skillA.Frontmatter.Name); - Assert.Equal("shared-skill", skillB.Frontmatter.Name); - Assert.Contains("Content from server A.", await skillA.GetContentAsync()); - Assert.Contains("Content from server B.", await skillB.GetContentAsync()); - } - - [Fact] - public async Task GetSkillsAsync_FirstRun_PrunesLeftoverSkillDirectoryAsync() - { - // Arrange - a leftover skill directory from a previous session sits in the provided directory. - string staleDir = Path.Combine(this._extractionRoot, "stale-skill"); - Directory.CreateDirectory(staleDir); - await File.WriteAllTextAsync(Path.Combine(staleDir, "SKILL.md"), StaleSkillMd); - - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - the leftover directory is pruned and only the advertised skill remains. - Assert.False(Directory.Exists(staleDir)); - var skill = Assert.Single(skills); - Assert.Equal("archived-skill", skill.Frontmatter.Name); - } - - [Fact] - public async Task GetSkillsAsync_SkillNoLongerAdvertised_IsPrunedAsync() - { - // Arrange - an earlier run extracts skill-a and skill-b into the shared directory. - await using var fullServer = new InMemoryMcpServer(builder => builder.WithResources()); - await using var fullClient = await fullServer.CreateClientAsync(); - var firstSource = new AgentMcpSkillsSource(fullClient, new() { ArchiveSkillsDirectory = this._extractionRoot }); - var firstSkills = await firstSource.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - Assert.Equal(2, firstSkills.Count); - Assert.True(Directory.Exists(Path.Combine(this._extractionRoot, "skill-b"))); - - // Act - a later run sees only skill-a. - await using var partialServer = new InMemoryMcpServer(builder => builder.WithResources()); - await using var partialClient = await partialServer.CreateClientAsync(); - var secondSource = new AgentMcpSkillsSource(partialClient, new() { ArchiveSkillsDirectory = this._extractionRoot }); - var secondSkills = await secondSource.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - skill-b's directory was pruned; only skill-a remains. - var skill = Assert.Single(secondSkills); - Assert.Equal("skill-a", skill.Frontmatter.Name); - Assert.False(Directory.Exists(Path.Combine(this._extractionRoot, "skill-b"))); - Assert.True(Directory.Exists(Path.Combine(this._extractionRoot, "skill-a"))); - } - - [Fact] - public async Task GetSkillsAsync_ServerListsNoArchives_PrunesLeftoversAsync() - { - // Arrange - a leftover skill directory exists but the server advertises no archive skills. - string staleDir = Path.Combine(this._extractionRoot, "stale-skill"); - Directory.CreateDirectory(staleDir); - await File.WriteAllTextAsync(Path.Combine(staleDir, "SKILL.md"), StaleSkillMd); - - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - the leftover directory is pruned and no skills are returned. - Assert.Empty(skills); - Assert.False(Directory.Exists(staleDir)); - } - - [Fact] - public async Task GetSkillsAsync_SecondDiscovery_ReExtractsContentAsync() - { - // Arrange - a first run extracts server A's content into the shared directory. - await using (var serverA = new InMemoryMcpServer(builder => builder.WithResources())) - await using (var clientA = await serverA.CreateClientAsync()) - { - var firstSource = new AgentMcpSkillsSource(clientA, new() { ArchiveSkillsDirectory = this._extractionRoot }); - Assert.Single(await firstSource.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - } - - // Act - a second run over the same directory re-extracts server B's content. - await using var serverB = new InMemoryMcpServer(builder => builder.WithResources()); - await using var clientB = await serverB.CreateClientAsync(); - var source = new AgentMcpSkillsSource(clientB, new() { ArchiveSkillsDirectory = this._extractionRoot }); - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - - // Assert - the content was replaced with server B's. - Assert.Contains("Content from server B.", await skill.GetContentAsync()); - } - - [Fact] - public async Task GetSkillsAsync_PreExtractionCleanupFails_DoesNotDiscoverStaleSkillAsync() - { - if (!OperatingSystem.IsWindows()) - { - return; - } - - // Arrange - a stale skill directory contains a locked file, causing recursive deletion to fail. - string skillDirectory = Path.Combine(this._extractionRoot, "shared-skill"); - Directory.CreateDirectory(skillDirectory); - await File.WriteAllTextAsync(Path.Combine(skillDirectory, "SKILL.md"), """ - --- - name: shared-skill - description: Shared. - --- - Stale content. - """); - - string lockedResourcePath = Path.Combine(skillDirectory, "locked.txt"); - await using var lockedResource = new FileStream(lockedResourcePath, FileMode.Create, FileAccess.ReadWrite, FileShare.None); - - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - failed cleanup prevents the stale directory from being proxied to AgentFileSkillsSource. - Assert.Empty(skills); - Assert.True(File.Exists(lockedResourcePath)); - } - - [Fact] - public void Extract_ZipSlipEntry_DoesNotEscapeTargetDirectory() - { - // Arrange - a malicious zip whose entry traverses out of the target directory. - byte[] zip = BuildZip(("../escaped.txt", "pwned"), ("SKILL.md", ArchivedSkillMd)); - string target = Path.Combine(this._extractionRoot, "skill"); - - // Act - AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target); - - // Assert - the traversal entry was skipped; only the safe entry was written. - Assert.False(File.Exists(Path.Combine(this._extractionRoot, "escaped.txt"))); - Assert.True(File.Exists(Path.Combine(target, "SKILL.md"))); - } - - [Fact] - public void Extract_CaseVariantZipSlipEntry_DoesNotEscapeTargetDirectory() - { - if (OperatingSystem.IsWindows()) - { - return; - } - - // Arrange - on case-sensitive file systems, "skill" is a sibling of "Skill", not the target. - byte[] zip = BuildZip(("../skill/escaped.txt", "pwned"), ("SKILL.md", ArchivedSkillMd)); - string target = Path.Combine(this._extractionRoot, "Skill"); - - // Act - AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target); - - // Assert - the case-variant traversal entry was skipped; only the safe entry was written. - Assert.False(File.Exists(Path.Combine(this._extractionRoot, "skill", "escaped.txt"))); - Assert.True(File.Exists(Path.Combine(target, "SKILL.md"))); - } - - [Fact] - public void Extract_ArchiveExceedsDefaultFileCount_Throws() - { - // Arrange - more files than the default cap. - var entries = Enumerable.Range(0, AgentMcpSkillArchiveExtractor.DefaultMaxFileCount + 1) - .Select(i => ($"file{i}.txt", "x")) - .ToArray(); - byte[] zip = BuildZip(entries); - string target = Path.Combine(this._extractionRoot, "skill"); - - // Act / Assert - Assert.Throws( - () => AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target)); - } - - [Fact] - public void Extract_ArchiveExceedsDefaultUncompressedSize_Throws() - { - // Arrange - a single file larger than the default uncompressed budget (1 MB). - string oversized = new('x', (int)AgentMcpSkillArchiveExtractor.DefaultMaxUncompressedSizeBytes + 1); - byte[] zip = BuildZip(("SKILL.md", oversized)); - string target = Path.Combine(this._extractionRoot, "skill"); - - // Act / Assert - Assert.Throws( - () => AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target)); - } - - [Fact] - public void Extract_UnknownFormat_ThrowsBeforeCreatingTarget() - { - // Arrange - string target = Path.Combine(this._extractionRoot, "skill"); - - // Act / Assert - var exception = Assert.Throws( - () => AgentMcpSkillArchiveExtractor.Extract([], ArchiveFormat.Unknown, target)); - Assert.Contains("Use ZIP instead", exception.Message); - Assert.False(Directory.Exists(target)); - } - - [Fact] - public void Extract_WithinDefaultLimits_Succeeds() - { - // Arrange - a typical small archive that is comfortably within the default limits. - byte[] zip = BuildZip(("SKILL.md", ArchivedSkillMd), ("reference.md", "Some reference content.")); - string target = Path.Combine(this._extractionRoot, "skill"); - - // Act - AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target); - - // Assert - Assert.True(File.Exists(Path.Combine(target, "SKILL.md"))); - Assert.True(File.Exists(Path.Combine(target, "reference.md"))); - } - - [Fact] - public void Extract_RaisedLimits_AllowsLargerArchive() - { - // Arrange - an archive that exceeds the default file count but fits within raised limits. - var entries = Enumerable.Range(0, AgentMcpSkillArchiveExtractor.DefaultMaxFileCount + 1) - .Select(i => ($"file{i}.txt", "x")) - .ToArray(); - byte[] zip = BuildZip(entries); - string target = Path.Combine(this._extractionRoot, "skill"); - - // Act - AgentMcpSkillArchiveExtractor.Extract( - zip, - ArchiveFormat.Zip, - target, - maxFileCount: entries.Length + 1); - - // Assert - every entry was extracted. - Assert.Equal(entries.Length, Directory.GetFiles(target).Length); - } - - [Fact] - public async Task GetSkillsAsync_ArchiveExceedsDefaultFileCount_SkillSkippedAsync() - { - // Arrange - the archive has more files than the default cap, so extraction fails and the skill is skipped. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_ArchiveExceedsConfiguredArchiveSize_SkillSkippedAsync() - { - // Arrange - the valid archive is larger than the configured downloaded-archive byte cap. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions - { - ArchiveSkillsDirectory = this._extractionRoot, - ArchiveMaxSizeBytes = 1, - }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_RaisedFileCountOption_LoadsLargerArchiveAsync() - { - // Arrange - raising ArchiveMaxFileCount lets the larger archive through. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions - { - ArchiveSkillsDirectory = this._extractionRoot, - ArchiveMaxFileCount = ManyFileArchiveFileCount + 1, - }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - - // Assert - Assert.Equal("archived-skill", skill.Frontmatter.Name); - } - - [Fact] - public async Task GetSkillsAsync_EntryMissingName_SkillSkippedAsync() - { - // Arrange - an archive index entry with a null/empty name is not actionable. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - the invalid entry is skipped; no skills are surfaced. - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_EntryWithInvalidNameChars_SkillSkippedAsync() - { - // Arrange - a name containing path separator characters is not a valid directory name. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_EntryMissingUrl_SkillSkippedAsync() - { - // Arrange - an archive entry without a url cannot be downloaded. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_ArchiveWithUnsupportedFormat_SkillSkippedAsync() - { - // Arrange - the archive payload does not match any known format (no magic bytes, no matching - // media type, no recognized URL extension). - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_ArchiveReturnsTextNotBlob_SkillSkippedAsync() - { - // Arrange - the archive resource returns text content instead of a binary blob. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_ConcurrentCalls_ReconcilesArchiveDirectorySafelyAsync() - { - // Arrange - a fixed extraction directory means every concurrent call reconciles the same - // on-disk location. The per-instance lock must serialize that reconcile/extract/read work so - // that no call observes a half-extracted or mid-prune directory. - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - var context = TestAgentSkillsSourceContextFactory.Create(); - - // Act - hammer the source from many threads at once. - var tasks = Enumerable.Range(0, 20) - .Select(_ => Task.Run(() => source.GetSkillsAsync(context))) - .ToArray(); - var results = await Task.WhenAll(tasks); - - // Assert - every call returns both skills with intact content. - foreach (var skills in results) - { - Assert.Equal(2, skills.Count); - - var byName = skills.ToDictionary(s => s.Frontmatter.Name); - Assert.Contains("Content A.", await byName["skill-a"].GetContentAsync()); - Assert.Contains("Content B.", await byName["skill-b"].GetContentAsync()); - } - } - - [Fact] - public async Task GetSkillsAsync_ArchiveUpdatedBetweenCalls_ReturnsUpdatedContentAsync() - { - // Arrange - a mutable server whose archive body the test can change to simulate the skill - // being republished. Because the loader re-extracts under its reconcile lock on every call, - // a query issued after the update (and any calls queued behind the lock) must observe the - // new content, never a stale pre-update snapshot. - MutableSkillServer.Body = "Version one."; - await using var server = new InMemoryMcpServer(builder => builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; - var source = new AgentMcpSkillsSource(client, options); - var context = TestAgentSkillsSourceContextFactory.Create(); - - // Act - the initial load observes the original content. - var before = await source.GetSkillsAsync(context); - - // The skill is republished with new content. - MutableSkillServer.Body = "Version two."; - - // Fire many post-update calls at once; they serialize behind the reconcile lock. - var tasks = Enumerable.Range(0, 10) - .Select(_ => Task.Run(() => source.GetSkillsAsync(context))) - .ToArray(); - var afterResults = await Task.WhenAll(tasks); - - // Assert - the first load saw the old content. - Assert.Contains("Version one.", await Assert.Single(before).GetContentAsync()); - - // Every post-update call sees the new content and not the stale pre-update body. - foreach (var skills in afterResults) - { - var content = await Assert.Single(skills).GetContentAsync(); - Assert.Contains("Version two.", content); - Assert.DoesNotContain("Version one.", content); - } - } - - public void Dispose() - { - try - { - if (Directory.Exists(this._extractionRoot)) - { - Directory.Delete(this._extractionRoot, recursive: true); - } - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - // Best-effort cleanup. - } - } - - private static byte[] BuildZip(params (string Path, string Content)[] entries) - { - using var ms = new MemoryStream(); - using (var archive = new ZipArchive(ms, ZipArchiveMode.Create, leaveOpen: true)) - { - foreach (var (path, content) in entries) - { - ZipArchiveEntry entry = archive.CreateEntry(path); - using Stream stream = entry.Open(); - byte[] bytes = Encoding.UTF8.GetBytes(content); - stream.Write(bytes, 0, bytes.Length); - } - } - - return ms.ToArray(); - } - - [SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "Archive digests require lowercase hexadecimal characters.")] - private static string ArchiveDigest(byte[] archive) => - "sha256:" + Convert.ToHexString(SHA256.HashData(archive)).ToLowerInvariant(); - - private static string ArchiveIndexWithDigest(string? digest) - { - JsonNode index = JsonNode.Parse(ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"))!; - index["skills"]![0]!["digest"] = digest; - return index.ToJsonString(); - } - - private static InMemoryMcpServer CreateArchiveServer(string index, IReadOnlyDictionary archives) => - new(builder => builder.WithResources(new DigestArchiveServer(index, archives))); - - private static string ArchiveIndex(string skillName, string url) => $$""" - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "{{skillName}}", - "type": "archive", - "description": "A skill delivered as an archive.", - "url": "{{url}}" - } - ] - } - """; - - #region Resource classes (registered with the MCP server via WithResources) - -#pragma warning disable CA1812 - - [McpServerResourceType] - private sealed class DigestArchiveServer(string index, IReadOnlyDictionary archives) - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public string Index() => index; - - [McpServerResource(UriTemplate = "skill://archives/{skillName}.zip", Name = "archive", MimeType = "application/zip")] - public BlobResourceContents Archive(string skillName) => BlobResourceContents.FromBytes( - archives[skillName], $"skill://archives/{skillName}.zip", "application/zip"); - } - - [McpServerResourceType] - private sealed class ZipArchiveServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"); - - [McpServerResource(UriTemplate = "skill://archives/archived-skill.zip", Name = "archive", MimeType = "application/zip")] - public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( - BuildZip(("SKILL.md", ArchivedSkillMd)), - "skill://archives/archived-skill.zip", - "application/zip"); - } - - [McpServerResourceType] - private sealed class TarGzArchiveServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.tar.gz"); - - [McpServerResource(UriTemplate = "skill://archives/archived-skill.tar.gz", Name = "archive", MimeType = "application/gzip")] - public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( - new byte[] { 0x1F, 0x8B }, - "skill://archives/archived-skill.tar.gz", - "application/gzip"); - } - - [McpServerResourceType] - private sealed class ZipWithScriptServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"); - - [McpServerResource(UriTemplate = "skill://archives/archived-skill.zip", Name = "archive", MimeType = "application/zip")] - public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( - BuildZip(("SKILL.md", ArchivedSkillMd), ("scripts/run.py", "print('hello')")), - "skill://archives/archived-skill.zip", - "application/zip"); - } - - [McpServerResourceType] - private sealed class ManyFileArchiveServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"); - - [McpServerResource(UriTemplate = "skill://archives/archived-skill.zip", Name = "archive", MimeType = "application/zip")] - public static BlobResourceContents Archive() - { - var entries = new List<(string Path, string Content)> { ("SKILL.md", ArchivedSkillMd) }; - for (int i = 0; i < ManyFileArchiveFileCount - 1; i++) - { - entries.Add(($"reference/file{i}.txt", "x")); - } - - return BlobResourceContents.FromBytes( - BuildZip(entries.ToArray()), - "skill://archives/archived-skill.zip", - "application/zip"); - } - } - - [McpServerResourceType] - private sealed class SharedNameServerA - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("shared-skill", "skill://archives/shared-skill.zip"); - - [McpServerResource(UriTemplate = "skill://archives/shared-skill.zip", Name = "archive", MimeType = "application/zip")] - public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( - BuildZip(("SKILL.md", """ - --- - name: shared-skill - description: Shared. - --- - Content from server A. - """)), - "skill://archives/shared-skill.zip", - "application/zip"); - } - - [McpServerResourceType] - private sealed class SharedNameServerB - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("shared-skill", "skill://archives/shared-skill.zip"); - - [McpServerResource(UriTemplate = "skill://archives/shared-skill.zip", Name = "archive", MimeType = "application/zip")] - public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( - BuildZip(("SKILL.md", """ - --- - name: shared-skill - description: Shared. - --- - Content from server B. - """)), - "skill://archives/shared-skill.zip", - "application/zip"); - } - - [McpServerResourceType] - private sealed class TwoSkillServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { "name": "skill-a", "type": "archive", "description": "Skill A.", "url": "skill://archives/skill-a.zip" }, - { "name": "skill-b", "type": "archive", "description": "Skill B.", "url": "skill://archives/skill-b.zip" } - ] - } - """; - - [McpServerResource(UriTemplate = "skill://archives/skill-a.zip", Name = "skill-a", MimeType = "application/zip")] - public static BlobResourceContents SkillA() => BlobResourceContents.FromBytes( - BuildZip(("SKILL.md", SkillAMd)), "skill://archives/skill-a.zip", "application/zip"); - - [McpServerResource(UriTemplate = "skill://archives/skill-b.zip", Name = "skill-b", MimeType = "application/zip")] - public static BlobResourceContents SkillB() => BlobResourceContents.FromBytes( - BuildZip(("SKILL.md", SkillBMd)), "skill://archives/skill-b.zip", "application/zip"); - } - - [McpServerResourceType] - private sealed class MutableSkillServer - { - // Toggled by the test to simulate the server republishing the skill with new content. - public static volatile string Body = "Version one."; - - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"); - - [McpServerResource(UriTemplate = "skill://archives/archived-skill.zip", Name = "archive", MimeType = "application/zip")] - public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( - BuildZip(("SKILL.md", $$""" - --- - name: archived-skill - description: A skill delivered as an archive. - --- - {{Body}} - """)), - "skill://archives/archived-skill.zip", - "application/zip"); - } - - [McpServerResourceType] - private sealed class OneSkillServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("skill-a", "skill://archives/skill-a.zip"); - - [McpServerResource(UriTemplate = "skill://archives/skill-a.zip", Name = "skill-a", MimeType = "application/zip")] - public static BlobResourceContents SkillA() => BlobResourceContents.FromBytes( - BuildZip(("SKILL.md", SkillAMd)), "skill://archives/skill-a.zip", "application/zip"); - } - - [McpServerResourceType] - private sealed class NoArchiveServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [] - } - """; - } - - [McpServerResourceType] - private sealed class MissingNameServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { "name": "", "type": "archive", "description": "No name.", "url": "skill://archives/x.zip" } - ] - } - """; - } - - [McpServerResourceType] - private sealed class InvalidNameCharsServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { "name": "../escape", "type": "archive", "description": "Bad name.", "url": "skill://archives/x.zip" } - ] - } - """; - } - - [McpServerResourceType] - private sealed class MissingUrlServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { "name": "good-name", "type": "archive", "description": "No url.", "url": "" } - ] - } - """; - } - - [McpServerResourceType] - private sealed class UnsupportedFormatServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("bad-format", "skill://archives/bad-format.bin"); - - [McpServerResource(UriTemplate = "skill://archives/bad-format.bin", Name = "archive", MimeType = "application/octet-stream")] - public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( - Encoding.UTF8.GetBytes("not an archive"), - "skill://archives/bad-format.bin", - "application/octet-stream"); - } - - [McpServerResourceType] - private sealed class TextOnlyArchiveServer - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => ArchiveIndex("text-skill", "skill://archives/text-skill.zip"); - - [McpServerResource(UriTemplate = "skill://archives/text-skill.zip", Name = "archive", MimeType = "application/zip")] - public static string Archive() => "this is not binary"; - } - -#pragma warning restore CA1812 - - #endregion -} diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceTests.cs deleted file mode 100644 index a43bb96b986..00000000000 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceTests.cs +++ /dev/null @@ -1,663 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System.Collections.Generic; -using System.Linq; -using System.Text.Json; -using System.Threading.Tasks; -using Microsoft.Extensions.AI; -using Microsoft.Extensions.DependencyInjection; -using ModelContextProtocol.Protocol; -using ModelContextProtocol.Server; - -namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; - -/// -/// Unit tests for . -/// -public sealed class AgentMcpSkillsSourceTests -{ - private const string SampleSkillMd = """ - --- - name: unit-converter - description: Convert between common units. - --- - # Unit Converter - - Body content here. - """; - - private const string SampleSkillIndex = """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "unit-converter", - "type": "skill-md", - "description": "Convert between common units.", - "url": "skill://unit-converter/SKILL.md" - } - ] - } - """; - - [Fact] - public async Task GetSkillsAsync_IndexBasedDiscovery_ReturnsSkillAsync() - { - // Arrange - server exposes both skill://index.json and the skill itself. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - frontmatter comes from index; Content is the actual SKILL.md body from the server. - var skill = Assert.Single(skills); - Assert.Equal("unit-converter", skill.Frontmatter.Name); - Assert.Equal("Convert between common units.", skill.Frontmatter.Description); - - string content = await skill.GetContentAsync(); - Assert.Contains("name: unit-converter", content); - Assert.Contains("description: Convert between common units.", content); - Assert.Contains("Body content here.", content); - } - - [Fact] - public async Task GetSkillsAsync_NoIndex_ReturnsEmptyAsync() - { - // Arrange - server only exposes SKILL.md, no skill://index.json. - // Per SEP-2640, discovery requires the index document; without it, no skills are surfaced. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetResourceAsync_SiblingText_ReturnsContentAsync() - { - // Arrange - server exposes index, SKILL.md, and a sibling reference file. - // The skill reads the sibling on demand via GetResourceAsync. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - var resource = await skill.GetResourceAsync("references/checklist.md"); - - // Assert - Assert.NotNull(resource); - var content = await resource!.ReadAsync(); - Assert.Equal("- check thing 1\n- check thing 2", content); - } - - [Fact] - public async Task GetResourceAsync_SiblingBinary_ReturnsDataContentAsync() - { - // Arrange - server exposes index, SKILL.md, and a binary sibling. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - var resource = await skill.GetResourceAsync("assets/icon.bin"); - - // Assert - Assert.NotNull(resource); - var content = await resource!.ReadAsync(); - var dataContent = Assert.IsType(content); - Assert.Equal("application/octet-stream", dataContent.MediaType); - Assert.Equal([0x01, 0x02, 0x03, 0x04], dataContent.Data.ToArray()); - } - - [Fact] - public async Task GetResourceAsync_UnknownName_ReturnsNullAsync() - { - // Arrange - index advertises a skill, but no sibling resource exists. - // GetResourceAsync eagerly fetches from the MCP server; a non-existent - // resource causes the server to return an error, so null is returned. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - var resource = await skill.GetResourceAsync("references/does-not-exist.md"); - - // Assert - resource does not exist on the server, so null is returned - Assert.Null(resource); - } - - [Theory] - [InlineData("../escape.md")] - [InlineData("references/../../escape.md")] - [InlineData("..")] - [InlineData("..\\escape.md")] - [InlineData("/etc/passwd")] - [InlineData("http://example.com/other")] - [InlineData("%2e%2e/escape.md")] - [InlineData("%2E./escape.md")] - [InlineData(".%2e/escape.md")] - [InlineData("references/%2e%2e/escape.md")] - [InlineData("references%2f..%2f..%2fescape.md")] - [InlineData("%2e%2e%5cescape.md")] - [InlineData("%252e%252e%252fescape.md")] - [InlineData("%25252e%25252e/escape.md")] - [InlineData("%2fescape.md")] - [InlineData("%5cescape.md")] - [InlineData("%68ttp%3a%2f%2fexample.com/other")] - [InlineData("..?download=1")] - [InlineData("..#fragment")] - [InlineData("%2e%2e%3fdownload=1")] - [InlineData("references%3f/../../escape.md")] - [InlineData("references%3f/%2e%2e/%2e%2e/escape.md")] - [InlineData("references%23/%2e%2e/%2e%2e/escape.md")] - [InlineData("references%3f%2f%2e%2e%2f%2e%2e%2fescape.md")] - [InlineData("references%23%5c%2e%2e%5c%2e%2e%5cescape.md")] - [InlineData("references%253f%252f%252e%252e%252f%252e%252e%252fescape.md")] - [InlineData("references%2523%252f%252e%252e%252f%252e%252e%252fescape.md")] - [InlineData("references%3f/%252e%252e/%252e%252e/escape.md")] - [InlineData("references%3f/%2e%2e/%2e%2e/escape.md?version=1")] - [InlineData("references%23/%2e%2e/%2e%2e/escape.md#section")] - [InlineData("references%3f%2f%2e%2e%20")] - [InlineData(".\t./escape.md")] - [InlineData(".%09./escape.md")] - [InlineData("references/\0/guide.md")] - [InlineData(".. ")] - [InlineData(".%2e ")] - [InlineData("%2e%2e ")] - [InlineData("..%20")] - [InlineData("%252e%252e%2520")] - [InlineData("references/.. ")] - [InlineData("references/.. ?version=1")] - [InlineData("references/guide.md?value=%00")] - [InlineData("references/guide.md#value=%2509")] - [InlineData("references/guide.md?value=%C2%85")] - [InlineData("references/%2500guide.md?version=1")] - [InlineData("references/guide.md?version=1#value=%2509")] - [InlineData("references/guide.md#section?value=%2509")] - public async Task GetResourceAsync_PathTraversalName_ReturnsNullAsync(string name) - { - foreach (string root in new[] - { - "skill://unit-converter/", - "skill://unit-converter/private/", - "https://example.com/skills/private/", - "file:///skills/private/", - "custom:skills/private/" - }) - { - // Arrange - accept every URI so rejection must happen before the MCP request. - List reads = []; - await using var server = CreatePermissiveSkillServer(root + "SKILL.md", reads); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - reads.Clear(); - - // Act - var resource = await skill.GetResourceAsync(name); - - // Assert - Assert.Null(resource); - Assert.Empty(reads); - } - } - - [Theory] - [InlineData("skill://unit-converter/")] - [InlineData("skill://unit-converter/private/")] - [InlineData("https://example.com/skills/private/")] - [InlineData("file:///skills/private/")] - [InlineData("custom:skills/private/")] - public async Task GetResourceAsync_SafeNamesAndSchemes_ArePreservedAsync(string root) - { - // Arrange - List reads = []; - await using var server = CreatePermissiveSkillServer(root + "SKILL.md", reads); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - - // Act - Assert.Equal(SampleSkillMd, await skill.GetContentAsync()); - - // Assert - Assert.Equal(root + "SKILL.md", reads[1]); - foreach (string name in new[] - { - "references/guide.md", - "references\\guide.md", - "references/guide%20one.md", - "references/v1.2/guide.md", - "references/%2520.md", - "references/100%.md", - "references/guide.md?version=1#section", - "references/guide%3fname.md", - "references/guide%23name.md", - "references/guide%253fname.md", - "references/guide.md?example=/../../other.md", - "references/guide.md#example=/../../other.md", - "references/guide.md?example=%2e%2e%2f%2e%2e%2fother.md", - "references/guide.md?src=https://example.com/other", - "references/guide%2520one.md?value=%2520#section%2520", - "references/guide%253fname.md#section?value=%2520", - "references/guide.md?", - "references/guide.md#", - "references/guide.md " - }) - { - reads.Clear(); - var resource = await skill.GetResourceAsync(name); - Assert.NotNull(resource); - Assert.Equal(name, resource.Name); - Assert.Equal("safe content", await resource.ReadAsync()); - Assert.Equal(root + name.Replace('\\', '/'), Assert.Single(reads)); - } - } - - [Theory] - [InlineData(1)] - [InlineData(31)] - [InlineData(32)] - [InlineData(33)] - [InlineData(4096)] - public async Task GetResourceAsync_DecodingDepthIsBoundedAsync(int depth) - { - // Arrange - const string Root = "skill://unit-converter/private/"; - List reads = []; - await using var server = CreatePermissiveSkillServer(Root + "SKILL.md", reads); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - string encoded = "%" + string.Concat(Enumerable.Repeat("25", depth - 1)) + "41"; - - foreach (string name in new[] - { - $"references/{encoded}.md", - $"references/guide.md?value={encoded}", - $"references/guide.md#value={encoded}", - $"../{encoded}.md" - }) - { - reads.Clear(); - - // Act - var resource = await skill.GetResourceAsync(name); - - // Assert - if (depth <= 32 && !name.StartsWith("../", System.StringComparison.Ordinal)) - { - Assert.NotNull(resource); - Assert.Equal(name, resource.Name); - Assert.Equal(Root + name, Assert.Single(reads)); - } - else - { - Assert.Null(resource); - Assert.Empty(reads); - } - } - } - - [Fact] - public async Task GetSkillsAsync_DoesNotReadSkillMdAsync() - { - // Arrange - index points to a non-existent SKILL.md URI. Because the source builds - // skills from index info only, discovery still succeeds. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - discovery succeeds from index alone. - var skill = Assert.Single(skills); - Assert.Equal("unit-converter", skill.Frontmatter.Name); - } - - [Fact] - public async Task GetSkillsAsync_IndexEntryWithInvalidName_IsSkippedAsync() - { - // Arrange - index entry has an invalid (uppercase) name, which AgentSkillFrontmatter rejects. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_IndexEntryWithMissingRequiredFields_IsSkippedAsync() - { - // Arrange - index entry is missing the required description and url fields. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_ArchiveEntryWithUnreadableResource_IsSkippedAsync() - { - // Arrange - index has an "archive" entry, but the referenced archive resource does not - // exist on the server, so reading it fails and the entry is skipped gracefully. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetSkillsAsync_IndexEntryWithTemplateType_IsSkippedAsync() - { - // Arrange - index has an "mcp-resource-template" entry (parameterized skill namespace). - // The current source skips template entries; they require user input to materialize. - await using var server = new InMemoryMcpServer(builder => - builder.WithResources()); - await using var client = await server.CreateClientAsync(); - var source = new AgentMcpSkillsSource(client); - - // Act - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - - // Assert - Assert.Empty(skills); - } - - [Fact] - public async Task GetContentAsync_IndexResponse_CombinesAllTextBlocksAsync() - { - // Arrange - byte[] bytes = [0xff]; - await using var server = new InMemoryMcpServer(builder => builder.WithReadResourceHandler((request, _) => - { - string uri = request.Params!.Uri; - return ValueTask.FromResult(new ReadResourceResult - { - Contents = uri == "skill://index.json" - ? [new TextResourceContents { Uri = uri, Text = SampleSkillIndex }] - : [ - new TextResourceContents { Uri = "skill://other/first.md", Text = "First." }, - new TextResourceContents { Uri = uri, Text = "Second." }, - BlobResourceContents.FromBytes(bytes, uri, "text/markdown"), - ], - }); - })); - await using var client = await server.CreateClientAsync(); - using var source = new AgentMcpSkillsSource(client); - var skill = Assert.IsType(Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); - - // Act - string content = await skill.GetContentAsync(); - - // Assert - Assert.Equal("First.\nSecond.", content); - } - - [Theory] - [InlineData("empty")] - [InlineData("text")] - [InlineData("binary")] - public async Task GetResourceAsync_IndexResponse_PreservesCompatibilityAsync(string kind) - { - // Arrange - byte[] bytes = [1, 2, 3, 4]; - await using var server = new InMemoryMcpServer(builder => builder.WithReadResourceHandler((request, _) => - { - string uri = request.Params!.Uri; - if (uri == "skill://index.json") - { - return ValueTask.FromResult(new ReadResourceResult - { - Contents = [new TextResourceContents { Uri = uri, Text = SampleSkillIndex }], - }); - } - - return ValueTask.FromResult(new ReadResourceResult - { - Contents = kind switch - { - "empty" => [new TextResourceContents { Uri = uri, Text = "" }], - "binary" => - [ - new TextResourceContents { Uri = uri, Text = "Ignored text." }, - BlobResourceContents.FromBytes(bytes, "skill://other/icon.bin", "application/octet-stream"), - ], - _ => - [ - new TextResourceContents { Uri = "skill://other/first.md", Text = "First." }, - new TextResourceContents { Uri = uri, Text = "Second." }, - ], - }, - }); - })); - await using var client = await server.CreateClientAsync(); - using var source = new AgentMcpSkillsSource(client); - var skill = Assert.IsType(Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); - - // Act - var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); - var content = await resource.ReadAsync(); - - // Assert - if (kind == "empty") - { - Assert.Null(content); - } - else if (kind == "binary") - { - Assert.Equal(bytes, Assert.IsType(content).Data.ToArray()); - } - else - { - Assert.Equal("First.\nSecond.", content); - } - } - - private static InMemoryMcpServer CreatePermissiveSkillServer(string skillMdUri, List reads) - { - string index = JsonSerializer.Serialize(new - { - skills = new[] - { - new { name = "unit-converter", type = "skill-md", description = "Convert between common units.", url = skillMdUri } - } - }); - return new InMemoryMcpServer(builder => builder.WithReadResourceHandler((request, cancellationToken) => - { - string uri = request.Params!.Uri; - reads.Add(uri); - return ValueTask.FromResult(new ReadResourceResult - { - Contents = [new TextResourceContents - { - Uri = uri, - Text = uri == "skill://index.json" ? index : uri == skillMdUri ? SampleSkillMd : "safe content", - MimeType = "text/plain" - }] - }); - })); - } - - #region Resource classes (registered with the MCP server via WithResources) - - // CA1812 flags these classes as "never instantiated", which is technically correct - - // they are never constructed because they only contain static methods (e.g. `public static string Index()`). - // The MCP framework discovers and invokes these static methods via the [McpServerResourceType] and - // [McpServerResource] attributes registered through WithResources(), without ever creating an instance. -#pragma warning disable CA1812 - - /// - /// Server type that exposes both skill://index.json and a single skill-md resource. - /// - [McpServerResourceType] - private sealed class IndexAndSkill - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => SampleSkillIndex; - - [McpServerResource(UriTemplate = "skill://unit-converter/SKILL.md", Name = "unit-converter", MimeType = "text/markdown")] - public static string Skill() => SampleSkillMd; - } - - /// Server type that exposes only SKILL.md (no index, no siblings). - [McpServerResourceType] - private sealed class SkillOnly - { - [McpServerResource(UriTemplate = "skill://unit-converter/SKILL.md", Name = "unit-converter", MimeType = "text/markdown")] - public static string Skill() => SampleSkillMd; - } - - /// Server type that exposes skill://index.json, SKILL.md, and one text sibling. - [McpServerResourceType] - private sealed class IndexAndSkillWithSibling - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => SampleSkillIndex; - - [McpServerResource(UriTemplate = "skill://unit-converter/SKILL.md", Name = "unit-converter", MimeType = "text/markdown")] - public static string Skill() => SampleSkillMd; - - [McpServerResource(UriTemplate = "skill://unit-converter/references/checklist.md", Name = "checklist", MimeType = "text/markdown")] - public static string Checklist() => "- check thing 1\n- check thing 2"; - } - - /// Server type that exposes skill://index.json, SKILL.md, and one binary sibling. - [McpServerResourceType] - private sealed class IndexAndSkillWithBinarySibling - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => SampleSkillIndex; - - [McpServerResource(UriTemplate = "skill://unit-converter/SKILL.md", Name = "unit-converter", MimeType = "text/markdown")] - public static string Skill() => SampleSkillMd; - - [McpServerResource(UriTemplate = "skill://unit-converter/assets/icon.bin", Name = "icon", MimeType = "application/octet-stream")] - public static BlobResourceContents Icon() => BlobResourceContents.FromBytes( - new byte[] { 0x01, 0x02, 0x03, 0x04 }, - "skill://unit-converter/assets/icon.bin", - "application/octet-stream"); - } - - /// Server type that exposes only the index (no concrete SKILL.md resource). - [McpServerResourceType] - private sealed class IndexWithoutSkillMdResource - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => SampleSkillIndex; - } - - /// Server type whose index entry has an invalid (uppercase) name. - [McpServerResourceType] - private sealed class IndexWithInvalidName - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "UnitConverter", - "type": "skill-md", - "description": "Convert between common units.", - "url": "skill://UnitConverter/SKILL.md" - } - ] - } - """; - } - - /// Server type whose index entry is missing required fields (description, url). - [McpServerResourceType] - private sealed class IndexWithIncompleteEntry - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "unit-converter", - "type": "skill-md" - } - ] - } - """; - } - - /// Server type whose index references only an archive entry. - [McpServerResourceType] - private sealed class IndexWithArchiveOnly - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "some-skill", - "type": "archive", - "description": "Packaged skill.", - "url": "skill://some-skill.tar.gz" - } - ] - } - """; - } - - /// Server type whose index references only an mcp-resource-template entry. - [McpServerResourceType] - private sealed class IndexWithTemplateOnly - { - [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] - public static string Index() => """ - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "type": "mcp-resource-template", - "description": "Per-product documentation skill", - "url": "skill://docs/{product}/SKILL.md" - } - ] - } - """; - } - -#pragma warning restore CA1812 - - #endregion -} diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.cs index 71f3bf550a8..49f51e06e40 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.IO; using System.Security.Cryptography; using System.Text; using System.Text.Json; @@ -233,7 +232,7 @@ public async Task GetSkillsAsync_InvalidLaterPage_ThrowsInsteadOfReturningPartia } [Fact] - public async Task GetSkillsAsync_LaterPageError_PropagatesWithoutReadingIndexAsync() + public async Task GetSkillsAsync_LaterPageError_PropagatesWithoutReadingResourcesAsync() { // Arrange int calls = 0; @@ -530,7 +529,7 @@ public async Task GetSkillsAsync_SingleListRequest_DoesNotReadResourcesAsync(boo } [Fact] - public async Task GetSkillsAsync_ListError_DoesNotReadIndexAsync() + public async Task GetSkillsAsync_ListError_DoesNotReadResourcesAsync() { // Arrange int calls = 0; @@ -542,7 +541,7 @@ public async Task GetSkillsAsync_ListError_DoesNotReadIndexAsync() }, (uri, _) => { reads.Add(uri); - return ValueTask.FromResult(Text(uri, "Index must not be read.")); + return ValueTask.FromResult(Text(uri, "Resource must not be read.")); }); await using var client = await server.CreateClientAsync(); using var source = new AgentMcpSkillsSource(client); @@ -596,17 +595,29 @@ public async Task GetSkillsAsync_ListCancellation_PropagatesAsync(bool laterPage } } - [Fact] - public async Task GetSkillsAsync_AlreadyCancelled_SendsNoRequestAsync() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_AlreadyCancelled_SendsNoRequestAsync(bool declaresSkills) { // Arrange int calls = 0; + List reads = []; await using var server = CreateServer((_, _) => { calls++; return ValueTask.FromResult(ListResult()); + }, (uri, _) => + { + reads.Add(uri); + return ValueTask.FromResult(Text(uri, "Must not be read.")); }); await using var client = await server.CreateClientAsync(); + if (!declaresSkills) + { + client.ServerCapabilities.Extensions = null; + } + using var source = new AgentMcpSkillsSource(client); using var cts = new CancellationTokenSource(); await cts.CancelAsync(); @@ -614,12 +625,13 @@ public async Task GetSkillsAsync_AlreadyCancelled_SendsNoRequestAsync() // Act & Assert await Assert.ThrowsAnyAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), cts.Token)); Assert.Equal(0, calls); + Assert.Empty(reads); } [Theory] [InlineData(false)] [InlineData(true)] - public async Task GetSkillsAsync_NoDeclaration_ReadsIndexInsteadOfListAsync(bool smallResourceLimit) + public async Task GetSkillsAsync_NoDeclaration_ReturnsEmptyWithoutRequestsAsync(bool smallResourceLimit) { // Arrange int calls = 0; @@ -644,15 +656,15 @@ public async Task GetSkillsAsync_NoDeclaration_ReadsIndexInsteadOfListAsync(bool var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); // Assert - Assert.Equal("older-skill", Assert.Single(skills).Frontmatter.Name); - Assert.Equal("skill://index.json", Assert.Single(reads)); + Assert.Empty(skills); + Assert.Empty(reads); Assert.Equal(0, calls); } [Theory] [InlineData("{}")] [InlineData("null")] - public async Task GetSkillsAsync_DeclaredButMissingList_DoesNotReadIndexAsync(string declaration) + public async Task GetSkillsAsync_DeclaredButMissingList_DoesNotReadResourcesAsync(string declaration) { // Arrange List reads = []; @@ -677,47 +689,34 @@ public async Task GetSkillsAsync_DeclaredButMissingList_DoesNotReadIndexAsync(st [Theory] [InlineData(false)] [InlineData(true)] - public async Task GetSkillsAsync_EmptyOrUnsupportedDynamicList_DoesNotReconcileArchivesAsync(bool dynamic) + public async Task GetSkillsAsync_EmptyOrUnsupportedDynamicList_DoesNotReadResourcesAsync(bool dynamic) { // Arrange - string directory = Path.Combine(Path.GetTempPath(), "af-mcp-list-tests-" + Guid.NewGuid().ToString("N")); - string staleDirectory = Path.Combine(directory, "stale-skill"); - Directory.CreateDirectory(staleDirectory); - string marker = Path.Combine(staleDirectory, "SKILL.md"); - await File.WriteAllTextAsync(marker, "Preserve this file."); - try - { - JsonObject entry = Entry(); - entry["resources"] = "dynamic"; - List reads = []; - await using var server = CreateServer((_, _) => ValueTask.FromResult(dynamic ? ListResult(entry) : ListResult()), - (uri, _) => - { - reads.Add(uri); - return ValueTask.FromResult(Text(uri, "{}")); - }); - await using var client = await server.CreateClientAsync(); - using var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = directory }); - - // Act - if (dynamic) - { - await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); - } - else + JsonObject entry = Entry(); + entry["resources"] = "dynamic"; + List reads = []; + await using var server = CreateServer((_, _) => ValueTask.FromResult(dynamic ? ListResult(entry) : ListResult()), + (uri, _) => { - var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); - Assert.Empty(skills); - } + reads.Add(uri); + return ValueTask.FromResult(Text(uri, "{}")); + }); + await using var client = await server.CreateClientAsync(); + using var source = new AgentMcpSkillsSource(client); - // Assert - Assert.Empty(reads); - Assert.Equal("Preserve this file.", await File.ReadAllTextAsync(marker)); + // Act + if (dynamic) + { + await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); } - finally + else { - Directory.Delete(directory, recursive: true); + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + Assert.Empty(skills); } + + // Assert + Assert.Empty(reads); } [Theory] From e7340a8b5a8919b02611a1b3b6a844bb5fbb44be Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 23 Oct 2026 17:12:51 +0100 Subject: [PATCH 2/6] Simplify MCP skill type names and folder structure Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...gentMcpListedSkill.cs => AgentMcpSkill.cs} | 14 +++--- ...llResource.cs => AgentMcpSkillResource.cs} | 4 +- .../Skills/AgentMcpSkillsSource.cs | 8 ++-- ...ListedSkillsCache.cs => McpSkillsCache.cs} | 14 +++--- ...JsonContext.cs => McpSkillsJsonContext.cs} | 8 ++-- .../{Listed => }/McpSkillsListResult.cs | 12 ++--- ...stedSkillsLoader.cs => McpSkillsLoader.cs} | 44 ++++++++--------- ...geLifetime.cs => McpSkillsPageLifetime.cs} | 4 +- ...SkillsSnapshot.cs => McpSkillsSnapshot.cs} | 6 +-- ...tedSkillTests.cs => AgentMcpSkillTests.cs} | 48 +++++++++---------- ...cs => AgentMcpSkillsSerializationTests.cs} | 40 ++++++++-------- ...ests.Ttl.cs => AgentMcpSkillsTests.Ttl.cs} | 38 +++++++-------- ...dSkillsTests.cs => AgentMcpSkillsTests.cs} | 32 ++++++------- ...lsCacheTests.cs => McpSkillsCacheTests.cs} | 42 ++++++++-------- 14 files changed, 157 insertions(+), 157 deletions(-) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{Listed/AgentMcpListedSkill.cs => AgentMcpSkill.cs} (94%) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{Listed/AgentMcpListedSkillResource.cs => AgentMcpSkillResource.cs} (90%) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{Listed/McpListedSkillsCache.cs => McpSkillsCache.cs} (86%) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{Listed/McpListedSkillsJsonContext.cs => McpSkillsJsonContext.cs} (68%) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{Listed => }/McpSkillsListResult.cs (86%) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{Listed/McpListedSkillsLoader.cs => McpSkillsLoader.cs} (85%) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{Listed/McpListedSkillsPageLifetime.cs => McpSkillsPageLifetime.cs} (86%) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{Listed/McpListedSkillsSnapshot.cs => McpSkillsSnapshot.cs} (78%) rename dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/{Listed/AgentMcpListedSkillTests.cs => AgentMcpSkillTests.cs} (94%) rename dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/{Listed/AgentMcpListedSkillsSerializationTests.cs => AgentMcpSkillsSerializationTests.cs} (85%) rename dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/{Listed/AgentMcpListedSkillsTests.Ttl.cs => AgentMcpSkillsTests.Ttl.cs} (93%) rename dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/{Listed/AgentMcpListedSkillsTests.cs => AgentMcpSkillsTests.cs} (98%) rename dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/{Listed/McpListedSkillsCacheTests.cs => McpSkillsCacheTests.cs} (80%) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/AgentMcpListedSkill.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs similarity index 94% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/AgentMcpListedSkill.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs index 7d3dd030a38..5f9c8aea8d9 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/AgentMcpListedSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkill.cs @@ -27,7 +27,7 @@ namespace Microsoft.Agents.AI; /// Each resource read must return exactly one text or binary content block. /// Only supporting resources listed by the server are available. /// -internal sealed class AgentMcpListedSkill : AgentSkill +internal sealed class AgentMcpSkill : AgentSkill { internal const long DefaultMaxResourceSizeBytes = 1024 * 1024; @@ -37,7 +37,7 @@ internal sealed class AgentMcpListedSkill : AgentSkill private readonly ILogger _logger; private readonly string _skillUri; private readonly string _skillRootUri; - private readonly IReadOnlyList _resources; + private readonly IReadOnlyList _resources; private readonly long _maxResourceSizeBytes; private string? _content; @@ -51,7 +51,7 @@ internal sealed class AgentMcpListedSkill : AgentSkill /// Optional logger for frontmatter parsing diagnostics. /// The maximum allowed resource size in bytes. /// is not positive. - public AgentMcpListedSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyList resources, McpClient client, ILogger? logger = null, long maxResourceSizeBytes = DefaultMaxResourceSizeBytes) + public AgentMcpSkill(AgentSkillFrontmatter frontmatter, string uri, IReadOnlyList resources, McpClient client, ILogger? logger = null, long maxResourceSizeBytes = DefaultMaxResourceSizeBytes) { this.Frontmatter = Throw.IfNull(frontmatter); this._skillUri = Throw.IfNullOrWhitespace(uri); @@ -83,7 +83,7 @@ public override async ValueTask GetContentAsync(CancellationToken cancel return this._content; } - McpListedSkillResourceEntry? skillMdResource = this._resources.FirstOrDefault(resource => string.Equals(resource.Uri, this._skillUri, StringComparison.Ordinal)); + McpSkillResourceEntry? skillMdResource = this._resources.FirstOrDefault(resource => string.Equals(resource.Uri, this._skillUri, StringComparison.Ordinal)); if (skillMdResource is null) { throw new InvalidOperationException($"The resource list must include the skill's SKILL.md resource '{this._skillUri}'."); @@ -149,7 +149,7 @@ public override async ValueTask GetContentAsync(CancellationToken cancel } // Supporting reads must stay within the held manifest. - McpListedSkillResourceEntry? resource = this._resources.FirstOrDefault(resource => string.Equals(resource.Uri, uri, StringComparison.Ordinal)); + McpSkillResourceEntry? resource = this._resources.FirstOrDefault(resource => string.Equals(resource.Uri, uri, StringComparison.Ordinal)); if (resource is null) { return null; @@ -159,7 +159,7 @@ public override async ValueTask GetContentAsync(CancellationToken cancel this.VerifyResourceIntegrity(resource, content); - return new AgentMcpListedSkillResource(name, content); + return new AgentMcpSkillResource(name, content); } private async Task ReadResourceContentAsync(string uri, CancellationToken cancellationToken) @@ -188,7 +188,7 @@ private async Task ReadResourceContentAsync(string uri, Cancel return content is BlobResourceContents binary ? this.DecodeBinaryContent(binary, uri) : content; } - private void VerifyResourceIntegrity(McpListedSkillResourceEntry resource, ResourceContents content) + private void VerifyResourceIntegrity(McpSkillResourceEntry resource, ResourceContents content) { int byteSize; diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/AgentMcpListedSkillResource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillResource.cs similarity index 90% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/AgentMcpListedSkillResource.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillResource.cs index b1429a624bb..04b6128cf1b 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/AgentMcpListedSkillResource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillResource.cs @@ -13,7 +13,7 @@ namespace Microsoft.Agents.AI; /// /// A supporting resource for a skill discovered through skills/list. /// -internal sealed class AgentMcpListedSkillResource : AgentSkillResource +internal sealed class AgentMcpSkillResource : AgentSkillResource { private readonly ResourceContents _content; @@ -23,7 +23,7 @@ internal sealed class AgentMcpListedSkillResource : AgentSkillResource /// The resource's relative name. /// The resource content that has passed size and digest verification. /// An optional description of the resource. - public AgentMcpListedSkillResource(string name, ResourceContents content, string? description = null) + public AgentMcpSkillResource(string name, ResourceContents content, string? description = null) : base(Throw.IfNullOrWhitespace(name), description) { this._content = Throw.IfNull(content); diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index 4b6ac041bb7..5ba382d46d8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -74,7 +74,7 @@ public sealed partial class AgentMcpSkillsSource : AgentSkillsSource [SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "The MCP client is supplied and owned by the caller, who is responsible for disposing it.")] private readonly McpClient _client; private readonly ILogger _logger; - private readonly McpListedSkillsLoader _listedSkillsLoader; + private readonly McpSkillsLoader _skillsLoader; /// /// Initializes a new instance of the class. @@ -87,7 +87,7 @@ public AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? optio this._client = Throw.IfNull(client); loggerFactory ??= NullLoggerFactory.Instance; this._logger = loggerFactory.CreateLogger(); - this._listedSkillsLoader = new McpListedSkillsLoader(this._client, this._logger, options); + this._skillsLoader = new McpSkillsLoader(this._client, this._logger, options); } /// @@ -105,7 +105,7 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo return []; } - IList skills = await this._listedSkillsLoader.DiscoverAsync(cancellationToken).ConfigureAwait(false); + IList skills = await this._skillsLoader.DiscoverAsync(cancellationToken).ConfigureAwait(false); LogSkillsLoadedTotal(this._logger, skills.Count); @@ -117,7 +117,7 @@ protected override void Dispose(bool disposing) { if (disposing) { - this._listedSkillsLoader.Dispose(); + this._skillsLoader.Dispose(); } base.Dispose(disposing); diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsCache.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCache.cs similarity index 86% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsCache.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCache.cs index 71c1b693af9..1888f010950 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsCache.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCache.cs @@ -11,12 +11,12 @@ namespace Microsoft.Agents.AI; /// /// Caches MCP-listed skills until any page's TTL expires. /// -internal sealed class McpListedSkillsCache : IDisposable +internal sealed class McpSkillsCache : IDisposable { private readonly TimeProvider _timeProvider; - private readonly Func> _discoverSkills; + private readonly Func> _discoverSkills; private readonly SemaphoreSlim _discoveryGate = new(1, 1); - private volatile McpListedSkillsSnapshot? _cachedSnapshot; + private volatile McpSkillsSnapshot? _cachedSnapshot; private bool _disposed; /// @@ -24,7 +24,7 @@ internal sealed class McpListedSkillsCache : IDisposable /// /// The clock for checking page freshness. /// Discovers and validates all pages, including their receipt timestamps and TTLs. - public McpListedSkillsCache(TimeProvider timeProvider, Func> discoverSkills) + public McpSkillsCache(TimeProvider timeProvider, Func> discoverSkills) { this._timeProvider = Throw.IfNull(timeProvider); this._discoverSkills = Throw.IfNull(discoverSkills); @@ -78,7 +78,7 @@ public async Task> GetOrCreateAsync(CancellationToken cancella /// /// Gets the cached snapshot, or if it is absent or any listing page has expired. /// - private McpListedSkillsSnapshot? GetCachedSnapshot() + private McpSkillsSnapshot? GetCachedSnapshot() { if (this._cachedSnapshot is not { } cached) { @@ -88,7 +88,7 @@ public async Task> GetOrCreateAsync(CancellationToken cancella long now = this._timeProvider.GetTimestamp(); // Check every page against the same timestamp. Any expired page makes the catalog stale. - foreach (McpListedSkillsPageLifetime page in cached.PageLifetimes) + foreach (McpSkillsPageLifetime page in cached.PageLifetimes) { if (this._timeProvider.GetElapsedTime(page.ReceivedTimestamp, now) >= page.TimeToLive) { @@ -103,7 +103,7 @@ public async Task> GetOrCreateAsync(CancellationToken cancella /// Stores the snapshot. /// /// The skill list and its listing-page lifetimes. - private void CacheSnapshot(McpListedSkillsSnapshot snapshot) + private void CacheSnapshot(McpSkillsSnapshot snapshot) { // Publish the complete catalog and page lifetimes together only after discovery and validation succeed. this._cachedSnapshot = snapshot; diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsJsonContext.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsJsonContext.cs similarity index 68% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsJsonContext.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsJsonContext.cs index a9be87ae474..b78e7218387 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsJsonContext.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsJsonContext.cs @@ -15,7 +15,7 @@ namespace Microsoft.Agents.AI; AllowTrailingCommas = true, ReadCommentHandling = JsonCommentHandling.Skip)] [JsonSerializable(typeof(McpSkillsListResult))] -[JsonSerializable(typeof(McpListedSkillEntry))] -[JsonSerializable(typeof(McpListedSkillFrontmatter))] -[JsonSerializable(typeof(McpListedSkillResourceEntry))] -internal sealed partial class McpListedSkillsJsonContext : JsonSerializerContext; +[JsonSerializable(typeof(McpSkillEntry))] +[JsonSerializable(typeof(McpSkillFrontmatter))] +[JsonSerializable(typeof(McpSkillResourceEntry))] +internal sealed partial class McpSkillsJsonContext : JsonSerializerContext; diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpSkillsListResult.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsListResult.cs similarity index 86% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpSkillsListResult.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsListResult.cs index 192c21208bc..32ca5ea312d 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpSkillsListResult.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsListResult.cs @@ -20,7 +20,7 @@ internal sealed class McpSkillsListResult /// /// Gets the skills listed by the server. /// - public required IReadOnlyList Skills { get; init; } + public required IReadOnlyList Skills { get; init; } /// /// Gets the opaque cursor to pass to the next request, or when no more pages are available. @@ -38,7 +38,7 @@ internal sealed class McpSkillsListResult /// /// A skill entry returned by skills/list. /// -internal sealed class McpListedSkillEntry +internal sealed class McpSkillEntry { /// /// Gets the resource URI of the skill's SKILL.md. @@ -48,18 +48,18 @@ internal sealed class McpListedSkillEntry /// /// Gets the skill's name and description. /// - public required McpListedSkillFrontmatter Frontmatter { get; init; } + public required McpSkillFrontmatter Frontmatter { get; init; } /// /// Gets the resources listed for the skill. /// - public required IReadOnlyList Resources { get; init; } + public required IReadOnlyList Resources { get; init; } } /// /// The name and description of a skill returned by skills/list. /// -internal sealed class McpListedSkillFrontmatter +internal sealed class McpSkillFrontmatter { /// /// Gets the skill's name. @@ -75,7 +75,7 @@ internal sealed class McpListedSkillFrontmatter /// /// A resource entry returned by skills/list. /// -internal sealed class McpListedSkillResourceEntry +internal sealed class McpSkillResourceEntry { /// /// Gets the file's resource URI. diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs similarity index 85% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs index acfc502b033..3b0e1a3e2e2 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs @@ -24,7 +24,7 @@ namespace Microsoft.Agents.AI; /// /// Instructions and resources are fetched later, when requested. /// -internal sealed partial class McpListedSkillsLoader : IDisposable +internal sealed partial class McpSkillsLoader : IDisposable { private const string CompleteResultType = "complete"; @@ -38,7 +38,7 @@ internal sealed partial class McpListedSkillsLoader : IDisposable private readonly int _maxPageCount; private readonly int _maxSkillCount; private readonly TimeProvider _timeProvider; - private readonly McpListedSkillsCache _cache; + private readonly McpSkillsCache _cache; /// /// Initializes a loader for skills discovered through skills/list. @@ -49,18 +49,18 @@ internal sealed partial class McpListedSkillsLoader : IDisposable /// Optional clock for checking page freshness. /// is . /// A configured limit is not positive. - public McpListedSkillsLoader(McpClient client, ILogger? logger = null, AgentMcpSkillsSourceOptions? options = null, TimeProvider? timeProvider = null) + public McpSkillsLoader(McpClient client, ILogger? logger = null, AgentMcpSkillsSourceOptions? options = null, TimeProvider? timeProvider = null) { this._client = Throw.IfNull(client); this._logger = logger ?? NullLogger.Instance; - this._maxResourceSizeBytes = options?.ListedResourceMaxSizeBytes ?? AgentMcpListedSkill.DefaultMaxResourceSizeBytes; + this._maxResourceSizeBytes = options?.ListedResourceMaxSizeBytes ?? AgentMcpSkill.DefaultMaxResourceSizeBytes; this._maxPageCount = options?.ListedMaxPageCount ?? DefaultMaxPageCount; this._maxSkillCount = options?.ListedMaxSkillCount ?? DefaultMaxSkillCount; this._timeProvider = timeProvider ?? TimeProvider.System; ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxResourceSizeBytes, nameof(options.ListedResourceMaxSizeBytes)); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxPageCount, nameof(options.ListedMaxPageCount)); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxSkillCount, nameof(options.ListedMaxSkillCount)); - this._cache = new McpListedSkillsCache(this._timeProvider, this.DiscoverCoreAsync); + this._cache = new McpSkillsCache(this._timeProvider, this.DiscoverCoreAsync); } /// @@ -79,29 +79,29 @@ public Task> DiscoverAsync(CancellationToken cancellationToken return this._cache.GetOrCreateAsync(cancellationToken); } - private async Task DiscoverCoreAsync(CancellationToken cancellationToken) + private async Task DiscoverCoreAsync(CancellationToken cancellationToken) { var (skillEntries, pageLifetimes) = await this.ListSkillEntriesAsync(cancellationToken).ConfigureAwait(false); List skills = []; - foreach (McpListedSkillEntry entry in skillEntries) + foreach (McpSkillEntry entry in skillEntries) { cancellationToken.ThrowIfCancellationRequested(); AgentSkillFrontmatter frontmatter = CreateFrontmatter(entry); - List resources = this.CreateSupportingResources(entry); + List resources = this.CreateSupportingResources(entry); - skills.Add(new AgentMcpListedSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes)); + skills.Add(new AgentMcpSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes)); } - return new McpListedSkillsSnapshot(skills, pageLifetimes); + return new McpSkillsSnapshot(skills, pageLifetimes); } - private async Task<(List Entries, List PageLifetimes)> ListSkillEntriesAsync(CancellationToken cancellationToken) + private async Task<(List Entries, List PageLifetimes)> ListSkillEntriesAsync(CancellationToken cancellationToken) { - List skillEntries = []; - List pageLifetimes = []; + List skillEntries = []; + List pageLifetimes = []; HashSet cursors = new(StringComparer.Ordinal); string? cursor = null; int pageCount = 0; @@ -141,7 +141,7 @@ private async Task DiscoverCoreAsync(CancellationToken throw new InvalidOperationException($"Skill discovery exceeds the maximum allowed skill count ({this._maxSkillCount})."); } - McpSkillsListResult result = resultNode.Deserialize(McpListedSkillsJsonContext.Default.McpSkillsListResult) + McpSkillsListResult result = resultNode.Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult) ?? throw new InvalidOperationException("The server returned no skills/list result."); if (result.ResultType != CompleteResultType) @@ -155,7 +155,7 @@ private async Task DiscoverCoreAsync(CancellationToken pageTimeToLive = TimeSpan.Zero; } - pageLifetimes.Add(new McpListedSkillsPageLifetime(receivedTimestamp, pageTimeToLive)); + pageLifetimes.Add(new McpSkillsPageLifetime(receivedTimestamp, pageTimeToLive)); cursor = result.NextCursor; if (cursor is not null && !cursors.Add(cursor)) @@ -163,7 +163,7 @@ private async Task DiscoverCoreAsync(CancellationToken throw new InvalidOperationException("The skills/list result contains a repeated pagination cursor."); } - foreach (McpListedSkillEntry? entry in result.Skills) + foreach (McpSkillEntry? entry in result.Skills) { // Nullable annotations do not reject null collection elements during deserialization. if (entry is null) @@ -179,7 +179,7 @@ private async Task DiscoverCoreAsync(CancellationToken return (skillEntries, pageLifetimes); } - private static AgentSkillFrontmatter CreateFrontmatter(McpListedSkillEntry entry) + private static AgentSkillFrontmatter CreateFrontmatter(McpSkillEntry entry) { // Instructions must be addressable as a safe, absolute resource, e.g. skill://unit-converter/SKILL.md. if (!Uri.TryCreate(entry.Uri, UriKind.Absolute, out _) || @@ -223,17 +223,17 @@ private static AgentSkillFrontmatter CreateFrontmatter(McpListedSkillEntry entry /// Validates and returns all resources listed for the skill, including SKILL.md, which contains /// the skill's instructions. /// - private List CreateSupportingResources(McpListedSkillEntry entry) + private List CreateSupportingResources(McpSkillEntry entry) { string skillUri = entry.Uri; // skill://unit-converter/SKILL.md -> skill://unit-converter/ (keep the trailing slash). string skillRootUri = skillUri[..^"SKILL.md".Length]; - List resources = []; + List resources = []; HashSet resourceUris = new(StringComparer.Ordinal); - foreach (McpListedSkillResourceEntry? resource in entry.Resources) + foreach (McpSkillResourceEntry? resource in entry.Resources) { this.ValidateResourceManifestEntry(resource, skillUri, skillRootUri); @@ -254,7 +254,7 @@ private List CreateSupportingResources(McpListedSki return resources; } - private void ValidateResourceManifestEntry([NotNull] McpListedSkillResourceEntry? resource, string skillUri, string skillRootUri) + private void ValidateResourceManifestEntry([NotNull] McpSkillResourceEntry? resource, string skillUri, string skillRootUri) { // Reject null entries, e.g. "resources": [null], which deserialization can accept. if (resource is null) @@ -265,7 +265,7 @@ private void ValidateResourceManifestEntry([NotNull] McpListedSkillResourceEntry // Resources must stay under the skill root, e.g. skill://unit-converter/references/rates.md. if (!resource.Uri.StartsWith(skillRootUri, StringComparison.Ordinal) || resource.Uri.Length == skillRootUri.Length || - !AgentMcpListedSkill.IsResourceNameSafe(resource.Uri[skillRootUri.Length..])) + !AgentMcpSkill.IsResourceNameSafe(resource.Uri[skillRootUri.Length..])) { throw new InvalidOperationException($"Skill '{skillUri}' contains an invalid resource manifest entry."); } diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsPageLifetime.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsPageLifetime.cs similarity index 86% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsPageLifetime.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsPageLifetime.cs index b0098c54e9b..60d29f1fa99 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsPageLifetime.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsPageLifetime.cs @@ -7,14 +7,14 @@ namespace Microsoft.Agents.AI; /// /// The receipt timestamp and TTL of a discovery page. /// -internal sealed class McpListedSkillsPageLifetime +internal sealed class McpSkillsPageLifetime { /// /// Initializes a page lifetime with its receipt timestamp and TTL. /// /// The timestamp recorded when the page response was received. /// The duration for which the page remains fresh. - public McpListedSkillsPageLifetime(long receivedTimestamp, TimeSpan timeToLive) + public McpSkillsPageLifetime(long receivedTimestamp, TimeSpan timeToLive) { this.ReceivedTimestamp = receivedTimestamp; this.TimeToLive = timeToLive; diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsSnapshot.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsSnapshot.cs similarity index 78% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsSnapshot.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsSnapshot.cs index 93c9b30e537..d88ad5883ee 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsSnapshot.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsSnapshot.cs @@ -7,14 +7,14 @@ namespace Microsoft.Agents.AI; /// /// A skill list and its listing-page lifetimes at a point in time. /// -internal sealed class McpListedSkillsSnapshot +internal sealed class McpSkillsSnapshot { /// /// Initializes a snapshot with its skills and listing-page lifetimes. /// /// The complete skill list for this snapshot. /// The receipt timestamp and TTL of each listing page. - public McpListedSkillsSnapshot(List skills, List pageLifetimes) + public McpSkillsSnapshot(List skills, List pageLifetimes) { this.Skills = skills; this.PageLifetimes = pageLifetimes; @@ -28,5 +28,5 @@ public McpListedSkillsSnapshot(List skills, List /// Gets the receipt timestamp and TTL of each listing page. /// - public List PageLifetimes { get; } + public List PageLifetimes { get; } } diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs similarity index 94% rename from dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillTests.cs rename to dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs index a74237a57b9..4bba698a922 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillTests.cs @@ -20,7 +20,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; /// /// Tests instruction and supporting resource access for listed skills. /// -public sealed class AgentMcpListedSkillTests +public sealed class AgentMcpSkillTests { private const string SkillRootUri = "skill://unit-converter/"; private const string SkillUri = SkillRootUri + "SKILL.md"; @@ -35,7 +35,7 @@ public async Task Constructor_PreservesProvidedFrontmatterAsync() var frontmatter = new AgentSkillFrontmatter("unit-converter", "Convert units.") { License = "MIT" }; // Act - var skill = new AgentMcpListedSkill(frontmatter, SkillUri, [Resource(SkillUri, SkillContent)], client); + var skill = new AgentMcpSkill(frontmatter, SkillUri, [Resource(SkillUri, SkillContent)], client); // Assert Assert.Same(frontmatter, skill.Frontmatter); @@ -53,7 +53,7 @@ public async Task GetContentAsync_ResourceList_FindsInstructionsByExactUriAsync( return ValueTask.FromResult(Text(uri, uri == SkillUri ? SkillContent : "Nested instructions.")); }); await using var client = await server.CreateClientAsync(); - var skill = new AgentMcpListedSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, + var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [ Resource(NestedUri, "Nested instructions."), Resource(SkillUri, SkillContent), @@ -84,10 +84,10 @@ public async Task GetContentAsync_MissingInstructionResource_ThrowsWithoutReadin }); await using var client = await server.CreateClientAsync(); var frontmatter = new AgentSkillFrontmatter("unit-converter", "Convert units."); - McpListedSkillResourceEntry[] resources = includeNestedInstructions + McpSkillResourceEntry[] resources = includeNestedInstructions ? [Resource(SkillRootUri + "nested/SKILL.md", SkillContent)] : []; - var skill = new AgentMcpListedSkill(frontmatter, SkillUri, resources, client); + var skill = new AgentMcpSkill(frontmatter, SkillUri, resources, client); // Act var exception = await Assert.ThrowsAsync(async () => await skill.GetContentAsync()); @@ -439,7 +439,7 @@ public async Task GetContentAsync_OptionalFrontmatter_IsNotComparedOrAppliedAsyn await using var server = CreateServer((uri, _) => ValueTask.FromResult(Text(uri, Content))); await using var client = await server.CreateClientAsync(); var frontmatter = new AgentSkillFrontmatter("unit-converter", "Convert units.") { License = "MIT" }; - var skill = new AgentMcpListedSkill(frontmatter, SkillUri, [Resource(SkillUri, Content)], client); + var skill = new AgentMcpSkill(frontmatter, SkillUri, [Resource(SkillUri, Content)], client); // Act string verified = await skill.GetContentAsync(); @@ -476,7 +476,7 @@ public async Task GetResourceAsync_ManifestFile_ReturnsTextOrBinaryAsync(string // Act var resource = await skill.GetResourceAsync(name); - var content = await Assert.IsType(resource).ReadAsync(); + var content = await Assert.IsType(resource).ReadAsync(); // Assert Assert.Equal(uri, Assert.Single(reads)); @@ -659,7 +659,7 @@ public async Task GetResourceAsync_TextBlock_DoesNotRequireUriMatchAsync() ]); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); + var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); var content = await resource.ReadAsync(); // Assert @@ -670,7 +670,7 @@ public async Task GetResourceAsync_TextBlock_DoesNotRequireUriMatchAsync() public async Task ReadAsync_EmptyResourceContent_ReturnsNullAsync() { // Arrange - var resource = new AgentMcpListedSkillResource("empty.txt", new TextResourceContents { Uri = SkillRootUri + "empty.txt", Text = "" }); + var resource = new AgentMcpSkillResource("empty.txt", new TextResourceContents { Uri = SkillRootUri + "empty.txt", Text = "" }); // Act var content = await resource.ReadAsync(); @@ -695,7 +695,7 @@ public async Task GetResourceAsync_TextContent_ReusesVerifiedTextAcrossReadsAsyn var skill = CreateSkill(client, resources: [Resource(Uri, Content)], maxResourceSizeBytes: Encoding.UTF8.GetByteCount(Content)); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); + var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); var first = await resource.ReadAsync(); var second = await resource.ReadAsync(); @@ -727,7 +727,7 @@ public async Task GetResourceAsync_BinaryContent_ReusesVerifiedDataAcrossReadsAs var skill = CreateSkill(client, resources: [Resource(Uri, bytes)], maxResourceSizeBytes: bytes.Length); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); + var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); var first = Assert.IsType(await resource.ReadAsync()); var second = Assert.IsType(await resource.ReadAsync()); @@ -764,7 +764,7 @@ public async Task GetResourceAsync_BinaryContent_PreservesMetadataAsync() var skill = CreateSkill(client, resources: [Resource(Uri, bytes)]); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); + var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); var content = Assert.IsType(await resource.ReadAsync()); // Assert @@ -792,7 +792,7 @@ public async Task GetResourceAsync_EmptyBlob_ReturnsBinaryContentAsync() var skill = CreateSkill(client, resources: [Resource(Uri, bytes)], maxResourceSizeBytes: 1); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); + var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); var content = Assert.IsType(await resource.ReadAsync()); // Assert @@ -869,7 +869,7 @@ public async Task GetContentAsync_Utf8Content_UsesByteCountAsync(bool useCharact await using var server = CreateServer((uri, _) => ValueTask.FromResult(Text(uri, Content))); await using var client = await server.CreateClientAsync(); - var skill = new AgentMcpListedSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [instructions], client); + var skill = new AgentMcpSkill(new AgentSkillFrontmatter("unit-converter", "Convert units."), SkillUri, [instructions], client); // Act & Assert if (useCharacterCount) @@ -900,7 +900,7 @@ public async Task GetResourceAsync_Utf8Content_VerifiesTextAndDecodedBlobAsync(b var skill = CreateSkill(client, resources: [Resource(Uri, bytes)]); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); + var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); var content = await resource.ReadAsync(); // Assert @@ -977,7 +977,7 @@ public async Task GetResourceAsync_ResourceLimit_EnforcesExactDefaultBoundaryAsy // Act & Assert if (excessBytes <= 0) { - var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); + var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); object? content = await resource.ReadAsync(); Assert.Equal(bytes.Length, binary ? Assert.IsType(content).Data.Length : Assert.IsType(content).Length); } @@ -1004,7 +1004,7 @@ public async Task GetResourceAsync_TextLimit_CountsUtf8BytesAsync(int limit, boo // Act & Assert if (accepted) { - var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); + var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); Assert.Equal(Content, await resource.ReadAsync()); } else @@ -1032,7 +1032,7 @@ public async Task GetResourceAsync_BinaryLimit_UsesDecodedSizeAsync(int length) var skill = CreateSkill(client, resources: [Resource(Uri, bytes)], maxResourceSizeBytes: length); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); + var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); // Assert Assert.Equal(length, Assert.IsType(await resource.ReadAsync()).Data.Length); @@ -1063,7 +1063,7 @@ public async Task GetResourceAsync_BinaryLimit_DoesNotRetainEncodedSizeBufferAsy var skill = CreateSkill(client, resources: [Resource(Uri, bytes)], maxResourceSizeBytes: Math.Max(1, bytes.Length)); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); + var resource = Assert.IsType(await skill.GetResourceAsync("assets/icon.bin")); var content = Assert.IsType(await resource.ReadAsync()); // Assert @@ -1119,12 +1119,12 @@ public async Task GetContentAsync_OversizedResponse_RetriesAndCachesOnlyVerified Assert.Equal(2, reads); } - private static AgentMcpListedSkill CreateSkill( + private static AgentMcpSkill CreateSkill( McpClient client, string root = SkillRootUri, - IReadOnlyList? resources = null, + IReadOnlyList? resources = null, string content = SkillContent, - long maxResourceSizeBytes = AgentMcpListedSkill.DefaultMaxResourceSizeBytes) => + long maxResourceSizeBytes = AgentMcpSkill.DefaultMaxResourceSizeBytes) => new(new AgentSkillFrontmatter("unit-converter", "Convert units."), root + "SKILL.md", [ Resource(root + "SKILL.md", content), @@ -1136,10 +1136,10 @@ .. resources ?? ], ], client, maxResourceSizeBytes: maxResourceSizeBytes); - private static McpListedSkillResourceEntry Resource(string uri, string text) => Resource(uri, Encoding.UTF8.GetBytes(text)); + private static McpSkillResourceEntry Resource(string uri, string text) => Resource(uri, Encoding.UTF8.GetBytes(text)); [SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "Resource digests require lowercase hexadecimal characters.")] - private static McpListedSkillResourceEntry Resource(string uri, byte[] bytes) => new() + private static McpSkillResourceEntry Resource(string uri, byte[] bytes) => new() { Uri = uri, Digest = "sha256:" + Convert.ToHexString(SHA256.HashData(bytes)).ToLowerInvariant(), diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsSerializationTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSerializationTests.cs similarity index 85% rename from dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsSerializationTests.cs rename to dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSerializationTests.cs index 85259abe7fe..3e80347df3d 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsSerializationTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSerializationTests.cs @@ -9,7 +9,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; /// /// Tests JSON deserialization of skills/list results. /// -public sealed class AgentMcpListedSkillsSerializationTests +public sealed class AgentMcpSkillsSerializationTests { [Theory] [InlineData("null", null)] @@ -27,7 +27,7 @@ public void Deserialize_TimeToLive_PreservesMilliseconds(string ttl, long? expec }; // Act - var result = node.Deserialize(McpListedSkillsJsonContext.Default.McpSkillsListResult); + var result = node.Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult); // Assert Assert.NotNull(result); @@ -41,7 +41,7 @@ public void Deserialize_MissingTimeToLive_PreservesAbsence() const string Json = """{"resultType":"complete","skills":[]}"""; // Act - var result = JsonSerializer.Deserialize(Json, McpListedSkillsJsonContext.Default.McpSkillsListResult); + var result = JsonSerializer.Deserialize(Json, McpSkillsJsonContext.Default.McpSkillsListResult); // Assert Assert.NotNull(result); @@ -64,7 +64,7 @@ public void Deserialize_InvalidTimeToLive_Throws(string ttl) }; // Act & Assert - Assert.Throws(() => node.Deserialize(McpListedSkillsJsonContext.Default.McpSkillsListResult)); + Assert.Throws(() => node.Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult)); } [Theory] @@ -93,7 +93,7 @@ public void Deserialize_InvalidTimeToLive_Throws(string ttl) public void Deserialize_InvalidResult_Throws(string json) { // Arrange - var context = McpListedSkillsJsonContext.Default; + var context = McpSkillsJsonContext.Default; // Act & Assert Assert.Throws(() => JsonSerializer.Deserialize(json, context.McpSkillsListResult)); @@ -114,7 +114,7 @@ public void Deserialize_NextCursor_PreservesValue(string? cursor) }; // Act - var result = node.Deserialize(McpListedSkillsJsonContext.Default.McpSkillsListResult); + var result = node.Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult); // Assert Assert.NotNull(result); @@ -142,7 +142,7 @@ public void Deserialize_InvalidSkillField_Throws(string field, string value) entry[field] = JsonNode.Parse(value); // Act & Assert - Assert.Throws(() => entry.Deserialize(McpListedSkillsJsonContext.Default.McpListedSkillEntry)); + Assert.Throws(() => entry.Deserialize(McpSkillsJsonContext.Default.McpSkillEntry)); } [Theory] @@ -156,7 +156,7 @@ public void Deserialize_MissingSkillField_Throws(string field) entry.Remove(field); // Act & Assert - JsonException exception = Assert.Throws(() => entry.Deserialize(McpListedSkillsJsonContext.Default.McpListedSkillEntry)); + JsonException exception = Assert.Throws(() => entry.Deserialize(McpSkillsJsonContext.Default.McpSkillEntry)); Assert.Contains(field, exception.Message); } @@ -174,10 +174,10 @@ public void Deserialize_MissingSkillField_Throws(string field) public void Deserialize_InvalidFrontmatter_Throws(string json) { // Arrange - var context = McpListedSkillsJsonContext.Default; + var context = McpSkillsJsonContext.Default; // Act & Assert - Assert.Throws(() => JsonSerializer.Deserialize(json, context.McpListedSkillFrontmatter)); + Assert.Throws(() => JsonSerializer.Deserialize(json, context.McpSkillFrontmatter)); } [Theory] @@ -190,10 +190,10 @@ public void Deserialize_InvalidFrontmatter_Throws(string json) public void Deserialize_InvalidResource_Throws(string json) { // Arrange - var context = McpListedSkillsJsonContext.Default; + var context = McpSkillsJsonContext.Default; // Act & Assert - Assert.Throws(() => JsonSerializer.Deserialize(json, context.McpListedSkillResourceEntry)); + Assert.Throws(() => JsonSerializer.Deserialize(json, context.McpSkillResourceEntry)); } [Fact] @@ -201,10 +201,10 @@ public void Deserialize_Resource_ReturnsResource() { // Arrange const string Json = """{"uri":"skill://unit-converter/file.txt","digest":"sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","size":0}"""; - var context = McpListedSkillsJsonContext.Default; + var context = McpSkillsJsonContext.Default; // Act - McpListedSkillResourceEntry? resource = JsonSerializer.Deserialize(Json, context.McpListedSkillResourceEntry); + McpSkillResourceEntry? resource = JsonSerializer.Deserialize(Json, context.McpSkillResourceEntry); // Assert Assert.NotNull(resource); @@ -223,7 +223,7 @@ public void Deserialize_Skill_UsesTypedFrontmatterAndResources(string resources) entry["resources"] = JsonNode.Parse(resources); // Act - McpListedSkillEntry? deserialized = entry.Deserialize(McpListedSkillsJsonContext.Default.McpListedSkillEntry); + McpSkillEntry? deserialized = entry.Deserialize(McpSkillsJsonContext.Default.McpSkillEntry); // Assert Assert.NotNull(deserialized); @@ -233,7 +233,7 @@ public void Deserialize_Skill_UsesTypedFrontmatterAndResources(string resources) Assert.Equal(expected.Count, deserialized.Resources.Count); if (expected.Count > 0) { - McpListedSkillResourceEntry resource = Assert.IsType(Assert.Single(deserialized.Resources)); + McpSkillResourceEntry resource = Assert.IsType(Assert.Single(deserialized.Resources)); Assert.Equal("skill://unit-converter/SKILL.md", resource.Uri); } } @@ -245,7 +245,7 @@ public void Deserialize_CaseVariants_IgnoresUnknownCaseVariants() const string Json = """{"resultType":"complete","ResultType":"unknown","skills":[],"Skills":null}"""; // Act - McpSkillsListResult? result = JsonSerializer.Deserialize(Json, McpListedSkillsJsonContext.Default.McpSkillsListResult); + McpSkillsListResult? result = JsonSerializer.Deserialize(Json, McpSkillsJsonContext.Default.McpSkillsListResult); // Assert Assert.NotNull(result); @@ -272,7 +272,7 @@ public void Deserialize_InvalidIntegrityField_Throws(string field, string value) resource[field] = JsonNode.Parse(value); // Act & Assert - Assert.Throws(() => resource.Deserialize(McpListedSkillsJsonContext.Default.McpListedSkillResourceEntry)); + Assert.Throws(() => resource.Deserialize(McpSkillsJsonContext.Default.McpSkillResourceEntry)); } [Theory] @@ -286,7 +286,7 @@ public void Deserialize_MissingResourceField_Throws(string field) resource.Remove(field); // Act & Assert - JsonException exception = Assert.Throws(() => resource.Deserialize(McpListedSkillsJsonContext.Default.McpListedSkillResourceEntry)); + JsonException exception = Assert.Throws(() => resource.Deserialize(McpSkillsJsonContext.Default.McpSkillResourceEntry)); Assert.Contains(field, exception.Message); } @@ -298,7 +298,7 @@ public void Deserialize_LargeSize_PreservesInt64() resource["size"] = long.MaxValue; // Act - var deserialized = resource.Deserialize(McpListedSkillsJsonContext.Default.McpListedSkillResourceEntry); + var deserialized = resource.Deserialize(McpSkillsJsonContext.Default.McpSkillResourceEntry); // Assert Assert.NotNull(deserialized); diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.Ttl.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Ttl.cs similarity index 93% rename from dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.Ttl.cs rename to dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Ttl.cs index a330d246b9a..20d9a5f0a1e 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.Ttl.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.Ttl.cs @@ -14,7 +14,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; -public sealed partial class AgentMcpListedSkillsTests +public sealed partial class AgentMcpSkillsTests { [Theory] [InlineData(false)] @@ -30,7 +30,7 @@ public async Task DiscoverAsync_PositiveTtl_ReusesCatalogUntilExactExpirationAsy long ttl = calls == 1 ? 1000 : 2000; return Task.FromResult(empty ? TtlListResult(ttl) : TtlListResult(ttl, Entry())); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); // Act var first = await loader.DiscoverAsync(CancellationToken.None); @@ -92,7 +92,7 @@ public async Task DiscoverAsync_MissingNullOrNonpositiveTtl_RefetchesOnEveryAcce return Task.FromResult(result); }); - using var loader = new McpListedSkillsLoader(client.Object); + using var loader = new McpSkillsLoader(client.Object); // Act var first = await loader.DiscoverAsync(CancellationToken.None); @@ -122,7 +122,7 @@ public async Task DiscoverAsync_ExtremelyLargeTtl_DoesNotOverflowFreshnessCalcul return Task.FromResult(result); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); // Act var first = await loader.DiscoverAsync(CancellationToken.None); @@ -168,7 +168,7 @@ public async Task DiscoverAsync_PaginatedTtl_ExpiresWhenAnyPageExpiresAsync(long return Task.FromResult(result); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); // Act var first = await loader.DiscoverAsync(CancellationToken.None); @@ -212,7 +212,7 @@ public async Task DiscoverAsync_StaleEmptyPage_PreventsCachingWholeCatalogAsync( return Task.FromResult(result); }); - using var loader = new McpListedSkillsLoader(client.Object); + using var loader = new McpSkillsLoader(client.Object); // Act await loader.DiscoverAsync(CancellationToken.None); @@ -243,7 +243,7 @@ public async Task DiscoverAsync_PageExpiresDuringPagination_DoesNotRestartItsClo return Task.FromResult(result); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); // Act await loader.DiscoverAsync(CancellationToken.None); @@ -265,7 +265,7 @@ public async Task DiscoverAsync_SlowRequest_TtlStartsAtResponseReceiptAsync() clock.Advance(TimeSpan.FromSeconds(2)); return Task.FromResult(TtlListResult(1000)); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); // Act var first = await loader.DiscoverAsync(CancellationToken.None); @@ -290,7 +290,7 @@ public async Task DiscoverAsync_ConcurrentCalls_ShareSuccessfulFreshDiscoveryAsy calls++; return refresh && calls == 1 ? Task.FromResult(TtlListResult(1000, Entry())) : release.Task; }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); if (refresh) { await loader.DiscoverAsync(CancellationToken.None); @@ -316,7 +316,7 @@ public async Task DiscoverAsync_ConcurrentCalls_ZeroTtlDoesNotShareStaleCatalogA int calls = 0; var client = CreateTtlClient((_, _) => ++calls == 1 ? release.Task : Task.FromResult(TtlListResult(0, Entry()))); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); // Act Task>[] pending = Enumerable.Range(0, 8) @@ -348,7 +348,7 @@ public async Task DiscoverAsync_CanceledWaiter_DoesNotCancelSharedDiscoveryAsync calls++; return release.Task.WaitAsync(token); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); using var cts = new CancellationTokenSource(); // Act @@ -378,7 +378,7 @@ public async Task DiscoverAsync_CanceledInitiator_WaiterRetriesDiscoveryAsync() int calls = 0; var client = CreateTtlClient((_, token) => ++calls == 1 ? release.Task.WaitAsync(token) : Task.FromResult(TtlListResult(1000, Entry()))); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); using var cts = new CancellationTokenSource(); // Act @@ -401,7 +401,7 @@ public async Task DiscoverAsync_CanceledToken_DoesNotDiscoverOrReturnFreshCacheA { // Arrange var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(1000, Entry()))); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); if (cached) { await loader.DiscoverAsync(CancellationToken.None); @@ -443,7 +443,7 @@ public async Task DiscoverAsync_FailedRefresh_PropagatesAndRetriesWithoutServing return Task.FromResult(TtlListResult(1000, Entry())); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); var first = await loader.DiscoverAsync(CancellationToken.None); clock.Advance(TimeSpan.FromSeconds(1)); @@ -482,7 +482,7 @@ public async Task DiscoverAsync_CanceledRefreshAfterResponse_DoesNotCacheReplace return Task.FromResult(result); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); var first = await loader.DiscoverAsync(CancellationToken.None); clock.Advance(TimeSpan.FromSeconds(1)); @@ -516,7 +516,7 @@ public async Task DiscoverAsync_TtlRefresh_ReplacesCatalogAsync(bool empty) added["frontmatter"]!["description"] = "Added skill."; return Task.FromResult(empty ? TtlListResult(1000) : TtlListResult(1000, updated, added)); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); var first = await loader.DiscoverAsync(CancellationToken.None); // Act @@ -563,7 +563,7 @@ public async Task DiscoverAsync_InvalidLaterRefreshPage_DoesNotCachePartialCatal return Task.FromResult(result); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); var first = await loader.DiscoverAsync(CancellationToken.None); clock.Advance(TimeSpan.FromSeconds(1)); @@ -592,7 +592,7 @@ public async Task DiscoverAsync_TtlRefresh_PreservesPreviouslyReturnedManifestAs return Task.FromResult(TtlListResult(1000, entry)); }); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: clock); + using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); var first = Assert.Single(await loader.DiscoverAsync(CancellationToken.None)); // Act @@ -612,7 +612,7 @@ public async Task DiscoverAsync_DisposedLoader_ThrowsWithoutDisposingClientAsync { // Arrange var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(1000, Entry()))); - using var loader = new McpListedSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); if (cached) { await loader.DiscoverAsync(CancellationToken.None); diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs similarity index 98% rename from dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.cs rename to dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs index 49f51e06e40..0b300e2b6bd 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs @@ -22,7 +22,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; /// /// Tests discovery and content access for skills returned by skills/list. /// -public sealed partial class AgentMcpListedSkillsTests +public sealed partial class AgentMcpSkillsTests { private const string SkillUri = "skill://unit-converter/SKILL.md"; private const string SkillContent = "---\nname: unit-converter\ndescription: Convert units.\n---\nInstructions."; @@ -44,7 +44,7 @@ public async Task GetSkillsAsync_ListRequest_ProjectsFrontmatterAndIncludesReque var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); // Assert - var skill = Assert.IsType(Assert.Single(skills)); + var skill = Assert.IsType(Assert.Single(skills)); Assert.Equal("unit-converter", skill.Frontmatter.Name); Assert.Equal("Convert units.", skill.Frontmatter.Description); Assert.Null(skill.Frontmatter.License); @@ -327,7 +327,7 @@ public async Task DiscoverAsync_LargeUnknownFields_AreAcceptedAcrossPagesAsync(i responses.ReturnsAsync(new JsonRpcResponse { Id = new RequestId(page + 1), Result = result }); } - using var loader = new McpListedSkillsLoader(client.Object); + using var loader = new McpSkillsLoader(client.Object); // Act var skills = await loader.DiscoverAsync(CancellationToken.None); @@ -344,7 +344,7 @@ public async Task DiscoverAsync_SkillLimit_RejectsExcessEntriesBeforeTypedDeseri var client = new Mock(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = ListResult(Entry(), new JsonObject()) }); - using var loader = new McpListedSkillsLoader(client.Object, options: new() { ListedMaxSkillCount = 1 }); + using var loader = new McpSkillsLoader(client.Object, options: new() { ListedMaxSkillCount = 1 }); // Act var exception = await Assert.ThrowsAsync(() => loader.DiscoverAsync(CancellationToken.None)); @@ -367,7 +367,7 @@ public async Task DiscoverAsync_LargeResourceManifest_HasNoSeparateResourceCount var client = new Mock(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = ListResult(entry) }); - using var loader = new McpListedSkillsLoader(client.Object); + using var loader = new McpSkillsLoader(client.Object); // Act var skills = await loader.DiscoverAsync(CancellationToken.None); @@ -457,7 +457,7 @@ public async Task DiscoverAsync_MissingResult_ThrowsAsync() var client = new Mock(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = null }); - using var loader = new McpListedSkillsLoader(client.Object); + using var loader = new McpSkillsLoader(client.Object); // Act InvalidOperationException exception = await Assert.ThrowsAsync(() => loader.DiscoverAsync(CancellationToken.None)); @@ -1042,7 +1042,7 @@ public async Task DiscoverAsync_LargeManifest_IsNotSubjectToResourceSizeLimitAsy var client = new Mock(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = result }); - using var loader = new McpListedSkillsLoader(client.Object); + using var loader = new McpSkillsLoader(client.Object); // Act var skills = await loader.DiscoverAsync(CancellationToken.None); @@ -1074,7 +1074,7 @@ public async Task GetSkillsAsync_Refresh_KeepsPreviouslyReturnedDigestsAndSizesA await Assert.ThrowsAsync(async () => await first.GetContentAsync()); await Assert.ThrowsAsync(async () => await first.GetResourceAsync("references/checklist.md")); Assert.Equal(UpdatedInstructions, await updated.GetContentAsync()); - var resource = Assert.IsType(await updated.GetResourceAsync("references/checklist.md")); + var resource = Assert.IsType(await updated.GetResourceAsync("references/checklist.md")); Assert.Equal("Updated checklist.", await resource.ReadAsync()); } @@ -1279,12 +1279,12 @@ public async Task GetSkillsAsync_DuplicateUri_ReturnsAllEntriesWithoutWarningsAs // Assert Assert.Equal(3, skills.Count); - var skill = Assert.IsType(skills[0]); + var skill = Assert.IsType(skills[0]); Assert.Equal("Convert units.", skill.Frontmatter.Description); Assert.Null(await skill.GetResourceAsync("references/changed.md")); - var secondSkill = Assert.IsType(skills[1]); + var secondSkill = Assert.IsType(skills[1]); Assert.Equal(conflicting ? "Changed description." : "Convert units.", secondSkill.Frontmatter.Description); - var thirdSkill = Assert.IsType(skills[2]); + var thirdSkill = Assert.IsType(skills[2]); Assert.Equal(conflicting ? "Another description." : "Convert units.", thirdSkill.Frontmatter.Description); logger.Verify( l => l.Log( @@ -1316,7 +1316,7 @@ public async Task GetContentAsync_LazyAndOriginBound_PreservesSchemeAsync(string using var source = new AgentMcpSkillsSource(client); // Act - var skill = Assert.IsType(Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); + var skill = Assert.IsType(Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); Assert.Empty(reads); string content = await skill.GetContentAsync(); string cached = await skill.GetContentAsync(); @@ -1558,7 +1558,7 @@ public async Task GetResourceAsync_ManifestFile_ReturnsTextOrBinaryAsync(string // Act var resource = await skill.GetResourceAsync(name); - var content = await Assert.IsType(resource).ReadAsync(); + var content = await Assert.IsType(resource).ReadAsync(); // Assert Assert.Equal(uri, Assert.Single(reads)); @@ -1600,7 +1600,7 @@ public async Task GetResourceAsync_MultipleContentBlocks_ThrowsAsync(bool firstT })); await using var client = await server.CreateClientAsync(); using var source = new AgentMcpSkillsSource(client); - var skill = Assert.IsType(Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); + var skill = Assert.IsType(Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()))); // Act var exception = await Assert.ThrowsAsync(async () => await skill.GetResourceAsync("references/checklist.md")); @@ -1736,7 +1736,7 @@ public async Task GetResourceAsync_TextBlock_DoesNotRequireUriMatchAsync() var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); // Act - var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); + var resource = Assert.IsType(await skill.GetResourceAsync("references/checklist.md")); var content = await resource.ReadAsync(); // Assert @@ -1747,7 +1747,7 @@ public async Task GetResourceAsync_TextBlock_DoesNotRequireUriMatchAsync() public async Task ReadAsync_EmptyResourceContent_ReturnsNullAsync() { // Arrange - var resource = new AgentMcpListedSkillResource("empty.txt", new TextResourceContents { Uri = "skill://unit-converter/empty.txt", Text = "" }); + var resource = new AgentMcpSkillResource("empty.txt", new TextResourceContents { Uri = "skill://unit-converter/empty.txt", Text = "" }); // Act var content = await resource.ReadAsync(); diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/McpListedSkillsCacheTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCacheTests.cs similarity index 80% rename from dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/McpListedSkillsCacheTests.cs rename to dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCacheTests.cs index 1b8aa7bdf78..0a716971870 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/McpListedSkillsCacheTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCacheTests.cs @@ -8,7 +8,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; -public sealed class McpListedSkillsCacheTests +public sealed class McpSkillsCacheTests { [Fact] public void Constructor_NullDiscoverSkills_ThrowsArgumentNullException() @@ -17,7 +17,7 @@ public void Constructor_NullDiscoverSkills_ThrowsArgumentNullException() var clock = new ManualTimeProvider(); // Act & Assert - Assert.Throws("discoverSkills", () => new McpListedSkillsCache(clock, null!)); + Assert.Throws("discoverSkills", () => new McpSkillsCache(clock, null!)); } [Fact] @@ -26,15 +26,15 @@ public async Task GetOrCreateAsync_PositiveTtl_ReturnsOriginalListUntilExactExpi // Arrange var clock = new ManualTimeProvider(); int calls = 0; - McpListedSkillsSnapshot? discovered = null; - Task DiscoverAsync(CancellationToken token) + McpSkillsSnapshot? discovered = null; + Task DiscoverAsync(CancellationToken token) { calls++; discovered = CreateSnapshot(clock, 1000); return Task.FromResult(discovered); } - using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + using var cache = new McpSkillsCache(clock, DiscoverAsync); // Act var first = await cache.GetOrCreateAsync(CancellationToken.None); @@ -67,7 +67,7 @@ public async Task GetOrCreateAsync_AnyExpiredPage_RefreshesEntireCatalogAsync(in // Arrange var clock = new ManualTimeProvider(); int calls = 0; - Task DiscoverAsync(CancellationToken token) + Task DiscoverAsync(CancellationToken token) { calls++; long[] ttls = [5000, 5000, 5000]; @@ -75,7 +75,7 @@ Task DiscoverAsync(CancellationToken token) return Task.FromResult(CreateSnapshot(clock, ttls)); } - using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + using var cache = new McpSkillsCache(clock, DiscoverAsync); var first = await cache.GetOrCreateAsync(CancellationToken.None); // Act @@ -94,11 +94,11 @@ public async Task GetOrCreateAsync_ConcurrentCalls_ReuseOnlyFreshResultsAsync(lo { // Arrange var clock = new ManualTimeProvider(); - var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); int calls = 0; - Task DiscoverAsync(CancellationToken token) => + Task DiscoverAsync(CancellationToken token) => ++calls == 1 ? release.Task : Task.FromResult(CreateSnapshot(clock, ttl)); - using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + using var cache = new McpSkillsCache(clock, DiscoverAsync); // Act Task>[] pending = Enumerable.Range(0, 8) @@ -125,12 +125,12 @@ public async Task GetOrCreateAsync_FailedDiscovery_PreservesErrorAndReleasesWait { // Arrange var clock = new ManualTimeProvider(); - var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var error = new InvalidOperationException("Discovery failed."); int calls = 0; - Task DiscoverAsync(CancellationToken token) => + Task DiscoverAsync(CancellationToken token) => ++calls == 1 ? release.Task : Task.FromResult(CreateSnapshot(clock, 1000)); - using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + using var cache = new McpSkillsCache(clock, DiscoverAsync); var active = cache.GetOrCreateAsync(CancellationToken.None); var waiting = cache.GetOrCreateAsync(CancellationToken.None); @@ -152,7 +152,7 @@ public async Task GetOrCreateAsync_CanceledAfterDiscovery_DoesNotCacheResultAsyn var clock = new ManualTimeProvider(); using var cts = new CancellationTokenSource(); int calls = 0; - Task DiscoverAsync(CancellationToken token) + Task DiscoverAsync(CancellationToken token) { if (++calls == 1) { @@ -163,7 +163,7 @@ Task DiscoverAsync(CancellationToken token) return Task.FromResult(CreateSnapshot(clock, 1000)); } - using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + using var cache = new McpSkillsCache(clock, DiscoverAsync); // Act & Assert await Assert.ThrowsAnyAsync(() => cache.GetOrCreateAsync(cts.Token)); @@ -181,13 +181,13 @@ public async Task GetOrCreateAsync_CanceledToken_DoesNotDiscoverOrReturnCachedSk // Arrange var clock = new ManualTimeProvider(); int calls = 0; - Task DiscoverAsync(CancellationToken token) + Task DiscoverAsync(CancellationToken token) { calls++; return Task.FromResult(CreateSnapshot(clock, 1000)); } - using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + using var cache = new McpSkillsCache(clock, DiscoverAsync); if (cached) { @@ -210,13 +210,13 @@ public async Task GetOrCreateAsync_DisposedCache_ThrowsWithoutInvokingDiscoveryA // Arrange var clock = new ManualTimeProvider(); int calls = 0; - Task DiscoverAsync(CancellationToken token) + Task DiscoverAsync(CancellationToken token) { calls++; return Task.FromResult(CreateSnapshot(clock, 1000)); } - using var cache = new McpListedSkillsCache(clock, DiscoverAsync); + using var cache = new McpSkillsCache(clock, DiscoverAsync); if (cached) { @@ -232,9 +232,9 @@ Task DiscoverAsync(CancellationToken token) Assert.Equal(cached ? 1 : 0, calls); } - private static McpListedSkillsSnapshot CreateSnapshot(ManualTimeProvider clock, params long[] ttls) => + private static McpSkillsSnapshot CreateSnapshot(ManualTimeProvider clock, params long[] ttls) => new([new AgentInlineSkill("test-skill", "A test skill.", "Test instructions.")], - ttls.Select(ttl => new McpListedSkillsPageLifetime(clock.GetTimestamp(), TimeSpan.FromMilliseconds(ttl))).ToList()); + ttls.Select(ttl => new McpSkillsPageLifetime(clock.GetTimestamp(), TimeSpan.FromMilliseconds(ttl))).ToList()); private sealed class ManualTimeProvider : TimeProvider { From 83c1749ce856d9539669b019126d277ca0461406 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 23 Oct 2026 17:23:06 +0100 Subject: [PATCH 3/6] Merge MCP skill discovery into AgentMcpSkillsSource Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsSource.cs | 245 ++++++++++++++- .../Skills/McpSkillsLoader.cs | 297 ------------------ .../Skills/AgentMcpSkillsTests.Ttl.cs | 191 +++++------ .../Skills/AgentMcpSkillsTests.cs | 44 +-- 4 files changed, 366 insertions(+), 411 deletions(-) delete mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index 5ba382d46d8..a13678f3fb6 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -1,13 +1,19 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Shared.Diagnostics; using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; namespace Microsoft.Agents.AI; @@ -70,11 +76,18 @@ namespace Microsoft.Agents.AI; public sealed partial class AgentMcpSkillsSource : AgentSkillsSource { private const string SkillsExtensionName = "io.modelcontextprotocol/skills"; + private const string CompleteResultType = "complete"; + private const int DefaultMaxPageCount = 10; + private const int DefaultMaxSkillCount = 100; [SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "The MCP client is supplied and owned by the caller, who is responsible for disposing it.")] private readonly McpClient _client; private readonly ILogger _logger; - private readonly McpSkillsLoader _skillsLoader; + private readonly long _maxResourceSizeBytes; + private readonly int _maxPageCount; + private readonly int _maxSkillCount; + private readonly TimeProvider _timeProvider; + private readonly McpSkillsCache _cache; /// /// Initializes a new instance of the class. @@ -82,12 +95,26 @@ public sealed partial class AgentMcpSkillsSource : AgentSkillsSource /// 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 options that control skill loading. /// Optional logger factory. + /// is . + /// A configured limit is not positive. public AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? options = null, ILoggerFactory? loggerFactory = null) + : this(client, options, loggerFactory, TimeProvider.System) + { + } + + internal AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? options, ILoggerFactory? loggerFactory, TimeProvider timeProvider) { this._client = Throw.IfNull(client); loggerFactory ??= NullLoggerFactory.Instance; this._logger = loggerFactory.CreateLogger(); - this._skillsLoader = new McpSkillsLoader(this._client, this._logger, options); + this._maxResourceSizeBytes = options?.ListedResourceMaxSizeBytes ?? AgentMcpSkill.DefaultMaxResourceSizeBytes; + this._maxPageCount = options?.ListedMaxPageCount ?? DefaultMaxPageCount; + this._maxSkillCount = options?.ListedMaxSkillCount ?? DefaultMaxSkillCount; + this._timeProvider = Throw.IfNull(timeProvider); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxResourceSizeBytes, nameof(options.ListedResourceMaxSizeBytes)); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxPageCount, nameof(options.ListedMaxPageCount)); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxSkillCount, nameof(options.ListedMaxSkillCount)); + this._cache = new McpSkillsCache(this._timeProvider, this.DiscoverCoreAsync); } /// @@ -105,7 +132,7 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo return []; } - IList skills = await this._skillsLoader.DiscoverAsync(cancellationToken).ConfigureAwait(false); + IList skills = await this._cache.GetOrCreateAsync(cancellationToken).ConfigureAwait(false); LogSkillsLoadedTotal(this._logger, skills.Count); @@ -117,12 +144,222 @@ protected override void Dispose(bool disposing) { if (disposing) { - this._skillsLoader.Dispose(); + this._cache.Dispose(); } base.Dispose(disposing); } + private async Task DiscoverCoreAsync(CancellationToken cancellationToken) + { + var (skillEntries, pageLifetimes) = await this.ListSkillEntriesAsync(cancellationToken).ConfigureAwait(false); + + List skills = []; + + foreach (McpSkillEntry entry in skillEntries) + { + cancellationToken.ThrowIfCancellationRequested(); + + AgentSkillFrontmatter frontmatter = CreateFrontmatter(entry); + List resources = this.CreateSupportingResources(entry); + + skills.Add(new AgentMcpSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes)); + } + + return new McpSkillsSnapshot(skills, pageLifetimes); + } + + private async Task<(List Entries, List PageLifetimes)> ListSkillEntriesAsync(CancellationToken cancellationToken) + { + List skillEntries = []; + List pageLifetimes = []; + HashSet cursors = new(StringComparer.Ordinal); + string? cursor = null; + int pageCount = 0; + + do + { + cancellationToken.ThrowIfCancellationRequested(); + + if (pageCount >= this._maxPageCount) + { + throw new InvalidOperationException($"Skill discovery exceeds the maximum allowed page count ({this._maxPageCount})."); + } + + JsonObject parameters = new(); + if (cursor is not null) + { + parameters["cursor"] = cursor; + } + + JsonRpcResponse response = await this._client.SendRequestAsync(new JsonRpcRequest + { + Method = "skills/list", + Params = parameters, + }, cancellationToken).ConfigureAwait(false); + + long receivedTimestamp = this._timeProvider.GetTimestamp(); + pageCount++; + + cancellationToken.ThrowIfCancellationRequested(); + + JsonNode resultNode = response.Result + ?? throw new InvalidOperationException("The server returned no skills/list result."); + + // Reject excessive catalogs before deserializing their typed metadata or constructing skills. + if (resultNode is JsonObject resultObject && resultObject["skills"] is JsonArray entries && entries.Count > this._maxSkillCount - skillEntries.Count) + { + throw new InvalidOperationException($"Skill discovery exceeds the maximum allowed skill count ({this._maxSkillCount})."); + } + + McpSkillsListResult result = resultNode.Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult) + ?? throw new InvalidOperationException("The server returned no skills/list result."); + + if (result.ResultType != CompleteResultType) + { + throw new InvalidOperationException($"A skills/list result must have resultType '{CompleteResultType}'. Received resultType '{result.ResultType}'."); + } + + TimeSpan pageTimeToLive = result.TimeToLive.GetValueOrDefault(); + if (pageTimeToLive < TimeSpan.Zero) + { + pageTimeToLive = TimeSpan.Zero; + } + + pageLifetimes.Add(new McpSkillsPageLifetime(receivedTimestamp, pageTimeToLive)); + + cursor = result.NextCursor; + if (cursor is not null && !cursors.Add(cursor)) + { + throw new InvalidOperationException("The skills/list result contains a repeated pagination cursor."); + } + + foreach (McpSkillEntry? entry in result.Skills) + { + // Nullable annotations do not reject null collection elements during deserialization. + if (entry is null) + { + throw new InvalidOperationException("The skills/list result contains a null skill entry."); + } + + skillEntries.Add(entry); + } + } + while (cursor is not null); + + return (skillEntries, pageLifetimes); + } + + private static AgentSkillFrontmatter CreateFrontmatter(McpSkillEntry entry) + { + // Instructions must be addressable as a safe, absolute resource, e.g. skill://unit-converter/SKILL.md. + if (!Uri.TryCreate(entry.Uri, UriKind.Absolute, out _) || + !entry.Uri.EndsWith("/SKILL.md", StringComparison.Ordinal) || + entry.Uri.Contains('\\') || entry.Uri.Any(char.IsControl)) + { + throw new InvalidOperationException("A skill entry must specify an absolute SKILL.md resource URI."); + } + + // Validate the metadata needed to advertise the skill. + if (!AgentSkillFrontmatter.ValidateName(entry.Frontmatter.Name, out string? reason) || + !AgentSkillFrontmatter.ValidateDescription(entry.Frontmatter.Description, out reason)) + { + throw new InvalidOperationException($"Skill '{entry.Uri}' has invalid frontmatter: {reason}"); + } + + // skill://unit-converter/SKILL.md -> skill://unit-converter + string directory = entry.Uri[..^"/SKILL.md".Length]; + + // Start of "unit-converter" in skill://unit-converter; 0 if no '/'. + int segmentStart = directory.LastIndexOf('/') + 1; + + // Opaque URI schemes can place the skill name directly after the scheme. + if (segmentStart == 0) + { + // Start of "unit-converter" in skill:unit-converter. + segmentStart = directory.IndexOf(':') + 1; + } + + // directory[segmentStart..] extracts "unit-converter", which must match the advertised skill name. + if (!string.Equals(directory[segmentStart..], entry.Frontmatter.Name, StringComparison.Ordinal)) + { + throw new InvalidOperationException($"Skill URI '{entry.Uri}' must end with its frontmatter name followed by /SKILL.md."); + } + + // Project the fields used to advertise the skill. + return new AgentSkillFrontmatter(entry.Frontmatter.Name, entry.Frontmatter.Description); + } + + /// + /// Validates and returns all resources listed for the skill, including SKILL.md, which contains + /// the skill's instructions. + /// + private List CreateSupportingResources(McpSkillEntry entry) + { + string skillUri = entry.Uri; + + // skill://unit-converter/SKILL.md -> skill://unit-converter/ (keep the trailing slash). + string skillRootUri = skillUri[..^"SKILL.md".Length]; + + List resources = []; + HashSet resourceUris = new(StringComparer.Ordinal); + + foreach (McpSkillResourceEntry? resource in entry.Resources) + { + this.ValidateResourceManifestEntry(resource, skillUri, skillRootUri); + + // Each resource URI must appear only once in the skill's resource list. + if (!resourceUris.Add(resource.Uri)) + { + throw new InvalidOperationException($"Skill '{skillUri}' contains a duplicate resource URI '{resource.Uri}'."); + } + + resources.Add(resource); + } + + if (!resourceUris.Contains(skillUri)) + { + throw new InvalidOperationException($"Skill '{skillUri}' must include SKILL.md in its resource manifest."); + } + + return resources; + } + + private void ValidateResourceManifestEntry([NotNull] McpSkillResourceEntry? resource, string skillUri, string skillRootUri) + { + // Reject null entries, e.g. "resources": [null], which deserialization can accept. + if (resource is null) + { + throw new InvalidOperationException($"Skill '{skillUri}' contains a null manifest entry."); + } + + // Resources must stay under the skill root, e.g. skill://unit-converter/references/rates.md. + if (!resource.Uri.StartsWith(skillRootUri, StringComparison.Ordinal) || + resource.Uri.Length == skillRootUri.Length || + !AgentMcpSkill.IsResourceNameSafe(resource.Uri[skillRootUri.Length..])) + { + throw new InvalidOperationException($"Skill '{skillUri}' contains an invalid resource manifest entry."); + } + + if (!ResourceDigestRegex().IsMatch(resource.Digest)) + { + throw new InvalidOperationException($"Resource '{resource.Uri}' must specify a digest as 'sha256:' followed by 64 lowercase hexadecimal characters."); + } + + if (resource.Size < 0) + { + throw new InvalidOperationException($"Resource '{resource.Uri}' must specify a nonnegative size in bytes."); + } + + if (resource.Size > this._maxResourceSizeBytes) + { + throw new InvalidOperationException($"Resource '{resource.Uri}' exceeds the maximum allowed size ({this._maxResourceSizeBytes} bytes)."); + } + } + + [GeneratedRegex(@"\Asha256:[0-9a-f]{64}\z", RegexOptions.CultureInvariant)] + private static partial Regex ResourceDigestRegex(); + [LoggerMessage(LogLevel.Information, "Successfully loaded {Count} skills from MCP server")] private static partial void LogSkillsLoadedTotal(ILogger logger, int count); diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs deleted file mode 100644 index 3b0e1a3e2e2..00000000000 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs +++ /dev/null @@ -1,297 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Text.Json; -using System.Text.Json.Nodes; -using System.Text.RegularExpressions; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Shared.Diagnostics; -using ModelContextProtocol; -using ModelContextProtocol.Client; -using ModelContextProtocol.Protocol; - -namespace Microsoft.Agents.AI; - -/// -/// Discovers skills through the MCP skills/list method. -/// -/// -/// Instructions and resources are fetched later, when requested. -/// -internal sealed partial class McpSkillsLoader : IDisposable -{ - private const string CompleteResultType = "complete"; - - private const int DefaultMaxPageCount = 10; - private const int DefaultMaxSkillCount = 100; - - [SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "The MCP client is supplied and owned by the caller, who is responsible for disposing it.")] - private readonly McpClient _client; - private readonly ILogger _logger; - private readonly long _maxResourceSizeBytes; - private readonly int _maxPageCount; - private readonly int _maxSkillCount; - private readonly TimeProvider _timeProvider; - private readonly McpSkillsCache _cache; - - /// - /// Initializes a loader for skills discovered through skills/list. - /// - /// The MCP client used to discover skills and fetch their content. - /// Optional logger for frontmatter parsing diagnostics. - /// Optional discovery limits and size limit for listed resources. - /// Optional clock for checking page freshness. - /// is . - /// A configured limit is not positive. - public McpSkillsLoader(McpClient client, ILogger? logger = null, AgentMcpSkillsSourceOptions? options = null, TimeProvider? timeProvider = null) - { - this._client = Throw.IfNull(client); - this._logger = logger ?? NullLogger.Instance; - this._maxResourceSizeBytes = options?.ListedResourceMaxSizeBytes ?? AgentMcpSkill.DefaultMaxResourceSizeBytes; - this._maxPageCount = options?.ListedMaxPageCount ?? DefaultMaxPageCount; - this._maxSkillCount = options?.ListedMaxSkillCount ?? DefaultMaxSkillCount; - this._timeProvider = timeProvider ?? TimeProvider.System; - ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxResourceSizeBytes, nameof(options.ListedResourceMaxSizeBytes)); - ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxPageCount, nameof(options.ListedMaxPageCount)); - ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxSkillCount, nameof(options.ListedMaxSkillCount)); - this._cache = new McpSkillsCache(this._timeProvider, this.DiscoverCoreAsync); - } - - /// - /// Gets the server's listed skills across all pages without fetching their instructions or resources. - /// - /// The token used to cancel discovery. - /// The skills discovered through skills/list. - /// The response cannot be deserialized into a skill list. - /// The response is missing or contains an unsupported result type, - /// invalid skill metadata or resource manifests, a repeated pagination cursor, or a configured limit is exceeded. - /// The MCP request fails. - /// Discovery is canceled. - /// The loader has been disposed. - public Task> DiscoverAsync(CancellationToken cancellationToken) - { - return this._cache.GetOrCreateAsync(cancellationToken); - } - - private async Task DiscoverCoreAsync(CancellationToken cancellationToken) - { - var (skillEntries, pageLifetimes) = await this.ListSkillEntriesAsync(cancellationToken).ConfigureAwait(false); - - List skills = []; - - foreach (McpSkillEntry entry in skillEntries) - { - cancellationToken.ThrowIfCancellationRequested(); - - AgentSkillFrontmatter frontmatter = CreateFrontmatter(entry); - List resources = this.CreateSupportingResources(entry); - - skills.Add(new AgentMcpSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes)); - } - - return new McpSkillsSnapshot(skills, pageLifetimes); - } - - private async Task<(List Entries, List PageLifetimes)> ListSkillEntriesAsync(CancellationToken cancellationToken) - { - List skillEntries = []; - List pageLifetimes = []; - HashSet cursors = new(StringComparer.Ordinal); - string? cursor = null; - int pageCount = 0; - - do - { - cancellationToken.ThrowIfCancellationRequested(); - - if (pageCount >= this._maxPageCount) - { - throw new InvalidOperationException($"Skill discovery exceeds the maximum allowed page count ({this._maxPageCount})."); - } - - JsonObject parameters = new(); - if (cursor is not null) - { - parameters["cursor"] = cursor; - } - - JsonRpcResponse response = await this._client.SendRequestAsync(new JsonRpcRequest - { - Method = "skills/list", - Params = parameters, - }, cancellationToken).ConfigureAwait(false); - - long receivedTimestamp = this._timeProvider.GetTimestamp(); - pageCount++; - - cancellationToken.ThrowIfCancellationRequested(); - - JsonNode resultNode = response.Result - ?? throw new InvalidOperationException("The server returned no skills/list result."); - - // Reject excessive catalogs before deserializing their typed metadata or constructing skills. - if (resultNode is JsonObject resultObject && resultObject["skills"] is JsonArray entries && entries.Count > this._maxSkillCount - skillEntries.Count) - { - throw new InvalidOperationException($"Skill discovery exceeds the maximum allowed skill count ({this._maxSkillCount})."); - } - - McpSkillsListResult result = resultNode.Deserialize(McpSkillsJsonContext.Default.McpSkillsListResult) - ?? throw new InvalidOperationException("The server returned no skills/list result."); - - if (result.ResultType != CompleteResultType) - { - throw new InvalidOperationException($"A skills/list result must have resultType '{CompleteResultType}'. Received resultType '{result.ResultType}'."); - } - - TimeSpan pageTimeToLive = result.TimeToLive.GetValueOrDefault(); - if (pageTimeToLive < TimeSpan.Zero) - { - pageTimeToLive = TimeSpan.Zero; - } - - pageLifetimes.Add(new McpSkillsPageLifetime(receivedTimestamp, pageTimeToLive)); - - cursor = result.NextCursor; - if (cursor is not null && !cursors.Add(cursor)) - { - throw new InvalidOperationException("The skills/list result contains a repeated pagination cursor."); - } - - foreach (McpSkillEntry? entry in result.Skills) - { - // Nullable annotations do not reject null collection elements during deserialization. - if (entry is null) - { - throw new InvalidOperationException("The skills/list result contains a null skill entry."); - } - - skillEntries.Add(entry); - } - } - while (cursor is not null); - - return (skillEntries, pageLifetimes); - } - - private static AgentSkillFrontmatter CreateFrontmatter(McpSkillEntry entry) - { - // Instructions must be addressable as a safe, absolute resource, e.g. skill://unit-converter/SKILL.md. - if (!Uri.TryCreate(entry.Uri, UriKind.Absolute, out _) || - !entry.Uri.EndsWith("/SKILL.md", StringComparison.Ordinal) || - entry.Uri.Contains('\\') || entry.Uri.Any(char.IsControl)) - { - throw new InvalidOperationException("A skill entry must specify an absolute SKILL.md resource URI."); - } - - // Validate the metadata needed to advertise the skill. - if (!AgentSkillFrontmatter.ValidateName(entry.Frontmatter.Name, out string? reason) || - !AgentSkillFrontmatter.ValidateDescription(entry.Frontmatter.Description, out reason)) - { - throw new InvalidOperationException($"Skill '{entry.Uri}' has invalid frontmatter: {reason}"); - } - - // skill://unit-converter/SKILL.md -> skill://unit-converter - string directory = entry.Uri[..^"/SKILL.md".Length]; - - // Start of "unit-converter" in skill://unit-converter; 0 if no '/'. - int segmentStart = directory.LastIndexOf('/') + 1; - - // Opaque URI schemes can place the skill name directly after the scheme. - if (segmentStart == 0) - { - // Start of "unit-converter" in skill:unit-converter. - segmentStart = directory.IndexOf(':') + 1; - } - - // directory[segmentStart..] extracts "unit-converter", which must match the advertised skill name. - if (!string.Equals(directory[segmentStart..], entry.Frontmatter.Name, StringComparison.Ordinal)) - { - throw new InvalidOperationException($"Skill URI '{entry.Uri}' must end with its frontmatter name followed by /SKILL.md."); - } - - // Project the fields used to advertise the skill. - return new AgentSkillFrontmatter(entry.Frontmatter.Name, entry.Frontmatter.Description); - } - - /// - /// Validates and returns all resources listed for the skill, including SKILL.md, which contains - /// the skill's instructions. - /// - private List CreateSupportingResources(McpSkillEntry entry) - { - string skillUri = entry.Uri; - - // skill://unit-converter/SKILL.md -> skill://unit-converter/ (keep the trailing slash). - string skillRootUri = skillUri[..^"SKILL.md".Length]; - - List resources = []; - HashSet resourceUris = new(StringComparer.Ordinal); - - foreach (McpSkillResourceEntry? resource in entry.Resources) - { - this.ValidateResourceManifestEntry(resource, skillUri, skillRootUri); - - // Each resource URI must appear only once in the skill's resource list. - if (!resourceUris.Add(resource.Uri)) - { - throw new InvalidOperationException($"Skill '{skillUri}' contains a duplicate resource URI '{resource.Uri}'."); - } - - resources.Add(resource); - } - - if (!resourceUris.Contains(skillUri)) - { - throw new InvalidOperationException($"Skill '{skillUri}' must include SKILL.md in its resource manifest."); - } - - return resources; - } - - private void ValidateResourceManifestEntry([NotNull] McpSkillResourceEntry? resource, string skillUri, string skillRootUri) - { - // Reject null entries, e.g. "resources": [null], which deserialization can accept. - if (resource is null) - { - throw new InvalidOperationException($"Skill '{skillUri}' contains a null manifest entry."); - } - - // Resources must stay under the skill root, e.g. skill://unit-converter/references/rates.md. - if (!resource.Uri.StartsWith(skillRootUri, StringComparison.Ordinal) || - resource.Uri.Length == skillRootUri.Length || - !AgentMcpSkill.IsResourceNameSafe(resource.Uri[skillRootUri.Length..])) - { - throw new InvalidOperationException($"Skill '{skillUri}' contains an invalid resource manifest entry."); - } - - if (!ResourceDigestRegex().IsMatch(resource.Digest)) - { - throw new InvalidOperationException($"Resource '{resource.Uri}' must specify a digest as 'sha256:' followed by 64 lowercase hexadecimal characters."); - } - - if (resource.Size < 0) - { - throw new InvalidOperationException($"Resource '{resource.Uri}' must specify a nonnegative size in bytes."); - } - - if (resource.Size > this._maxResourceSizeBytes) - { - throw new InvalidOperationException($"Resource '{resource.Uri}' exceeds the maximum allowed size ({this._maxResourceSizeBytes} bytes)."); - } - } - - [GeneratedRegex(@"\Asha256:[0-9a-f]{64}\z", RegexOptions.CultureInvariant)] - private static partial Regex ResourceDigestRegex(); - - /// - public void Dispose() - { - this._cache.Dispose(); - } -} 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 20d9a5f0a1e..453e5fe536c 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 @@ -19,7 +19,7 @@ public sealed partial class AgentMcpSkillsTests [Theory] [InlineData(false)] [InlineData(true)] - public async Task DiscoverAsync_PositiveTtl_ReusesCatalogUntilExactExpirationAsync(bool empty) + public async Task GetSkillsAsync_PositiveTtl_ReusesCatalogUntilExactExpirationAsync(bool empty) { // Arrange var clock = new ManualTimeProvider(); @@ -30,12 +30,12 @@ public async Task DiscoverAsync_PositiveTtl_ReusesCatalogUntilExactExpirationAsy long ttl = calls == 1 ? 1000 : 2000; return Task.FromResult(empty ? TtlListResult(ttl) : TtlListResult(ttl, Entry())); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); // Act - var first = await loader.DiscoverAsync(CancellationToken.None); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); clock.Advance(TimeSpan.FromMilliseconds(999)); - var fresh = await loader.DiscoverAsync(CancellationToken.None); + var fresh = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Same(first, fresh); @@ -45,7 +45,7 @@ public async Task DiscoverAsync_PositiveTtl_ReusesCatalogUntilExactExpirationAsy // Act clock.Advance(TimeSpan.FromMilliseconds(1)); - var refreshed = await loader.DiscoverAsync(CancellationToken.None); + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.NotSame(first, refreshed); @@ -54,7 +54,7 @@ public async Task DiscoverAsync_PositiveTtl_ReusesCatalogUntilExactExpirationAsy // Act clock.Advance(TimeSpan.FromMilliseconds(1999)); - var stillFresh = await loader.DiscoverAsync(CancellationToken.None); + var stillFresh = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Same(refreshed, stillFresh); @@ -62,12 +62,13 @@ public async Task DiscoverAsync_PositiveTtl_ReusesCatalogUntilExactExpirationAsy // Act clock.Advance(TimeSpan.FromMilliseconds(1)); - var expired = await loader.DiscoverAsync(CancellationToken.None); + var expired = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.NotSame(refreshed, expired); Assert.Equal(3, calls); client.Verify(c => c.SendRequestAsync(It.Is(request => request.Method == "skills/list"), It.IsAny()), Times.Exactly(3)); + client.VerifyGet(c => c.ServerCapabilities, Times.Exactly(5)); client.VerifyNoOtherCalls(); } @@ -77,7 +78,7 @@ public async Task DiscoverAsync_PositiveTtl_ReusesCatalogUntilExactExpirationAsy [InlineData("0")] [InlineData("-1")] [InlineData("-9223372036854775808")] - public async Task DiscoverAsync_MissingNullOrNonpositiveTtl_RefetchesOnEveryAccessAsync(string? ttl) + public async Task GetSkillsAsync_MissingNullOrNonpositiveTtl_RefetchesOnEveryAccessAsync(string? ttl) { // Arrange int calls = 0; @@ -92,11 +93,11 @@ public async Task DiscoverAsync_MissingNullOrNonpositiveTtl_RefetchesOnEveryAcce return Task.FromResult(result); }); - using var loader = new McpSkillsLoader(client.Object); + using var source = new AgentMcpSkillsSource(client.Object); // Act - var first = await loader.DiscoverAsync(CancellationToken.None); - var second = await loader.DiscoverAsync(CancellationToken.None); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.NotSame(first, second); @@ -106,7 +107,7 @@ public async Task DiscoverAsync_MissingNullOrNonpositiveTtl_RefetchesOnEveryAcce [Theory] [InlineData(false)] [InlineData(true)] - public async Task DiscoverAsync_ExtremelyLargeTtl_DoesNotOverflowFreshnessCalculationAsync(bool paginated) + public async Task GetSkillsAsync_ExtremelyLargeTtl_DoesNotOverflowFreshnessCalculationAsync(bool paginated) { // Arrange var clock = new ManualTimeProvider(); @@ -122,12 +123,12 @@ public async Task DiscoverAsync_ExtremelyLargeTtl_DoesNotOverflowFreshnessCalcul return Task.FromResult(result); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); // Act - var first = await loader.DiscoverAsync(CancellationToken.None); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); clock.Advance(TimeSpan.FromDays(365)); - var second = await loader.DiscoverAsync(CancellationToken.None); + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Same(first, second); @@ -139,7 +140,7 @@ public async Task DiscoverAsync_ExtremelyLargeTtl_DoesNotOverflowFreshnessCalcul [InlineData(5000L, 1000L, 5000L, 800L)] [InlineData(5000L, 5000L, 1000L, 1000L)] [InlineData(1000L, 900L, 800L, 600L)] - public async Task DiscoverAsync_PaginatedTtl_ExpiresWhenAnyPageExpiresAsync(long firstTtl, long secondTtl, long thirdTtl, long remaining) + public async Task GetSkillsAsync_PaginatedTtl_ExpiresWhenAnyPageExpiresAsync(long firstTtl, long secondTtl, long thirdTtl, long remaining) { // Arrange var clock = new ManualTimeProvider(); @@ -168,12 +169,12 @@ public async Task DiscoverAsync_PaginatedTtl_ExpiresWhenAnyPageExpiresAsync(long return Task.FromResult(result); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); // Act - var first = await loader.DiscoverAsync(CancellationToken.None); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); clock.Advance(TimeSpan.FromMilliseconds(remaining - 1)); - var fresh = await loader.DiscoverAsync(CancellationToken.None); + var fresh = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Same(first, fresh); @@ -182,7 +183,7 @@ public async Task DiscoverAsync_PaginatedTtl_ExpiresWhenAnyPageExpiresAsync(long // Act clock.Advance(TimeSpan.FromMilliseconds(1)); - var refreshed = await loader.DiscoverAsync(CancellationToken.None); + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.NotSame(first, refreshed); @@ -196,7 +197,7 @@ public async Task DiscoverAsync_PaginatedTtl_ExpiresWhenAnyPageExpiresAsync(long [InlineData(0L, true)] [InlineData(-1L, false)] [InlineData(-1L, true)] - public async Task DiscoverAsync_StaleEmptyPage_PreventsCachingWholeCatalogAsync(long? ttl, bool emptyFirstPage) + public async Task GetSkillsAsync_StaleEmptyPage_PreventsCachingWholeCatalogAsync(long? ttl, bool emptyFirstPage) { // Arrange int calls = 0; @@ -212,18 +213,18 @@ public async Task DiscoverAsync_StaleEmptyPage_PreventsCachingWholeCatalogAsync( return Task.FromResult(result); }); - using var loader = new McpSkillsLoader(client.Object); + using var source = new AgentMcpSkillsSource(client.Object); // Act - await loader.DiscoverAsync(CancellationToken.None); - await loader.DiscoverAsync(CancellationToken.None); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Equal(4, calls); } [Fact] - public async Task DiscoverAsync_PageExpiresDuringPagination_DoesNotRestartItsClockAsync() + public async Task GetSkillsAsync_PageExpiresDuringPagination_DoesNotRestartItsClockAsync() { // Arrange var clock = new ManualTimeProvider(); @@ -243,18 +244,18 @@ public async Task DiscoverAsync_PageExpiresDuringPagination_DoesNotRestartItsClo return Task.FromResult(result); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); // Act - await loader.DiscoverAsync(CancellationToken.None); - await loader.DiscoverAsync(CancellationToken.None); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Equal(4, calls); } [Fact] - public async Task DiscoverAsync_SlowRequest_TtlStartsAtResponseReceiptAsync() + public async Task GetSkillsAsync_SlowRequest_TtlStartsAtResponseReceiptAsync() { // Arrange var clock = new ManualTimeProvider(); @@ -265,11 +266,11 @@ public async Task DiscoverAsync_SlowRequest_TtlStartsAtResponseReceiptAsync() clock.Advance(TimeSpan.FromSeconds(2)); return Task.FromResult(TtlListResult(1000)); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); // Act - var first = await loader.DiscoverAsync(CancellationToken.None); - var second = await loader.DiscoverAsync(CancellationToken.None); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); + var second = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Same(first, second); @@ -279,7 +280,7 @@ public async Task DiscoverAsync_SlowRequest_TtlStartsAtResponseReceiptAsync() [Theory] [InlineData(false)] [InlineData(true)] - public async Task DiscoverAsync_ConcurrentCalls_ShareSuccessfulFreshDiscoveryAsync(bool refresh) + public async Task GetSkillsAsync_ConcurrentCalls_ShareSuccessfulFreshDiscoveryAsync(bool refresh) { // Arrange var clock = new ManualTimeProvider(); @@ -290,16 +291,16 @@ public async Task DiscoverAsync_ConcurrentCalls_ShareSuccessfulFreshDiscoveryAsy calls++; return refresh && calls == 1 ? Task.FromResult(TtlListResult(1000, Entry())) : release.Task; }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); if (refresh) { - await loader.DiscoverAsync(CancellationToken.None); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); clock.Advance(TimeSpan.FromSeconds(1)); } // Act Task>[] pending = Enumerable.Range(0, 8) - .Select(_ => loader.DiscoverAsync(CancellationToken.None)).ToArray(); + .Select(_ => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)).ToArray(); release.SetResult(TtlListResult(1000, Entry())); var results = await Task.WhenAll(pending).WaitAsync(TimeSpan.FromSeconds(10)); @@ -309,18 +310,18 @@ public async Task DiscoverAsync_ConcurrentCalls_ShareSuccessfulFreshDiscoveryAsy } [Fact] - public async Task DiscoverAsync_ConcurrentCalls_ZeroTtlDoesNotShareStaleCatalogAsync() + public async Task GetSkillsAsync_ConcurrentCalls_ZeroTtlDoesNotShareStaleCatalogAsync() { // Arrange var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); int calls = 0; var client = CreateTtlClient((_, _) => ++calls == 1 ? release.Task : Task.FromResult(TtlListResult(0, Entry()))); - using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: new ManualTimeProvider()); // Act Task>[] pending = Enumerable.Range(0, 8) - .Select(_ => loader.DiscoverAsync(CancellationToken.None)).ToArray(); + .Select(_ => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)).ToArray(); // Assert Assert.Equal(1, calls); @@ -338,7 +339,7 @@ public async Task DiscoverAsync_ConcurrentCalls_ZeroTtlDoesNotShareStaleCatalogA } [Fact] - public async Task DiscoverAsync_CanceledWaiter_DoesNotCancelSharedDiscoveryAsync() + public async Task GetSkillsAsync_CanceledWaiter_DoesNotCancelSharedDiscoveryAsync() { // Arrange var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -348,12 +349,12 @@ public async Task DiscoverAsync_CanceledWaiter_DoesNotCancelSharedDiscoveryAsync calls++; return release.Task.WaitAsync(token); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: new ManualTimeProvider()); using var cts = new CancellationTokenSource(); // Act - var initiating = loader.DiscoverAsync(CancellationToken.None); - var waiting = loader.DiscoverAsync(cts.Token); + var initiating = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); + var waiting = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), cts.Token); await cts.CancelAsync(); try { @@ -366,52 +367,52 @@ public async Task DiscoverAsync_CanceledWaiter_DoesNotCancelSharedDiscoveryAsync } var first = await initiating.WaitAsync(TimeSpan.FromSeconds(10)); - Assert.Same(first, await loader.DiscoverAsync(CancellationToken.None)); + Assert.Same(first, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); Assert.Equal(1, calls); } [Fact] - public async Task DiscoverAsync_CanceledInitiator_WaiterRetriesDiscoveryAsync() + public async Task GetSkillsAsync_CanceledInitiator_WaiterRetriesDiscoveryAsync() { // Arrange var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); int calls = 0; var client = CreateTtlClient((_, token) => ++calls == 1 ? release.Task.WaitAsync(token) : Task.FromResult(TtlListResult(1000, Entry()))); - using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: new ManualTimeProvider()); using var cts = new CancellationTokenSource(); // Act - var initiating = loader.DiscoverAsync(cts.Token); - var waiting = loader.DiscoverAsync(CancellationToken.None); + var initiating = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), cts.Token); + var waiting = source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); await cts.CancelAsync(); // Assert await Assert.ThrowsAnyAsync(() => initiating.WaitAsync(TimeSpan.FromSeconds(10))); var skills = await waiting.WaitAsync(TimeSpan.FromSeconds(10)); Assert.Single(skills); - Assert.Same(skills, await loader.DiscoverAsync(CancellationToken.None)); + Assert.Same(skills, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); Assert.Equal(2, calls); } [Theory] [InlineData(false)] [InlineData(true)] - public async Task DiscoverAsync_CanceledToken_DoesNotDiscoverOrReturnFreshCacheAsync(bool cached) + public async Task GetSkillsAsync_CanceledToken_DoesNotDiscoverOrReturnFreshCacheAsync(bool cached) { // Arrange var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(1000, Entry()))); - using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: new ManualTimeProvider()); if (cached) { - await loader.DiscoverAsync(CancellationToken.None); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); } using var cts = new CancellationTokenSource(); await cts.CancelAsync(); // Act & Assert - await Assert.ThrowsAnyAsync(() => loader.DiscoverAsync(cts.Token)); + await Assert.ThrowsAnyAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), cts.Token)); client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Exactly(cached ? 1 : 0)); } @@ -420,7 +421,7 @@ public async Task DiscoverAsync_CanceledToken_DoesNotDiscoverOrReturnFreshCacheA [InlineData("page", typeof(JsonException))] [InlineData("entry", typeof(InvalidOperationException))] [InlineData("manifest", typeof(InvalidOperationException))] - public async Task DiscoverAsync_FailedRefresh_PropagatesAndRetriesWithoutServingStaleCatalogAsync(string failure, Type exceptionType) + public async Task GetSkillsAsync_FailedRefresh_PropagatesAndRetriesWithoutServingStaleCatalogAsync(string failure, Type exceptionType) { // Arrange var clock = new ManualTimeProvider(); @@ -443,22 +444,22 @@ public async Task DiscoverAsync_FailedRefresh_PropagatesAndRetriesWithoutServing return Task.FromResult(TtlListResult(1000, Entry())); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); - var first = await loader.DiscoverAsync(CancellationToken.None); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); clock.Advance(TimeSpan.FromSeconds(1)); // Act & Assert - await Assert.ThrowsAsync(exceptionType, () => loader.DiscoverAsync(CancellationToken.None)); - var refreshed = await loader.DiscoverAsync(CancellationToken.None); + await Assert.ThrowsAsync(exceptionType, () => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); Assert.NotSame(first, refreshed); Assert.Equal(3, calls); - Assert.Same(refreshed, await loader.DiscoverAsync(CancellationToken.None)); + Assert.Same(refreshed, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); } [Theory] [InlineData(false)] [InlineData(true)] - public async Task DiscoverAsync_CanceledRefreshAfterResponse_DoesNotCacheReplacementAsync(bool laterPage) + public async Task GetSkillsAsync_CanceledRefreshAfterResponse_DoesNotCacheReplacementAsync(bool laterPage) { // Arrange var clock = new ManualTimeProvider(); @@ -482,23 +483,23 @@ public async Task DiscoverAsync_CanceledRefreshAfterResponse_DoesNotCacheReplace return Task.FromResult(result); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); - var first = await loader.DiscoverAsync(CancellationToken.None); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); clock.Advance(TimeSpan.FromSeconds(1)); // Act & Assert - await Assert.ThrowsAnyAsync(() => loader.DiscoverAsync(cts.Token)); - var refreshed = await loader.DiscoverAsync(CancellationToken.None); + await Assert.ThrowsAnyAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), cts.Token)); + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); Assert.NotSame(first, refreshed); Assert.Equal(laterPage ? 6 : 3, calls); Assert.Equal(laterPage ? [null, "next", null, "next", null, "next"] : new string?[] { null, null, null }, cursors); - Assert.Same(refreshed, await loader.DiscoverAsync(CancellationToken.None)); + Assert.Same(refreshed, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); } [Theory] [InlineData(false)] [InlineData(true)] - public async Task DiscoverAsync_TtlRefresh_ReplacesCatalogAsync(bool empty) + public async Task GetSkillsAsync_TtlRefresh_ReplacesCatalogAsync(bool empty) { // Arrange var clock = new ManualTimeProvider(); @@ -516,12 +517,12 @@ public async Task DiscoverAsync_TtlRefresh_ReplacesCatalogAsync(bool empty) added["frontmatter"]!["description"] = "Added skill."; return Task.FromResult(empty ? TtlListResult(1000) : TtlListResult(1000, updated, added)); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); - var first = await loader.DiscoverAsync(CancellationToken.None); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Act clock.Advance(TimeSpan.FromSeconds(1)); - var refreshed = await loader.DiscoverAsync(CancellationToken.None); + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.NotSame(first, refreshed); @@ -537,12 +538,12 @@ public async Task DiscoverAsync_TtlRefresh_ReplacesCatalogAsync(bool empty) Assert.All(refreshed, skill => Assert.DoesNotContain(skill, first)); } - Assert.Same(refreshed, await loader.DiscoverAsync(CancellationToken.None)); + Assert.Same(refreshed, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); Assert.Equal(2, calls); } [Fact] - public async Task DiscoverAsync_InvalidLaterRefreshPage_DoesNotCachePartialCatalogAsync() + public async Task GetSkillsAsync_InvalidLaterRefreshPage_DoesNotCachePartialCatalogAsync() { // Arrange var clock = new ManualTimeProvider(); @@ -563,21 +564,21 @@ public async Task DiscoverAsync_InvalidLaterRefreshPage_DoesNotCachePartialCatal return Task.FromResult(result); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); - var first = await loader.DiscoverAsync(CancellationToken.None); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); + var first = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); clock.Advance(TimeSpan.FromSeconds(1)); // Act & Assert - await Assert.ThrowsAsync(() => loader.DiscoverAsync(CancellationToken.None)); - var refreshed = await loader.DiscoverAsync(CancellationToken.None); + await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); + var refreshed = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); Assert.Single(refreshed); Assert.NotSame(first, refreshed); Assert.Equal([null, "next", null, "next", null, "next"], cursors); - Assert.Same(refreshed, await loader.DiscoverAsync(CancellationToken.None)); + Assert.Same(refreshed, await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); } [Fact] - public async Task DiscoverAsync_TtlRefresh_PreservesPreviouslyReturnedManifestAsync() + public async Task GetSkillsAsync_TtlRefresh_PreservesPreviouslyReturnedManifestAsync() { // Arrange var clock = new ManualTimeProvider(); @@ -592,12 +593,12 @@ public async Task DiscoverAsync_TtlRefresh_PreservesPreviouslyReturnedManifestAs return Task.FromResult(TtlListResult(1000, entry)); }); - using var loader = new McpSkillsLoader(client.Object, timeProvider: clock); - var first = Assert.Single(await loader.DiscoverAsync(CancellationToken.None)); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: clock); + var first = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); // Act clock.Advance(TimeSpan.FromSeconds(1)); - var updated = Assert.Single(await loader.DiscoverAsync(CancellationToken.None)); + var updated = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); // Assert Assert.NotSame(first, updated); @@ -608,28 +609,29 @@ public async Task DiscoverAsync_TtlRefresh_PreservesPreviouslyReturnedManifestAs [Theory] [InlineData(false)] [InlineData(true)] - public async Task DiscoverAsync_DisposedLoader_ThrowsWithoutDisposingClientAsync(bool cached) + public async Task GetSkillsAsync_DisposedSource_ThrowsWithoutDisposingClientAsync(bool cached) { // Arrange var client = CreateTtlClient((_, _) => Task.FromResult(TtlListResult(1000, Entry()))); - using var loader = new McpSkillsLoader(client.Object, timeProvider: new ManualTimeProvider()); + using var source = new AgentMcpSkillsSource(client.Object, options: null, loggerFactory: null, timeProvider: new ManualTimeProvider()); if (cached) { - await loader.DiscoverAsync(CancellationToken.None); + await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); } // Act - loader.Dispose(); - loader.Dispose(); + source.Dispose(); + source.Dispose(); // Assert - await Assert.ThrowsAsync(() => loader.DiscoverAsync(CancellationToken.None)); + await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Exactly(cached ? 1 : 0)); + client.VerifyGet(c => c.ServerCapabilities, Times.Exactly(cached ? 2 : 1)); client.VerifyNoOtherCalls(); } [Fact] - public async Task GetSkillsAsync_DisposedSource_DisposesLoaderButNotClientAsync() + public async Task GetSkillsAsync_DisposedSource_DisposesCacheButNotClientAsync() { // Arrange int calls = 0; @@ -740,9 +742,22 @@ private static JsonObject TtlListResult(long? ttl, params JsonNode?[] entries) return result; } - private static Mock CreateTtlClient(Func> handler) + private static Mock CreateSkillsClient() { var client = new Mock(); + client.SetupGet(c => c.ServerCapabilities).Returns(new ServerCapabilities + { + Extensions = new Dictionary + { + ["io.modelcontextprotocol/skills"] = new JsonObject(), + }, + }); + return client; + } + + private static Mock CreateTtlClient(Func> handler) + { + var client = CreateSkillsClient(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .Returns((JsonRpcRequest request, CancellationToken token) => RespondAsync(request, token)); return client; 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 0b300e2b6bd..d81128586ec 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs @@ -309,11 +309,11 @@ public async Task GetSkillsAsync_SkillLimit_CountsEntriesAcrossPagesIncludingDup [Theory] [InlineData(1)] [InlineData(2)] - public async Task DiscoverAsync_LargeUnknownFields_AreAcceptedAcrossPagesAsync(int pageCount) + public async Task GetSkillsAsync_LargeUnknownFields_AreAcceptedAcrossPagesAsync(int pageCount) { // Arrange string padding = new('x', 10 * 1024 * 1024 / pageCount); - var client = new Mock(); + var client = CreateSkillsClient(); var responses = client.SetupSequence(c => c.SendRequestAsync(It.IsAny(), It.IsAny())); for (int page = 0; page < pageCount; page++) { @@ -327,10 +327,10 @@ public async Task DiscoverAsync_LargeUnknownFields_AreAcceptedAcrossPagesAsync(i responses.ReturnsAsync(new JsonRpcResponse { Id = new RequestId(page + 1), Result = result }); } - using var loader = new McpSkillsLoader(client.Object); + using var source = new AgentMcpSkillsSource(client.Object); // Act - var skills = await loader.DiscoverAsync(CancellationToken.None); + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Equal(pageCount, skills.Count); @@ -338,23 +338,23 @@ public async Task DiscoverAsync_LargeUnknownFields_AreAcceptedAcrossPagesAsync(i } [Fact] - public async Task DiscoverAsync_SkillLimit_RejectsExcessEntriesBeforeTypedDeserializationAsync() + public async Task GetSkillsAsync_SkillLimit_RejectsExcessEntriesBeforeTypedDeserializationAsync() { // Arrange - var client = new Mock(); + var client = CreateSkillsClient(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = ListResult(Entry(), new JsonObject()) }); - using var loader = new McpSkillsLoader(client.Object, options: new() { ListedMaxSkillCount = 1 }); + using var source = new AgentMcpSkillsSource(client.Object, options: new() { ListedMaxSkillCount = 1 }); // Act - var exception = await Assert.ThrowsAsync(() => loader.DiscoverAsync(CancellationToken.None)); + var exception = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); // Assert Assert.Contains("skill count (1)", exception.Message); } [Fact] - public async Task DiscoverAsync_LargeResourceManifest_HasNoSeparateResourceCountLimitAsync() + public async Task GetSkillsAsync_LargeResourceManifest_HasNoSeparateResourceCountLimitAsync() { // Arrange JsonObject entry = Entry(); @@ -364,13 +364,13 @@ public async Task DiscoverAsync_LargeResourceManifest_HasNoSeparateResourceCount resources.Add(Resource($"skill://unit-converter/file-{i}.txt")); } - var client = new Mock(); + var client = CreateSkillsClient(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = ListResult(entry) }); - using var loader = new McpSkillsLoader(client.Object); + using var source = new AgentMcpSkillsSource(client.Object); // Act - var skills = await loader.DiscoverAsync(CancellationToken.None); + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Single(skills); @@ -384,7 +384,7 @@ public async Task DiscoverAsync_LargeResourceManifest_HasNoSeparateResourceCount public void Constructor_InvalidDiscoveryLimit_Throws(string option, int limit) { // Arrange - var client = new Mock(); + var client = CreateSkillsClient(); var options = new AgentMcpSkillsSourceOptions { ListedMaxPageCount = option == nameof(AgentMcpSkillsSourceOptions.ListedMaxPageCount) ? limit : null, @@ -451,16 +451,16 @@ public async Task GetSkillsAsync_InvalidListResult_ThrowsAsync(string result) } [Fact] - public async Task DiscoverAsync_MissingResult_ThrowsAsync() + public async Task GetSkillsAsync_MissingResult_ThrowsAsync() { // Arrange - var client = new Mock(); + var client = CreateSkillsClient(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = null }); - using var loader = new McpSkillsLoader(client.Object); + using var source = new AgentMcpSkillsSource(client.Object); // Act - InvalidOperationException exception = await Assert.ThrowsAsync(() => loader.DiscoverAsync(CancellationToken.None)); + InvalidOperationException exception = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None)); // Assert Assert.Equal("The server returned no skills/list result.", exception.Message); @@ -1020,7 +1020,7 @@ public async Task GetSkillsAsync_RaisedResourceLimit_AllowsLargerReadsAsync() public void Constructor_InvalidResourceLimit_Throws(long limit) { // Arrange - var client = new Mock(); + var client = CreateSkillsClient(); var options = new AgentMcpSkillsSourceOptions { ListedResourceMaxSizeBytes = limit, @@ -1034,18 +1034,18 @@ public void Constructor_InvalidResourceLimit_Throws(long limit) } [Fact] - public async Task DiscoverAsync_LargeManifest_IsNotSubjectToResourceSizeLimitAsync() + public async Task GetSkillsAsync_LargeManifest_IsNotSubjectToResourceSizeLimitAsync() { // Arrange JsonObject result = ListResult(Entry()); result["padding"] = new string('x', 1_048_577); - var client = new Mock(); + var client = CreateSkillsClient(); client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = result }); - using var loader = new McpSkillsLoader(client.Object); + using var source = new AgentMcpSkillsSource(client.Object); // Act - var skills = await loader.DiscoverAsync(CancellationToken.None); + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create(), CancellationToken.None); // Assert Assert.Single(skills); From 2eb91b86da35eba4574d0fa769a383cd489b9be1 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 23 Oct 2026 17:29:27 +0100 Subject: [PATCH 4/6] Move MCP skill caching into McpSkillsLoader Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsSource.cs | 8 +-- .../{McpSkillsCache.cs => McpSkillsLoader.cs} | 4 +- ...sCacheTests.cs => McpSkillsLoaderTests.cs} | 56 +++++++++---------- 3 files changed, 34 insertions(+), 34 deletions(-) rename dotnet/src/Microsoft.Agents.AI.Mcp/Skills/{McpSkillsCache.cs => McpSkillsLoader.cs} (95%) rename dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/{McpSkillsCacheTests.cs => McpSkillsLoaderTests.cs} (78%) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index a13678f3fb6..98377748572 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -87,7 +87,7 @@ public sealed partial class AgentMcpSkillsSource : AgentSkillsSource private readonly int _maxPageCount; private readonly int _maxSkillCount; private readonly TimeProvider _timeProvider; - private readonly McpSkillsCache _cache; + private readonly McpSkillsLoader _skillsLoader; /// /// Initializes a new instance of the class. @@ -114,7 +114,7 @@ internal AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? opt ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxResourceSizeBytes, nameof(options.ListedResourceMaxSizeBytes)); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxPageCount, nameof(options.ListedMaxPageCount)); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxSkillCount, nameof(options.ListedMaxSkillCount)); - this._cache = new McpSkillsCache(this._timeProvider, this.DiscoverCoreAsync); + this._skillsLoader = new McpSkillsLoader(this._timeProvider, this.DiscoverCoreAsync); } /// @@ -132,7 +132,7 @@ public override async Task> GetSkillsAsync(AgentSkillsSourceCo return []; } - IList skills = await this._cache.GetOrCreateAsync(cancellationToken).ConfigureAwait(false); + IList skills = await this._skillsLoader.GetOrCreateAsync(cancellationToken).ConfigureAwait(false); LogSkillsLoadedTotal(this._logger, skills.Count); @@ -144,7 +144,7 @@ protected override void Dispose(bool disposing) { if (disposing) { - this._cache.Dispose(); + this._skillsLoader.Dispose(); } base.Dispose(disposing); diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCache.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs similarity index 95% rename from dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCache.cs rename to dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs index 1888f010950..8552db36e85 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsCache.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/McpSkillsLoader.cs @@ -11,7 +11,7 @@ namespace Microsoft.Agents.AI; /// /// Caches MCP-listed skills until any page's TTL expires. /// -internal sealed class McpSkillsCache : IDisposable +internal sealed class McpSkillsLoader : IDisposable { private readonly TimeProvider _timeProvider; private readonly Func> _discoverSkills; @@ -24,7 +24,7 @@ internal sealed class McpSkillsCache : IDisposable /// /// The clock for checking page freshness. /// Discovers and validates all pages, including their receipt timestamps and TTLs. - public McpSkillsCache(TimeProvider timeProvider, Func> discoverSkills) + public McpSkillsLoader(TimeProvider timeProvider, Func> discoverSkills) { this._timeProvider = Throw.IfNull(timeProvider); this._discoverSkills = Throw.IfNull(discoverSkills); diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCacheTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsLoaderTests.cs similarity index 78% rename from dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCacheTests.cs rename to dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsLoaderTests.cs index 0a716971870..f11c326bf03 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsCacheTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/McpSkillsLoaderTests.cs @@ -8,7 +8,7 @@ namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; -public sealed class McpSkillsCacheTests +public sealed class McpSkillsLoaderTests { [Fact] public void Constructor_NullDiscoverSkills_ThrowsArgumentNullException() @@ -17,7 +17,7 @@ public void Constructor_NullDiscoverSkills_ThrowsArgumentNullException() var clock = new ManualTimeProvider(); // Act & Assert - Assert.Throws("discoverSkills", () => new McpSkillsCache(clock, null!)); + Assert.Throws("discoverSkills", () => new McpSkillsLoader(clock, null!)); } [Fact] @@ -34,12 +34,12 @@ Task DiscoverAsync(CancellationToken token) return Task.FromResult(discovered); } - using var cache = new McpSkillsCache(clock, DiscoverAsync); + using var loader = new McpSkillsLoader(clock, DiscoverAsync); // Act - var first = await cache.GetOrCreateAsync(CancellationToken.None); + var first = await loader.GetOrCreateAsync(CancellationToken.None); clock.Advance(TimeSpan.FromMilliseconds(999)); - var fresh = await cache.GetOrCreateAsync(CancellationToken.None); + var fresh = await loader.GetOrCreateAsync(CancellationToken.None); // Assert Assert.NotNull(discovered); @@ -50,7 +50,7 @@ Task DiscoverAsync(CancellationToken token) // Act clock.Advance(TimeSpan.FromMilliseconds(1)); - var refreshed = await cache.GetOrCreateAsync(CancellationToken.None); + var refreshed = await loader.GetOrCreateAsync(CancellationToken.None); // Assert Assert.NotSame(first, refreshed); @@ -75,12 +75,12 @@ Task DiscoverAsync(CancellationToken token) return Task.FromResult(CreateSnapshot(clock, ttls)); } - using var cache = new McpSkillsCache(clock, DiscoverAsync); - var first = await cache.GetOrCreateAsync(CancellationToken.None); + using var loader = new McpSkillsLoader(clock, DiscoverAsync); + var first = await loader.GetOrCreateAsync(CancellationToken.None); // Act clock.Advance(TimeSpan.FromSeconds(1)); - var refreshed = await cache.GetOrCreateAsync(CancellationToken.None); + var refreshed = await loader.GetOrCreateAsync(CancellationToken.None); // Assert Assert.NotSame(first, refreshed); @@ -98,11 +98,11 @@ public async Task GetOrCreateAsync_ConcurrentCalls_ReuseOnlyFreshResultsAsync(lo int calls = 0; Task DiscoverAsync(CancellationToken token) => ++calls == 1 ? release.Task : Task.FromResult(CreateSnapshot(clock, ttl)); - using var cache = new McpSkillsCache(clock, DiscoverAsync); + using var loader = new McpSkillsLoader(clock, DiscoverAsync); // Act Task>[] pending = Enumerable.Range(0, 8) - .Select(_ => cache.GetOrCreateAsync(CancellationToken.None)).ToArray(); + .Select(_ => loader.GetOrCreateAsync(CancellationToken.None)).ToArray(); Assert.Equal(1, calls); release.SetResult(CreateSnapshot(clock, ttl)); var results = await Task.WhenAll(pending).WaitAsync(TimeSpan.FromSeconds(10)); @@ -130,9 +130,9 @@ public async Task GetOrCreateAsync_FailedDiscovery_PreservesErrorAndReleasesWait int calls = 0; Task DiscoverAsync(CancellationToken token) => ++calls == 1 ? release.Task : Task.FromResult(CreateSnapshot(clock, 1000)); - using var cache = new McpSkillsCache(clock, DiscoverAsync); - var active = cache.GetOrCreateAsync(CancellationToken.None); - var waiting = cache.GetOrCreateAsync(CancellationToken.None); + using var loader = new McpSkillsLoader(clock, DiscoverAsync); + var active = loader.GetOrCreateAsync(CancellationToken.None); + var waiting = loader.GetOrCreateAsync(CancellationToken.None); // Act release.SetException(error); @@ -141,7 +141,7 @@ Task DiscoverAsync(CancellationToken token) => Assert.Same(error, await Assert.ThrowsAsync(() => active.WaitAsync(TimeSpan.FromSeconds(10)))); var skills = await waiting.WaitAsync(TimeSpan.FromSeconds(10)); Assert.Single(skills); - Assert.Same(skills, await cache.GetOrCreateAsync(CancellationToken.None)); + Assert.Same(skills, await loader.GetOrCreateAsync(CancellationToken.None)); Assert.Equal(2, calls); } @@ -163,14 +163,14 @@ Task DiscoverAsync(CancellationToken token) return Task.FromResult(CreateSnapshot(clock, 1000)); } - using var cache = new McpSkillsCache(clock, DiscoverAsync); + using var loader = new McpSkillsLoader(clock, DiscoverAsync); // Act & Assert - await Assert.ThrowsAnyAsync(() => cache.GetOrCreateAsync(cts.Token)); - var skills = await cache.GetOrCreateAsync(CancellationToken.None); + await Assert.ThrowsAnyAsync(() => loader.GetOrCreateAsync(cts.Token)); + var skills = await loader.GetOrCreateAsync(CancellationToken.None); Assert.Single(skills); Assert.Equal(2, calls); - Assert.Same(skills, await cache.GetOrCreateAsync(CancellationToken.None)); + Assert.Same(skills, await loader.GetOrCreateAsync(CancellationToken.None)); } [Theory] @@ -187,25 +187,25 @@ Task DiscoverAsync(CancellationToken token) return Task.FromResult(CreateSnapshot(clock, 1000)); } - using var cache = new McpSkillsCache(clock, DiscoverAsync); + using var loader = new McpSkillsLoader(clock, DiscoverAsync); if (cached) { - await cache.GetOrCreateAsync(CancellationToken.None); + await loader.GetOrCreateAsync(CancellationToken.None); } using var cts = new CancellationTokenSource(); await cts.CancelAsync(); // Act & Assert - await Assert.ThrowsAnyAsync(() => cache.GetOrCreateAsync(cts.Token)); + await Assert.ThrowsAnyAsync(() => loader.GetOrCreateAsync(cts.Token)); Assert.Equal(cached ? 1 : 0, calls); } [Theory] [InlineData(false)] [InlineData(true)] - public async Task GetOrCreateAsync_DisposedCache_ThrowsWithoutInvokingDiscoveryAsync(bool cached) + public async Task GetOrCreateAsync_DisposedLoader_ThrowsWithoutInvokingDiscoveryAsync(bool cached) { // Arrange var clock = new ManualTimeProvider(); @@ -216,19 +216,19 @@ Task DiscoverAsync(CancellationToken token) return Task.FromResult(CreateSnapshot(clock, 1000)); } - using var cache = new McpSkillsCache(clock, DiscoverAsync); + using var loader = new McpSkillsLoader(clock, DiscoverAsync); if (cached) { - await cache.GetOrCreateAsync(CancellationToken.None); + await loader.GetOrCreateAsync(CancellationToken.None); } // Act - cache.Dispose(); - cache.Dispose(); + loader.Dispose(); + loader.Dispose(); // Assert - await Assert.ThrowsAsync(() => cache.GetOrCreateAsync(CancellationToken.None)); + await Assert.ThrowsAsync(() => loader.GetOrCreateAsync(CancellationToken.None)); Assert.Equal(cached ? 1 : 0, calls); } From f4cd0be1d4f46960cbab5d9042280525fe3c2975 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 23 Oct 2026 17:36:02 +0100 Subject: [PATCH 5/6] Remove unused MCP client import from skills tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsTests.cs | 1 - 1 file changed, 1 deletion(-) 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 d81128586ec..58d11c56ff3 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsTests.cs @@ -13,7 +13,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using ModelContextProtocol; -using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; using Moq; From b5642d5b393a06d81512323bfbd63499516af65c Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 23 Oct 2026 19:11:45 +0100 Subject: [PATCH 6/6] Update Foundry MCP skills sample documentation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../foundry/Agent_Step26_FoundryToolboxMcpSkills/README.md | 4 ++-- dotnet/samples/02-agents/AgentProviders/foundry/README.md | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/dotnet/samples/02-agents/AgentProviders/foundry/Agent_Step26_FoundryToolboxMcpSkills/README.md b/dotnet/samples/02-agents/AgentProviders/foundry/Agent_Step26_FoundryToolboxMcpSkills/README.md index 9896fe978af..61b6ab1de15 100644 --- a/dotnet/samples/02-agents/AgentProviders/foundry/Agent_Step26_FoundryToolboxMcpSkills/README.md +++ b/dotnet/samples/02-agents/AgentProviders/foundry/Agent_Step26_FoundryToolboxMcpSkills/README.md @@ -8,7 +8,7 @@ and inject them as `AIContextProviders` so the agent can discover and use them a - Connecting to a Foundry toolbox's MCP endpoint via Streamable HTTP transport - Injecting a fresh Azure AI bearer token (`https://ai.azure.com/.default`) on every MCP request -- Using `AgentSkillsProviderBuilder.UseMcpSkills(client)` to discover skills from the toolbox +- Using `AgentSkillsProviderBuilder.UseMcpSkills(client)` to discover skills from the toolbox through `skills/list` - Injecting the discovered skills into `AIProjectClient.AsAIAgent(...)` via `AIContextProviders` - Auto-approving skill tools with `UseToolApproval` and `AgentSkillsProvider.AllToolsAutoApprovalRule` @@ -19,7 +19,7 @@ for a walkthrough of the full approval flow. ## Prerequisites - A Microsoft Foundry project with a toolbox already configured -- The toolbox MCP endpoint must expose `skill://index.json` with `skill-md` entries (SEP-2640). If the resource is absent, the sample runs but the skills provider will be empty. +- The toolbox MCP endpoint must declare the `io.modelcontextprotocol/skills` extension and implement `skills/list` and resource reads for the advertised skills. If the extension is not declared, the skills provider will be empty. - An authenticated Azure identity (for example, sign in with `az login`) Set the following environment variables: diff --git a/dotnet/samples/02-agents/AgentProviders/foundry/README.md b/dotnet/samples/02-agents/AgentProviders/foundry/README.md index eaae6be594f..40c75f4113f 100644 --- a/dotnet/samples/02-agents/AgentProviders/foundry/README.md +++ b/dotnet/samples/02-agents/AgentProviders/foundry/README.md @@ -74,7 +74,7 @@ Some samples require extra tool-specific environment variables. See each sample | [Local MCP](./Agent_Step23_LocalMCP/) | Local MCP client with HTTP transport | | [Code interpreter file download](./Agent_Step24_CodeInterpreterFileDownload/) | Download container files generated by code interpreter | | [Foundry toolbox via MCP](./Agent_Step25_FoundryToolboxMcp/README.md) | Use a Foundry Toolbox from a non-hosted agent via its MCP endpoint | -| [Foundry toolbox MCP skills](./Agent_Step26_FoundryToolboxMcpSkills/README.md) | Use a Foundry Toolbox with MCP-based skills discovery (SEP-2640) via AIContextProviders | +| [Foundry toolbox MCP skills](./Agent_Step26_FoundryToolboxMcpSkills/README.md) | Use a Foundry Toolbox with MCP Skills extension discovery through `skills/list` via AIContextProviders | ## Running the samples @@ -87,4 +87,3 @@ dotnet run --project .\Agent_Step01_Basics If you want to exercise the full create-run-delete lifecycle, run `Agent_Step00_FoundryAgentLifecycle`. -