Add the Skills extension (SEP-2640) as ModelContextProtocol.Extensions.Skills by PederHP · Pull Request #1864 · modelcontextprotocol/csharp-sdk

PederHP · 2026-09-09T05:34:20Z

Closes #1863.

Summary

Adds ModelContextProtocol.Extensions.Skills, an implementation of the MCP Skills extension (SEP-2640, io.modelcontextprotocol/skills, specification) as a separate package alongside Extensions.Apps and Extensions.Tasks.

This is an alternative to #1856 and reuses several parts of it (see Relationship to #1856). Compared to that PR it adds a client API, a server authoring model that computes manifests from the served bytes, entry validation against the specification's MUSTs, protocol-version gating that matches Core, a server and a client sample, and a docs page.

What's in the package

Protocol types (Skill, SkillResource, SkillResources, ListSkillsRequestParams/Result, GetSkillRequestParams/Result), source-generated for AOT. SkillResources is a closed array-or-"dynamic" union; an entry with a missing, null, or otherwise malformed resources fails deserialization, as the spec requires hosts to reject it.

Server

WithSkillsFromDirectory(path) serves every skill directory under a folder in one call, reading each SKILL.md's frontmatter and publishing at skill://{name}/SKILL.md (or under a uriPrefix).
McpServerSkill.CreateFromDirectory(path) and McpServerSkill.Create(uri, files) build a skill's entry and its file resources together, reading the frontmatter from SKILL.md. Overloads taking an explicit JsonObject remain for frontmatter the reader cannot handle; when it can read the file, a supplied object that differs from it is rejected at construction. File bytes are copied once and digests and sizes are computed from that copy, which is also what the resources serve, so they cannot disagree. File path segments are percent-encoded in resource URIs so a name like {name}.md stays a concrete resource. CreateFromDirectory does not follow symbolic links and throws on one, since a link can publish a file from outside the skill. Text files are served as TextResourceContents only when the bytes round-trip through UTF-8 exactly (a host hashes the UTF-8 of the text it receives); everything else is served as a blob.
WithSkills(IEnumerable<McpServerSkill>) registers skills/list, skills/get, the extension capability, the resources capability, and every file resource. Files shared between nested skills are registered once and must agree on digest.
WithSkills(IMcpSkillCatalog) for catalogs that are large, generated, per-tenant, or backed by storage. Both catalog methods receive an McpSkillRequestContext with the JSON-RPC request and the caller's ClaimsPrincipal (populated by the ASP.NET Core transport), because the skills methods are raw handlers and do not pass through the request filters that guard resources/*; a per-caller catalog decides from that context, and the docs say so. InMemoryMcpSkillCatalog is the built-in implementation with keyset cursors and keeps its own copy of every entry.
Every entry is validated at construction against the spec's structural MUSTs: URIs are absolute with no query, fragment, or ./.. segments and end in /SKILL.md; frontmatter has string name and description, name matches the final path segment and the Agent Skills naming rules, description is at most 1024 characters, compatibility 1 to 500 if present, and metadata a mapping if present (other fields and metadata values pass through verbatim, since hosts compare them against the file and ignore fields like allowed-tools for MCP-origin skills); the manifest lists the skill's own SKILL.md exactly once, every file is under the skill root, digests are sha256: + 64 lowercase hex, and the 512-file / 16 MiB limits hold without overflow.

Client

client.SupportsSkills(), client.ListSkillsAsync() (follows pagination), client.ListSkillsAsync(ListSkillsRequestParams) (one page, exposes caching hints), client.GetSkillAsync(uri).
client.ReadSkillResourceAsync(skill, uri) reads through resources/read and verifies size and digest against the held entry, throwing SkillVerificationException on mismatch. A URI the manifest does not list is refused before any request is sent, per the spec's "unlisted file is a change to the skill" rule, and the response must contain the requested URI, so a server cannot satisfy a read of one file with another correctly digested file of the same skill. SkillVerifier exposes the same checks for hosts that read resources another way.

Protocol-version handling. resultType, ttlMs, and cacheScope are emitted only on requests negotiated under 2026-07-28 or later, where ttlMs and cacheScope are required and default to 0 / private when unset, exactly as Core does for tools/list. On 2025-11-25 sessions they are omitted (#1721). Both directions are tested.

Also: conformance server fixture, docs page (docs/concepts/skills), README/PACKAGE.md entries, and two samples (samples/SkillsServer, samples/SkillsClient).

Core change

PaginatedRequestParams and PaginatedResult had private protected constructors, so a package outside Core could not implement a paginated method. They are widened to protected. Source and binary compatible; package validation passes against the 2.0.0 baseline. Happy to split this into its own PR if preferred.

CacheScopeConverter (Core's lenient read-side converter for cacheScope) was internal; it is now public so that extension packages defining their own ICacheableResult types get the same read-side leniency. This was originally done by compiling the Core source file into the package; @girishkvs pointed out that made Skills the only package reaching into Core's source tree, and making the type public is cleaner.

Frontmatter without a YAML dependency. SkillFrontmatter.Parse reads the YAML subset that Agent Skills frontmatter uses (nested block mappings, block and flow sequences, plain/quoted/block scalars, comments) and resolves unquoted scalars per the YAML 1.2 core schema, which is what the yaml package, yaml.v3, and therefore other SDKs and hosts do. Anchors, aliases, tags, multi-document streams, complex keys, and tabs are rejected with a FormatException naming the construct rather than guessed at, since a wrong rendering is a host-side verification failure. It is public so hosts can use the same reader for the spec's frontmatter verification. Its output is checked against the yaml npm package's core-schema resolution on a 92-case corpus (block and flow collections, every block-scalar style and chomping mode, comments, quoting, and numeric edge cases); the 66 cases both accept are committed as SkillFrontmatterCorpusTests with the reference parser's output as expected values, and the rest are the reader's deliberate rejections.

Deliberately not included

Automatic frontmatter verification on the client. The pieces are there (SkillFrontmatter.Parse + JsonNode.DeepEquals), but a strict subset reader rejecting an exotic-but-valid file would look like a failed verification, so that call is left to the host for now.
resources/directory/read and the directoryRead setting. Servers built with this package do not declare it.

Open questions for maintainers

Authorization of skill entries. skills/list and skills/get bypass AddAuthorizationFilters() because Core has no filter hook for custom request handlers. This PR gives catalogs the request context so they can decide per caller, and documents the gap. A general filter hook for McpServerOptions.RequestHandlers in Core would be the cleaner fix and would help Tasks too; happy to follow up on that separately.
Placement. This is a separate package, matching Extensions.Apps and Extensions.Tasks. If you would rather see it in Core, the code moves without API changes.
CacheScopeConverter is now public in Core (see above); shout if you would rather keep it internal and I will go back to linking the source.
Whether the protected widening should land separately.
Whether a strict YAML-subset reader in the package is acceptable versus requiring explicit frontmatter. The convenience case (point at a directory) seemed too important to leave out, and the reader refuses rather than guesses.

Relationship to #1856

@girishkvs opened #1856 first, and I reviewed it before deciding that the shape changes I wanted (client API, authoring model, frontmatter reading, validation, samples, docs) were better written than requested through review rounds. Girish has agreed to the co-authorship on Discord. I reused these parts of #1856, and the commits carrying them name @girishkvs as co-author:

the SkillResources array-or-"dynamic" union and its converter, including HandleNull;
the keyset-cursor design of the in-memory catalog;
the conformance fixture's skill set;
the serialization tests;
the protected widening in Core.

Authorship

This PR was written by Claude (Claude Fable 5.1, via Claude Code) with me directing the design, reviewing every file, and making the calls on scope and API shape. The commit trailers carry the session link. I am listing this explicitly because I think who and what wrote a PR should be visible to reviewers.

Validation

dotnet build clean on net10.0, net9.0, net8.0, netstandard2.0 with TreatWarningsAsErrors, 0 warnings
dotnet pack with package validation: Core against the 2.0.0 baseline, and the new package across target frameworks
278 new tests (serialization, frontmatter reader with a 66-case reference-parser corpus, catalog, McpServerSkill, SkillVerifier, six end-to-end classes including 2025-11-25 and 2026-07-28 wire-shape checks, the no-options defaults, a caller-mutated buffer, a server that answers a read with a different file, and a directory of skills served through WithSkillsFromDirectory)
Full ModelContextProtocol.Tests on net10.0: 2637 passed, 3 skipped, 1 failed. The failure is DockerEverythingServerTests.Sampling_Sse_EverythingServer (Execution=Manual, Docker image of the TS everything server), failing with Unknown tool: trigger-sampling-request; it fails identically on main and is unrelated.
Existing server conformance suite (ServerConformanceTests, pinned npm version): 83 passed, 0 failed, no regression from the added skill resources
SEP-2640 scenarios from feat(sep-2640): skills server conformance against the Accepted SEP, enumeration + manifest + directory conformance#330 against ConformanceServer:

Scenario	`/stateless` (2026-07-28)	`/` with `--spec-version 2025-11-25`
`sep-2640-skills-enumeration`	30/30	29/29 (`ttlMs`/`cacheScope` check skipped below 2026-07-28)
`sep-2640-skills-manifest`	6/6	6/6
`sep-2640-skills-directory`	1/1, 6 skipped (`directoryRead` not declared)	1/1, 6 skipped

NativeAOT publish of ModelContextProtocol.AotCompatibility.TestApp (now also lists, gets, and verifies a skill): no trim or AOT warnings, runs to Success!
SkillsServer + SkillsClient samples run end to end over Streamable HTTP on 2026-07-28

…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

SkillsServer is a Streamable HTTP server serving two skills from directories on disk through McpServerSkill.CreateFromDirectory, one with a nested skill path, and pointing at one of them from its server instructions. SkillsClient connects to it and walks through the host side: capability check, listing with caching hints, retrieval by URI, verified reads of SKILL.md and a supporting file, and the refusal of an unlisted file. The docs page covers serving skills, custom catalogs, dynamic skills, protocol-version handling of the caching hints, the client API, verification, security considerations, and what is deliberately not implemented. README and PACKAGE.md list the new package. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

Addresses four review findings against the previous commits. McpServerSkill.Create now copies each file's bytes once and uses that copy for both the manifest digest and the served resource, so a caller that mutates its buffer after construction cannot make the served content diverge from the published digest. File path segments are percent-encoded when building resource URIs. A file named "{name}.md" previously registered as a resource template instead of a concrete resource and was missing from resources/list. CreateFromDirectory no longer follows symbolic links or other reparse points, and throws when it meets one. A link inside a skill directory could publish a file from outside it under a URI that appears to be inside. SkillVerifier gains Verify(skill, uri, result), which additionally requires the read's result to contain the requested URI. ReadSkillResourceAsync uses it, so a server cannot satisfy a read of one file by returning another, correctly digested, file of the same skill. Regression tests cover each case, including an end-to-end substitution via a read-resource filter and a caller-owned buffer mutated after creation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

Requiring authors to restate a SKILL.md's frontmatter as a JsonObject was awkward and error-prone. SkillFrontmatter.Parse now reads it from the file without a YAML dependency, accepting the subset Agent Skills frontmatter uses (nested block mappings, block and flow sequences, plain, quoted, and block scalars, comments) and resolving unquoted scalars per the YAML 1.2 core schema, which matches the YAML libraries other SDKs and hosts use. Anchors, aliases, tags, multi-document streams, complex keys, and tab indentation are rejected with a FormatException naming the construct, since a guessed rendering would fail host-side verification. McpServerSkill gains Create(files), Create(uri, files), CreateFromDirectory(path), and CreateFromDirectory(uri, path), which read the frontmatter from SKILL.md and, where no URI is given, derive it from the frontmatter name as skill://{name}/SKILL.md. The overloads that take an explicit JsonObject remain for frontmatter the reader cannot handle; when the reader can parse the file, a supplied object that differs from it is rejected at construction, since hosts compare the two field by field. WithSkillsFromDirectory(path, uriPrefix) serves every immediate subdirectory containing a SKILL.md in one call. The server sample shrinks to a single WithSkillsFromDirectory call, the conformance fixture reads its frontmatter from the files, and the docs describe the reader, its limits, and the escape hatch. ClientServerTestBase gains a virtual DisposeAsync so fixtures can clean up temporary directories. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

Addresses four review findings and adds a differential corpus. WithSkillsFromDirectory now rejects a skill directory that is itself a symbolic link, closing the gap left by the per-skill loader, which only checked links inside a skill. Comment removal no longer treats an apostrophe inside a plain scalar as the start of a quoted string, so "the team's workflow # note" yields the text without the comment. Quoted values and flow collections are handed to their own parsers unstripped. A tab before '#' starts a comment, as in YAML. Folded block scalars keep leading empty lines as line breaks, matching the literal style and reference parsers. Hexadecimal and octal integers take no sign under the YAML 1.2 core schema, so "+0x10" is a string, and values beyond 64 bits keep their full unsigned magnitude instead of wrapping. Two further divergences surfaced by comparing against the yaml npm package are fixed as well: a plain scalar starting with '@', '`', or '%' is an error rather than text, and a '- ' entry on the same line as its key is an error rather than a string. SkillFrontmatterCorpusTests holds 66 cases whose expected values were produced by the yaml package's core schema, so the agreement is checked in CI without a Node dependency. The remaining differences are deliberate rejections of constructs the reader does not support. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

The remarks still said the package does not parse YAML. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

PederHP · 2026-09-09T05:38:54Z

The inclusion of the YAML parsing code (4 above) is probably the biggest thing to decide on. I think the developer experience is really bad without it (having to repeat frontmatter in code, with risk of drift, and having to roll one's own loader for anything dynamic), but I am not too happy about almost 1000 lines of YAML related code being added - I just don't see a good alternative.

The absolute links pointed at main, where the samples do not exist yet, and failed the markdown link check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

Copilot

🟡 Changes recommended

Unresolved critical integrity, authorization, path-safety, and validation issues block approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds the standalone SEP-2640 Skills extension, including server authoring, client verification, protocol models, documentation, samples, conformance tests, and AOT support.

Changes:

Adds Skills catalogs, resource serving, validation, frontmatter parsing, and client APIs.
Widens Core pagination constructors for external extensions.
Adds comprehensive tests, samples, documentation, and project wiring.

File summaries

File	Description
`tests/ModelContextProtocol.Tests/Server/SkillFrontmatterTests.cs`	Tests frontmatter parsing.
`tests/ModelContextProtocol.Tests/Server/SkillFrontmatterCorpusTests.cs`	Adds differential YAML cases.
`tests/ModelContextProtocol.Tests/Server/McpServerSkillTests.cs`	Tests skill construction.
`tests/ModelContextProtocol.Tests/Server/McpServerSkillsTests.cs`	Tests end-to-end behavior.
`tests/ModelContextProtocol.Tests/Server/McpServerSkillsIntegrityTests.cs`	Tests integrity behavior.
`tests/ModelContextProtocol.Tests/Server/McpServerSkillsFromDirectoryTests.cs`	Tests directory-backed registration.
`tests/ModelContextProtocol.Tests/Server/McpServerSkillsDefaultsTests.cs`	Tests default caching hints.
`tests/ModelContextProtocol.Tests/Server/McpServerSkillsCatalogTests.cs`	Tests catalogs and protocol versions.
`tests/ModelContextProtocol.Tests/Server/InMemoryMcpSkillCatalogTests.cs`	Tests in-memory catalog behavior.
`tests/ModelContextProtocol.Tests/Protocol/SkillSerializationTests.cs`	Tests protocol serialization.
`tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj`	References the Skills package.
`tests/ModelContextProtocol.Tests/ClientServerTestBase.cs`	Supports asynchronous cleanup overrides.
`tests/ModelContextProtocol.Tests/Client/SkillVerifierTests.cs`	Tests resource verification.
`tests/ModelContextProtocol.ConformanceServer/Skills/ConformanceSkills.cs`	Defines conformance fixtures.
`tests/ModelContextProtocol.ConformanceServer/Program.cs`	Registers conformance skills.
`tests/ModelContextProtocol.ConformanceServer/ModelContextProtocol.ConformanceServer.csproj`	References the Skills package.
`tests/ModelContextProtocol.AotCompatibility.TestApp/Program.cs`	Exercises Skills under AOT.
`tests/ModelContextProtocol.AotCompatibility.TestApp/ModelContextProtocol.AotCompatibility.TestApp.csproj`	Adds the AOT package reference.
`src/PACKAGE.md`	Documents the new package.
`src/ModelContextProtocol.Extensions.Skills/SkillValidation.cs`	Validates skill entries and manifests.
`src/ModelContextProtocol.Extensions.Skills/SkillsProtocol.cs`	Defines protocol constants and limits.
`src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsOptions.cs`	Configures caching hints.
`src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsBuilderExtensions.cs`	Registers Skills handlers and resources.
`src/ModelContextProtocol.Extensions.Skills/Server/McpSkillPage.cs`	Models catalog pages.
`src/ModelContextProtocol.Extensions.Skills/Server/McpServerSkillFile.cs`	Models authored skill files.
`src/ModelContextProtocol.Extensions.Skills/Server/McpServerSkill.cs`	Builds skill manifests and resources.
`src/ModelContextProtocol.Extensions.Skills/Server/InMemoryMcpSkillCatalog.cs`	Implements keyset pagination.
`src/ModelContextProtocol.Extensions.Skills/Server/IMcpSkillCatalog.cs`	Defines the catalog contract.
`src/ModelContextProtocol.Extensions.Skills/Protocol/SkillResourcesConverter.cs`	Serializes the resources union.
`src/ModelContextProtocol.Extensions.Skills/Protocol/SkillResources.cs`	Models static or dynamic resources.
`src/ModelContextProtocol.Extensions.Skills/Protocol/SkillResource.cs`	Models manifest files.
`src/ModelContextProtocol.Extensions.Skills/Protocol/Skill.cs`	Models skill entries.
`src/ModelContextProtocol.Extensions.Skills/Protocol/ListSkillsResult.cs`	Models paginated list results.
`src/ModelContextProtocol.Extensions.Skills/Protocol/ListSkillsRequestParams.cs`	Models list parameters.
`src/ModelContextProtocol.Extensions.Skills/Protocol/GetSkillResult.cs`	Models retrieval results.
`src/ModelContextProtocol.Extensions.Skills/Protocol/GetSkillRequestParams.cs`	Models retrieval parameters.
`src/ModelContextProtocol.Extensions.Skills/ModelContextProtocol.Extensions.Skills.csproj`	Defines the Skills package.
`src/ModelContextProtocol.Extensions.Skills/McpSkillsJsonContext.cs`	Adds AOT serialization metadata.
`src/ModelContextProtocol.Extensions.Skills/Client/SkillVerifier.cs`	Verifies resource integrity.
`src/ModelContextProtocol.Extensions.Skills/Client/SkillVerificationException.cs`	Defines verification failures.
`src/ModelContextProtocol.Extensions.Skills/Client/McpSkillsClientExtensions.cs`	Adds client Skills operations.
`src/ModelContextProtocol.Core/Protocol/PaginatedResult.cs`	Enables external paginated results.
`src/ModelContextProtocol.Core/Protocol/PaginatedRequest.cs`	Enables external pagination subclasses.
`src/Directory.Build.targets`	Integrates Skills package references.
`samples/SkillsServer/SkillsServer.csproj`	Defines the server sample.
`samples/SkillsServer/Skills/refunds/SKILL.md`	Adds a refunds skill.
`samples/SkillsServer/Skills/refunds/policy/REFUND_POLICY.md`	Adds refund policy content.
`samples/SkillsServer/Skills/refunds/examples/declined.md`	Adds a rejection example.
`samples/SkillsServer/Skills/refunds/examples/approved.md`	Adds an approval example.
`samples/SkillsServer/Skills/git-workflow/templates/PULL_REQUEST.md`	Adds a pull-request template.
`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/README.md`	Documents the server sample.
`samples/SkillsServer/Properties/launchSettings.json`	Configures the sample endpoint.
`samples/SkillsServer/Program.cs`	Demonstrates directory-backed skills.
`samples/SkillsClient/SkillsClient.csproj`	Defines the client sample.
`samples/SkillsClient/README.md`	Documents the client sample.
`samples/SkillsClient/Program.cs`	Demonstrates discovery and verification.
`README.md`	Lists the Skills package.
`ModelContextProtocol.slnx`	Adds package and sample projects.
`docs/concepts/toc.yml`	Adds Skills navigation.
`docs/concepts/skills/skills.md`	Documents Skills usage and security.
`docs/concepts/index.md`	Links the Skills guide.

Review details

Files reviewed: 64/64 changed files
Comments generated: 11
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Catalogs now receive an McpSkillRequestContext carrying the JSON-RPC request, the caller's ClaimsPrincipal when the transport supplies one, and per-request items. The skills methods are raw handlers and do not pass through the request filters that guard the resource methods, so a catalog whose skills are not visible to every caller needs the caller to decide with; the docs and API remarks now say so plainly. The interface changes before it ships rather than after. InMemoryMcpSkillCatalog keeps its own copy of each entry, so mutating a Skill (or McpServerSkill.ProtocolSkill) after registration cannot make the published manifest disagree with the served bytes. Validation tightens to what the specifications require: resource and skill URIs must be absolute, without query or fragment, and without empty, '.', or '..' path segments, so a prefix check establishes containment; the total size check cannot overflow; and the Agent Skills frontmatter limits are enforced (description at most 1024 characters, compatibility 1 to 500, license and allowed-tools strings, metadata a map of strings). CreateFromDirectory rejects a root directory that is itself a link, to match its documented behaviour. SkillFrontmatter rejects malformed block scalar headers (repeated indicators or trailing text), comments inside flow collections (which YAML treats as running to the end of the line), and \U escapes that are not Unicode scalar values, each with a FormatException. The catalog contract now describes cursors as opaque positions a catalog may resume from rather than tokens it must have issued. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

Copilot

🟡 Changes recommended

Moderate authorization, validation, frontmatter parsing, and protocol-conformance issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

src/ModelContextProtocol.Extensions.Skills/Client/McpSkillsClientExtensions.cs:169

skills/get likewise returns an unvalidated remote entry, so callers can receive a Skill that violates the extension's required structure. Apply the same response validation used for list results before returning this object.

        JsonRpcResponse response = await client.SendRequestAsync(request, cancellationToken).ConfigureAwait(false);
        return response.Result?.Deserialize(McpSkillsJsonContext.Default.GetSkillResult) ??
            throw new JsonException($"Unexpected JSON result in the response to '{SkillsProtocol.MethodSkillsGet}'.");

src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsBuilderExtensions.cs:254

These handlers are registered through the raw custom-handler path, so existing request filters—including ASP.NET Core authorization filters—never run for skills/list or skills/get. A server that protects its resources with those filters can still disclose skill frontmatter, file names, sizes, and digests to an unauthorized caller. Route extension handlers through the filter/authorization pipeline before exposing this API.
src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsBuilderExtensions.cs:300
The skills/get path also returns a custom catalog entry without validating it, allowing a non-conforming entry to be emitted even if list validation is added. Validate the resolved skill before serializing it.

Files reviewed: 65/65 changed files
Comments generated: 9
Review effort level: Balanced

The client now validates every entry returned by skills/list and skills/get against the specification's structural requirements and throws SkillVerificationException for one a host must not load. The server-side handlers apply the same validation to entries from a custom catalog and report a failure as an internal error rather than publishing it. The explicit-frontmatter escape hatch now covers only valid YAML the reader does not support (anchors, aliases, tags, complex keys, nested flow collections, multi-line quoted scalars). A SKILL.md that is not UTF-8, has no frontmatter block, or is malformed is rejected regardless, since no host could parse it either. SkillFrontmatter rejects a compact mapping inside a flow sequence ("[a: b]"), which reference parsers read as a mapping, and unpaired surrogates from \u escapes, while still accepting a surrogate pair. URI validation uses System.Uri for syntax and then checks the raw segments, so "1 bad://x/SKILL.md" is rejected and "file:///x/SKILL.md" is accepted. Optional frontmatter fields that are present but null are rejected rather than treated as absent. A self-comparing test assertion is fixed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

Copilot

🟡 Changes recommended

skills/get does not bind responses to requested URIs, and the frontmatter parser accepts malformed flow sequences.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/ModelContextProtocol.Extensions.Skills/SkillFrontmatter.cs:601

An empty flow-sequence element is currently parsed as JSON null: inputs such as [a,,b] or [,a] reach ParseFlowScalar at a comma, which resolves the empty substring as null. These inputs are malformed YAML 1.2, so the server can publish frontmatter that a conforming host cannot parse and verify. Reject a comma where an entry is expected while continuing to allow a trailing comma before ].

Files reviewed: 66/66 changed files
Comments generated: 2
Review effort level: Balanced

A skills/get response is only useful if it describes the skill that was asked for. The client now rejects a valid entry for a different URI with SkillVerificationException, and the server handler reports a custom catalog that answers for the wrong URI as an internal error rather than publishing it. Both directions are tested. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

Copilot

🟡 Changes recommended

Malformed YAML can be published, URI-equivalent resources can collide, and directory loading has unresolved correctness and resource-bound issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsBuilderExtensions.cs:164

The reparse-point check runs before determining whether the directory contains SKILL.md. As a result, a symlink to an unrelated non-skill directory aborts registration, contradicting this API's contract that subdirectories without SKILL.md are ignored. Check for SKILL.md first, and reject a link only when that directory would otherwise be loaded as a skill.

[!NOTE]
AI-generated by GitHub Copilot.

src/ModelContextProtocol.Extensions.Skills/Server/McpServerSkill.cs:334

UnsupportedYamlException does not guarantee that the YAML is valid: for example, SkillFrontmatter.Parse throws it for an unterminated quoted scalar (SkillFrontmatter.cs:810) and for anchors/aliases before validating their syntax. Swallowing every such exception therefore lets the explicit-frontmatter overload publish a malformed SKILL.md, which a conforming host cannot parse and verify. Distinguish syntactically valid unsupported constructs from malformed input before accepting the supplied object.

[!NOTE]
AI-generated by GitHub Copilot.

Files reviewed: 66/66 changed files
Comments generated: 2
Review effort level: Balanced

…valence CreateFromDirectory now applies the 512-file and 16 MiB limits while walking the directory, using each file's reported length, so an oversized or overly broad directory fails naming the limit before anything is read. The manifest built from the bytes actually read is still validated against the same limits afterwards. WithSkills detects files shared between skills with the same equivalence the server's resource collection uses for concrete URIs, System.Uri equality, under which scheme and authority are case-insensitive. Two skills whose file URIs differ only in authority case previously passed the ordinal check and were then merged silently by the collection, so reads of the second returned the first's bytes. Same content still registers once; different content is an error that names both URIs. 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

PederHP · 2026-09-09T19:39:48Z

@girishkvs Your review should now be addressed. I have read and approved Claude's response to your copilot. Thank you for the quick corrections.

girishkvs · 2026-09-09T21:19:08Z

+        }
+
+        /// <summary>Parses the block node formed by the lines at <paramref name="indent"/>.</summary>
+        private JsonNode? ParseBlock(int indent)


Can we cap nesting before recursing? About 2,500 nested mappings is well under the size limit and overflows the stack, which ends the process.

— girishkvs's Copilot 🤖

girishkvs · 2026-09-09T21:19:08Z

+                    break;
+                }
+
+                var keyNode = ParseFlowScalar(text, ref pos, lineNumber, ":");


Can we honor YAML’s separator rules here? {version:1} is a key with a null value, not version: 1.

— girishkvs's Copilot 🤖

girishkvs · 2026-09-09T21:19:08Z

+                }
+                else
+                {
+                    int separator = FindKeySeparator(content);


Can we resolve unquoted mapping keys too? TRUE: value and 0x10: value currently produce different keys from the reference parser.

— girishkvs's Copilot 🤖

girishkvs · 2026-09-09T21:19:09Z

+                    break;
+                }
+
+                string text = StripComment(line.Content).Trim();


Can we stop plain-scalar continuation after an inline comment? An indented line after description: first # note is invalid YAML but gets published.

— girishkvs's Copilot 🤖

girishkvs · 2026-09-09T21:19:09Z

+                return rest.TrimEnd();
+            }
+
+            return StripComment(rest).Trim();


Can we trim only YAML whitespace? Trim() removes non-breaking spaces that are part of the scalar, so hosts get different frontmatter.

— girishkvs's Copilot 🤖

girishkvs · 2026-09-09T21:19:09Z

+        }
+
+        char previous = '-';
+        foreach (char c in name)


Can we accept Unicode skill names? The Agent Skills spec permits café and 数据分析, but this rejects both.

— girishkvs's Copilot 🤖

girishkvs · 2026-09-09T21:19:09Z

+        Assert.True(client.SupportsSkills());
+        Assert.NotNull(client.ServerCapabilities.Resources);
+
+        var settings = System.Text.Json.JsonSerializer.SerializeToNode(client.ServerCapabilities.Extensions![SkillsProtocol.ExtensionId])?.AsObject();


Can we use source-generated JSON metadata here? This test fails on net9.0, where reflection serialization is disabled.

— girishkvs's Copilot 🤖

girishkvs · 2026-09-09T21:19:09Z

+    private const string SkillUri = "skill://git-workflow/SKILL.md";
+    private const string SkillMarkdown = "---\nname: git-workflow\ndescription: Git conventions\n---\n\n# Git workflow\n";
+
+    private static JsonObject Frontmatter(string name = "git-workflow", string? description = "Git conventions") => new()


nit: Can Frontmatter be an instance helper?

— girishkvs's Copilot 🤖

girishkvs · 2026-09-09T21:26:01Z

These threads are marked resolved but the code still has the issue. I replied in each one with what I found:

Mutable entries — InMemoryMcpSkillCatalog.cs:94,115 returns entries without copying, so mutating a GetAsync result changes later responses. Add the Skills extension (SEP-2640) as ModelContextProtocol.Extensions.Skills #1864 (comment)
URI containment — https://example.com/alpha/%2e%2e/secret.md passes validation and resolves outside the skill. Add the Skills extension (SEP-2640) as ModelContextProtocol.Extensions.Skills #1864 (comment)
Explicit-frontmatter escape hatch — An unterminated quoted scalar is still classified as unsupported and swallowed at McpServerSkill.cs:331. Add the Skills extension (SEP-2640) as ModelContextProtocol.Extensions.Skills #1864 (comment)
Bounded directory reads — File.ReadAllBytes can still allocate past 16 MiB if the file grows after the length check. Add the Skills extension (SEP-2640) as ModelContextProtocol.Extensions.Skills #1864 (comment)
URI-equivalent resources — Identical-content aliases still fail ReadSkillResourceAsync because the response uses the first URI. Add the Skills extension (SEP-2640) as ModelContextProtocol.Extensions.Skills #1864 (comment)

I can’t reopen them myself, no write access.

— girishkvs's Copilot 🤖

PederHP and others added 8 commits September 8, 2026 22:53

Point SkillVerifier remarks at SkillFrontmatter for frontmatter checks

f24ab73

The remarks still said the package does not parse YAML. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6omaTvQiryHtzFHRqUE3b

PederHP requested review from halter73 and a balanced review from Copilot September 9, 2026 06:18

Copilot started reviewing on behalf of PederHP September 9, 2026 06:21 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

PederHP requested a balanced review from Copilot September 9, 2026 07:02

Copilot started reviewing on behalf of PederHP September 9, 2026 07:03 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

PederHP requested a balanced review from Copilot September 9, 2026 16:26

Copilot started reviewing on behalf of PederHP September 9, 2026 16:27 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread src/ModelContextProtocol.Extensions.Skills/Client/McpSkillsClientExtensions.cs

Comment thread src/ModelContextProtocol.Extensions.Skills/Server/McpSkillsBuilderExtensions.cs

PederHP requested a balanced review from Copilot September 9, 2026 17:44

Copilot started reviewing on behalf of PederHP September 9, 2026 17:44 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread src/ModelContextProtocol.Extensions.Skills/Server/McpServerSkill.cs

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

girishkvs reviewed Sep 9, 2026

View reviewed changes

Merge branch 'main' into feat/skills-extension

8e8379a

girishkvs reviewed Sep 9, 2026

View reviewed changes

Uh oh!

Conversation

PederHP commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What's in the package

Core change

Deliberately not included

Open questions for maintainers

Relationship to #1856

Authorship

Validation

Uh oh!

PederHP commented Sep 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PederHP commented Sep 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PederHP commented Sep 9, 2026 •

edited

Loading