feat: Skills extension for server and client (ext/skills subpaths)

[mattzcarey]

Adds the Skills extension (io.modelcontextprotocol/skills, SEP-2640) as @modelcontextprotocol/server/ext/skills and @modelcontextprotocol/client/ext/skills. Stacked on #2820; review the top commit.

// server
const skills = new SkillsExtension({
    list: () => ({ skills: [gitWorkflow] }),
    get: ({ uri }) => (uri === gitWorkflow.uri ? gitWorkflow : undefined)
});
const server = new McpServer(info, { extensions: [skills] });
server.registerResource(uri, uri, { mimeType: 'text/markdown' }, () => ({ contents: [{ uri, text }] }));

// client
const skills = new SkillsClientExtension();
const client = new Client(info, { extensions: [skills] });
const { skills: entries } = await skills.list();
const manifest = await skills.read(entries[0], entries[0].uri); // verified against size and digest

The SDK handles the wire protocol. The server supplies the catalog:

• SkillsExtension declares the extension and the resources capability, and serves skills/list and skills/get from a SkillSource. It also serves resources/directory/read, declared as directoryRead, when the source implements readDirectory. Unknown skills and directories return -32602. An entry that breaks the spec's structural rules returns -32603 instead of going out on the wire.
• Skill files are ordinary resources. skillResourceOf(uri, content) computes the digest and size an entry lists for each file.
• SkillsClientExtension wraps list, get and readDirectory, each refused unless the server declared support. read(skill, uri) fetches a file with resources/read and rejects it unless the entry lists it and its size and SHA-256 digest match.

Scope follows the Go SDK's split (go-sdk#1238): protocol support here, with a filesystem provider left for a follow-up. The schemas don't enforce the 512-file and 16 MiB limits, because those are a floor hosts must accept. Frontmatter checks against SKILL.md need a YAML parser and are left to the host. Schemas and constants are adapted from #2818 (co-authored). Refs #2798.

pnpm check:all and the core-internal, client and server suites ran. The end-to-end test runs a real Client against createMcpHandler using the spec's worked example. It checks that our digests and sizes equal the spec's values, and covers pagination, unlisted skills/get, -32602, -32603, directory reads, verified reads with tampered and unlisted files, and resultType plus cache fields on the wire.

[changeset-bot]

🦋 Changeset detected

Latest commit: 0f7df1f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/core-internal	Minor
@modelcontextprotocol/client	Minor
@modelcontextprotocol/server	Minor
@modelcontextprotocol/codemod	Minor
@modelcontextprotocol/core	Minor
@modelcontextprotocol/server-legacy	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2972

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2972

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2972

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2972

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2972

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2972

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2972

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2972

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2972

commit: 0f7df1f

[claude]

Beyond the inline findings, I also checked the base64 blob decoding in the client's read (atob plus codePointAt yields the raw bytes, so digests over binary files are computed correctly), the loose-object directory-child schema (extra fields are passed through to clients, not dropped), and the ext/skills index re-exports (the dts bundling of Server is only a type re-export, so no duplicate class type is emitted).

Extended reasoning...

The change adds a new Skills extension across core-internal, server and client, with new package subpaths, schemas, digest helpers and an e2e test. The inline findings cover path containment in the resource schema, the resources capability being advertised without handlers, the skills/get URI mismatch, unchecked cacheHint and a docs mismatch; this note only records what else was examined and ruled out.

2 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.

[claude]

🔴 Hosts and servers get skill entries whose files sit outside the skill directory even though the SDK and docs say that rule is enforced. The check at packages/core-internal/src/ext/skills/schemas.ts:62 only tests startsWith(${root}/), so a resource URI such as skill://pdf-processing/../other/secret.md passes, as do . segments and percent-encoded forms like %2e%2e. Fix: reject any resource whose path, after decoding and segment normalisation, contains . or .. segments or does not stay under the skill root, and apply the same normalisation to the frontmatter.name segment check. Entries from a SkillSource go out on the wire, and entries from a server pass listSkillsResultSchema on the client, with these URIs intact. [also at: packages/core-internal/src/ext/skills/schemas.ts:69 - Hosts can be handed a skill whose listed files resolve outside the skill directory, which the base branch never served since the extension did not exist.]

Why this was flagged

A SkillSource (or a remote server answering skills/list) returns an entry with uri: 'skill://pdf-processing/SKILL.md' and a resource uri: 'skill://pdf-processing/../etc/passwd'. skillSchema.superRefine at packages/core-internal/src/ext/skills/schemas.ts:61-65 derives root as skill://pdf-processing and accepts the resource because the string starts with skill://pdf-processing/. validEntry at packages/server/src/ext/skills/skillsExtension.ts:80 therefore sends the entry instead of answering -32603, and the client's listSkillsResultSchema parse at packages/client/src/ext/skills/skillsClientExtension.ts:74 accepts it. docs/servers/skills.md:29 tells users the server checks the structural rules, so a host that strips the root to get a relative path and writes files under a skill folder can be sent outside it. The base branch has no skills extension, so nothing on the base makes this claim. The test 'rejects a resource outside the skill directory' at packages/core-internal/test/ext/skills/schemas.test.ts:31 only covers a URI with a different prefix, not traversal segments.

Verification: packages/core-internal/src/ext/skills/schemas.ts:53 derives root = skill.uri.slice(0, -'/SKILL.md'.length) and line 62 checks only resource.uri.startsWith(${root}/); no decoding or dot-segment normalisation is done, so skill://acme/billing/refunds/../../other/x.md passes. The only test for this rule (packages/core-internal/test/ext/skills/schemas.test.ts:31) uses a plain prefix mismatch and does not cover traversal.

[claude]

🔴 Clients of an McpServer that installs SkillsExtension without registering a resource get "Method not found" (-32601) for resources/list and resources/read, although the server advertised the resources capability. packages/server/src/ext/skills/skillsExtension.ts:99 registers resources: {} on the low-level Server, but McpServer only installs the resource handlers eagerly when options.capabilities.resources is set (packages/server/src/server/mcp.ts:164), otherwise lazily on the first registerResource. Fix: make a capability an extension declares during install get the same eager handlers as one passed in options, e.g. have mcp.ts:164 consult this.server.getCapabilities().resources after the Server constructor has run the extensions. [also at: packages/server/src/ext/skills/skillsExtension.ts:98 - Hosts connecting to a McpServer that installs SkillsExtension but has not registered any resource see the resources capability yet get -32601 "Method not found" for resources/list.]

Why this was flagged

A user builds new McpServer(info, { extensions: [new SkillsExtension(source)] }) with an empty or fully 'dynamic' catalog, or registers resources only later, and connects a client. During the Server constructor (packages/server/src/server/server.ts:367-370) SkillsExtension.install runs packages/server/src/ext/skills/skillsExtension.ts:99 and merges resources: {} into the advertised capabilities. McpServer's constructor at packages/server/src/server/mcp.ts:161-169 decides whether to install the resources/list and resources/read handlers only from options?.capabilities?.resources, which is undefined here. The initialize result therefore declares resources while no handler exists, so a client sending resources/list receives a -32601 error. The comment at mcp.ts:156-160 says a server that declares a primitive capability MUST answer its list method, which this path breaks. On the base branch no extension registers a primitive capability, so the declared capabilities and the installed handlers always agree.

Verification: packages/server/src/ext/skills/skillsExtension.ts:98 runs server.registerCapabilities({ resources: {}, extensions: {...} }) on the low-level Server. McpServer decides whether to eagerly install the resource handlers at mcp.ts:164 with if (options?.capabilities?.resources), so the capability the extension merged is never seen. The base branch has no route to this inconsistency through McpServer; the extension adds one.

[claude]

🔴 Hosts whose client cached a skill file keep getting InvalidResult from read after the skill is updated, even after refreshing the entry as the docs advise. packages/client/src/ext/skills/skillsClientExtension.ts:112 calls readResource with plain RequestOptions, so a still-fresh cached body (stored when the server set a positive ttlMs on resources/read) is served and compared against the new entry's digest, and get cannot clear it. Fix: on a size or digest mismatch against a cache-served body, evict and re-read once with cacheMode: 'refresh' (as callTool does for a stale schema) before failing, and accept CacheableRequestOptions so hosts can bypass the cache themselves. The failure lasts until the body's TTL expires, up to MAX_CACHE_TTL_MS (24 h). [also at: packages/client/src/ext/skills/skillsClientExtension.ts:118 - Hosts whose client cached a skill file get a spurious verification failure from read after the skill changes, and the documented recovery (refresh the entry with get) cannot clear it. skillsClientExtension.ts:112 calls readResource with plain RequestOptions, so a resources/read body cached under a positive ttlMs is served instead of refetched, then fails the new entry's size/digest check at :117-118.]

Why this was flagged

A server registers a skill file with registerResource(uri, uri, { cacheHint: { ttlMs: 300_000, cacheScope: 'public' } }, cb); packages/server/src/server/mcp.ts:598 attaches that hint and the client's readResource at packages/client/src/client/client.ts:1912-1914 stores the body. The operator then changes the file and the entry's digest. The host calls skills.list() and gets the new digest, then skills.read(skill, uri). packages/client/src/ext/skills/skillsClientExtension.ts:112 passes options typed RequestOptions (no cacheMode), so client.ts:1902 _serveFromCache returns the old body; line 117/118 then throw SdkError(InvalidResult, 'digest mismatch'). docs/clients/skills.md:33 tells the host to refresh the entry with get, but get touches no resources/read cache key, so every retry fails the same way until the TTL (capped at MAX_CACHE_TTL_MS = 86_400_000) expires. The base branch has no read, so hosts there hand-roll readResource and can pass cacheMode.

Verification: normal — when a server attaches a positive ttlMs cache hint to a skill file and later updates it within the TTL. skillsClientExtension.ts:112 calls readResource with options?: RequestOptions; client.ts:1901-1903 serves a still-fresh cached body first. skills/get never evicts the response cache, so the recovery docs/clients/skills.md:33 advises does not clear it; lines 117-118 then throw InvalidResult until the TTL expires.

[claude]

🟡 nit (optional): a server configured with an out-of-range cacheHint (e.g. ttlMs: 1.5 or -1) sends it on the wire, and every client then rejects skills/list and skills/get responses. The option at skillsExtension.ts:73 redefines the SDK's existing CacheHint/CacheScope shape with a new SkillsCacheScope type and never validates it, unlike ServerOptions.cacheHints, which throws a RangeError at construction. Fix: type the option as the existing CacheHint (drop SkillsCacheScope) and run assertValidCacheHint(hint, 'cacheHint') in the constructor so bad values fail at construction like the core option does.

Why this was flagged

A server author passes new SkillsExtension(source, { cacheHint: { ttlMs: 1.5 } }) (or a negative number). skillsExtension.ts:92 stores the value unchecked and spreads it into every skills/list and skills/get result at skillsExtension.ts:102 and :110. On the client, listSkillsResultSchema/getSkillResultSchema (schemas.ts:20, z.number().int().nonnegative()) reject the result, so SkillsClientExtension.list() and get() fail for every caller with a parse error, far from the misconfiguration. The base branch's ServerOptions.cacheHints (server.ts:342-346) rejects the same value at construction with RangeError, and exposes the CacheHint/CacheScope types from core-internal/src/shared/resultCacheHints.ts:33; the new SkillsCacheScope at types.ts:67 duplicates CacheScope rather than reusing it.

Verification: Triggers when a server author constructs SkillsExtension with a cacheHint.ttlMs that is not a non-negative safe integer. packages/server/src/ext/skills/skillsExtension.ts:92 stores the value with no check, and it is spread into every result at lines 105 and 112. Client schemas require ttlMs: z.number().int().nonnegative() (schemas.ts:20), so every SDK client's list()/get() fails.

[claude]

🟡 (optional) Hosts asking for one skill can be handed a different skill's entry, and every later verification then passes against the wrong entry. skillsExtension.ts:110-112 returns whatever the source gives for skills/get without checking skill.uri equals params.uri, and the client's get at skillsClientExtension.ts:82-85 does not check either. A buggy or hostile server answers skills/get for X with entry Y; read(skill, Y's files) verifies cleanly, so the host runs Y believing it fetched X. Fix: on both sides reject a skills/get result whose skill.uri differs from the requested uri (server: -32603 via invalidEntry; client: InvalidResult), so the entry a host keys by uri is the one it asked for. [also at: packages/server/src/ext/skills/skillsExtension.ts:112 - Hosts that call skills.get(uriA) can silently receive the entry for a different skill, and then read and verify that other skill's files as if they were A's.]

Why this was flagged

A host calls skills.get('file:///skills/git-workflow/SKILL.md') through packages/client/src/ext/skills/skillsClientExtension.ts:82-85. The server handler at packages/server/src/ext/skills/skillsExtension.ts:110-112 passes the SkillSource's return through validEntry (schemas.ts:42-66), which checks only structural rules on the entry itself, never that skill.uri === params.uri; the client get applies getSkillResultSchema (schemas.ts:83) with the same gap. A source bug (wrong map key, cursor mix-up) or a non-SDK server returns a different skill's entry; it passes both checks. The host then stores the entry under the uri it requested and calls read(skill, skill.uri), which verifies against the substituted entry's digests at skillsClientExtension.ts:116-118 and succeeds, so the host loads and executes skill Y's instructions under X's name with no error. Remedy: compare skill.uri to params.uri on the server (throw via invalidEntry) and on the client (InvalidResult).

Verification: nit — triggers only when a SkillSource answers skills/get with an entry whose uri differs from the requested one. packages/server/src/ext/skills/skillsExtension.ts:110-112 returns validEntry(skill), and validEntry runs only skillSchema; nothing compares skill.uri to params.uri. The client at packages/client/src/ext/skills/skillsClientExtension.ts:82-85 has no cross-check against the requested uri.

[claude]

🟡 nit (optional): readers of the client docs are told every method is refused unless the server declared the extension, but read is not gated. docs/clients/skills.md:20 says "Every method refuses with CapabilityNotSupported", while read at packages/client/src/ext/skills/skillsClientExtension.ts:108 never calls #require and issues resources/read regardless of what the server declared. Fix: make the words match the code, either by saying list, get and readDirectory are refused (as the changeset and the class JSDoc already do) or by gating read on the same check. [also at: docs/clients/skills.md:20 - nit: CLAUDE.md asks that docs say what the code does: this line states "Every method refuses with CapabilityNotSupported unless the server declared the extension and the resources capability", but SkillsClientExtension.read() (packages/client/src/ext/skills/skillsClientExtension.ts:108-120) never calls #require — it goes straight to client.readResource, so a server that declared resources but not the skills extension is read from without refusal.]

Why this was flagged

A user reads docs/clients/skills.md:20 and expects skills.read(skill, uri) to throw CapabilityNotSupported against a server that did not declare io.modelcontextprotocol/skills. The method at packages/client/src/ext/skills/skillsClientExtension.ts:108-120 performs no capability check and calls this.client.readResource directly, so the call proceeds and either succeeds or fails with whatever resources/read returns. The changeset and the class JSDoc at skillsClientExtension.ts:4-6 describe only list, get and readDirectory as refused, so the docs page alone is wrong. Nothing breaks at runtime; the doc misstates the behaviour.

Verification: nit. The doc line is docs/clients/skills.md:20: "Every method refuses with CapabilityNotSupported unless the server declared the extension and the resources capability." In packages/client/src/ext/skills/skillsClientExtension.ts, list (line 73), get (line 83) and readDirectory (line 89) call this.#require(...), but read (lines 108-120) never does. Nothing breaks at runtime, so this is a documentation inaccuracy.

[claude]

One part of this review's analysis that stopped early was run once more. It finished.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Still open from earlier reviews (6):

• 🔴 packages/client/src/ext/skills/skillsClientExtension.ts:112 — Hosts whose client cached a skill file keep getting InvalidResult from read after the skill is updated, even after refr…
• 🔴 packages/core-internal/src/ext/skills/schemas.ts:65 — Hosts and servers get skill entries whose files sit outside the skill directory even though the SDK and docs say that r…
• 🔴 packages/server/src/ext/skills/skillsExtension.ts:99 — Clients of an McpServer that installs SkillsExtension without registering a resource get "Method not found" (-32601) fo…
• Also unresolved: 3 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.

[claude]

🟡 nit (optional): maintainers take on a wider public API than the feature needs, since both subpaths re-export every zod schema and constant from core-internal. The client subpath at packages/client/src/ext/skills/index.ts:24-43 exports params schemas (listSkillsParamsSchema, getSkillParamsSchema, readResourceDirectoryParamsSchema) a client never parses, and MAX_SKILL_RESOURCES / MAX_SKILL_TOTAL_BYTES, which no SDK code enforces. The server subpath mirrors this with result schemas at packages/server/src/ext/skills/index.ts:26-45. Fix: export from each subpath only what its users call (types, SkillsExtension/SkillsClientExtension, skillResourceOf, skillDigest, SKILLS_EXTENSION_ID, DIRECTORY_MIME_TYPE), or state in the PR why each schema and limit constant is public.

Why this was flagged

A user imports @ modelcontextprotocol/client/ext/skills or @ modelcontextprotocol/server/ext/skills. Each index (packages/client/src/ext/skills/index.ts:24-43, packages/server/src/ext/skills/index.ts:26-45) re-exports all ten zod schemas and all five constants from @ modelcontextprotocol/core-internal/ext/skills, including params schemas on the client side and result schemas on the server side that the respective extension never uses, plus MAX_SKILL_RESOURCES and MAX_SKILL_TOTAL_BYTES, which packages/core-internal/src/ext/skills/schemas.ts:8-9 says are deliberately not enforced and which only a test reads. Once published these become semver-bound surface that the SDK must keep. On the base branch neither subpath exists, so nothing is exposed. CLAUDE.md principle 1 asks to prefer changes that add no API unless justified; the PR description gives no reason for exporting the schemas or limits. Nothing fails at runtime.

Verification: nit. Triggering condition: any consumer of the new subpaths sees the full re-exported surface. Mechanism verified: packages/client/src/ext/skills/index.ts:24-44 re-exports all ten zod schemas and all five constants; packages/server/src/ext/skills/index.ts:26-46 does the same. Nothing fails at runtime; this is API-surface commitment only, hence nit.