.NET: [BREAKING] Support archive-type skills in AgentMcpSkillsSource

dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs

@@ -2,7 +2,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Text.Json;
 using System.Threading;
@@ -22,19 +21,22 @@ namespace Microsoft.Agents.AI;
 /// <remarks>
 /// <para>
 /// Discovery follows the SEP-2640 recommended approach: the source reads the well-known
-/// <c>skill://index.json</c> resource and constructs one <see cref="AgentSkill"/> per
-/// <c>skill-md</c> entry directly from the entry's <c>name</c>, <c>description</c>, and <c>url</c> fields.
-/// The referenced <c>SKILL.md</c> resource is not read during discovery; hosts fetch its body on
-/// demand via <c>resources/read</c> against the URI exposed on the resulting skill.
+/// <c>skill://index.json</c> resource and constructs one <see cref="AgentSkill"/> per index entry.
 /// </para>
 /// <para>
-/// Only index entries of type <c>skill-md</c> are supported at the moment; entries of any other
-/// type are skipped.
+/// Index entries are dispatched to an <see cref="IMcpSkillEntryLoader"/> by their <c>type</c>:
+/// <list type="bullet">
+///   <item><description><c>skill-md</c> - handled by <see cref="SkillMdEntryLoader"/>; the skill's
+///   <c>SKILL.md</c> and sibling resources are fetched on demand from the MCP server.</description></item>
+///   <item><description><c>archive</c> - handled by <see cref="ArchiveEntryLoader"/>; the entry's
+///   <c>url</c> points to a single archive resource whose content unpacks into the skill's
+///   namespace.</description></item>
+/// </list>
+/// Entries whose type has no registered loader (e.g. <c>mcp-resource-template</c>) are skipped.
 /// </para>
 /// <para>
-/// If <c>skill://index.json</c> is absent, unreadable, empty, or fails to parse, this source
-/// returns an empty list. Discovered skills serve their referenced resources on demand via
-/// <see cref="AgentSkill.GetResourceAsync"/>; they do not enumerate sibling files up front.
+/// If <c>skill://index.json</c> is absent, unreadable, empty, or fails to parse, this source returns an
+/// empty list.
 /// </para>
 /// </remarks>
 internal sealed partial class AgentMcpSkillsSource : AgentSkillsSource
@@ -44,42 +46,148 @@ internal sealed partial class AgentMcpSkillsSource : AgentSkillsSource
     /// </summary>
     private const string IndexUri = "skill://index.json";
 
-    private const string SkillMdEntryType = "skill-md";
-
     private readonly McpClient _client;
     private readonly ILogger _logger;
+    private readonly Dictionary<string, IMcpSkillEntryLoader> _loaders;
+    private readonly TimeSpan? _refreshInterval;
+
+    private IList<AgentSkill>? _cachedSkills;
+    private DateTime _lastRefreshedUtc;
+    private Task<IList<AgentSkill>>? _refreshTask;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="AgentMcpSkillsSource"/> class.
     /// </summary>
     /// <param name="client">An MCP client connected to a server that exposes Agent Skills resources.</param>
+    /// <param name="options">Optional options that control archive-distributed skill handling.</param>
     /// <param name="loggerFactory">Optional logger factory.</param>
-    public AgentMcpSkillsSource(McpClient client, ILoggerFactory? loggerFactory = null)
+    public AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? options = null, ILoggerFactory? loggerFactory = null)
     {
         this._client = Throw.IfNull(client);
-        this._logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger<AgentMcpSkillsSource>();
+        loggerFactory ??= NullLoggerFactory.Instance;
+        this._logger = loggerFactory.CreateLogger<AgentMcpSkillsSource>();
+
+        IMcpSkillEntryLoader[] loaders =
+        [
+            new SkillMdEntryLoader(this._client, loggerFactory),
+            new ArchiveEntryLoader(this._client, options, loggerFactory),
+        ];
+
+        this._loaders = loaders.ToDictionary(l => l.EntryType, StringComparer.OrdinalIgnoreCase);
+        this._refreshInterval = options?.RefreshInterval;
     }
 
     /// <inheritdoc/>
     public override async Task<IList<AgentSkill>> GetSkillsAsync(CancellationToken cancellationToken = default)
+    {
+        if (this.TryGetCachedSkills() is { } cached)
+        {
+            return cached;
+        }
+
+        // Use CAS to ensure only one concurrent refresh runs; other callers await the same task.
+        var tcs = new TaskCompletionSource<IList<AgentSkill>>(TaskCreationOptions.RunContinuationsAsynchronously);
+
+        if (Interlocked.CompareExchange(ref this._refreshTask, tcs.Task, null) is { } existing)
+        {
+            // Wait for the in-flight refresh but let this caller cancel its own wait independently
+            // without aborting the shared refresh work.
+            return await existing.WaitAsync(cancellationToken).ConfigureAwait(false);
+        }
+
+        try
+        {
+            // The refresh owner uses CancellationToken.None so that a single caller's cancellation
+            // does not abort the shared refresh for all concurrent waiters.
+            var skills = await this.GetCoreSkillsAsync(CancellationToken.None).ConfigureAwait(false);
+
+            this.UpdateCache(skills);
+
+            tcs.SetResult(skills);
+
+            // Allow the current caller to observe cancellation without impacting other awaiters.
+            cancellationToken.ThrowIfCancellationRequested();
+
+            return skills;
+        }
+        catch (Exception ex)
+        {
+            tcs.TrySetException(ex);
+            throw;
+        }
+        finally
+        {
+            this._refreshTask = null;
+        }
+    }
+
+    /// <summary>
+    /// Returns the cached skill list if caching is enabled and the cache is still fresh;
+    /// otherwise returns <see langword="null"/>.
+    /// </summary>
+    private IList<AgentSkill>? TryGetCachedSkills()
+    {
+        if (this._refreshInterval is null || this._cachedSkills is null)
+        {
+            return null;
+        }
+
+        TimeSpan cacheAge = DateTime.UtcNow - this._lastRefreshedUtc;
+
+        if (cacheAge >= this._refreshInterval.Value)
+        {
+            return null;
+        }
+
+        return this._cachedSkills;
+    }
+
+    /// <summary>
+    /// Stores the skill list and records the refresh timestamp for cache freshness checks.
+    /// </summary>
+    private void UpdateCache(IList<AgentSkill> skills)
+    {
+        this._cachedSkills = skills;
+        this._lastRefreshedUtc = DateTime.UtcNow;
+    }
+
+    /// <summary>
+    /// Reads the skill index from the MCP server, dispatches entries to registered loaders, and
+    /// returns the aggregated skill list.
+    /// </summary>
+    private async Task<IList<AgentSkill>> GetCoreSkillsAsync(CancellationToken cancellationToken)
     {
         McpSkillIndex? index = await this.TryReadIndexAsync(cancellationToken).ConfigureAwait(false);
 
-        var skills = new List<AgentSkill>();
+        // 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<string, List<McpSkillIndexEntry>>(StringComparer.OrdinalIgnoreCase);
 
-        foreach (var entry in index?.Skills ?? [])
+        foreach (var group in (index?.Skills ?? []).GroupBy(e => e.Type ?? string.Empty, StringComparer.OrdinalIgnoreCase))
         {
-            if (this.TryCreateSkill(entry, out AgentMcpSkill? skill, out string skipReason))
+            if (this._loaders.ContainsKey(group.Key))
             {
-                skills.Add(skill);
-                LogSkillLoaded(this._logger, skill.Frontmatter.Name);
+                entriesByType[group.Key] = group.ToList();
             }
             else
             {
-                LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", skipReason);
+                foreach (var entry in group)
+                {
+                    LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", $"unsupported type '{entry.Type ?? "(none)"}'");
+                }
             }
         }
 
+        // 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<AgentSkill>();
+
+        foreach (var loader in this._loaders.Values)
+        {
+            var entries = entriesByType.TryGetValue(loader.EntryType, out List<McpSkillIndexEntry>? matched) ? matched : [];
+            skills.AddRange(await loader.LoadAsync(entries, cancellationToken).ConfigureAwait(false));
+        }
+
         LogSkillsLoadedTotal(this._logger, skills.Count);
 
         return skills;
@@ -124,44 +232,6 @@ public override async Task<IList<AgentSkill>> GetSkillsAsync(CancellationToken c
         }
     }
 
-    private bool TryCreateSkill(
-        McpSkillIndexEntry entry,
-        [NotNullWhen(true)] out AgentMcpSkill? skill,
-        out string skipReason)
-    {
-        skill = null;
-
-        if (!string.Equals(entry.Type, SkillMdEntryType, StringComparison.Ordinal))
-        {
-            skipReason = $"unsupported type '{entry.Type ?? "(none)"}'";
-            return false;
-        }
-
-        if (string.IsNullOrWhiteSpace(entry.Url))
-        {
-            skipReason = "missing required 'url' field";
-            return false;
-        }
-
-        AgentSkillFrontmatter frontmatter;
-        try
-        {
-            frontmatter = new AgentSkillFrontmatter(entry.Name!, entry.Description!);
-        }
-        catch (ArgumentException ex)
-        {
-            skipReason = $"invalid metadata: {ex.Message}";
-            return false;
-        }
-
-        skill = new AgentMcpSkill(frontmatter, entry.Url!, this._client);
-        skipReason = string.Empty;
-        return true;
-    }
-
-    [LoggerMessage(LogLevel.Information, "Loaded MCP skill: {SkillName}")]
-    private static partial void LogSkillLoaded(ILogger logger, string skillName);
-
     [LoggerMessage(LogLevel.Information, "Successfully loaded {Count} skills from MCP server")]
     private static partial void LogSkillsLoadedTotal(ILogger logger, int count);
 

dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs

@@ -0,0 +1,98 @@
+// Copyright (c) Microsoft. All rights reserved.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using Microsoft.Shared.DiagnosticIds;
+
+namespace Microsoft.Agents.AI;
+
+/// <summary>
+/// Configuration options for <see cref="AgentMcpSkillsSource"/>.
+/// </summary>
+[Experimental(DiagnosticIds.Experiments.AgentsAIExperiments)]
+public sealed class AgentMcpSkillsSourceOptions
+{
+    /// <summary>
+    /// Gets or sets the base directory that archive-type skills are extracted to and served from.
+    /// </summary>
+    /// <remarks>
+    /// Archives are extracted beneath this directory as <c>{ArchiveSkillsDirectory}/{skill-name}/</c>.
+    /// When <see langword="null"/>, the source extracts to a per-instance unique location of
+    /// <c>{currentDirectory}/{guid}/{skill-name}/</c>, where the GUID is generated once per
+    /// <see cref="AgentMcpSkillsSource"/> 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 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.
+    /// </remarks>
+    public string? ArchiveSkillsDirectory { get; set; }
+
+    /// <summary>
+    /// Gets or sets the allowed file extensions for resources discovered in extracted archive-type skills.
+    /// </summary>
+    /// <remarks>
+    /// When <see langword="null"/>, defaults to <c>.md</c>, <c>.json</c>, <c>.yaml</c>, <c>.yml</c>,
+    /// <c>.csv</c>, <c>.xml</c>, and <c>.txt</c>.
+    /// </remarks>
+    public IEnumerable<string>? ArchiveResourceExtensions { get; set; }
+
+    /// <summary>
+    /// Gets or sets the maximum depth to search for resource files within each extracted archive-type
+    /// skill directory. A value of <c>1</c> searches only the skill root directory. A value of <c>2</c>
+    /// searches the root and one level of subdirectories.
+    /// </summary>
+    /// <remarks>
+    /// When <see langword="null"/>, the source uses the default depth of <c>2</c>.
+    /// </remarks>
+    public int? ArchiveResourceSearchDepth { get; set; }
+
+    /// <summary>
+    /// Gets or sets the maximum number of files that may be extracted from a single archive-type skill.
+    /// </summary>
+    /// <remarks>
+    /// Guards against excessive-file-count denial-of-service archives. When <see langword="null"/>, the
+    /// source uses a default of <c>20</c>, 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.
+    /// </remarks>
+    public int? ArchiveMaxFileCount { get; set; }
+
+    /// <summary>
+    /// Gets or sets the maximum size, in bytes, of a downloaded archive-type skill resource.
+    /// </summary>
+    /// <remarks>
+    /// Guards against archive resources that are too large to materialize safely. When
+    /// <see langword="null"/>, the source uses a default of <c>1 MB</c>, 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.
+    /// </remarks>
+    public long? ArchiveMaxSizeBytes { get; set; }
+
+    /// <summary>
+    /// Gets or sets the maximum total uncompressed size, in bytes, of all files extracted from a single
+    /// archive-type skill.
+    /// </summary>
+    /// <remarks>
+    /// Guards against decompression-bomb archives. When <see langword="null"/>, the source uses a default
+    /// of <c>1 MB</c>, 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.
+    /// </remarks>
+    public long? ArchiveMaxUncompressedSizeBytes { get; set; }
+
+    /// <summary>
+    /// Gets or sets the interval at which cached skills are considered fresh. When a caller invokes
+    /// <see cref="AgentMcpSkillsSource.GetSkillsAsync"/> and the cached result is younger than this
+    /// interval, the cached list is returned without contacting the MCP server.
+    /// </summary>
+    /// <remarks>
+    /// When <see langword="null"/> (the default), caching is disabled and every call fetches from
+    /// the MCP server. Set to a positive <see cref="TimeSpan"/> to enable caching. Values of
+    /// <see cref="TimeSpan.Zero"/> or negative durations effectively disable caching because the
+    /// cache age will always be greater than or equal to the interval.
+    /// </remarks>
+    public TimeSpan? RefreshInterval { get; set; }
+}

dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs

@@ -15,12 +15,13 @@ public static class AgentSkillsProviderBuilderMcpExtensions
     /// </summary>
     /// <param name="builder">The builder to extend.</param>
     /// <param name="client">An MCP client connected to a server exposing Agent Skills resources.</param>
+    /// <param name="options">Optional options that control archive-distributed skill handling.</param>
     /// <returns>The builder instance for chaining.</returns>
-    public static AgentSkillsProviderBuilder UseMcpSkills(this AgentSkillsProviderBuilder builder, McpClient client)
+    public static AgentSkillsProviderBuilder UseMcpSkills(this AgentSkillsProviderBuilder builder, McpClient client, AgentMcpSkillsSourceOptions? options = null)
     {
         _ = Throw.IfNull(builder);
         _ = Throw.IfNull(client);
 
-        return builder.UseSource(new AgentMcpSkillsSource(client));
+        return builder.UseSource(loggerFactory => new AgentMcpSkillsSource(client, options, loggerFactory));
     }
 }