Skip to content

Commit ea7ae1c

Browse files
SergeyMenshykhsemenshiCopilot
authored
.NET: [BREAKING] Support archive-type skills in AgentMcpSkillsSource (#6631)
* NET: Support archive-type skills in AgentMcpSkillsSource Add archive-type skill discovery to the MCP skills source. Index entries are dispatched to per-type loaders (skill-md and archive) via a new IMcpSkillEntryLoader strategy. The archive loader downloads, safely unpacks, and serves packaged skills through an internal file skills source, while ensuring MCP-delivered scripts are never executed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix CS0121 ambiguity in UseSource null test Cast null! to AgentSkillsSource to disambiguate from the new Func<ILoggerFactory?, AgentSkillsSource> overload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review: fix misleading comment and catch UnauthorizedAccessException in Dispose - Remove hardcoded '50' from test comment; it now says 'default cap' without citing a specific number that can drift from the constant. - Catch UnauthorizedAccessException alongside IOException in test Dispose for robust cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Decouple shared refresh from per-caller cancellation Use CancellationToken.None for the shared refresh so one caller's cancellation does not abort work for all concurrent waiters. Waiters use WaitAsync(cancellationToken) to cancel independently. The refresh owner checks its own token after publishing the result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix file encoding: add UTF-8 BOM to archive tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix file encoding: add UTF-8 BOM to ArchiveFormat.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify pruning doc: covers non-actionable entries too Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add branch-coverage tests and drop [Experimental] attribute - Add 5 unit tests covering FilterValidEntries/download condition branches (missing name, invalid name chars, missing url, unsupported format, text-only blob) - Remove [Experimental] attribute from AgentMcpSkillsSourceOptions (alpha package suffices) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: SergeyMenshykh <SergeMenshikh@outlook.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e049bb5 commit ea7ae1c

13 files changed

Lines changed: 1924 additions & 79 deletions

File tree

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

Lines changed: 128 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
using System;
44
using System.Collections.Generic;
5-
using System.Diagnostics.CodeAnalysis;
65
using System.Linq;
76
using System.Text.Json;
87
using System.Threading;
@@ -22,19 +21,22 @@ namespace Microsoft.Agents.AI;
2221
/// <remarks>
2322
/// <para>
2423
/// Discovery follows the SEP-2640 recommended approach: the source reads the well-known
25-
/// <c>skill://index.json</c> resource and constructs one <see cref="AgentSkill"/> per
26-
/// <c>skill-md</c> entry directly from the entry's <c>name</c>, <c>description</c>, and <c>url</c> fields.
27-
/// The referenced <c>SKILL.md</c> resource is not read during discovery; hosts fetch its body on
28-
/// demand via <c>resources/read</c> against the URI exposed on the resulting skill.
24+
/// <c>skill://index.json</c> resource and constructs one <see cref="AgentSkill"/> per index entry.
2925
/// </para>
3026
/// <para>
31-
/// Only index entries of type <c>skill-md</c> are supported at the moment; entries of any other
32-
/// type are skipped.
27+
/// Index entries are dispatched to an <see cref="IMcpSkillEntryLoader"/> by their <c>type</c>:
28+
/// <list type="bullet">
29+
/// <item><description><c>skill-md</c> - handled by <see cref="SkillMdEntryLoader"/>; the skill's
30+
/// <c>SKILL.md</c> and sibling resources are fetched on demand from the MCP server.</description></item>
31+
/// <item><description><c>archive</c> - handled by <see cref="ArchiveEntryLoader"/>; the entry's
32+
/// <c>url</c> points to a single archive resource whose content unpacks into the skill's
33+
/// namespace.</description></item>
34+
/// </list>
35+
/// Entries whose type has no registered loader (e.g. <c>mcp-resource-template</c>) are skipped.
3336
/// </para>
3437
/// <para>
35-
/// If <c>skill://index.json</c> is absent, unreadable, empty, or fails to parse, this source
36-
/// returns an empty list. Discovered skills serve their referenced resources on demand via
37-
/// <see cref="AgentSkill.GetResourceAsync"/>; they do not enumerate sibling files up front.
38+
/// If <c>skill://index.json</c> is absent, unreadable, empty, or fails to parse, this source returns an
39+
/// empty list.
3840
/// </para>
3941
/// </remarks>
4042
internal sealed partial class AgentMcpSkillsSource : AgentSkillsSource
@@ -44,42 +46,148 @@ internal sealed partial class AgentMcpSkillsSource : AgentSkillsSource
4446
/// </summary>
4547
private const string IndexUri = "skill://index.json";
4648

47-
private const string SkillMdEntryType = "skill-md";
48-
4949
private readonly McpClient _client;
5050
private readonly ILogger _logger;
51+
private readonly Dictionary<string, IMcpSkillEntryLoader> _loaders;
52+
private readonly TimeSpan? _refreshInterval;
53+
54+
private IList<AgentSkill>? _cachedSkills;
55+
private DateTime _lastRefreshedUtc;
56+
private Task<IList<AgentSkill>>? _refreshTask;
5157

5258
/// <summary>
5359
/// Initializes a new instance of the <see cref="AgentMcpSkillsSource"/> class.
5460
/// </summary>
5561
/// <param name="client">An MCP client connected to a server that exposes Agent Skills resources.</param>
62+
/// <param name="options">Optional options that control archive-distributed skill handling.</param>
5663
/// <param name="loggerFactory">Optional logger factory.</param>
57-
public AgentMcpSkillsSource(McpClient client, ILoggerFactory? loggerFactory = null)
64+
public AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? options = null, ILoggerFactory? loggerFactory = null)
5865
{
5966
this._client = Throw.IfNull(client);
60-
this._logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger<AgentMcpSkillsSource>();
67+
loggerFactory ??= NullLoggerFactory.Instance;
68+
this._logger = loggerFactory.CreateLogger<AgentMcpSkillsSource>();
69+
70+
IMcpSkillEntryLoader[] loaders =
71+
[
72+
new SkillMdEntryLoader(this._client, loggerFactory),
73+
new ArchiveEntryLoader(this._client, options, loggerFactory),
74+
];
75+
76+
this._loaders = loaders.ToDictionary(l => l.EntryType, StringComparer.OrdinalIgnoreCase);
77+
this._refreshInterval = options?.RefreshInterval;
6178
}
6279

6380
/// <inheritdoc/>
6481
public override async Task<IList<AgentSkill>> GetSkillsAsync(CancellationToken cancellationToken = default)
82+
{
83+
if (this.TryGetCachedSkills() is { } cached)
84+
{
85+
return cached;
86+
}
87+
88+
// Use CAS to ensure only one concurrent refresh runs; other callers await the same task.
89+
var tcs = new TaskCompletionSource<IList<AgentSkill>>(TaskCreationOptions.RunContinuationsAsynchronously);
90+
91+
if (Interlocked.CompareExchange(ref this._refreshTask, tcs.Task, null) is { } existing)
92+
{
93+
// Wait for the in-flight refresh but let this caller cancel its own wait independently
94+
// without aborting the shared refresh work.
95+
return await existing.WaitAsync(cancellationToken).ConfigureAwait(false);
96+
}
97+
98+
try
99+
{
100+
// The refresh owner uses CancellationToken.None so that a single caller's cancellation
101+
// does not abort the shared refresh for all concurrent waiters.
102+
var skills = await this.GetCoreSkillsAsync(CancellationToken.None).ConfigureAwait(false);
103+
104+
this.UpdateCache(skills);
105+
106+
tcs.SetResult(skills);
107+
108+
// Allow the current caller to observe cancellation without impacting other awaiters.
109+
cancellationToken.ThrowIfCancellationRequested();
110+
111+
return skills;
112+
}
113+
catch (Exception ex)
114+
{
115+
tcs.TrySetException(ex);
116+
throw;
117+
}
118+
finally
119+
{
120+
this._refreshTask = null;
121+
}
122+
}
123+
124+
/// <summary>
125+
/// Returns the cached skill list if caching is enabled and the cache is still fresh;
126+
/// otherwise returns <see langword="null"/>.
127+
/// </summary>
128+
private IList<AgentSkill>? TryGetCachedSkills()
129+
{
130+
if (this._refreshInterval is null || this._cachedSkills is null)
131+
{
132+
return null;
133+
}
134+
135+
TimeSpan cacheAge = DateTime.UtcNow - this._lastRefreshedUtc;
136+
137+
if (cacheAge >= this._refreshInterval.Value)
138+
{
139+
return null;
140+
}
141+
142+
return this._cachedSkills;
143+
}
144+
145+
/// <summary>
146+
/// Stores the skill list and records the refresh timestamp for cache freshness checks.
147+
/// </summary>
148+
private void UpdateCache(IList<AgentSkill> skills)
149+
{
150+
this._cachedSkills = skills;
151+
this._lastRefreshedUtc = DateTime.UtcNow;
152+
}
153+
154+
/// <summary>
155+
/// Reads the skill index from the MCP server, dispatches entries to registered loaders, and
156+
/// returns the aggregated skill list.
157+
/// </summary>
158+
private async Task<IList<AgentSkill>> GetCoreSkillsAsync(CancellationToken cancellationToken)
65159
{
66160
McpSkillIndex? index = await this.TryReadIndexAsync(cancellationToken).ConfigureAwait(false);
67161

68-
var skills = new List<AgentSkill>();
162+
// Group entries by type and set aside those a registered loader can handle; entries of any
163+
// other type are unsupported and logged.
164+
var entriesByType = new Dictionary<string, List<McpSkillIndexEntry>>(StringComparer.OrdinalIgnoreCase);
69165

70-
foreach (var entry in index?.Skills ?? [])
166+
foreach (var group in (index?.Skills ?? []).GroupBy(e => e.Type ?? string.Empty, StringComparer.OrdinalIgnoreCase))
71167
{
72-
if (this.TryCreateSkill(entry, out AgentMcpSkill? skill, out string skipReason))
168+
if (this._loaders.ContainsKey(group.Key))
73169
{
74-
skills.Add(skill);
75-
LogSkillLoaded(this._logger, skill.Frontmatter.Name);
170+
entriesByType[group.Key] = group.ToList();
76171
}
77172
else
78173
{
79-
LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", skipReason);
174+
foreach (var entry in group)
175+
{
176+
LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", $"unsupported type '{entry.Type ?? "(none)"}'");
177+
}
80178
}
81179
}
82180

181+
// Invoke every registered loader, even when the server advertises no entries of its type, so
182+
// each type's lifecycle still runs (e.g. the archive loader prunes leftover directories).
183+
var skills = new List<AgentSkill>();
184+
185+
foreach (var loader in this._loaders.Values)
186+
{
187+
var entries = entriesByType.TryGetValue(loader.EntryType, out List<McpSkillIndexEntry>? matched) ? matched : [];
188+
skills.AddRange(await loader.LoadAsync(entries, cancellationToken).ConfigureAwait(false));
189+
}
190+
83191
LogSkillsLoadedTotal(this._logger, skills.Count);
84192

85193
return skills;
@@ -124,44 +232,6 @@ public override async Task<IList<AgentSkill>> GetSkillsAsync(CancellationToken c
124232
}
125233
}
126234

127-
private bool TryCreateSkill(
128-
McpSkillIndexEntry entry,
129-
[NotNullWhen(true)] out AgentMcpSkill? skill,
130-
out string skipReason)
131-
{
132-
skill = null;
133-
134-
if (!string.Equals(entry.Type, SkillMdEntryType, StringComparison.Ordinal))
135-
{
136-
skipReason = $"unsupported type '{entry.Type ?? "(none)"}'";
137-
return false;
138-
}
139-
140-
if (string.IsNullOrWhiteSpace(entry.Url))
141-
{
142-
skipReason = "missing required 'url' field";
143-
return false;
144-
}
145-
146-
AgentSkillFrontmatter frontmatter;
147-
try
148-
{
149-
frontmatter = new AgentSkillFrontmatter(entry.Name!, entry.Description!);
150-
}
151-
catch (ArgumentException ex)
152-
{
153-
skipReason = $"invalid metadata: {ex.Message}";
154-
return false;
155-
}
156-
157-
skill = new AgentMcpSkill(frontmatter, entry.Url!, this._client);
158-
skipReason = string.Empty;
159-
return true;
160-
}
161-
162-
[LoggerMessage(LogLevel.Information, "Loaded MCP skill: {SkillName}")]
163-
private static partial void LogSkillLoaded(ILogger logger, string skillName);
164-
165235
[LoggerMessage(LogLevel.Information, "Successfully loaded {Count} skills from MCP server")]
166236
private static partial void LogSkillsLoadedTotal(ILogger logger, int count);
167237

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
3+
using System;
4+
using System.Collections.Generic;
5+
6+
namespace Microsoft.Agents.AI;
7+
8+
/// <summary>
9+
/// Configuration options for <see cref="AgentMcpSkillsSource"/>.
10+
/// </summary>
11+
public sealed class AgentMcpSkillsSourceOptions
12+
{
13+
/// <summary>
14+
/// Gets or sets the base directory that archive-type skills are extracted to and served from.
15+
/// </summary>
16+
/// <remarks>
17+
/// Archives are extracted beneath this directory as <c>{ArchiveSkillsDirectory}/{skill-name}/</c>.
18+
/// When <see langword="null"/>, the source extracts to a per-instance unique location of
19+
/// <c>{currentDirectory}/{guid}/{skill-name}/</c>, where the GUID is generated once per
20+
/// <see cref="AgentMcpSkillsSource"/> instance so that multiple sources never overwrite one
21+
/// another. Set this to a fixed value to get a predictable, reusable extraction location.
22+
/// When set, each source must use its own unique directory: the source treats the directory as
23+
/// exclusively its own and, on every discovery, prunes any sub-directory that the MCP server no
24+
/// longer advertises or whose index entry is not actionable (e.g., missing a required field).
25+
/// Pointing two sources at the same directory would therefore cause them to
26+
/// delete each other's extracted skills.
27+
/// </remarks>
28+
public string? ArchiveSkillsDirectory { get; set; }
29+
30+
/// <summary>
31+
/// Gets or sets the allowed file extensions for resources discovered in extracted archive-type skills.
32+
/// </summary>
33+
/// <remarks>
34+
/// When <see langword="null"/>, defaults to <c>.md</c>, <c>.json</c>, <c>.yaml</c>, <c>.yml</c>,
35+
/// <c>.csv</c>, <c>.xml</c>, and <c>.txt</c>.
36+
/// </remarks>
37+
public IEnumerable<string>? ArchiveResourceExtensions { get; set; }
38+
39+
/// <summary>
40+
/// Gets or sets the maximum depth to search for resource files within each extracted archive-type
41+
/// skill directory. A value of <c>1</c> searches only the skill root directory. A value of <c>2</c>
42+
/// searches the root and one level of subdirectories.
43+
/// </summary>
44+
/// <remarks>
45+
/// When <see langword="null"/>, the source uses the default depth of <c>2</c>.
46+
/// </remarks>
47+
public int? ArchiveResourceSearchDepth { get; set; }
48+
49+
/// <summary>
50+
/// Gets or sets the maximum number of files that may be extracted from a single archive-type skill.
51+
/// </summary>
52+
/// <remarks>
53+
/// Guards against excessive-file-count denial-of-service archives. When <see langword="null"/>, the
54+
/// source uses a default of <c>20</c>, sized for a typical well-formed skill (a handful of files).
55+
/// Raise this for archive-type skills that legitimately bundle many files. An archive that exceeds
56+
/// the limit is skipped.
57+
/// </remarks>
58+
public int? ArchiveMaxFileCount { get; set; }
59+
60+
/// <summary>
61+
/// Gets or sets the maximum size, in bytes, of a downloaded archive-type skill resource.
62+
/// </summary>
63+
/// <remarks>
64+
/// Guards against archive resources that are too large to materialize safely. When
65+
/// <see langword="null"/>, the source uses a default of <c>1 MB</c>, sized for a typical
66+
/// well-formed skill archive. Raise this for archive-type skills that legitimately require
67+
/// larger archive payloads. An archive that exceeds the limit is skipped.
68+
/// </remarks>
69+
public long? ArchiveMaxSizeBytes { get; set; }
70+
71+
/// <summary>
72+
/// Gets or sets the maximum total uncompressed size, in bytes, of all files extracted from a single
73+
/// archive-type skill.
74+
/// </summary>
75+
/// <remarks>
76+
/// Guards against decompression-bomb archives. When <see langword="null"/>, the source uses a default
77+
/// of <c>1 MB</c>, sized for a typical well-formed skill (well under ~1 MB). Raise this for
78+
/// archive-type skills that legitimately bundle larger content. An archive that exceeds the limit is
79+
/// skipped.
80+
/// </remarks>
81+
public long? ArchiveMaxUncompressedSizeBytes { get; set; }
82+
83+
/// <summary>
84+
/// Gets or sets the interval at which cached skills are considered fresh. When a caller invokes
85+
/// <see cref="AgentMcpSkillsSource.GetSkillsAsync"/> and the cached result is younger than this
86+
/// interval, the cached list is returned without contacting the MCP server.
87+
/// </summary>
88+
/// <remarks>
89+
/// When <see langword="null"/> (the default), caching is disabled and every call fetches from
90+
/// the MCP server. Set to a positive <see cref="TimeSpan"/> to enable caching. Values of
91+
/// <see cref="TimeSpan.Zero"/> or negative durations effectively disable caching because the
92+
/// cache age will always be greater than or equal to the interval.
93+
/// </remarks>
94+
public TimeSpan? RefreshInterval { get; set; }
95+
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,13 @@ public static class AgentSkillsProviderBuilderMcpExtensions
1515
/// </summary>
1616
/// <param name="builder">The builder to extend.</param>
1717
/// <param name="client">An MCP client connected to a server exposing Agent Skills resources.</param>
18+
/// <param name="options">Optional options that control archive-distributed skill handling.</param>
1819
/// <returns>The builder instance for chaining.</returns>
19-
public static AgentSkillsProviderBuilder UseMcpSkills(this AgentSkillsProviderBuilder builder, McpClient client)
20+
public static AgentSkillsProviderBuilder UseMcpSkills(this AgentSkillsProviderBuilder builder, McpClient client, AgentMcpSkillsSourceOptions? options = null)
2021
{
2122
_ = Throw.IfNull(builder);
2223
_ = Throw.IfNull(client);
2324

24-
return builder.UseSource(new AgentMcpSkillsSource(client));
25+
return builder.UseSource(loggerFactory => new AgentMcpSkillsSource(client, options, loggerFactory));
2526
}
2627
}

0 commit comments

Comments
 (0)