Skip to content

Add SEP-2640 Skills extension support - #1856

Merged
tarekgh merged 5 commits into
modelcontextprotocol:mainfrom
girishkvs:skills_sep2640
Oct 23, 2026
Merged

tarekgh merged 5 commits into
modelcontextprotocol:mainfrom
girishkvs:skills_sep2640

Conversation

@girishkvs

@girishkvs girishkvs commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds ModelContextProtocol.Extensions.Skills, implementing the skills/list and skills/get methods 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 through McpServerOptions.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. PaginatedResult and PaginatedRequestParams had private protected constructors, so a package outside Core cannot implement a paginated method. skills/list is paginated. Widening those to protected is source and binary compatible, and package validation passes against the 2.0.0 baseline. CacheScopeConverter is also public so extensions can reuse Core's read-side leniency for unrecognized scope values.

A skill's manifest uses a closed SkillResources type admitting only an enumerated file list or the "dynamic" marker, and "resources": null is 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, cacheScope and resultType are emitted on both list/get results for protocol revision 2026-07-28 and 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. ListSkillsWithDiagnosticsAsync returns the valid entries plus per-entry failures; there's no warn-and-load mode.

Earlier review follow-ups added optional entry-level _meta with owned snapshots, Unicode-scalar limits for description and compatibility, 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 with SkillVerificationException.

The latest follow-ups address Peder's and Tarek's reviews:

  • Skills can be registered once per server; a second WithSkills/WithSkillsFromDirectory call fails at registration with a Skills-specific error.
  • A Skills file whose URI collides with another registered resource fails when the server options are built, in either registration order, instead of serving different bytes than the manifest declares.
  • The frontmatter reader resolves hexadecimal and octal integers within 64 bits, matching gopkg.in/yaml.v3. Wider literals are reported as unsupported rather than converted, which removes the expensive BigInteger path. 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.
  • The frontmatter reader is internal. The client sample verifies fetched frontmatter with YamlDotNet, used only by the sample and its test.
  • On .NET Framework, the conformance-process test helper drains redirected output before reading it; strict exits are unchanged.
  • The guide notes that a custom catalog's own scopes are separate from the request scope and that a shared catalog must not capture scoped services.
  • The client sample reads cache hints and entries from the same page responses.

The previous update, rebased on main with MEAI 10.11.0, addressed the Copilot review:

  • Every frontmatter field the Agent Skills specification defines is type-checked. A license or allowed-tools that is not a string is rejected in authoring, catalog construction and client validation, and Skill exposes License, Compatibility and AllowedTools as typed properties. Unknown fields still pass through.
  • WithSkills adds file resources only after every skill, the catalog and configure succeed, so a failed call leaves nothing behind and a retry with new skills works.
  • The client sample's YamlDotNet check 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.
  • The conformance-process helper waits for the runner's exit and both output streams under one deadline: WaitForExitAsync on .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 main at v2.3.0, addressed further Copilot and Tarek comments:

  • The client sample keeps the line break before the closing ---, so a final kept block scalar (|+) reads the same as on the server. A new test compiles the sample's YamlFrontmatterReader and checks that its frontmatter equals the server's for kept, clipped and stripped final blocks, and for a kept block with CRLF.
  • A skill entry with a string that is not valid Unicode text, such as an escaped unpaired surrogate, counts as malformed JSON: ListSkillsWithDiagnosticsAsync rejects that entry and returns the others, and the strict list and get helpers throw JsonException instead of InvalidOperationException.

This update is rebased on main with #1946 and addresses the two new Copilot comments:

  • The server's frontmatter reader rejects a raw character outside YAML's printable set, such as a NUL, on any frontmatter line, comments included, before parsing, as libyaml, Go's yaml.v3 and 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 \0 or \L in double-quoted strings stay valid.
  • The client sample rejects the same raw characters, because YamlDotNet accepts most of them, can skip a NUL and reads a lone CR, NEL, LS or PS as a line break. It also accepts a tag on a collection only when the conversion keeps its meaning (!, or !!seq on a list and !!map on a mapping), so !!str [one] or a root tagged !!str fails verification.

The Core discovery/OAuth changes are no longer in this PR. #1719 fixes the server/discover probe interrupting OAuth, including a finite probe timeout when InitializationTimeout is infinite, and #1891 handles metadata-fetch cancellation. The injected-parameter schema workaround was removed after the MEAI 10.11.0 upgrade (#1940); InjectedParameterSchemaTests stays.

I would still like maintainer input on the authorization boundary and public Core API shape:

  • Skills served through RequestHandlers do not automatically pass through AuthorizationFilterSetup. 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 from resources/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?
  • Making CacheScopeConverter public is deliberate, alongside the pagination constructors. I can split these API changes if that is the preferred package boundary.

Following CONTRIBUTING.md I 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

File What
Extensions.Skills/SkillsProtocol.cs New. Method names, extension id, limits, sentinels
Extensions.Skills/Protocol/Skill.cs, SkillResource.cs New. Entry and manifest file shapes, including optional entry _meta
Extensions.Skills/Protocol/SkillResources.cs, SkillResourcesConverter.cs New. Array-or-"dynamic" union and its converter
Extensions.Skills/Protocol/ListSkills*.cs, GetSkill*.cs New. Required request/result shapes and protocol-gated cache hints
Extensions.Skills/McpSkillsJsonContext.cs New. Source-generated serialization for AOT
Extensions.Skills/Server/IMcpSkillCatalog.cs, McpSkillPage.cs, McpSkillRequestContext.cs New. Request-aware catalog abstraction and caller context
Extensions.Skills/Server/InMemoryMcpSkillCatalog.cs New. Keyset cursors, strict UTF-8 decoding, validated owned snapshots and a narrow built-in fast path
Extensions.Skills/Server/McpSkillsBuilderExtensions.cs, McpSkillsOptions.cs New. WithSkills(), directory discovery, safe selection defaults, one registration per server, conflict check against other resources, and options
Extensions.Skills/Server/McpServerSkill*.cs, SkillFrontmatter.cs New. Manifests computed from served bytes; selected directory files, encoded path names, link rejection, and internal bounded-subset frontmatter parsing with a raw-character check, 64-bit hex/octal integers and unsupported-syntax classification
Extensions.Skills/SkillValidation.cs New. Structural validation, Unicode-scalar bounds, string-valued known metadata and native/encoded URI checks without losing traversal guards
Extensions.Skills/Client New. Capability/list/get helpers, bounded aggregation, diagnostic pages and manifest-bound resource verification, including explicit null-content errors and unreadable text reported as malformed JSON
Core/Protocol/PaginatedResult.cs, PaginatedRequest.cs, CacheScopeConverter.cs Widen pagination constructors to protected; expose the existing cache-scope converter
ConformanceServer/Skills/ConformanceSkills.cs New. Skill fixture; digests computed from the same bytes the resources serve
ConformanceServer/Program.cs, conformance fixture/tests Register skills/resources, await owned host readiness, use assigned loopback ports and retain bounded startup/cleanup ownership
Skills protocol, server and client tests New. Serialization, hostile responses, file/catalog ownership, pagination, policy alignment, registration/collision, Unicode/native URI wire behavior, unreadable-text responses and parser regressions
Conformance package / runner tests Pin alpha.12, select all three Skills scenarios explicitly for both revisions, require successful native process exits and a single positive, complete summary
samples/SkillsServer, samples/SkillsClient, guide and link workflow New. Serving and verified-consumption samples (the client sample uses YamlDotNet; the test project compiles the sample's reader to compare it with the server); preserve main links and validate same-repository paths against the proposed checkout

Validation

Local Windows results for committed source f84344ec, tree eafdb888, on main 2bd715b3 (#1946):

  • All six packages build and pack with warnings as errors across net10.0, net9.0, net8.0 and netstandard2.0, with APICompat clean; all 21 sample projects build (24 builds, with TestServerWithHosting on four targets)
  • The whole ModelContextProtocol.Tests project and the HTTP Skills tests in ModelContextProtocol.AspNetCore.Tests, zero failures:
Target ModelContextProtocol.Tests HTTP Skills
.NET 10 3,358 passed, 4 skipped 23 passed
.NET 9 3,358 passed, 4 skipped 23 passed
.NET 8 3,358 passed, 4 skipped 23 passed
.NET Framework 4.7.2 2,988 passed, 288 skipped Not an HTTP project target

No Skills test is skipped. The skips are existing upstream conditions: OpenAI API key, platform, and an AIJsonUtilities limitation, plus #587 (280), non-serializable test data (2) and manual Docker tests (2) on .NET Framework.

  • Complete server and client conformance selection: 86/86 on .NET 10/9/8 with Node 22.23.3 and strict process exits, no runner crash
  • Native AOT win-x64 publish and execution, and DocFX 2.78.5 warnings-as-errors
  • InjectedParameterSchemaTests passes on all four targets with MEAI 10.11.0 and no workaround; with 10.8.3 its re-enumerated-parameter cases fail

The .NET 9 reflection-disabled setting remains unchanged.

SEP-2640 conformance scenarios in pinned 0.2.0-alpha.12, run explicitly against the correct ConformanceServer endpoints:

Scenario 2025-11-25 2026-07-28
sep-2640-skills-enumeration 30/30 32/32
sep-2640-skills-manifest 6/6 6/6
sep-2640-skills-directory 1/1, 6 not applicable (directoryRead not declared) 1/1, 6 not applicable

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/read is not implemented in this PR, so directoryRead is 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 fetched SKILL.md with 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.

@Alumniminium

Copy link
Copy Markdown

Lets get this merged asap please

@PederHP

PederHP commented Sep 8, 2026

Copy link
Copy Markdown
Member

At least one sample should be included imo.

PederHP added a commit to PederHP/csharp-sdk that referenced this pull request Sep 9, 2026
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
@tarekgh

tarekgh commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

@PederHP I see that you’ve opened PR #1864. Do you think we should keep both PRs, or would it make sense to close one in favor of the other?

@PederHP

PederHP commented Oct 9, 2026

Copy link
Copy Markdown
Member

@PederHP I see that you’ve opened PR #1864: #1864. Do you think we should keep both PRs, or would it make sense to close one in favor of the other?

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.

PederHP added a commit to PederHP/csharp-sdk that referenced this pull request Oct 10, 2026
…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
PederHP added a commit to PederHP/csharp-sdk that referenced this pull request Oct 10, 2026
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
PederHP added a commit to PederHP/csharp-sdk that referenced this pull request Oct 10, 2026
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
PederHP added a commit to PederHP/csharp-sdk that referenced this pull request Oct 10, 2026
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
girishkvs added a commit to girishkvs/csharp-sdk that referenced this pull request Oct 11, 2026
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>
@girishkvs

Copy link
Copy Markdown
Contributor Author

@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 skills/get cache hints missing from both PRs.

Comparison and fixes

Compared sources

Both PRs were compared on the same main. Both original heads had the old SourceLink NU1902 failure. The update already on main clears it; that isn't a new Skills fix.

Implementation comparison

Area Original #1856 Peder's #1864 This update
Protocol foundation Request/result types, SkillResources union, source-generated serialization, catalog and conformance fixtures Reuses those foundations, with additional APIs and validation Kept once, with the existing credit
Capabilities and version handling Bare catalog registration lacks the resources declaration; current defaults and legacy resultType behavior have gaps Declares resources, supplies list defaults and gates current-only fields Peder's behavior retained, including older-protocol compatibility
skills/get cache hints Missing Missing Added required ttlMs and cacheScope on current requests, including defaults and configured values; omitted on legacy requests
Request handling Incorrectly typed uri/cursor can return InternalError Typed parameter handling returns InvalidParams Peder's handling retained
Catalog ownership Caller-owned mutable entries Defensive snapshots and keyset pagination Peder's implementation retained
Caller context Catalog interface has no request context Catalog receives request context and caller identity Retained; applications still need to apply their authorization policy consistently to entries and resources
Client APIs No dedicated client helpers Capability checks, list/get helpers and manifest-bound resource reads Retained, including rejection of unlisted files and binding reads to the requested URI
Byte/file authoring Application supplies metadata and resources Manifest and served resources use the same copied bytes; directory helpers enforce file/URI boundaries Retained
YAML convenience Application supplies a JsonObject Optional hand-written subset reader and automatic authoring Reader retained; confirmed parsing errors fixed; unsupported-format and caller responsibilities stated explicitly
Samples and docs No standalone Skills client/server samples Both samples and a guide Retained and updated; the client sample now parses fetched frontmatter and compares it before displaying the file
AOT and packaging Generated metadata and pagination constructor changes Adds Skills-specific AOT coverage and package integration Retained, with additional assertions for our cache/parser changes

The stable spec added the skills/get cache requirement after both implementations. Both needed this fix; it doesn't invalidate their earlier conformance results.

Additional corrections

  1. skills/get cache hints. Added ICacheableResult and set the fields in the raw handler. Defaults are 0 / private; configured values are honored. The interface alone wouldn't set them.
  2. Next-line YAML values. Valid plain, quoted and nested values no longer fail because they start on the next line.
  3. Folded TAB-led content. Kept the line breaks instead of silently replacing them with spaces.
  4. Trailing comments. Require whitespace after a quoted or flow value before its comment. Plain values containing # still work.
  5. Flow-collection keys. Kept the existing unsupported-YAML path so explicit metadata still works. Added regression cases alongside ordinary flow values and quoted keys.
  6. Client sample verification. Kept the manifest, URI, digest and size checks. The sample now also parses fetched SKILL.md and compares its frontmatter before displaying it.
  7. Docs and links. Clarified what the application, authoring helper and host each validate. Fixed the two sample links using a published source snapshot.

The trailing-comment check matches the yaml reference parser. Some other YAML libraries accept those forms, so this isn't a failure in every host.

Parser responsibility

I kept Peder's parser as an optional authoring helper. There's no new YAML dependency.

  • Automatic authoring requires the bounded reader to succeed.
  • With explicit metadata, the application must validate the complete file and supply matching JSON. Existing checks remain, but an unsupported-parser exception doesn't prove the rest of the YAML is valid.
  • Hosts must independently parse fetched frontmatter and compare it field by field before loading. Digest verification alone is not enough.

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 credit

3338e88e  Current main
  47b9db7d  Original implementation - Girish
    f3d028b7  Peder's additional implementation - Peder
      6b0870cb  Added fixes and responsibility alignment - Girish

Peder is the author of the import commit, which records #1864 and the source commits. Before our fixes, its full tree is ba9234da7a7a3ff5ad2a4e78ca9308a72ad004d9, identical to his selected head. Our fixes are separately authored.

Validation of the consolidated commit

These results are for 6b0870cb on Windows x64.

Check Result
Complete Skills suite, .NET 10 / 9 / 8 470 passed on each target; zero failures or skips
Complete Skills suite, .NET Framework 4.7.2 466 passed; four modern-.NET-only symlink cases are not compiled on this target
Full non-manual .NET 10 suite 2,860 passed, zero failures, four declared skips
Pack and API compatibility Passed; Core checked against the existing 2.0.0 baseline
NativeAOT, Windows x64 Published and executed the Skills list/get/read/verify and parser/cache assertions successfully
Source/artifact identity Package metadata, informational versions and production SourceLink identify 6b0870cb; 31 PE/PDB pairs passed source/binary inspection
Client/server samples and documentation links Samples passed; normal related and full documentation link checks reported zero errors
Skills conformance, 2026-07-28 39 applicable checks passed; all three scenario processes exited zero. Six optional directoryRead checks were not applicable
Skills conformance, 2025-11-25 Enumeration exited zero with 30 checks passed and two version-inapplicable cache checks. Manifest and directory processes aborted with UV_HANDLE_CLOSING after printing successful checks; neither process is counted green

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

  • No new general YAML dependency, directoryRead support, full host implementation or general Core authorization hook is included.
  • Package placement, the public reader and Core API changes still need maintainer review.
  • Hosted CI hasn't run on this head. The earlier Ubuntu Debug OAuth failure isn't fixed by these Skills changes.
  • Add the Skills extension (SEP-2640) as ModelContextProtocol.Extensions.Skills #1864 remains open; this update doesn't assume agreement to close it.

— girishkvs's Copilot 🤖

@PederHP

PederHP commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Protocol conformance, authorization exposure, validation gaps, and inaccurate PR metadata remain unresolved.

7 open findings
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.

Comment thread src/ModelContextProtocol.Extensions.Skills/SkillValidation.cs Outdated
Comment thread src/ModelContextProtocol.Extensions.Skills/SkillValidation.cs Outdated
Comment thread src/ModelContextProtocol.Core/Protocol/CacheScopeConverter.cs
@tarekgh

tarekgh commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Should we upgrade the conformance test package to 0.2.0-alpha.12 to ensure the added skills tests are covered? By the way, I already ran these tests against the PR, and all passed.

Comment thread src/ModelContextProtocol.Extensions.Skills/Server/McpServerSkill.cs
Comment thread src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsBuilderExtensions.cs Outdated
@girishkvs

Copy link
Copy Markdown
Contributor Author

@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 🤖

Comment thread src/ModelContextProtocol.Extensions.Skills/SkillValidation.cs Outdated
Comment thread tests/ModelContextProtocol.AspNetCore.Tests/SkillsAuthorizationTests.cs Outdated
Comment thread src/ModelContextProtocol.Extensions.Skills/Protocol/Skill.cs
Comment thread src/ModelContextProtocol.Core/Client/McpClientOptions.cs Outdated
@girishkvs

Copy link
Copy Markdown
Contributor Author

@tarekgh FYI: the Windows Sampling_Stdio failure in the last CI run is the same discovery race as #1943. That test passes its own options, so it still uses the 5-second probe; #1950 applies #1943's timeouts to it.

— girishkvs's Copilot 🤖

@tarekgh

tarekgh commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

@tarekgh FYI: the Windows Sampling_Stdio failure in the last CI run is the same discovery race as #1943. That test passes its own options, so it still uses the 5-second probe; #1950 applies #1943's timeouts to it.

— girishkvs's Copilot 🤖

Thanks, I already have the PR #1946 to address all timeout issues.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Frontmatter validation needs correction, and the expanded public API and authorization boundary still require maintainer review.

2 open issues
1 resolved since last review

Review effort: Balanced

Comment thread samples/SkillsClient/YamlFrontmatterReader.cs
Comment thread src/ModelContextProtocol.Extensions.Skills/SkillFrontmatter.cs
@tarekgh

tarekgh commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

@girishkvs thanks again. We got a couple of copilot comments we need to address before merging. We almost there 😄

girishkvs added a commit to girishkvs/csharp-sdk that referenced this pull request Oct 22, 2026
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>
@girishkvs

Copy link
Copy Markdown
Contributor Author

@tarekgh Done: rebased on #1946 and fixed both Copilot comments. Sampling_Stdio is still exposed after #1946: it passes its own options, so its test_server case keeps the 5-second probe and still fails when the server/discover answer is held 7 s. #1950 now only starts those options from TestMcpClient.CreateOptions(), as CONTRIBUTING.md asks.

— girishkvs's Copilot 🤖

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The public API additions and parsing and authorization boundaries require maintainer review and current-head cross-platform CI confirmation.

1 open issue
2 resolved since last review

Review effort: Balanced

Comment thread tests/ModelContextProtocol.AspNetCore.Tests/SkillsAuthorizationTests.cs Outdated
girishkvs and others added 4 commits October 22, 2026 16:05
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.
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.
@girishkvs

Copy link
Copy Markdown
Contributor Author

@tarekgh Fixed a couple of bugs in this PR's test helper RunServerConformanceAsync that broke the Windows jobs: its output reads could block each other and starve the thread pool, so they now run on their own threads. The Windows Debug job on 8398c300 failed before any AspNetCore net9 test started (xUnit's 60 s startup timeout); could you re-run it?

— girishkvs's Copilot 🤖

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The public API expansion, separate catalog/resource authorization boundary, and YAML verification design warrant maintainer sign-off.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@tarekgh tarekgh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@tarekgh
tarekgh merged commit 8b00f20 into modelcontextprotocol:main Oct 23, 2026
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants