Repository navigation
Add SEP-2640 Skills extension support - #1856
Conversation
|
Lets get this merged asap please |
|
At least one sample should be included imo. |
Make CacheScopeConverter public in Core instead of compiling its source into the Skills package, which was the only package doing so. Adding a public type passes package validation against the 2.0.0 baseline. Validate only what the Agent Skills specification states as requirements beyond name and description: compatibility is 1 to 500 characters if provided, and metadata is a mapping. license, allowed-tools, and the values inside metadata pass through verbatim. Hosts compare frontmatter against the file and ignore allowed-tools for MCP-origin skills, so rejecting a whole skill over the shape of such a field would help nobody. Repeat the authorization caveat on WithSkills(IEnumerable<McpServerSkill>) and WithSkillsFromDirectory, whose in-memory catalog serves every caller, and explain why a directory without SKILL.md is skipped while an invalid skill is an error. Guard the public verifier and client read path against a Skill whose required Resources property was assigned null, and document SkillVerificationException on the remaining GetSkillAsync overload. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b
I think we should pick one. I was a little impatient when I made the other, as it represented what I'd like to have merged before the MCPCon in September. From what I remember the one I opened was more comprehensive, building on this one, and I put the author of this one as co-author of the foundational elements that I reused. I would suggest having one or more agents compare the two and pick based on that. |
…ected Both base classes had private protected constructors, so a package outside Core could not implement a paginated method. The Skills extension's skills/list is paginated. Widening to protected is source and binary compatible and passes package validation against the 2.0.0 baseline. Carried over from modelcontextprotocol#1856. Co-authored-by: Girish Konda <girish.sai1@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b
Implements the MCP Skills extension (io.modelcontextprotocol/skills) as a separate package alongside Extensions.Apps and Extensions.Tasks. Protocol: Skill, SkillResource, SkillResources (a closed array-or-"dynamic" union whose converter rejects null and every other shape), and the skills/list and skills/get request and result types, source-generated for AOT. ListSkillsResult links Core's internal CacheScopeConverter so unknown cacheScope values do not break deserialization of a listing. Server: McpServerSkill.Create and CreateFromDirectory build a skill's entry and its file resources together, computing each digest and size from the bytes the resource serves. Text is served as TextResourceContents only when the bytes round-trip through UTF-8, since hosts hash the UTF-8 of the text they receive; everything else is a blob. WithSkills(IEnumerable<McpServerSkill>) registers the methods, the extension and resources capabilities, and every file resource; WithSkills(IMcpSkillCatalog) backs the methods with a custom catalog. InMemoryMcpSkillCatalog uses keyset cursors. Every entry is validated at construction against the specification's structural MUSTs. Client: SupportsSkills, ListSkillsAsync (all pages or one), GetSkillAsync, and ReadSkillResourceAsync, which reads through resources/read and verifies size and digest against the held entry, refusing unlisted URIs before any request is sent. SkillVerifier exposes the same checks. resultType, ttlMs, and cacheScope are emitted only on requests negotiated under 2026-07-28 or later, where the latter two default to 0 and private when unset, matching Core's handling of the built-in list methods (modelcontextprotocol#1721). YAML frontmatter parsing and resources/directory/read are deliberately not included; see the docs page and PR description. The SkillResources union and converter and the keyset-cursor catalog design are carried over from modelcontextprotocol#1856. Co-authored-by: Girish Konda <girish.sai1@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b
Unit tests cover serialization (including every invalid manifest shape and frontmatter pass-through), the in-memory catalog (ordering, keyset pagination, cursor validation, and each structural validation rule), McpServerSkill (manifest computation, path normalization, resource metadata, directory loading), and SkillVerifier (known digest vectors, size and digest mismatches, text versus blob, unlisted files). End-to-end tests drive the client extensions against an in-process server for both WithSkills overloads: listing and pagination, skills served but not listed, dynamic skills, verified reads including a tampered resource, the -32602 error contract, and the wire shape of resultType, ttlMs, and cacheScope on 2025-11-25 and 2026-07-28 sessions, including the defaults applied when no options are set. The conformance server gains the SEP-2640 fixture: three static skills built with McpServerSkill plus a dynamic one, paged two at a time. The SEP-2640 scenarios from modelcontextprotocol/conformance#330 pass 30/30, 6/6, 1/1 stateless and 29/29, 6/6, 1/1 on a 2025-11-25 session. The AOT compatibility app now also lists, gets, and verifies a skill. The serialization tests and the fixture's skill set are carried over from modelcontextprotocol#1856. Co-authored-by: Girish Konda <girish.sai1@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b
Make CacheScopeConverter public in Core instead of compiling its source into the Skills package, which was the only package doing so. Adding a public type passes package validation against the 2.0.0 baseline. Validate only what the Agent Skills specification states as requirements beyond name and description: compatibility is 1 to 500 characters if provided, and metadata is a mapping. license, allowed-tools, and the values inside metadata pass through verbatim. Hosts compare frontmatter against the file and ignore allowed-tools for MCP-origin skills, so rejecting a whole skill over the shape of such a field would help nobody. Repeat the authorization caveat on WithSkills(IEnumerable<McpServerSkill>) and WithSkillsFromDirectory, whose in-memory catalog serves every caller, and explain why a directory without SKILL.md is skipped while an invalid skill is an error. Guard the public verifier and client read path against a Skill whose required Resources property was assigned null, and document SkillVerificationException on the remaining GetSkillAsync overload. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b
Import the net changes from PR modelcontextprotocol#1864 on top of the rebased modelcontextprotocol#1856 foundation, including the client, request-aware catalogs, validation, byte-snapshot authoring, YAML reader, samples, docs and regression tests. The shared foundation is retained once, not replayed. Source PR: modelcontextprotocol#1864 Source head: ecb11e6 Same-main source tree: ba9234d Source lineage (including already-shared foundation): 4a7d3f5 Widen PaginatedRequestParams and PaginatedResult constructors to protected 9569706 Add ModelContextProtocol.Extensions.Skills (SEP-2640) 50f7cd7 Add tests and a conformance fixture for the Skills extension 44d80ae Add Skills samples and docs 53e5425 Harden skill authoring and verified reads b2e92aa Read skill frontmatter from SKILL.md 9dc02bb Align the frontmatter reader with reference YAML parsers 72a0406 Point SkillVerifier remarks at SkillFrontmatter for frontmatter checks 932c318 Link the Skills docs page to the samples by relative path f18d7dc Address the Copilot review of modelcontextprotocol#1864 8c39436 Address the second Copilot review of modelcontextprotocol#1864 8593f0d Bind skills/get answers to the requested URI 82f0943 Enforce skill limits while reading a directory; match Core's URI equivalence fe89998 Address review feedback from the author of modelcontextprotocol#1856 4813b72 Complete earlier fixes and align the reader further with reference parsers ecb11e6 Close the neighbouring cases a differential YAML run and a second review pass found This local squash preserves human authorship, not source signatures. No consolidation repairs are included in this import. Co-authored-by: Girish Konda <girish.sai1@gmail.com>
cbd8a58 to
6b0870c
Compare
|
@PederHP @tarekgh @Alumniminium I compared #1856 and #1864 and found a few remaining gaps in #1864. I pulled Peder's changes into this #1856, preserving his authorship and crediting him as co-author, and then added my fixes on top of it. I kept his client helpers, entry validation, file authoring, samples and docs. I also fixed the Comparison and fixesCompared sources
Both PRs were compared on the same Implementation comparison
The stable spec added the Additional corrections
The trailing-comment check matches the Parser responsibilityI kept Peder's parser as an optional authoring helper. There's no new YAML dependency.
This keeps the separation used in the TypeScript and Python proposals without dropping the authoring helpers. Go also separates digest checks from frontmatter verification. Those peer PRs were open when inspected, not released implementations. The explicit-metadata fallback still can't validate the rest of a file after unsupported syntax. I haven't claimed that as a parser fix. The docs now state the caller's responsibility, and hosts must still verify the fetched file. A maintained YAML-library/provider can be a separate follow-up. History and creditPeder is the author of the import commit, which records #1864 and the source commits. Before our fixes, its full tree is Validation of the consolidated commitThese results are for
The full-suite filter excludes two declared manual Docker cases. The four skips are two API-key-dependent cases, one unsupported-platform case and one optional-default-extraction limitation. No failing assertion or audit check was disabled. The Windows legacy runner shutdown failure is still open. This isn't an all-green conformance matrix, and local results don't replace hosted CI. Remaining scope and review decisions
— girishkvs's Copilot 🤖 |
|
Looks good. I made a PR (girishkvs#1) with some suggestions mostly related to docs and description. I was going to close #1864 so we can focus on getting this one over the finish line, but I was beat to it by a few minutes. Very much agree on joining forces and thank you for taking in what you did from the other one. |
There was a problem hiding this comment.
🟡 Changes recommended
Protocol conformance, authorization exposure, validation gaps, and inaccurate PR metadata remain unresolved.
7 open findings
Apply authorization to skill catalog handlers · New Require skills property to reject omitted response members · New Reject invalid UTF-8 in cursors · New Validate Unicode lowercase letters by scalar value · New Enforce string-only metadata values · New Document or revert unintended public converter API · New Document newly added public client wrappers · New
What changed in this PR
Adds SEP-2640 Skills support, including protocol models, server/client APIs, validation, samples, conformance fixtures, and AOT coverage.
Changes:
- Adds Skills discovery, retrieval, authoring, and integrity verification.
- Adds directory-backed and custom catalogs with pagination.
- Expands Core pagination and cache-converter APIs.
| File | Description |
|---|---|
README.md |
Lists the Skills package. |
ModelContextProtocol.slnx |
Adds Skills projects and samples. |
docs/concepts/index.md |
Links Skills documentation. |
docs/concepts/toc.yml |
Adds Skills navigation. |
docs/concepts/skills/skills.md |
Documents serving, consuming, and security. |
src/Directory.Build.targets |
Includes Skills in package dependency versioning. |
src/PACKAGE.md |
Lists the Skills package. |
src/ModelContextProtocol.Core/Protocol/CacheScopeConverter.cs |
Makes the converter public. |
src/ModelContextProtocol.Core/Protocol/PaginatedRequest.cs |
Allows external derivation. |
src/ModelContextProtocol.Core/Protocol/PaginatedResult.cs |
Allows external derivation. |
src/ModelContextProtocol.Extensions.Skills/ModelContextProtocol.Extensions.Skills.csproj |
Defines the new package. |
src/ModelContextProtocol.Extensions.Skills/SkillsProtocol.cs |
Defines protocol constants and limits. |
src/ModelContextProtocol.Extensions.Skills/McpSkillsJsonContext.cs |
Adds AOT serialization metadata. |
src/ModelContextProtocol.Extensions.Skills/SkillFrontmatter.cs |
Implements bounded YAML frontmatter parsing. |
src/ModelContextProtocol.Extensions.Skills/SkillValidation.cs |
Validates skill entries and manifests. |
src/ModelContextProtocol.Extensions.Skills/Protocol/Skill.cs |
Defines skill entries. |
src/ModelContextProtocol.Extensions.Skills/Protocol/SkillResource.cs |
Defines manifest files. |
src/ModelContextProtocol.Extensions.Skills/Protocol/SkillResources.cs |
Models static or dynamic manifests. |
src/ModelContextProtocol.Extensions.Skills/Protocol/SkillResourcesConverter.cs |
Serializes the manifest union. |
src/ModelContextProtocol.Extensions.Skills/Protocol/ListSkillsRequestParams.cs |
Defines list parameters. |
src/ModelContextProtocol.Extensions.Skills/Protocol/ListSkillsResult.cs |
Defines paginated list results. |
src/ModelContextProtocol.Extensions.Skills/Protocol/GetSkillRequestParams.cs |
Defines get parameters. |
src/ModelContextProtocol.Extensions.Skills/Protocol/GetSkillResult.cs |
Defines get results. |
src/ModelContextProtocol.Extensions.Skills/Server/IMcpSkillCatalog.cs |
Defines the catalog contract. |
src/ModelContextProtocol.Extensions.Skills/Server/InMemoryMcpSkillCatalog.cs |
Implements keyset pagination. |
src/ModelContextProtocol.Extensions.Skills/Server/McpSkillPage.cs |
Models catalog pages. |
src/ModelContextProtocol.Extensions.Skills/Server/McpSkillRequestContext.cs |
Exposes caller context to catalogs. |
src/ModelContextProtocol.Extensions.Skills/Server/McpServerSkill.cs |
Authors entries and file resources. |
src/ModelContextProtocol.Extensions.Skills/Server/McpServerSkillFile.cs |
Models authored files. |
src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsBuilderExtensions.cs |
Registers Skills handlers and resources. |
src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsOptions.cs |
Configures caching hints. |
src/ModelContextProtocol.Extensions.Skills/Client/McpSkillsClientExtensions.cs |
Adds client Skills operations. |
src/ModelContextProtocol.Extensions.Skills/Client/SkillVerifier.cs |
Verifies resource integrity. |
src/ModelContextProtocol.Extensions.Skills/Client/SkillVerificationException.cs |
Defines verification failures. |
samples/SkillsClient/SkillsClient.csproj |
Defines the client sample. |
samples/SkillsClient/Program.cs |
Demonstrates discovery and verification. |
samples/SkillsClient/README.md |
Documents the client sample. |
samples/SkillsServer/SkillsServer.csproj |
Defines the server sample. |
samples/SkillsServer/Program.cs |
Configures directory-backed skills. |
samples/SkillsServer/README.md |
Documents the server sample. |
samples/SkillsServer/Properties/launchSettings.json |
Configures the sample endpoint. |
samples/SkillsServer/Skills/git-workflow/SKILL.md |
Adds a Git workflow skill. |
samples/SkillsServer/Skills/git-workflow/references/COMMIT_STYLE.md |
Adds commit guidance. |
samples/SkillsServer/Skills/git-workflow/templates/PULL_REQUEST.md |
Adds a pull-request template. |
samples/SkillsServer/Skills/refunds/SKILL.md |
Adds a refunds skill. |
samples/SkillsServer/Skills/refunds/policy/REFUND_POLICY.md |
Adds refund policy data. |
samples/SkillsServer/Skills/refunds/examples/approved.md |
Adds an approval response. |
samples/SkillsServer/Skills/refunds/examples/declined.md |
Adds a decline response. |
tests/ModelContextProtocol.AotCompatibility.TestApp/ModelContextProtocol.AotCompatibility.TestApp.csproj |
Roots the Skills assembly for AOT. |
tests/ModelContextProtocol.AotCompatibility.TestApp/Program.cs |
Exercises Skills under AOT. |
tests/ModelContextProtocol.ConformanceServer/ModelContextProtocol.ConformanceServer.csproj |
References the Skills package. |
tests/ModelContextProtocol.ConformanceServer/Program.cs |
Registers conformance skills. |
tests/ModelContextProtocol.ConformanceServer/Skills/ConformanceSkills.cs |
Defines conformance fixtures. |
tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj |
References the Skills package. |
tests/ModelContextProtocol.Tests/ClientServerTestBase.cs |
Enables derived async cleanup. |
tests/ModelContextProtocol.Tests/Protocol/SkillSerializationTests.cs |
Tests protocol serialization. |
tests/ModelContextProtocol.Tests/Client/McpSkillsClientValidationTests.cs |
Tests hostile server responses. |
tests/ModelContextProtocol.Tests/Client/SkillVerifierTests.cs |
Tests digest verification. |
tests/ModelContextProtocol.Tests/Server/InMemoryMcpSkillCatalogTests.cs |
Tests catalog behavior. |
tests/ModelContextProtocol.Tests/Server/McpServerSkillAuthoringBoundaryTests.cs |
Tests authoring boundaries. |
tests/ModelContextProtocol.Tests/Server/McpServerSkillTests.cs |
Tests skill authoring. |
tests/ModelContextProtocol.Tests/Server/McpServerSkillsCatalogTests.cs |
Tests custom catalogs. |
tests/ModelContextProtocol.Tests/Server/McpServerSkillsDefaultsTests.cs |
Tests default cache hints. |
tests/ModelContextProtocol.Tests/Server/McpServerSkillsFromDirectoryTests.cs |
Tests directory loading. |
tests/ModelContextProtocol.Tests/Server/McpServerSkillsIntegrityTests.cs |
Tests content integrity. |
tests/ModelContextProtocol.Tests/Server/McpServerSkillsOwnershipTests.cs |
Tests metadata and byte ownership. |
tests/ModelContextProtocol.Tests/Server/McpServerSkillsTests.cs |
Tests end-to-end Skills behavior. |
tests/ModelContextProtocol.Tests/Server/SkillFrontmatterCorpusTests.cs |
Tests parser compatibility corpus. |
tests/ModelContextProtocol.Tests/Server/SkillFrontmatterNodeClassificationTests.cs |
Tests YAML node classification. |
tests/ModelContextProtocol.Tests/Server/SkillFrontmatterRegressionTests.cs |
Adds parser regression coverage. |
tests/ModelContextProtocol.Tests/Server/SkillFrontmatterTests.cs |
Tests frontmatter parsing. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Should we upgrade the conformance test package to |
|
@tarekgh Re your alpha.12 question: Pinned alpha.12 and added explicit Skills scenarios for both protocol revisions; a nonzero runner exit now fails even if stdout reports success, and the tests require actual executed checks. — girishkvs's Copilot 🤖 |
Thanks, I already have the PR #1946 to address all timeout issues. |
|
@girishkvs thanks again. We got a couple of copilot comments we need to address before merging. We almost there 😄 |
Import the net changes from PR modelcontextprotocol#1864 on top of the rebased modelcontextprotocol#1856 foundation, including the client, request-aware catalogs, validation, byte-snapshot authoring, YAML reader, samples, docs and regression tests. The shared foundation is retained once, not replayed. Source PR: modelcontextprotocol#1864 Source head: ecb11e6 Same-main source tree: ba9234d Source lineage (including already-shared foundation): 4a7d3f5 Widen PaginatedRequestParams and PaginatedResult constructors to protected 9569706 Add ModelContextProtocol.Extensions.Skills (SEP-2640) 50f7cd7 Add tests and a conformance fixture for the Skills extension 44d80ae Add Skills samples and docs 53e5425 Harden skill authoring and verified reads b2e92aa Read skill frontmatter from SKILL.md 9dc02bb Align the frontmatter reader with reference YAML parsers 72a0406 Point SkillVerifier remarks at SkillFrontmatter for frontmatter checks 932c318 Link the Skills docs page to the samples by relative path f18d7dc Address the Copilot review of modelcontextprotocol#1864 8c39436 Address the second Copilot review of modelcontextprotocol#1864 8593f0d Bind skills/get answers to the requested URI 82f0943 Enforce skill limits while reading a directory; match Core's URI equivalence fe89998 Address review feedback from the author of modelcontextprotocol#1856 4813b72 Complete earlier fixes and align the reader further with reference parsers ecb11e6 Close the neighbouring cases a differential YAML run and a second review pass found This local squash preserves human authorship, not source signatures. No consolidation repairs are included in this import. Co-authored-by: Girish Konda <girish.sai1@gmail.com>
68acfb2 to
f84344e
Compare
|
@tarekgh Done: rebased on #1946 and fixed both Copilot comments. — girishkvs's Copilot 🤖 |
Adds ModelContextProtocol.Extensions.Skills implementing the skills/list and skills/get methods of SEP-2640, alongside the existing Tasks and Apps extension packages. Declares the io.modelcontextprotocol/skills capability inline and registers both methods through McpServerOptions.RequestHandlers, so Core is unchanged. Skill manifests are modelled as SkillResources, a closed type admitting only an enumerated file list or the dynamic marker, so an invalid manifest cannot be constructed. Pagination uses keyset cursors over URI order. skills/get sets RoutingNameParameter to uri, matching how Core routes resources/read for the Mcp-Name header. ttlMs and cacheScope are gated on the negotiated protocol version by default, which SEP-2640 currently requires, but the gate is configurable while the working group settles whether that condition is removed.
Import the net changes from PR modelcontextprotocol#1864 on top of the rebased modelcontextprotocol#1856 foundation, including the client, request-aware catalogs, validation, byte-snapshot authoring, YAML reader, samples, docs and regression tests. The shared foundation is retained once, not replayed. Source PR: modelcontextprotocol#1864 Source head: ecb11e6 Same-main source tree: ba9234d Source lineage (including already-shared foundation): 4a7d3f5 Widen PaginatedRequestParams and PaginatedResult constructors to protected 9569706 Add ModelContextProtocol.Extensions.Skills (SEP-2640) 50f7cd7 Add tests and a conformance fixture for the Skills extension 44d80ae Add Skills samples and docs 53e5425 Harden skill authoring and verified reads b2e92aa Read skill frontmatter from SKILL.md 9dc02bb Align the frontmatter reader with reference YAML parsers 72a0406 Point SkillVerifier remarks at SkillFrontmatter for frontmatter checks 932c318 Link the Skills docs page to the samples by relative path f18d7dc Address the Copilot review of modelcontextprotocol#1864 8c39436 Address the second Copilot review of modelcontextprotocol#1864 8593f0d Bind skills/get answers to the requested URI 82f0943 Enforce skill limits while reading a directory; match Core's URI equivalence fe89998 Address review feedback from the author of modelcontextprotocol#1856 4813b72 Complete earlier fixes and align the reader further with reference parsers ecb11e6 Close the neighbouring cases a differential YAML run and a second review pass found This local squash preserves human authorship, not source signatures. No consolidation repairs are included in this import. Co-authored-by: Girish Konda <girish.sai1@gmail.com>
Add ICacheableResult and source-generated cache-hint serialization to skills/get. Apply the configured hints or zero/private defaults on 2026-07-28 requests, and keep all versioned fields off 2025-11-25 replies. Read scalar values starting on the next line, preserve folded line breaks around TAB-led content, and reject comments without whitespace after a quoted scalar or flow collection. Keep plain hashes, existing chomping rules and non-JSON number rejection intact. Add parser, authoring, wire and serialization regressions. Exercise the new cache fields and authoring cases in the native AOT app. Update the cache option docs and pin both sample links to their published source.
Consolidate frontmatter verification responsibilities and use main-branch sample links. Include both commits from PederHP's documentation pull request, without our follow-up changes.
54eaf86 to
5f5e548
Compare
5f5e548 to
2317ca8
Compare
Address Skills review feedback with strict entry validation, safe directory selection, bounded aggregation, explicit diagnostic pages, and validated catalog snapshots. Exercise Skills conformance with strict process exits. Count Unicode scalars consistently, retain entry metadata in owned snapshots, and authorize catalog entries before pagination. Support encoded names and native absolute resource URIs without rewriting resource identity. Type-check every frontmatter field the Agent Skills specification defines: reject a license or allowed-tools value that is not a string, and read the license, compatibility, and allowed-tools fields through typed Skill properties. Unknown fields still pass through. Reject a second Skills registration for the same services, and fail when another resource is registered at a Skills file URI. A registration that fails leaves no file resources behind. Treat a received string that is not valid Unicode text, such as an escaped unpaired surrogate, as malformed JSON: the diagnostic listing rejects the entry, and the strict list and get methods throw JsonException. Reject unsupported flow mappings instead of publishing incorrect frontmatter, and report valid YAML the reader does not support as unsupported rather than malformed. Read indented root mappings and single-line root flow mappings, reject any other root as not a mapping, and report multi-line flow collections, comments included, as unsupported only when they are valid YAML. Resolve hexadecimal and octal integers within 64 bits and report wider literals as unsupported. Reject a raw character outside YAML's printable set, such as a NUL, and a lone CR, NEL, LS or PS, on any frontmatter line before parsing; escapes such as \0 and \L stay valid. Report null read contents as verification failures. Strengthen conformance summary assertions and cover nested Unicode and native URI resource reads. Keep the frontmatter reader internal to server authoring; the client sample verifies fetched frontmatter with YamlDotNet, reading its parsing events within limits on nesting depth and alias expansion and rejecting alias cycles. The sample keeps the line break before the closing delimiter, so a final kept block scalar reads as it does on the server; a test compiles the sample's reader and compares it with the server. The sample also rejects the same raw characters as the server, strips one leading byte order mark as the server does, and rejects a tag on a collection that the conversion would change, such as !!str on a list. Document that a shared skill catalog must not capture scoped services. Test that injected parameters stay out of wire schemas when parameters are re-enumerated. Connect the Skills authorization and conformance fixture tests to their controlled test servers with TestMcpClient.CreateAsync, as the contributing guide asks. Use owned conformance-host readiness, bounded startup and cleanup, and deterministic cancellation-observation tests. Wait for the conformance runner's exit and the end of both output streams within one deadline. Kill a runner that is still running at the deadline together with its child processes. Read the runner's output streams on dedicated threads, so reads held open by a descendant do not tie up thread-pool threads. Validate repository links against the proposed checkout.
2317ca8 to
8398c30
Compare
|
@tarekgh Fixed a couple of bugs in this PR's test helper — girishkvs's Copilot 🤖 |
tarekgh
left a comment
There was a problem hiding this comment.
Thanks a lot @girishkvs for your effort getting this done in such shape. Thanks to @PederHP too for his input and feedback on the changes.
LGTM!



Summary
Adds
ModelContextProtocol.Extensions.Skills, implementing theskills/listandskills/getmethods of SEP-2640 (io.modelcontextprotocol/skills, accepted 2026-09-01), alongside the existing Tasks and Apps extension packages. The capability is declared inline and both methods are registered throughMcpServerOptions.RequestHandlers, so the extension consumes only public Core API.The Core API changes are intentional, and I am happy to split them into their own PR.
PaginatedResultandPaginatedRequestParamshadprivate protectedconstructors, so a package outside Core cannot implement a paginated method.skills/listis paginated. Widening those toprotectedis source and binary compatible, and package validation passes against the 2.0.0 baseline.CacheScopeConverteris also public so extensions can reuse Core's read-side leniency for unrecognized scope values.A skill's manifest uses a closed
SkillResourcestype admitting only an enumerated file list or the"dynamic"marker, and"resources": nullis rejected on read. Entries receive structural validation, and pagination uses keyset cursors over URI order. The built-in catalog owns validated snapshots; custom catalogs remain checked on each request.ttlMs,cacheScopeandresultTypeare emitted on both list/get results for protocol revision2026-07-28and omitted on older revisions. Cache values are configurable and default to immediately stale/private. Client-side wrappers, byte-consistent file authoring, an internal bounded YAML reader and both client/server samples are included.Directory loading excludes dot-prefixed, hidden and system entries by default, with explicit opt-in. Aggregating client listing defaults to 100 pages and 1,000 entries, with configurable positive limits and cursor-cycle errors rather than silent truncation. Validation is strict: server authoring rejects skills that break the Agent Skills format, and the list helpers reject a page with an invalid entry.
ListSkillsWithDiagnosticsAsyncreturns the valid entries plus per-entry failures; there's no warn-and-load mode.Earlier review follow-ups added optional entry-level
_metawith owned snapshots, Unicode-scalar limits fordescriptionandcompatibility, and authorization-safe pagination in the shared-policy example. Encoded Unicode names and native absolute resource URIs are supported without rewriting resource identity, and null resource-read contents fail withSkillVerificationException.The latest follow-ups address Peder's and Tarek's reviews:
WithSkills/WithSkillsFromDirectorycall fails at registration with a Skills-specific error.gopkg.in/yaml.v3. Wider literals are reported as unsupported rather than converted, which removes the expensiveBigIntegerpath. Valid YAML outside the bounded reader, such as explicit?keys or comments inside multi-line flow collections, is reported as unsupported rather than malformed, so the explicit-frontmatter overload accepts it. Indented root mappings now parse; malformed YAML and a non-mapping root still fail.YamlDotNet, used only by the sample and its test.The previous update, rebased on
mainwith MEAI 10.11.0, addressed the Copilot review:licenseorallowed-toolsthat is not a string is rejected in authoring, catalog construction and client validation, andSkillexposesLicense,CompatibilityandAllowedToolsas typed properties. Unknown fields still pass through.WithSkillsadds file resources only after every skill, the catalog andconfiguresucceed, so a failed call leaves nothing behind and a retry with new skills works.YamlDotNetcheck rejects alias cycles and caps nesting at 32 levels and the node count at 10,000, counting each alias as the nodes it expands to, reporting each as a verification failure.WaitForExitAsyncon .NET, exit and end-of-stream signals on .NET Framework. A timeout fails even after exit, and a runner that is still running is killed with its child processes.The next update, rebased on
mainat v2.3.0, addressed further Copilot and Tarek comments:---, so a final kept block scalar (|+) reads the same as on the server. A new test compiles the sample'sYamlFrontmatterReaderand checks that its frontmatter equals the server's for kept, clipped and stripped final blocks, and for a kept block with CRLF.ListSkillsWithDiagnosticsAsyncrejects that entry and returns the others, and the strict list and get helpers throwJsonExceptioninstead ofInvalidOperationException.This update is rebased on
mainwith #1946 and addresses the two new Copilot comments:yaml.v3and PyYAML do. It also rejects a lone CR, NEL, LS or PS, which some YAML parsers read as a line break and others as text, so these characters can't make two parsers read different keys from the same file. Both authoring overloads refuse such a file; escapes such as\0or\Lin double-quoted strings stay valid.!, or!!seqon a list and!!mapon a mapping), so!!str [one]or a root tagged!!strfails verification.The Core discovery/OAuth changes are no longer in this PR. #1719 fixes the
server/discoverprobe interrupting OAuth, including a finite probe timeout whenInitializationTimeoutis infinite, and #1891 handles metadata-fetch cancellation. The injected-parameter schema workaround was removed after the MEAI 10.11.0 upgrade (#1940);InjectedParameterSchemaTestsstays.I would still like maintainer input on the authorization boundary and public Core API shape:
RequestHandlersdo not automatically pass throughAuthorizationFilterSetup. The current implementation keeps one shared application policy for catalog visibility and resource access, with HTTP allow/deny and cursor-disclosure coverage. It does not derive authorization fromresources/list, since that listing may be partial while direct reads remain valid. Would you prefer a general filter hook for custom request handlers in Core?CacheScopeConverterpublic is deliberate, alongside the pagination constructors. I can split these API changes if that is the preferred package boundary.Following
CONTRIBUTING.mdI would normally open an issue first. I am opening this as a PR because the working group asked for a reference implementation in a Tier 1 SDK and Go already has one in flight. Happy to convert this to an issue, split it, or reshape the API if you would prefer.Changes
Extensions.Skills/SkillsProtocol.csExtensions.Skills/Protocol/Skill.cs,SkillResource.cs_metaExtensions.Skills/Protocol/SkillResources.cs,SkillResourcesConverter.cs"dynamic"union and its converterExtensions.Skills/Protocol/ListSkills*.cs,GetSkill*.csExtensions.Skills/McpSkillsJsonContext.csExtensions.Skills/Server/IMcpSkillCatalog.cs,McpSkillPage.cs,McpSkillRequestContext.csExtensions.Skills/Server/InMemoryMcpSkillCatalog.csExtensions.Skills/Server/McpSkillsBuilderExtensions.cs,McpSkillsOptions.csWithSkills(), directory discovery, safe selection defaults, one registration per server, conflict check against other resources, and optionsExtensions.Skills/Server/McpServerSkill*.cs,SkillFrontmatter.csExtensions.Skills/SkillValidation.csExtensions.Skills/ClientCore/Protocol/PaginatedResult.cs,PaginatedRequest.cs,CacheScopeConverter.csprotected; expose the existing cache-scope converterConformanceServer/Skills/ConformanceSkills.csConformanceServer/Program.cs, conformance fixture/testssamples/SkillsServer,samples/SkillsClient, guide and link workflowYamlDotNet; the test project compiles the sample's reader to compare it with the server); preservemainlinks and validate same-repository paths against the proposed checkoutValidation
Local Windows results for committed source
f84344ec, treeeafdb888, onmain2bd715b3(#1946):net10.0,net9.0,net8.0andnetstandard2.0, with APICompat clean; all 21 sample projects build (24 builds, withTestServerWithHostingon four targets)ModelContextProtocol.Testsproject and the HTTP Skills tests inModelContextProtocol.AspNetCore.Tests, zero failures:ModelContextProtocol.TestsNo Skills test is skipped. The skips are existing upstream conditions: OpenAI API key, platform, and an
AIJsonUtilitieslimitation, plus #587 (280), non-serializable test data (2) and manual Docker tests (2) on .NET Framework.win-x64publish and execution, and DocFX 2.78.5 warnings-as-errorsInjectedParameterSchemaTestspasses on all four targets with MEAI 10.11.0 and no workaround; with 10.8.3 its re-enumerated-parameter cases failThe .NET 9 reflection-disabled setting remains unchanged.
SEP-2640 conformance scenarios in pinned
0.2.0-alpha.12, run explicitly against the correctConformanceServerendpoints:sep-2640-skills-enumerationsep-2640-skills-manifestsep-2640-skills-directorydirectoryReadnot declared)A printed success summary cannot override a failed process. The summary assertion also rejects zero checks, unequal passed/total counts and duplicate summaries. Earlier Node 24 aborts remain recorded, not retroactively green.
New-head hosted CI, including Linux/macOS, must be checked separately after publication.
resources/directory/readis not implemented in this PR, sodirectoryReadis not declared. Client wrappers are included; the results above are server-side conformance, not a claim to implement every host policy. Hosts must still parse fetchedSKILL.mdwith a full YAML parser and compare frontmatter before loading; the client sample shows one. For YAML outside the bounded reader, explicit metadata remains the caller's full-validation responsibility. A maintained YAML library/provider, full host approval binding and origin isolation remain separate scope.