diff --git a/dotnet/Directory.Packages.props b/dotnet/Directory.Packages.props index 87413586448..dd7d30dd3f2 100644 --- a/dotnet/Directory.Packages.props +++ b/dotnet/Directory.Packages.props @@ -22,7 +22,7 @@ - + diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index 07ccfb73eb9..33e8ad5ab37 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -27,15 +27,17 @@ namespace Microsoft.Agents.AI; /// /// Listed skill discovery is based on the Skills extension specification incorporating the /// working group's decision dated 2026-09-08. -/// Support currently covers a single skills/list result with resource lists and reading skill files, -/// not the full extension. +/// Support currently includes skill discovery through skills/list and reading listed skill resources, +/// but not the full extension. /// /// /// 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. -/// Listed resource sizes are bounded by . +/// 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. /// These checks do not limit incoming messages before the supplied client receives and parses them. /// /// @@ -95,6 +97,14 @@ namespace Microsoft.Agents.AI; /// not trustworthiness: a server controlling both the index and archive can replace both or omit /// the digest. /// +/// +/// This source does not impose a byte limit on individual or combined skills/list responses. +/// Page-count, skill-count, and resource-size limits do not bound incoming message size or total +/// memory consumption. A malicious, compromised, or misconfigured server can return oversized +/// responses that cause excessive memory consumption. Application developers are responsible for +/// configuring appropriate input-size and resource limits through their chosen MCP client, transport, +/// or hosting environment. +/// /// internal sealed partial class AgentMcpSkillsSource : AgentSkillsSource { diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs index 5d5570f5014..29c4913a9c1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -8,8 +8,12 @@ namespace Microsoft.Agents.AI; /// Configuration options for . /// /// -/// The listed resource size limit bounds resource content processed by this source. -/// These checks do not limit incoming messages before the supplied client receives and parses them. +/// Listed discovery limits bound page counts and total skill counts. +/// The listed resource size limit separately bounds resource content processed by this source. +/// These limits do not bound incoming message size, list-response JSON size, or total memory consumption. +/// 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 @@ -18,6 +22,25 @@ namespace Microsoft.Agents.AI; /// public sealed class AgentMcpSkillsSourceOptions { + /// + /// Gets or sets the maximum number of skills/list pages requested per discovery. + /// + /// + /// When , defaults to 10. Must be positive. + /// Empty pages count toward this limit. Discovery fails rather than returning a partial catalog + /// when the last allowed page includes a continuation cursor. + /// + public int? ListedMaxPageCount { get; set; } + + /// + /// Gets or sets the maximum number of skills accepted across all skills/list pages per discovery. + /// + /// + /// When , defaults to 100. Must be positive. + /// Every entry counts, including duplicate skill URIs. Discovery fails if the limit is exceeded. + /// + public int? ListedMaxSkillCount { get; set; } + /// /// Gets or sets the maximum size, in bytes, of a resource discovered through skills/list, /// including SKILL.md. diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs index 7a8eef5491c..8f5e69b33b5 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs @@ -23,14 +23,15 @@ public static class AgentSkillsProviderBuilderMcpExtensions /// io.modelcontextprotocol/skills extension. Otherwise, discovers skills through /// skill://index.json. /// Listed skills fetch instructions and supporting resources when requested. - /// Their resource sizes are bounded by . + /// Page counts, total skill counts, and individual resource sizes + /// are bounded by . /// These checks do not limit incoming messages before the supplied client receives and parses them. /// /// /// Listed skill discovery is based on the Skills extension specification incorporating the /// working group's decision dated 2026-09-08. - /// Support currently covers a single skills/list result with resource lists and reading skill files, - /// not the full extension. + /// Support currently includes skill discovery through skills/list and reading listed skill resources, + /// but not the full extension. /// /// /// Index-based discovery and archive-distributed skills are compatibility features and will be deprecated @@ -44,6 +45,14 @@ public static class AgentSkillsProviderBuilderMcpExtensions /// or to exfiltrate data through instructions or scripts the agent is induced to run. Only connect /// to MCP servers you trust and have evaluated, and treat their responses as untrusted input. /// + /// + /// This source does not impose a byte limit on individual or combined skills/list responses. + /// Page-count, skill-count, and resource-size limits do not bound incoming message size or total + /// memory consumption. A malicious, compromised, or misconfigured server can return oversized + /// responses that cause excessive memory consumption. Application developers are responsible for + /// configuring appropriate input-size and resource limits through their chosen MCP client, transport, + /// or hosting environment. + /// /// public static AgentSkillsProviderBuilder UseMcpSkills(this AgentSkillsProviderBuilder builder, McpClient client, AgentMcpSkillsSourceOptions? options = null) { diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs index b2342ab9947..1ce32a3e08c 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs @@ -28,74 +28,136 @@ internal sealed partial class McpListedSkillsLoader { private const string CompleteResultType = "complete"; + private const int DefaultMaxPageCount = 10; + private const int DefaultMaxSkillCount = 100; + private readonly McpClient _client; private readonly ILogger _logger; private readonly long _maxResourceSizeBytes; + private readonly int _maxPageCount; + private readonly int _maxSkillCount; /// /// 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 size limit for listed resources. + /// Optional discovery limits and size limit for listed resources. /// is . - /// A configured size limit is not positive. + /// A configured limit is not positive. public McpListedSkillsLoader(McpClient client, ILogger? logger = null, AgentMcpSkillsSourceOptions? options = null) { this._client = Throw.IfNull(client); this._logger = logger ?? NullLogger.Instance; this._maxResourceSizeBytes = options?.ListedResourceMaxSizeBytes ?? AgentMcpListedSkill.DefaultMaxResourceSizeBytes; + this._maxPageCount = options?.ListedMaxPageCount ?? DefaultMaxPageCount; + this._maxSkillCount = options?.ListedMaxSkillCount ?? DefaultMaxSkillCount; ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxResourceSizeBytes, nameof(options.ListedResourceMaxSizeBytes)); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxPageCount, nameof(options.ListedMaxPageCount)); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(this._maxSkillCount, nameof(options.ListedMaxSkillCount)); } /// - /// Gets the server's listed skills without fetching their instructions or resources. + /// 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, or a resource exceeding the configured size limit. + /// invalid skill metadata or resource manifests, a repeated pagination cursor, or a configured limit is exceeded. /// The MCP request fails. /// Discovery is canceled. public async Task> DiscoverAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - JsonRpcResponse response = await this._client.SendRequestAsync(new JsonRpcRequest + List skillEntries = await this.ListSkillEntriesAsync(cancellationToken).ConfigureAwait(false); + List skills = []; + + foreach (McpListedSkillEntry entry in skillEntries) { - Method = "skills/list", - Params = new JsonObject(), - }, cancellationToken).ConfigureAwait(false); + cancellationToken.ThrowIfCancellationRequested(); - McpSkillsListResult result = response.Result?.Deserialize(McpListedSkillsJsonContext.Default.McpSkillsListResult) - ?? throw new InvalidOperationException("The server returned no skills/list result."); + AgentSkillFrontmatter frontmatter = CreateFrontmatter(entry); + List resources = this.CreateSupportingResources(entry); - if (result.ResultType != CompleteResultType) - { - throw new InvalidOperationException($"A skills/list result must have resultType '{CompleteResultType}'. Received resultType '{result.ResultType}'."); + skills.Add(new AgentMcpListedSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes)); } - List skills = []; + cancellationToken.ThrowIfCancellationRequested(); - foreach (McpListedSkillEntry? entry in result.Skills) + return skills; + } + + private async Task> ListSkillEntriesAsync(CancellationToken cancellationToken) + { + List skillEntries = []; + HashSet cursors = new(StringComparer.Ordinal); + string? cursor = null; + int pageCount = 0; + + do { cancellationToken.ThrowIfCancellationRequested(); - // Nullable annotations do not reject null collection elements during deserialization. - if (entry is null) + if (pageCount >= this._maxPageCount) { - throw new InvalidOperationException("The skills/list result contains a null skill entry."); + throw new InvalidOperationException($"Skill discovery exceeds the maximum allowed page count ({this._maxPageCount})."); } - AgentSkillFrontmatter frontmatter = CreateFrontmatter(entry); + JsonObject parameters = new(); + if (cursor is not null) + { + parameters["cursor"] = cursor; + } - List resources = this.CreateSupportingResources(entry); + JsonRpcResponse response = await this._client.SendRequestAsync(new JsonRpcRequest + { + Method = "skills/list", + Params = parameters, + }, cancellationToken).ConfigureAwait(false); - skills.Add(new AgentMcpListedSkill(frontmatter, entry.Uri, resources, this._client, this._logger, this._maxResourceSizeBytes)); + 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(McpListedSkillsJsonContext.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}'."); + } + + cursor = result.NextCursor; + if (cursor is not null && !cursors.Add(cursor)) + { + throw new InvalidOperationException("The skills/list result contains a repeated pagination cursor."); + } + + foreach (McpListedSkillEntry? 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 skills; + return skillEntries; } private static AgentSkillFrontmatter CreateFrontmatter(McpListedSkillEntry entry) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpSkillsListResult.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpSkillsListResult.cs index 7fecfbd22a7..0cd6b6e2fcd 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpSkillsListResult.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpSkillsListResult.cs @@ -19,6 +19,11 @@ internal sealed class McpSkillsListResult /// Gets the skills listed by the server. /// public required IReadOnlyList Skills { get; init; } + + /// + /// Gets the opaque cursor to pass to the next request, or when no more pages are available. + /// + public string? NextCursor { get; init; } } /// diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsSerializationTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsSerializationTests.cs index 948e67aff41..73e7250b9ac 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsSerializationTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsSerializationTests.cs @@ -29,6 +29,10 @@ public sealed class AgentMcpListedSkillsSerializationTests [InlineData("""{"resultType":[],"skills":[]}""")] [InlineData("""{"ResultType":"complete","skills":[]}""")] [InlineData("""{"resultType":"complete","Skills":[]}""")] + [InlineData("""{"resultType":"complete","skills":[],"nextCursor":42}""")] + [InlineData("""{"resultType":"complete","skills":[],"nextCursor":true}""")] + [InlineData("""{"resultType":"complete","skills":[],"nextCursor":[]}""")] + [InlineData("""{"resultType":"complete","skills":[],"nextCursor":{}}""")] public void Deserialize_InvalidResult_Throws(string json) { // Arrange @@ -38,6 +42,28 @@ public void Deserialize_InvalidResult_Throws(string json) Assert.Throws(() => JsonSerializer.Deserialize(json, context.McpSkillsListResult)); } + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("opaque cursor:+/=\u00e9")] + public void Deserialize_NextCursor_PreservesValue(string? cursor) + { + // Arrange + JsonObject node = new() + { + ["resultType"] = "complete", + ["skills"] = new JsonArray(), + ["nextCursor"] = cursor, + }; + + // Act + var result = node.Deserialize(McpListedSkillsJsonContext.Default.McpSkillsListResult); + + // Assert + Assert.NotNull(result); + Assert.Equal(cursor, result.NextCursor); + } + [Theory] [InlineData("uri", "null")] [InlineData("uri", "42")] @@ -168,6 +194,7 @@ public void Deserialize_CaseVariants_IgnoresUnknownCaseVariants() Assert.NotNull(result); Assert.Equal("complete", result.ResultType); Assert.Empty(result.Skills); + Assert.Null(result.NextCursor); } [Theory] 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 fb6c0a75bf6..77b4fdb2589 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 @@ -56,6 +56,349 @@ public async Task GetSkillsAsync_ListRequest_ProjectsFrontmatterAndIncludesReque Assert.NotNull(request["_meta"]![MetaKeys.ClientInfo]); } + [Theory] + [InlineData("")] + [InlineData("opaque cursor:+/=\u00e9")] + public async Task GetSkillsAsync_PaginatedList_FollowsOpaqueCursorsAndEmptyPagesAsync(string firstCursor) + { + // Arrange + List requests = []; + List reads = []; + await using var server = CreateServer((request, _) => + { + requests.Add(request.Params?.DeepClone()); + JsonObject result = requests.Count switch + { + 1 => ListResult(Entry("skill://first/unit-converter/")), + 2 => ListResult(), + _ => ListResult(Entry("skill://second/unit-converter/")), + }; + if (requests.Count < 3) + { + result["nextCursor"] = requests.Count == 1 ? firstCursor : "last-page"; + } + + return ValueTask.FromResult(result); + }, (uri, _) => + { + reads.Add(uri); + return ValueTask.FromResult(Text(uri, "Checklist.")); + }); + await using var client = await server.CreateClientAsync(); + using var source = new AgentMcpSkillsSource(client); + + // Act + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.Equal(3, requests.Count); + Assert.Null(requests[0]!["cursor"]); + Assert.Equal(firstCursor, requests[1]!["cursor"]!.GetValue()); + Assert.Equal("last-page", requests[2]!["cursor"]!.GetValue()); + foreach (JsonNode? request in requests) + { + Assert.Equal(client.NegotiatedProtocolVersion, request!["_meta"]![MetaKeys.ProtocolVersion]!.GetValue()); + Assert.NotNull(request["_meta"]![MetaKeys.ClientCapabilities]); + Assert.NotNull(request["_meta"]![MetaKeys.ClientInfo]); + } + + Assert.Equal(2, skills.Count); + Assert.Empty(reads); + Assert.NotNull(await skills[0].GetResourceAsync("references/checklist.md")); + Assert.NotNull(await skills[1].GetResourceAsync("references/checklist.md")); + Assert.Equal( + ["skill://first/unit-converter/references/checklist.md", "skill://second/unit-converter/references/checklist.md"], + reads); + } + + [Theory] + [InlineData(null, 10, false)] + [InlineData(null, 10, true)] + [InlineData(2, 2, false)] + [InlineData(2, 2, true)] + [InlineData(11, 11, false)] + [InlineData(11, 11, true)] + public async Task GetSkillsAsync_PageLimit_IncludesEmptyPagesAndDoesNotSendExtraRequestAsync(int? limit, int expectedPages, bool exceedsLimit) + { + // Arrange + int calls = 0; + await using var server = CreateServer((_, _) => + { + JsonObject result = ListResult(); + calls++; + if (exceedsLimit || calls < expectedPages) + { + result["nextCursor"] = $"page-{calls}"; + } + + return ValueTask.FromResult(result); + }); + await using var client = await server.CreateClientAsync(); + using var source = new AgentMcpSkillsSource(client, new() { ListedMaxPageCount = limit }); + + // Act & Assert + if (exceedsLimit) + { + var exception = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Contains($"page count ({expectedPages})", exception.Message); + } + else + { + Assert.Empty(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + } + + Assert.Equal(expectedPages, calls); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_RepeatedCursor_ThrowsAsync(bool cycle) + { + // Arrange + int calls = 0; + await using var server = CreateServer((_, _) => + { + calls++; + JsonObject result = ListResult(Entry()); + result["nextCursor"] = cycle && calls == 2 ? "b" : "a"; + return ValueTask.FromResult(result); + }); + await using var client = await server.CreateClientAsync(); + using var source = new AgentMcpSkillsSource(client); + + // Act + var exception = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + + // Assert + Assert.Contains("repeated pagination cursor", exception.Message); + Assert.Equal(cycle ? 3 : 2, calls); + } + + [Fact] + public async Task GetSkillsAsync_CursorsDifferingOnlyByCase_AreDistinctAsync() + { + // Arrange + int calls = 0; + await using var server = CreateServer((_, _) => + { + JsonObject result = ListResult(); + calls++; + result["nextCursor"] = calls switch { 1 => "A", 2 => "a", _ => null }; + return ValueTask.FromResult(result); + }); + await using var client = await server.CreateClientAsync(); + using var source = new AgentMcpSkillsSource(client); + + // Act + var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()); + + // Assert + Assert.Empty(skills); + Assert.Equal(3, calls); + } + + [Theory] + [InlineData("{}", typeof(JsonException))] + [InlineData("""{"resultType":"unknown","skills":[]}""", typeof(InvalidOperationException))] + [InlineData("""{"resultType":"complete","skills":[null]}""", typeof(InvalidOperationException))] + [InlineData("""{"resultType":"complete","skills":[],"nextCursor":42}""", typeof(JsonException))] + public async Task GetSkillsAsync_InvalidLaterPage_ThrowsInsteadOfReturningPartialCatalogAsync(string invalidPage, Type exceptionType) + { + // Arrange + int calls = 0; + List reads = []; + await using var server = CreateServer((_, _) => + { + if (++calls > 1) + { + return ValueTask.FromResult(JsonNode.Parse(invalidPage)); + } + + JsonObject result = ListResult(Entry()); + result["nextCursor"] = "next"; + return ValueTask.FromResult(result); + }, (uri, _) => + { + reads.Add(uri); + return ValueTask.FromResult(Text(uri, "Must not be read.")); + }); + await using var client = await server.CreateClientAsync(); + using var source = new AgentMcpSkillsSource(client); + + // Act & Assert + await Assert.ThrowsAsync(exceptionType, () => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Equal(2, calls); + Assert.Empty(reads); + } + + [Fact] + public async Task GetSkillsAsync_LaterPageError_PropagatesWithoutReadingIndexAsync() + { + // Arrange + int calls = 0; + List reads = []; + await using var server = CreateServer((_, _) => + { + if (++calls > 1) + { + throw new McpProtocolException("Cannot list next page.", McpErrorCode.InternalError); + } + + JsonObject result = ListResult(Entry()); + result["nextCursor"] = "next"; + return ValueTask.FromResult(result); + }, (uri, _) => + { + reads.Add(uri); + return ValueTask.FromResult(Text(uri, "Must not be read.")); + }); + await using var client = await server.CreateClientAsync(); + using var source = new AgentMcpSkillsSource(client); + + // Act & Assert + await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Equal(2, calls); + Assert.Empty(reads); + } + + [Theory] + [InlineData(null, 99, false)] + [InlineData(null, 100, false)] + [InlineData(null, 101, true)] + [InlineData(2, 2, false)] + [InlineData(2, 3, true)] + [InlineData(101, 101, false)] + public async Task GetSkillsAsync_SkillLimit_CountsEntriesAcrossPagesIncludingDuplicatesAsync(int? limit, int count, bool exceedsLimit) + { + // Arrange + int calls = 0; + await using var server = CreateServer((_, _) => + { + calls++; + JsonObject result = ListResult(); + var entries = Assert.IsType(result["skills"]); + for (int i = 0; i < (calls == 1 ? 1 : count - 1); i++) + { + entries.Add(Entry()); + } + + if (calls == 1) + { + result["nextCursor"] = "next"; + } + + return ValueTask.FromResult(result); + }); + await using var client = await server.CreateClientAsync(); + using var source = new AgentMcpSkillsSource(client, new() { ListedMaxSkillCount = limit }); + + // Act & Assert + if (exceedsLimit) + { + var exception = await Assert.ThrowsAsync(() => source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())); + Assert.Contains($"skill count ({limit ?? 100})", exception.Message); + } + else + { + Assert.Equal(count, (await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create())).Count); + } + + Assert.Equal(2, calls); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + public async Task DiscoverAsync_LargeUnknownFields_AreAcceptedAcrossPagesAsync(int pageCount) + { + // Arrange + string padding = new('x', 10 * 1024 * 1024 / pageCount); + var client = new Mock(); + var responses = client.SetupSequence(c => c.SendRequestAsync(It.IsAny(), It.IsAny())); + for (int page = 0; page < pageCount; page++) + { + JsonObject result = ListResult(Entry()); + result["unknown"] = padding; + if (page < pageCount - 1) + { + result["nextCursor"] = $"page-{page + 1}"; + } + + responses.ReturnsAsync(new JsonRpcResponse { Id = new RequestId(page + 1), Result = result }); + } + + var loader = new McpListedSkillsLoader(client.Object); + + // Act + var skills = await loader.DiscoverAsync(CancellationToken.None); + + // Assert + Assert.Equal(pageCount, skills.Count); + client.Verify(c => c.SendRequestAsync(It.IsAny(), It.IsAny()), Times.Exactly(pageCount)); + } + + [Fact] + public async Task DiscoverAsync_SkillLimit_RejectsExcessEntriesBeforeTypedDeserializationAsync() + { + // Arrange + 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()) }); + var loader = new McpListedSkillsLoader(client.Object, options: new() { ListedMaxSkillCount = 1 }); + + // Act + var exception = await Assert.ThrowsAsync(() => loader.DiscoverAsync(CancellationToken.None)); + + // Assert + Assert.Contains("skill count (1)", exception.Message); + } + + [Fact] + public async Task DiscoverAsync_LargeResourceManifest_HasNoSeparateResourceCountLimitAsync() + { + // Arrange + JsonObject entry = Entry(); + var resources = Assert.IsType(entry["resources"]); + for (int i = 0; i < 101; i++) + { + resources.Add(Resource($"skill://unit-converter/file-{i}.txt")); + } + + var client = new Mock(); + client.Setup(c => c.SendRequestAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new JsonRpcResponse { Id = new RequestId(1), Result = ListResult(entry) }); + var loader = new McpListedSkillsLoader(client.Object); + + // Act + var skills = await loader.DiscoverAsync(CancellationToken.None); + + // Assert + Assert.Single(skills); + } + + [Theory] + [InlineData(nameof(AgentMcpSkillsSourceOptions.ListedMaxPageCount), 0)] + [InlineData(nameof(AgentMcpSkillsSourceOptions.ListedMaxPageCount), -1)] + [InlineData(nameof(AgentMcpSkillsSourceOptions.ListedMaxSkillCount), 0)] + [InlineData(nameof(AgentMcpSkillsSourceOptions.ListedMaxSkillCount), -1)] + public void Constructor_InvalidDiscoveryLimit_Throws(string option, int limit) + { + // Arrange + var client = new Mock(); + var options = new AgentMcpSkillsSourceOptions + { + ListedMaxPageCount = option == nameof(AgentMcpSkillsSourceOptions.ListedMaxPageCount) ? limit : null, + ListedMaxSkillCount = option == nameof(AgentMcpSkillsSourceOptions.ListedMaxSkillCount) ? limit : null, + }; + + // Act + var exception = Assert.Throws(() => new AgentMcpSkillsSource(client.Object, options)); + + // Assert + Assert.Equal(option, exception.ParamName); + } + [Fact] public async Task GetSkillsAsync_Refresh_KeepsPreviouslyReturnedManifestAsync() { @@ -210,14 +553,24 @@ public async Task GetSkillsAsync_ListError_DoesNotReadIndexAsync() Assert.Empty(reads); } - [Fact] - public async Task GetSkillsAsync_ListCancellation_PropagatesAsync() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetSkillsAsync_ListCancellation_PropagatesAsync(bool laterPage) { // Arrange var started = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + int calls = 0; await using var server = CreateServer(async (_, token) => { + if (++calls == 1 && laterPage) + { + JsonObject result = ListResult(Entry()); + result["nextCursor"] = "next"; + return result; + } + started.SetResult(); await release.Task.WaitAsync(token); return ListResult(Entry()); @@ -235,6 +588,7 @@ public async Task GetSkillsAsync_ListCancellation_PropagatesAsync() // Assert await Assert.ThrowsAnyAsync(() => operation.WaitAsync(TimeSpan.FromSeconds(10))); + Assert.Equal(laterPage ? 2 : 1, calls); } finally { @@ -768,7 +1122,7 @@ public async Task GetSkillsAsync_UnknownFieldsAndCaseVariants_AreIgnoredAsync() JsonObject listResult = ListResult(entry); listResult["ttlMs"] = 300000; listResult["cacheScope"] = "public"; - listResult["nextCursor"] = "next-page"; + listResult["NextCursor"] = "ignored-case-variant"; listResult["future-field"] = true; await using var server = CreateServer((_, _) => ValueTask.FromResult(listResult)); await using var client = await server.CreateClientAsync();