Repository navigation
feat: Skills extension (SEP-2640) for server and client - #3047
Conversation
…, server handlers) Adds the first phase of first-class Skills extension support behind new `/ext/skills` subpath exports. Root barrels are unchanged. - `@modelcontextprotocol/core/ext/skills` — shared, runtime-neutral schemas, inferred types, wire constants and `skillsCapabilityOf()`. - `@modelcontextprotocol/client/ext/skills` — `listSkills()`, `getSkill()` and `getSkillsCapability()`, gated on the server advertising both `io.modelcontextprotocol/skills` and `resources`. - `@modelcontextprotocol/server/ext/skills` — `installSkills()` declares the extension capability and serves `skills/list` / `skills/get` from caller-provided skill definitions. The skills result schemas deliberately carry no `resultType` member: the 2026-07-28 era codec validates that discriminator on decode and consumes it before any caller-supplied result schema runs, so a schema that re-declares it can never match (#2789). Regression tests pin this end to end. Metadata surface only — filesystem discovery, digest-verified resource reads and `resources/directory/read` follow separately. Refs #2798 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`packageTopologyPins` asserts each published package's export-map keys exactly, so the new `./ext/skills` subpath on core, client and server trips it by design. Per docs/behavior-surface-pins.md the pin is updated in the same PR rather than loosened, with a note on each entry recording why the subpath is public and why it stays off the root barrel. No behavior change: this updates the expectation only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lity fix(skills): default cache hints and preserve legacy wire responses
Loose object schemas throughout, _meta on Skill, resources/directory/read params and result schemas with caching fields, SKILL_LIMITS in place of the schema-level 512-entry cap. Co-authored-by: tobi-oye <tobiloba5000@gmail.com>
Built-in YAML-subset reader for SKILL.md frontmatter (the subset the C# SDK reads, YAML 1.2 core-schema typing, fail-closed on anchors, aliases, tags, duplicate keys and deep nesting), with a differential test against the yaml library (devDependency only).
A ':' before whitespace, a flow indicator or the end of the line is an
indicator in flow collections, and a flow key that starts with '[' or '{' is
a complex key; both are now rejected instead of read as plain text. Plain
flow scalars stop at ',[]{}' and ' #'.
Runtime-neutral UTF-8, base64 and SHA-256 helpers; skill URI rules (segment encoding, canonical form, containment, reference resolution, one code-unit order); a MIME table; and entry validation and building shared by the client and server. Every tsconfig that compiles core-internal sources maps @modelcontextprotocol/core/ext/skills.
isWithin rejects segments that decode to a '/' or NUL, and skillRootUri rejects roots that end in '/' (an empty segment). Tests restore the escapes and exercise the resource sort.
One reusable, extension-shaped object per catalog: attach(mcp) declares io.modelcontextprotocol/skills, registers a skill:// resource template and the skills/list, skills/get and resources/directory/read handlers. Entries from custom sources are validated; caching fields only on 2026-07-28. Replaces installSkills from #2818. Co-authored-by: Matt Carey <mcarey@cloudflare.com>
A SkillFiles adapter (list and read over relative paths) with an in-memory implementation, and buildCatalogSnapshot: it walks the adapter, builds and checks every skill (invalid ones skipped with a warning, or collected under strict), and assembles the entries, file metadata, directories and listing a catalog serves. Metadata only; file contents are never held.
…memorySkillFiles A symlinked SKILL.md marks its folder as a skill, so it is skipped and reported like any other special file; special files outside every skill are reported too. A name that cannot be encoded as a URI skips its skill with a warning instead of aborting the build. memorySkillFiles reads paths directly and re-indexes only when the record's keys change.
memorySkillFiles re-reads its record once per walk (on list(''), where every
load and reload() starts) instead of on every call, and the catalog build
indexes the walked tree once instead of rescanning it per skill and per
directory. Output and ordering are unchanged; 10,000 files build in about
0.1 s instead of several seconds.
SkillsExtension.load(files) builds a SkillCatalog through a two-method SkillFiles adapter (memorySkillFiles for in-code files). It finds every SKILL.md, hashes the files, checks the SEP and Agent Skills rules, skips invalid skills with a warning (or fails under strict), lists only SKILL.md in resources/list, pages with position cursors and re-checks every read against the published digest. reload() is serialized and all-or-nothing, and sends resources/list_changed when the listing changes.
A connected server whose transport rejects resources/list_changed now produces a warning instead of an unhandled rejection. Adds a same-size edit test so the digest check on reads is exercised.
loadSkills('./skills') and nodeSkillFiles(root) at a Node-only subpath, so
ext/skills itself stays free of Node built-ins (pinned by barrelClean).
Symlinks are never followed: a skill containing one is skipped with a warning.
barrelClean follows relative from- and side-effect imports with a single walker and pins the root entry and ext/skills against every node: import. Adds a real-filesystem test for a symlinked skill folder and narrows the nodeSkillFiles doc to what O_NOFOLLOW guarantees.
new SkillsClient(client) reports supported and directoryRead, lists every page (bounded by listMaxPages, stopping at a repeated cursor) with invalid entries and every copy of a repeated URI returned in invalid (or thrown under strict), gets one entry and reads skill directories. Replaces listSkills and getSkill from #2818. Co-authored-by: Matt Carey <mcarey@cloudflare.com>
new SkillsClient(client) lists (every page, invalid and duplicate entries reported in invalid, or thrown under strict), gets and reads skills. read(skill, ref) resolves ref against the skill root, refuses unlisted files before any request, checks size, digest and SKILL.md frontmatter, and fetches once more with cacheMode 'refresh' when a cached copy fails. Replaces listSkills/getSkill from #2818. Co-authored-by: Matt Carey <mcarey@cloudflare.com>
…s by file key encodeSegment now emits each path segment the way WHATWG URL serialises it (always encoding % / ? # and \), so a host that resolves "references/Q&A.md" gets the published URI. SkillsClient.read matches refs to the listing by a normalised file key (fragment dropped, segments re-encoded) so servers that encode differently still verify, refuses unparsable refs as unlisted, and decides "own SKILL.md" by the same key.
…signs joinUri encodes the segment that lands in the authority with encodeURIComponent, so grouping folders and prefixes such as me@home or acme:prod keep publishing. '^' is always encoded, whatever the runtime's URL parser does. skillFileKey decodes each valid escape on its own, so names with a stray '%' match by their plain names.
A %XX that does not start a valid UTF-8 sequence is keyed as %25 plus the original two characters, exactly as encodeSegment publishes such a name, so plain-name reads of files like "10%Dec.md" match their listing.
…C errors A 2026-07-28 Streamable HTTP server answers an unimplemented method with HTTP 404 and a -32601 JSON-RPC body. The transport now surfaces that body in-band, as it already did for 400, so callers get ProtocolError(-32601) instead of an opaque SdkHttpError. Legacy-era exchanges are unchanged.
End to end through public import paths: loadSkills on a temp folder, SkillsClient verifying every file, over createMcpHandler (one server per request) and stdio, on 2025-11-25 and 2026-07-28. The wire suite pins resources/list, resources/read, resources/templates/list, skills/* and the error codes, including app-resource precedence, listResources and baseUri; the SEP-3371 middleware version must pass it unchanged. Co-authored-by: Matt Carey <mcarey@cloudflare.com>
Bumps @modelcontextprotocol/conformance to 0.2.0-alpha.12. The everything server serves a SkillsExtension catalog (built from #2797's fixture idea with the SDK's own API); the everything client loads skills through SkillsClient for the verify-digest, verify-size, verify-frontmatter and no-prefetch scenarios. Co-authored-by: tobi-oye <tobiloba5000@gmail.com>
Without --spec-version the conformance runner runs extension scenarios at 2026-07-28 for servers and 2025-11-25 for clients. Two forced legs cover the other version on each side, and CI runs them.
The server serves the skills/ folder next to it with loadSkills (prefix "examples"); the client lists the catalog, loads git-workflow with verified reads, follows a reference, browses the refunds folder and catches a verification failure, over stdio and HTTP on both protocol eras.
docs/servers/skills.md and docs/clients/skills.md are synced from executed companions under examples/guides. The skills changeset credits every author explicitly and replaces #2818's; a separate patch changeset covers the client's 2026-07-28 method-not-found handling, and the protocol-versions era table now lists HTTP 400/404.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, two things that looked suspicious were checked and ruled out: nested skills being folded into the parent skill's file list (catalogBuild.ts:137-146) is deliberate and matches the spec's directory model, and the ttlMs: 0 / cacheScope: 'private' defaults filled in when no cacheHint is given (extension.ts:100) are the spec's own defaults, not a stricter choice by this SDK.
Extended reasoning...
The change adds the SEP-2640 Skills extension as new ext/skills subpaths across core, core-internal, server and client, plus a client.ts refactor exposing internal paging and readResource seams; it serves files from disk via a skill:// resource template, so path containment and digest verification are the sensitive surface. Seven verified findings (including a half-configured server on failed attach and a possible crash when onWarning throws after reload) rule out approval; the ruled-out note above records what else was examined.
3 optional notes from this repository's REVIEW.md or CLAUDE.md checks were not posted as comments, over this review's limit for such notes; they are on this commit's check card.
Findings marked 🟡 are optional suggestions and need no follow-up push.
CodeQL (js/polynomial-redos) flagged /\/+$/ on baseUri input in joinUri: it takes quadratic time on a long run of '/'. trimTrailingSlashes does the same with a loop, and SkillsExtension's baseUri handling uses it too.
Multi-line SKILL.md and markdown strings in the docs companions, test fixtures, conformance fixture and integration suites read as the files they stand for. Byte-identical; parser corpora, the BOM/CRLF fixture and single-line strings stay escaped.
There was a problem hiding this comment.
Code review found no new issues
No new issues were found in this update; 7 findings from earlier reviews are still open above.
Still open from earlier reviews (7):
- 🔴
packages/client/src/ext/skills/skillsClient.ts:107—Hosts reading a dynamic skill from a third-party server get resources/read not-found (or InvalidResult) for files whose… - Also unresolved: 6 minor or pre-existing.
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
mimeTypeFor read the extension off a plain object literal, so a file named notes.__proto__ was published with mimeType {} and x.constructor with a function, and clients rejected the read. Inherited names now fall back to application/octet-stream.
…ification When a list_changed send failed and onWarning threw, the throw escaped: on a synchronous send failure it rejected reload() and skipped the remaining servers, and on an asynchronous one it became an unhandled rejection. The warning call is now guarded, as the comment on #notify already promised.
For a dynamic skill, read() requested the normalised key rather than the resolved ref, turning Q%26A.md into Q&A.md. RFC 3986 treats a reserved character and its percent-encoding as different URIs, so a server that keys files by exact URI answered not-found. read() now requests the resolved URI (fragment dropped) and matches the returned contents by key, so a reply that spells the file differently is still accepted. Listed files keep requesting the entry's spelling.
…alid to keep valid entries list() returned the valid entries and moved invalid ones into invalid unless the caller opted into strict, the only list verb in the SDK that tolerated a spec-breaking peer by default. It now throws an SdkError naming every invalid entry, as listTools and listResources reject a malformed result and as the Go and C# skills clients do; skipInvalid: true keeps the valid entries and returns the rest in invalid. The strict option is replaced by skipInvalid.
The client compares an entry's frontmatter with its own YAML 1.2 core-schema parse, as SEP-2640 requires field by field, so a server whose YAML parser types values differently (YAML 1.1 reads yes as true) fails verification. The client guide and the parseFrontmatter JSDoc now say so and point at parseFrontmatter.
SKILL_URI_SCHEME, SKILL_MANIFEST_FILENAME and SKILLS_META_PREFIX were exported from three ext/skills subpaths, yet nothing in the SDK read them; the code spells the values inline, and SKILL_URI_SCHEME ('skill:') did not even match the 'skill://' the code uses. The extension has not shipped, so they go before they become API.
The repository keeps inline comments to one line; the reasoning (skills on the root barrel put the frontmatter parser in every package's root module graph) is in 7ee8d91's message.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline note, I re-read the fix commits since my last review (own-key MIME lookup in mime.ts, the try/catch around the warn sink in SkillCatalog#notifyFailed, dynamic-skill reads now requesting the resolved URI with contents matched by skillFileKey, and strict-by-default list() with skipInvalid) and found no regressions in them.
Extended reasoning...
The push since the last review changes 15 files (134 insertions, 58 deletions), mostly in packages/client/src/ext/skills/skillsClient.ts, packages/core-internal/src/ext/skills/mime.ts, packages/server/src/ext/skills/extension.ts and docs/clients/skills.md, plus tests for each. It touches no auth, crypto or injection surface; the SHA-256 digest verification path in the client is unchanged by this push. The PR remains a large multi-package feature with one earlier inline thread still open, so it is not an approve candidate; this note only records that the fix commits were checked.
Still open from earlier reviews (1):
- Unresolved: 1 minor or pre-existing.
parseFrontmatter received the whole SKILL.md, so a host following the guide and passing a YAML parser got a multi-document error, and every read() failed frontmatter verification; the client exposes no splitter to work around it. The client now splits the file itself (BOM and CRLF removed) and passes the YAML block, so parseFrontmatter: yaml => YAML.parse(yaml) works as the guide describes.
claude
Bot
added
the
v2
felixweinberger
left a comment
felixweinberger
left a comment
There was a problem hiding this comment.
One thing before I stamp:
ttlMs and cacheScope on 2026-07-28. The stable spec page makes both REQUIRED on all three results (list, get, directory read), but the client schemas have them optional on every version, and skills/get doesn't check them at all. Please:
- use a strict schema for the three results when the negotiated version is 2026-07-28 (derive it from the current one; keep the loose one for 2025-11-25), so we reject servers that omit them;
- return the fields from
get()andreadDirectory()aslist()already does, so hosts can honour them.
Generated by Claude Code
|
Non-blocking, fine as a follow-up PR: the skills behaviours aren't in Generated by Claude Code |
The stable spec makes ttlMs and cacheScope REQUIRED on skills/list, skills/get and resources/directory/read results. On 2026-07-28 SkillsClient now checks each result, every page of a walk included, against a schema with both required, and throws InvalidResult when one is missing; on 2025-11-25 they stay optional. get() now returns { skill, ttlMs?, cacheScope? } and readDirectory() { resources, ttlMs?, cacheScope? }, as list() returns its fields.
The stable spec defines the Skills extension on 2026-07-28, where ListSkillsResult, GetSkillResult and ReadResourceDirectoryResult extend CacheableResult, so the exported schemas now require ttlMs and cacheScope. Reading the 2025-11-25 backport, which has no caching fields, is SkillsClient's own choice: it derives its loose schemas with .partial(), and get() and readDirectory() return the fields as optional, as list() does.
Done in 1b4e941 and 02582e8. The |
Agreed. I'll add the skills behaviours to |
5 participants
Adds the Skills extension (SEP-2640,
io.modelcontextprotocol/skills) to the server and client SDKs, behind newext/skillssubpaths. A server publishes a folder of skills withloadSkills('./skills')andattach(server). A client lists skills and reads their files with size, digest and frontmatter checks. Package root exports are unchanged.Motivation and Context
SEP-2640 is an accepted extension that lets servers ship agent skills over MCP: a
SKILL.mdplus its supporting files, served as resources and discovered throughskills/list,skills/getandresources/directory/read.This PR implements it in the TypeScript SDK:
skills/list,skills/getandresources/directory/read, and expose every file through askill://resource template.strict: true. A file is served only while its bytes match the published size and digest, andreload()swaps the whole catalog at once.c1aa87e.How Has This Been Tested?
Run at 8afe260. The client, integration and client conformance runs were repeated after the transport revert (62fee61):
0.2.0-alpha.12:Breaking Changes
None. The skills API is new and lives only in the
ext/skillssubpaths, and the package roots load no skills code.Types of changes
Checklist
Additional context
Design decisions:
list()is strict by default. When a server'sskills/listresponse contains an entry that fails validation, or two entries that share a URI,SkillsClient.list()throws anSdkErrornaming every such entry. A host that would rather keep the valid skills creates the client withnew SkillsClient(client, { skipInvalid: true }).list()then returns them and reports each rejected entry in aninvalidarray, with its position in the listing, its URI if it has one, and the reason it was rejected.yaml. It adds about 5 KB gzipped, only in the skills subpaths, and can be swapped foryamlif you prefer the dependency.resources/listshows only each skill'sSKILL.mdfor built-in catalogs, which the spec allows. The full file list is in each skill's entry, so listing every file would only makeresources/listlonger.@experimental. The API is shaped to move onto SEP-3371'sextensions: [skills]without a breaking change.Credits: