From 957fa6949c19f6a2070809ca00ffdd37372ecad8 Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Thu, 22 Oct 2026 12:17:13 +0100 Subject: [PATCH 1/4] feat(server): serve the Skills extension through extensions: [skills] SkillsExtension is now a ServerExtension: setup(ctx) declares the capability and registers the skill resource template and the skills methods through the extension context, so the SDK checks the id and the method names. attach(mcp) and its guards (one per server, before connecting, idempotent) are gone; the extensions option covers them. SkillCatalog tracks every server it is set up on for reload(). --- .changeset/skills-as-extension.md | 5 + examples/guides/clients/skills.examples.ts | 3 +- examples/guides/servers/skills.examples.ts | 16 +- examples/skills/server.ts | 8 +- packages/server/src/ext/skills/extension.ts | 77 ++++----- packages/server/src/ext/skills/node.ts | 2 +- .../server/test/ext/skills/catalog.test.ts | 28 ++- .../server/test/ext/skills/extension.test.ts | 162 ++++++++---------- packages/server/test/ext/skills/harness.ts | 6 +- packages/server/test/ext/skills/node.test.ts | 3 +- .../server/test/server/barrelClean.test.ts | 6 +- skills/mcp-typescript-sdk-skills/SKILL.md | 17 +- .../references/server.md | 19 +- test/conformance/src/everythingServer.ts | 7 +- test/e2e/requirements.ts | 10 +- test/e2e/scenarios/skills.test.ts | 38 ++-- .../test/__fixtures__/skillsStdioServer.ts | 10 +- .../test/server/cloudflareWorkers.test.ts | 7 +- .../test/skills/skillsEndToEnd.test.ts | 15 +- .../test/skills/skillsWire.test.ts | 30 ++-- 20 files changed, 195 insertions(+), 274 deletions(-) create mode 100644 .changeset/skills-as-extension.md diff --git a/.changeset/skills-as-extension.md b/.changeset/skills-as-extension.md new file mode 100644 index 0000000000..6eec5511fc --- /dev/null +++ b/.changeset/skills-as-extension.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/server': minor +--- + +`SkillsExtension` is now a server extension: serve it with `new McpServer(info, { extensions: [skills] })`. This removes the experimental `SkillsExtension.attach(server)`; replace `skills.attach(server)` with the `extensions` option. Passing two skills extensions to one server now fails construction (a duplicate extension id) instead of throwing from the second `attach`. diff --git a/examples/guides/clients/skills.examples.ts b/examples/guides/clients/skills.examples.ts index 7c2a86d651..28a4f21913 100644 --- a/examples/guides/clients/skills.examples.ts +++ b/examples/guides/clients/skills.examples.ts @@ -38,8 +38,7 @@ description: Process customer refunds. 'acme/billing/refunds/templates/refund-email.md': 'Your refund is on its way.\n' }) ); -const server = new McpServer({ name: 'team-skills', version: '1.0.0' }); -catalog.attach(server); +const server = new McpServer({ name: 'team-skills', version: '1.0.0' }, { extensions: [catalog] }); const client = new Client({ name: 'skills-docs-host', version: '1.0.0' }); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); await server.connect(serverTransport); diff --git a/examples/guides/servers/skills.examples.ts b/examples/guides/servers/skills.examples.ts index 4bfb84cd55..1f2e9d99ab 100644 --- a/examples/guides/servers/skills.examples.ts +++ b/examples/guides/servers/skills.examples.ts @@ -23,23 +23,18 @@ import { loadSkills } from '@modelcontextprotocol/server/ext/skills/node'; const skills = await loadSkills('./skills'); -const server = new McpServer({ name: 'team-skills', version: '1.0.0' }); -skills.attach(server); +const server = new McpServer({ name: 'team-skills', version: '1.0.0' }, { extensions: [skills] }); //#endregion loadSkills_basic //#region entries console.log(skills.entries.map(skill => skill.uri).join('\n')); //#endregion entries -//#region attach_perRequest +//#region extensions_perRequest import { createMcpHandler } from '@modelcontextprotocol/server'; -const handler = createMcpHandler(() => { - const mcp = new McpServer({ name: 'team-skills', version: '1.0.0' }); - skills.attach(mcp); - return mcp; -}); -//#endregion attach_perRequest +const handler = createMcpHandler(() => new McpServer({ name: 'team-skills', version: '1.0.0' }, { extensions: [skills] })); +//#endregion extensions_perRequest // What the handler publishes to `subscriptions/listen` streams, recorded for the checks at the end. const published: string[] = []; @@ -164,6 +159,7 @@ if (!liveChanges.listChanged || JSON.stringify(published) !== JSON.stringify(['r if (inline.entries[0]?.frontmatter.name !== 'release-notes' || fromBucket.entries[0]?.uri !== inline.entries[0]?.uri) { throw new Error('skills.md: in-code and adapter catalogs must match'); } -for (const extension of [inline, fromBucket, generated]) extension.attach(new McpServer({ name: 'docs-check', version: '1.0.0' })); +for (const extension of [inline, fromBucket, generated]) + new McpServer({ name: 'docs-check', version: '1.0.0' }, { extensions: [extension] }); await handler.close(); await server.close(); diff --git a/examples/skills/server.ts b/examples/skills/server.ts index 91d330f214..008206de12 100644 --- a/examples/skills/server.ts +++ b/examples/skills/server.ts @@ -1,7 +1,7 @@ /** * Serves the `skills/` folder next to this file as MCP skills (SEP-2640), - * under the `examples` prefix: one `loadSkills` call at startup, attached to - * every server instance. + * under the `examples` prefix: one `loadSkills` call at startup, passed to + * every server instance in `extensions`. * * One binary, either transport — selected by `--http --port ` (defaults to * stdio). See `examples/CONTRIBUTING.md` for the canonical shape. @@ -17,9 +17,7 @@ import { serveStdio } from '@modelcontextprotocol/server/stdio'; const skills = await loadSkills(siblingPath(import.meta.url, 'skills'), { prefix: 'examples' }); function buildServer(): McpServer { - const mcp = new McpServer({ name: 'skills-example', version: '1.0.0' }); - skills.attach(mcp); - return mcp; + return new McpServer({ name: 'skills-example', version: '1.0.0' }, { extensions: [skills] }); } const { transport, port } = parseExampleArgs(); diff --git a/packages/server/src/ext/skills/extension.ts b/packages/server/src/ext/skills/extension.ts index 0e10494acf..5cd087cfa6 100644 --- a/packages/server/src/ext/skills/extension.ts +++ b/packages/server/src/ext/skills/extension.ts @@ -25,6 +25,7 @@ import { trimTrailingSlashes } from '@modelcontextprotocol/core-internal/ext/skills'; +import type { ServerExtension, ServerExtensionContext } from '../../server/extension'; import type { McpServer } from '../../server/mcp'; import { ResourceTemplate } from '../../server/mcp'; import type { Server } from '../../server/server'; @@ -58,23 +59,18 @@ function toContents(uri: string, result: SkillReadResult): ReadResourceResult['c return 'text' in result ? { uri, ...mimeType, text: result.text } : { uri, ...mimeType, blob: encodeBase64(result.bytes) }; } -// Servers with a skills extension attached: one per server, as SEP-3371 rejects a duplicate extension identifier. -const skillsServers = new WeakSet(); - -// Runs after a successful `attach`, by extension (`SkillCatalog` tracks servers); off the class so no subclass hooks it. -const afterAttach = new WeakMap void>(); - -const ONE_PER_SERVER = - 'another skills extension is already attached to this server; attach one per server ' + - '(to serve several folders or sources, combine them behind one SkillFiles adapter or SkillSource)'; +// Runs after a successful `setup`, by extension (`SkillCatalog` tracks servers); off the class so no subclass hooks it. +const afterSetup = new WeakMap void>(); /** - * The MCP Skills extension (SEP-2640) for one catalog of skills. One instance - * serves any number of servers: call {@linkcode SkillsExtension.attach} on each - * (before connecting). Build it from a {@linkcode SkillSource}, or with - * `SkillsExtension.load(files)` from storage. + * The MCP Skills extension (SEP-2640) for one catalog of skills, as a server + * extension: `new McpServer(info, { extensions: [skills] })`. One instance + * serves any number of servers. Build it from a {@linkcode SkillSource}, or + * with `SkillsExtension.load(files)` from storage. To serve several folders + * or sources on one server, combine them behind one `SkillFiles` adapter or + * `SkillSource`: a server takes one extension per id. */ -export class SkillsExtension { +export class SkillsExtension implements ServerExtension { readonly id = SKILLS_EXTENSION_ID; readonly #source: SkillSource; readonly #trusted: boolean; @@ -83,7 +79,6 @@ export class SkillsExtension { readonly #cache: { ttlMs: number; cacheScope: 'public' | 'private' }; readonly #strict: boolean; readonly #warn: (warning: SkillWarning) => void; - readonly #attached = new WeakSet(); /** * Builds a catalog from storage through a {@linkcode SkillFiles} adapter @@ -109,47 +104,39 @@ export class SkillsExtension { } /** - * Serves these skills on `mcp`: registers the skill resource template and - * the skills methods, then declares the capability. Must run before - * connecting. Calling it again for the same server does nothing. One - * skills extension per server: attaching another one throws and leaves the - * server as it was, as does attaching to a server that already handles one - * of the skills methods. + * Serves these skills on the server being constructed: declares the + * capability, then registers the skill resource template and the skills + * methods. The server calls this for each entry in `extensions`. * * @experimental How an extension is wired to a server may change in a minor release. */ - attach(mcp: McpServer): void { - if (this.#attached.has(mcp)) return; - if (mcp.isConnected()) throw new Error('attach skills before connecting the server'); - if (skillsServers.has(mcp)) throw new Error(ONE_PER_SERVER); + setup(ctx: ServerExtensionContext): void { + const mcp = ctx.mcp; const server = mcp.server; const directoryRead = this.#source.readDirectory !== undefined; - const list = this.#source.listResources === undefined ? undefined : (ctx: ServerContext) => this.#listResources(ctx); - server.assertCanSetRequestHandler(SKILLS_LIST_METHOD); - server.assertCanSetRequestHandler(SKILLS_GET_METHOD); - if (directoryRead) server.assertCanSetRequestHandler(RESOURCES_DIRECTORY_READ_METHOD); - // Check before changing anything; declare the capability last, so a server never advertises skills it does not serve. + if (directoryRead) ctx.capability({ directoryRead: true }); + + const list = this.#source.listResources === undefined ? undefined : (requestCtx: ServerContext) => this.#listResources(requestCtx); mcp.registerResource( SKILLS_EXTENSION_ID, new ResourceTemplate(`${this.#baseRoot}{+path}`, { list }), this.#cacheHint === undefined ? {} : { cacheHint: this.#cacheHint }, - (uri, _variables, ctx) => this.#read(uri, ctx) + (uri, _variables, requestCtx) => this.#read(uri, requestCtx) ); - server.setRequestHandler(SKILLS_LIST_METHOD, { params: ListSkillsRequestParamsSchema }, (params, ctx) => - this.#list(server, params.cursor, ctx) + ctx.setRequestHandler(SKILLS_LIST_METHOD, { params: ListSkillsRequestParamsSchema }, (params, requestCtx) => + this.#list(server, params.cursor, requestCtx) ); - server.setRequestHandler(SKILLS_GET_METHOD, { params: GetSkillRequestParamsSchema }, (params, ctx) => - this.#get(server, params.uri, ctx) + ctx.setRequestHandler(SKILLS_GET_METHOD, { params: GetSkillRequestParamsSchema }, (params, requestCtx) => + this.#get(server, params.uri, requestCtx) ); if (directoryRead) { - server.setRequestHandler(RESOURCES_DIRECTORY_READ_METHOD, { params: ReadResourceDirectoryRequestParamsSchema }, (params, ctx) => - this.#readDirectory(server, params.uri, params.cursor, ctx) + ctx.setRequestHandler( + RESOURCES_DIRECTORY_READ_METHOD, + { params: ReadResourceDirectoryRequestParamsSchema }, + (params, requestCtx) => this.#readDirectory(server, params.uri, params.cursor, requestCtx) ); } - server.registerCapabilities({ extensions: { [SKILLS_EXTENSION_ID]: directoryRead ? { directoryRead: true } : {} } }); - this.#attached.add(mcp); - skillsServers.add(mcp); - afterAttach.get(this)?.(mcp); + afterSetup.get(this)?.(mcp); } #cacheFields(server: Server): { ttlMs?: number; cacheScope?: 'public' | 'private' } { @@ -236,7 +223,7 @@ interface CatalogInit { readonly sourceOptions: SkillSourceOptions; } -// Fewest tracked servers at which `attach` drops references to collected ones; then again at twice the survivors. +// Fewest tracked servers at which `setup` drops references to collected ones; then again at twice the survivors. const PRUNE_THRESHOLD = 1024; function resolveLimits(limits: SkillCatalogOptions['limits']): { files: number; bytes: number } { @@ -280,7 +267,7 @@ export class SkillCatalog extends SkillsExtension { this.#state = init.state; this.#files = init.files; this.#build = init.build; - afterAttach.set(this, mcp => this.#track(mcp)); + afterSetup.set(this, mcp => this.#track(mcp)); } static override async load(files: SkillFiles, options: SkillCatalogOptions = {}): Promise { @@ -318,12 +305,12 @@ export class SkillCatalog extends SkillsExtension { * * When skills are added or removed, or a `SKILL.md`'s name or description * changes, the result has `listChanged: true` and `resources/list_changed` - * goes out through every attached server that is still connected (stdio, + * goes out through every server it is set up on that is still connected (stdio, * stateful 2025-11-25 HTTP sessions). Servers that `createMcpHandler` builds per * request are closed by then, so publish the change on the handler: * `if (changes.listChanged) handler.notify.resourcesChanged()`. Where the * runtime has no `WeakRef` (Cloudflare Workers with a compatibility date - * before 2025-05-05), attached servers are not tracked and nothing is sent. + * before 2025-05-05), servers are not tracked and nothing is sent. */ reload(): Promise { const run = this.#queue.then(() => this.#reloadNow()); diff --git a/packages/server/src/ext/skills/node.ts b/packages/server/src/ext/skills/node.ts index 12cb203f23..f136419da3 100644 --- a/packages/server/src/ext/skills/node.ts +++ b/packages/server/src/ext/skills/node.ts @@ -59,7 +59,7 @@ export function nodeSkillFiles(root: string): SkillFiles { * @example * ```ts * const skills = await loadSkills('./skills'); - * skills.attach(mcp); + * const server = new McpServer(info, { extensions: [skills] }); * ``` */ export function loadSkills(root: string, options?: SkillCatalogOptions): Promise { diff --git a/packages/server/test/ext/skills/catalog.test.ts b/packages/server/test/ext/skills/catalog.test.ts index 6f3894ffa2..37c51d71c8 100644 --- a/packages/server/test/ext/skills/catalog.test.ts +++ b/packages/server/test/ext/skills/catalog.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it, vi } from 'vitest'; import type { SkillCatalogOptions, SkillWarning } from '../../../src/ext/skills/index'; import { memorySkillFiles, SkillCatalogError, SkillsExtension } from '../../../src/ext/skills/index'; +import type { McpServer } from '../../../src/server/mcp'; import { ESCALATION, fixtureFiles, GIT, GIT_SKILL_MD, LOGO, PNG, PURCHASE_ORDER, REFUNDS, REFUNDS_SKILL_MD, REMBOURSE } from './fixtures'; import { errorOf, LEGACY, newServer, resultOf } from './harness'; @@ -13,8 +14,7 @@ async function loaded(options: SkillCatalogOptions = {}) { const files = fixtureFiles(); const warnings: SkillWarning[] = []; const catalog = await SkillsExtension.load(memorySkillFiles(files), { onWarning: warning => warnings.push(warning), ...options }); - const mcp = newServer(); - catalog.attach(mcp); + const mcp = newServer(catalog); return { catalog, mcp, warnings, files }; } @@ -162,14 +162,13 @@ describe('SkillCatalog serving', () => { it('serves many servers from one catalog', async () => { const { catalog } = await loaded(); - const servers = [newServer(), newServer(), newServer()]; - for (const server of servers) catalog.attach(server); + const servers = [newServer(catalog), newServer(catalog), newServer(catalog)]; for (const server of servers) expect(await resultOf(server, 'skills/get', { uri: GIT })).toMatchObject({ skill: { uri: GIT } }); }); - it('keeps attach cheap while many attached servers are still alive', async () => { + it('keeps setup cheap while many servers it is set up on are still alive', async () => { const { catalog } = await loaded(); - const servers = Array.from({ length: 1100 }, () => newServer()); + const servers: McpServer[] = []; const deref = WeakRef.prototype.deref; let derefs = 0; WeakRef.prototype.deref = function (this: WeakRef) { @@ -177,7 +176,7 @@ describe('SkillCatalog serving', () => { return deref.call(this); }; try { - for (const server of servers) catalog.attach(server); + for (let index = 0; index < 1100; index++) servers.push(newServer(catalog)); } finally { WeakRef.prototype.deref = deref; } @@ -240,14 +239,13 @@ describe('SkillCatalog.reload', () => { expect(await catalog.reload()).toMatchObject({ added: ['skill://aaa/SKILL.md'], listChanged: true }); }); - it('attaches and reloads where the runtime has no WeakRef, tracking no servers (Workers before 2025-05-05)', async () => { + it('sets up and reloads where the runtime has no WeakRef, tracking no servers (Workers before 2025-05-05)', async () => { const { catalog, files } = await loaded(); const descriptor = Object.getOwnPropertyDescriptor(globalThis, 'WeakRef')!; Reflect.deleteProperty(globalThis, 'WeakRef'); try { expect(typeof WeakRef).toBe('undefined'); - const mcp = newServer(); - expect(() => catalog.attach(mcp)).not.toThrow(); + const mcp = newServer(catalog); expect(await resultOf(mcp, 'skills/get', { uri: GIT })).toMatchObject({ skill: { uri: GIT } }); const notify = vi.spyOn(mcp.server, 'sendResourceListChanged'); const [, serverSide] = InMemoryTransport.createLinkedPair(); @@ -283,9 +281,7 @@ description: d it('notifies each connected server once, and only when the listing changes', async () => { const { catalog, files, mcp: disconnected } = await loaded(); - const mcp = newServer(); - catalog.attach(mcp); - catalog.attach(mcp); + const mcp = newServer(catalog); const notify = vi.spyOn(mcp.server, 'sendResourceListChanged'); const skipped = vi.spyOn(disconnected.server, 'sendResourceListChanged'); const [, serverSide] = InMemoryTransport.createLinkedPair(); @@ -302,8 +298,7 @@ description: d it('reports a failed list_changed send as a warning, never an unhandled rejection', async () => { const { catalog, files, warnings } = await loaded(); - const mcp = newServer(); - catalog.attach(mcp); + const mcp = newServer(catalog); const [, serverSide] = InMemoryTransport.createLinkedPair(); await mcp.connect(serverSide); vi.spyOn(serverSide, 'send').mockRejectedValue(new Error('transport down')); @@ -330,9 +325,8 @@ description: d throw new Error('sink failed'); }); const { catalog, files } = await loaded({ onWarning: sink }); - const servers = [newServer(), newServer(), newServer()]; + const servers = [newServer(catalog), newServer(catalog), newServer(catalog)]; for (const server of servers) { - catalog.attach(server); await server.connect(InMemoryTransport.createLinkedPair()[1]); } const [throwsSync, rejects, healthy] = servers.map(server => vi.spyOn(server.server, 'sendResourceListChanged')); diff --git a/packages/server/test/ext/skills/extension.test.ts b/packages/server/test/ext/skills/extension.test.ts index 33b06c1d81..1e126cb57f 100644 --- a/packages/server/test/ext/skills/extension.test.ts +++ b/packages/server/test/ext/skills/extension.test.ts @@ -2,11 +2,11 @@ import { createHash } from 'node:crypto'; import type { Skill } from '@modelcontextprotocol/core/ext/skills'; import { ListSkillsRequestParamsSchema, ReadResourceDirectoryRequestParamsSchema } from '@modelcontextprotocol/core/ext/skills'; -import { InMemoryTransport } from '@modelcontextprotocol/core-internal'; import { describe, expect, it } from 'vitest'; import type { SkillSource, SkillWarning } from '../../../src/ext/skills/index'; import { memorySkillFiles, SkillCatalogError, skillEntry, SkillsExtension } from '../../../src/ext/skills/index'; +import type { ServerExtension } from '../../../src/server/extension'; import { ResourceTemplate } from '../../../src/server/mcp'; import { GIT, GIT_SKILL_MD } from './fixtures'; import { errorOf, LEGACY, newServer, resultOf } from './harness'; @@ -27,17 +27,16 @@ function sourceOf(overrides: Partial = {}): SkillSource { }; } -function attached(options: Partial[0]> = {}) { +function served(options: Partial[0]> = {}) { const warnings: SkillWarning[] = []; - const mcp = newServer(); - new SkillsExtension({ source: sourceOf(), onWarning: warning => warnings.push(warning), ...options }).attach(mcp); + const mcp = newServer(new SkillsExtension({ source: sourceOf(), onWarning: warning => warnings.push(warning), ...options })); return { mcp, warnings }; } describe('capability', () => { it('declares the extension, with directoryRead only when the source reads directories', () => { - expect(attached().mcp.server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': {} }); - const withDirectories = attached({ source: sourceOf({ readDirectory: async () => ({ resources: [] }) }) }); + expect(served().mcp.server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': {} }); + const withDirectories = served({ source: sourceOf({ readDirectory: async () => ({ resources: [] }) }) }); expect(withDirectories.mcp.server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': { directoryRead: true } }); @@ -47,7 +46,7 @@ describe('capability', () => { describe('skills/list', () => { it('serves the page with caching fields on 2026-07-28 only', async () => { - const { mcp } = attached({ cacheHint: { ttlMs: 60_000, cacheScope: 'public' } }); + const { mcp } = served({ cacheHint: { ttlMs: 60_000, cacheScope: 'public' } }); expect(await resultOf(mcp, 'skills/list')).toMatchObject({ skills: [gitWorkflow], ttlMs: 60_000, cacheScope: 'public' }); const legacy = await resultOf(mcp, 'skills/list', {}, LEGACY); expect(legacy['skills']).toEqual([gitWorkflow]); @@ -56,32 +55,32 @@ describe('skills/list', () => { }); it('defaults the caching fields to 0 / private', async () => { - expect(await resultOf(attached().mcp, 'skills/list')).toMatchObject({ ttlMs: 0, cacheScope: 'private' }); + expect(await resultOf(served().mcp, 'skills/list')).toMatchObject({ ttlMs: 0, cacheScope: 'private' }); }); it('passes nextCursor through', async () => { - const { mcp } = attached({ source: sourceOf({ list: async () => ({ skills: [gitWorkflow], nextCursor: 'n' }) }) }); + const { mcp } = served({ source: sourceOf({ list: async () => ({ skills: [gitWorkflow], nextCursor: 'n' }) }) }); expect(await resultOf(mcp, 'skills/list')).toMatchObject({ nextCursor: 'n' }); }); it('drops invalid entries from a source and warns, or fails with -32603 when strict', async () => { const broken = { ...gitWorkflow, uri: 'skill://broken/SKILL.md' }; - const lenient = attached({ source: sourceOf({ list: async () => ({ skills: [gitWorkflow, broken] }) }) }); + const lenient = served({ source: sourceOf({ list: async () => ({ skills: [gitWorkflow, broken] }) }) }); const { skills } = await resultOf(lenient.mcp, 'skills/list'); expect(skills).toEqual([gitWorkflow]); expect(lenient.warnings[0]).toMatchObject({ path: 'skill://broken/SKILL.md' }); - const strict = attached({ strict: true, source: sourceOf({ list: async () => ({ skills: [broken] }) }) }); + const strict = served({ strict: true, source: sourceOf({ list: async () => ({ skills: [broken] }) }) }); expect(await errorOf(strict.mcp, 'skills/list')).toMatchObject({ code: -32_603 }); }); }); describe('skills/get', () => { it('returns the entry with caching fields', async () => { - expect(await resultOf(attached().mcp, 'skills/get', { uri: gitWorkflow.uri })).toMatchObject({ skill: gitWorkflow, ttlMs: 0 }); + expect(await resultOf(served().mcp, 'skills/get', { uri: gitWorkflow.uri })).toMatchObject({ skill: gitWorkflow, ttlMs: 0 }); }); it('answers -32602 for a skill it does not serve', async () => { - expect(await errorOf(attached().mcp, 'skills/get', { uri: 'skill://nope/SKILL.md' })).toMatchObject({ code: -32_602 }); + expect(await errorOf(served().mcp, 'skills/get', { uri: 'skill://nope/SKILL.md' })).toMatchObject({ code: -32_602 }); }); it('answers -32603 when the source returns a different or invalid skill', async () => { @@ -91,10 +90,10 @@ describe('skills/get', () => { frontmatter: { name: 'other', description: 'd' }, resources: [{ uri: 'skill://other/SKILL.md', digest, size: 1 }] }; - const differs = attached({ source: sourceOf({ get: async () => other }) }); + const differs = served({ source: sourceOf({ get: async () => other }) }); expect(await errorOf(differs.mcp, 'skills/get', { uri: gitWorkflow.uri })).toMatchObject({ code: -32_603 }); const invalid = { ...gitWorkflow, frontmatter: { name: 'Wrong', description: 'd' } }; - const broken = attached({ source: sourceOf({ get: async () => invalid }) }); + const broken = served({ source: sourceOf({ get: async () => invalid }) }); expect(await errorOf(broken.mcp, 'skills/get', { uri: gitWorkflow.uri })).toMatchObject({ code: -32_603 }); }); }); @@ -102,7 +101,7 @@ describe('skills/get', () => { describe('resources/read through the skill template', () => { it('serves text, serves bytes as base64, and answers -32602 for unknown files', async () => { const bytes = Uint8Array.from([0, 1, 2, 255]); - const { mcp } = attached({ + const { mcp } = served({ source: sourceOf({ read: async uri => { if (uri === gitWorkflow.uri) return { text: '# Git', mimeType: 'text/markdown' }; @@ -121,7 +120,7 @@ describe('resources/read through the skill template', () => { }); it('registers the skill:// template under the extension id', async () => { - const { resourceTemplates } = await resultOf(attached().mcp, 'resources/templates/list'); + const { resourceTemplates } = await resultOf(served().mcp, 'resources/templates/list'); expect(resourceTemplates).toEqual([ expect.objectContaining({ name: 'io.modelcontextprotocol/skills', uriTemplate: 'skill://{+path}' }) ]); @@ -131,7 +130,7 @@ describe('resources/read through the skill template', () => { describe('resources/directory/read', () => { it('passes pages through with caching fields, answers -32602 for non-directories, and is absent without readDirectory', async () => { const child = { uri: 'skill://git-workflow/SKILL.md', name: 'SKILL.md', mimeType: 'text/markdown' }; - const { mcp } = attached({ + const { mcp } = served({ source: sourceOf({ readDirectory: async uri => (uri === 'skill://git-workflow' ? { resources: [child], nextCursor: 'n' } : undefined) }) @@ -143,17 +142,17 @@ describe('resources/directory/read', () => { cacheScope: 'private' }); expect(await errorOf(mcp, 'resources/directory/read', { uri: 'skill://git-workflow/SKILL.md' })).toMatchObject({ code: -32_602 }); - expect(await errorOf(attached().mcp, 'resources/directory/read', { uri: 'skill://git-workflow' })).toMatchObject({ code: -32_601 }); + expect(await errorOf(served().mcp, 'resources/directory/read', { uri: 'skill://git-workflow' })).toMatchObject({ code: -32_601 }); }); }); describe('resources/list', () => { it('lists nothing from a source by default and what listResources returns otherwise', async () => { - const { resources: none } = await resultOf(attached().mcp, 'resources/list'); + const { resources: none } = await resultOf(served().mcp, 'resources/list'); expect(none).toEqual([]); const listed = { uri: gitWorkflow.uri, name: 'git-workflow', description: 'd', mimeType: 'text/markdown' }; const outside = { uri: 'other://x/SKILL.md', name: 'x' }; - const { mcp, warnings } = attached({ source: sourceOf({ listResources: async () => [listed, outside] }) }); + const { mcp, warnings } = served({ source: sourceOf({ listResources: async () => [listed, outside] }) }); const { resources } = await resultOf(mcp, 'resources/list'); expect(resources).toEqual([listed]); expect(warnings[0]).toMatchObject({ path: 'other://x/SKILL.md' }); @@ -167,7 +166,7 @@ describe('baseUri', () => { frontmatter: { name: 'refunds', description: 'Refunds.' }, resources: [{ uri: 'github://owner/repo/skills/refunds/SKILL.md', digest, size: 1 }] }; - const { mcp } = attached({ + const { mcp } = served({ baseUri: 'github://owner/repo/skills', source: sourceOf({ list: async () => ({ skills: [github] }), @@ -183,7 +182,7 @@ describe('baseUri', () => { it('matches requests however the baseUri spells its host and port', async () => { const uri = 'https://example.com/skills/refunds/SKILL.md'; const listed = { uri, name: 'refunds' }; - const { mcp, warnings } = attached({ + const { mcp, warnings } = served({ baseUri: 'https://Example.com:443/skills/', source: sourceOf({ read: async requested => (requested === uri ? { text: 'x' } : undefined), @@ -206,100 +205,79 @@ describe('baseUri', () => { }); }); -describe('attach', () => { - it('is a no-op the second time on the same server and serves many servers', async () => { +describe('setup', () => { + it('serves many servers from one instance', async () => { const extension = new SkillsExtension({ source: sourceOf() }); - const first = newServer(); - const second = newServer(); - extension.attach(first); - extension.attach(first); - extension.attach(second); + const first = newServer(extension); + const second = newServer(extension); expect(await resultOf(first, 'skills/get', { uri: gitWorkflow.uri })).toMatchObject({ skill: gitWorkflow }); expect(await resultOf(second, 'skills/get', { uri: gitWorkflow.uri })).toMatchObject({ skill: gitWorkflow }); expect(first.server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': {} }); }); - it('refuses a second skills extension on a server, leaving the first one serving with its capability unchanged', async () => { - const mcp = newServer(); + it('refuses a second skills extension on one server; each still serves a server of its own', async () => { const catalog = await SkillsExtension.load(memorySkillFiles({ 'git-workflow/SKILL.md': GIT_SKILL_MD })); - catalog.attach(mcp); - const declared = { 'io.modelcontextprotocol/skills': { directoryRead: true } }; - expect(mcp.server.getCapabilities().extensions).toEqual(declared); - // A source without readDirectory would declare `{}`: the probe that used to overwrite `directoryRead`. const second = new SkillsExtension({ source: sourceOf() }); - expect(() => second.attach(mcp)).toThrow(/another skills extension is already attached/); - expect(() => second.attach(mcp)).toThrow(/another skills extension is already attached/); - expect(mcp.server.getCapabilities().extensions).toEqual(declared); - expect(await resultOf(mcp, 'skills/get', { uri: GIT })).toMatchObject({ skill: { uri: GIT } }); - expect(await resultOf(mcp, 'resources/directory/read', { uri: 'skill://git-workflow' })).toMatchObject({ + expect(() => newServer(catalog, second)).toThrow('Extension "io.modelcontextprotocol/skills" is listed more than once'); + const own = newServer(catalog); + expect(own.server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': { directoryRead: true } }); + expect(await resultOf(own, 'resources/directory/read', { uri: 'skill://git-workflow' })).toMatchObject({ resources: [{ name: 'SKILL.md' }] }); - // The refused extension still attaches to a server of its own. - const own = newServer(); - second.attach(own); - expect(own.server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': {} }); + expect(newServer(second).server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': {} }); }); - it('refuses to replace a handler the server already has for a skills method, changing nothing', async () => { - const mcp = newServer(); - mcp.server.setRequestHandler('skills/list', { params: ListSkillsRequestParamsSchema }, () => ({ skills: [] })); - const extension = new SkillsExtension({ source: sourceOf() }); - expect(() => extension.attach(mcp)).toThrow('A request handler for skills/list already exists, which would be overridden'); - expect(mcp.server.getCapabilities().extensions).toBeUndefined(); - expect(await resultOf(mcp, 'skills/list')).toMatchObject({ skills: [] }); - mcp.server.removeRequestHandler('skills/list'); - extension.attach(mcp); - expect(await resultOf(mcp, 'skills/list')).toMatchObject({ skills: [gitWorkflow] }); + it('refuses a skills method another extension already serves', () => { + const claimsList: ServerExtension = { + id: 'com.example/list', + setup: ctx => ctx.setRequestHandler('skills/list', { params: ListSkillsRequestParamsSchema }, () => ({ skills: [] })) + }; + expect(() => newServer(claimsList, new SkillsExtension({ source: sourceOf() }))).toThrow( + 'Extension "io.modelcontextprotocol/skills" cannot register "skills/list": another handler already serves it' + ); }); - it('refuses to replace a resources/directory/read handler when the source reads directories', () => { - const mcp = newServer(); - mcp.server.setRequestHandler('resources/directory/read', { params: ReadResourceDirectoryRequestParamsSchema }, () => ({ - resources: [] - })); + it('refuses resources/directory/read another extension serves only when the source reads directories', () => { + const claimsDirectories: ServerExtension = { + id: 'com.example/directories', + setup: ctx => + ctx.setRequestHandler('resources/directory/read', { params: ReadResourceDirectoryRequestParamsSchema }, () => ({ + resources: [] + })) + }; const withDirectories = new SkillsExtension({ source: sourceOf({ readDirectory: async () => ({ resources: [] }) }) }); - expect(() => withDirectories.attach(mcp)).toThrow(/resources\/directory\/read already exists/); - expect(mcp.server.getCapabilities().extensions).toBeUndefined(); - new SkillsExtension({ source: sourceOf() }).attach(mcp); - expect(mcp.server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': {} }); + expect(() => newServer(claimsDirectories, withDirectories)).toThrow(/cannot register "resources\/directory\/read"/); + // Without readDirectory, Skills never registers the method, so the two coexist. + const mcp = newServer(claimsDirectories, new SkillsExtension({ source: sourceOf() })); + expect(mcp.server.getCapabilities().extensions).toEqual({ 'com.example/directories': {}, 'io.modelcontextprotocol/skills': {} }); }); - it('declares nothing when the template name is already taken', () => { - const mcp = newServer(); - mcp.registerResource( - 'io.modelcontextprotocol/skills', - new ResourceTemplate('app://{+path}', { list: undefined }), - {}, - async uri => ({ - contents: [{ uri: uri.href, text: 'app' }] - }) - ); - expect(() => new SkillsExtension({ source: sourceOf() }).attach(mcp)).toThrow(/already registered/); - expect(mcp.server.getCapabilities().extensions?.['io.modelcontextprotocol/skills']).toBeUndefined(); + it('fails when the template name is already taken', () => { + const takesName: ServerExtension = { + id: 'com.example/app', + setup: ctx => + ctx.mcp.registerResource( + 'io.modelcontextprotocol/skills', + new ResourceTemplate('app://{+path}', { list: undefined }), + {}, + async uri => ({ + contents: [{ uri: uri.href, text: 'app' }] + }) + ) + }; + expect(() => newServer(takesName, new SkillsExtension({ source: sourceOf() }))).toThrow(/already registered/); }); - it('has no attach hook a subclass can override', () => { + it('has no setup hook a subclass can override', () => { let calls = 0; class Hooked extends SkillsExtension { - afterAttach(): void { + afterSetup(): void { calls++; } } - new Hooked({ source: sourceOf() }).attach(newServer()); + newServer(new Hooked({ source: sourceOf() })); expect(calls).toBe(0); - expect('afterAttach' in SkillsExtension.prototype).toBe(false); - }); - - it('throws after the server connected', async () => { - const mcp = newServer(); - const [, serverSide] = InMemoryTransport.createLinkedPair(); - await mcp.connect(serverSide); - expect(() => new SkillsExtension({ source: sourceOf() }).attach(mcp)).toThrow(/before connecting/); - await mcp.close(); - }); - - it('rejects an invalid cacheHint at construction', () => { - expect(() => new SkillsExtension({ source: sourceOf(), cacheHint: { ttlMs: -1 } })).toThrow(); + expect('afterSetup' in SkillsExtension.prototype).toBe(false); }); }); diff --git a/packages/server/test/ext/skills/harness.ts b/packages/server/test/ext/skills/harness.ts index 12009433ce..4c46aa52a5 100644 --- a/packages/server/test/ext/skills/harness.ts +++ b/packages/server/test/ext/skills/harness.ts @@ -6,6 +6,7 @@ import { setNegotiatedProtocolVersion } from '@modelcontextprotocol/core-internal'; +import type { ServerExtension } from '../../../src/server/extension'; import { invoke } from '../../../src/server/invoke'; import { McpServer } from '../../../src/server/mcp'; @@ -19,8 +20,9 @@ export interface Reply { let nextId = 1; -export function newServer(): McpServer { - return new McpServer({ name: 'skills-test', version: '1.0.0' }); +/** A server with `extensions` set up on it, as `new McpServer(info, { extensions })` does. */ +export function newServer(...extensions: ServerExtension[]): McpServer { + return new McpServer({ name: 'skills-test', version: '1.0.0' }, { extensions }); } /** Sends one JSON-RPC request through the real dispatch path, on the given protocol revision. */ diff --git a/packages/server/test/ext/skills/node.test.ts b/packages/server/test/ext/skills/node.test.ts index 427b245a89..bbd9e4c1cb 100644 --- a/packages/server/test/ext/skills/node.test.ts +++ b/packages/server/test/ext/skills/node.test.ts @@ -130,8 +130,7 @@ description: Lives outside the root. it('refuses a file edited on disk until reload() publishes it', async () => { const catalog = await loadSkills(root, { onWarning: () => undefined }); - const mcp = newServer(); - catalog.attach(mcp); + const mcp = newServer(catalog); await writeFile(path.join(root, 'git-workflow', 'references', 'REVIEW.md'), '# Review v2\n'); const uri = 'skill://git-workflow/references/REVIEW.md'; expect(await errorOf(mcp, 'resources/read', { uri })).toMatchObject({ code: -32_603 }); diff --git a/packages/server/test/server/barrelClean.test.ts b/packages/server/test/server/barrelClean.test.ts index 72fc4221bf..27d35b97f3 100644 --- a/packages/server/test/server/barrelClean.test.ts +++ b/packages/server/test/server/barrelClean.test.ts @@ -164,13 +164,13 @@ describe('@modelcontextprotocol/server root and ext/skills are runtime-neutral', expect(graph.some(content => SKILLS_CODE.test(content))).toBe(true); }); - test('the ext/skills declarations give subclasses no attach hook to override', () => { - // SEP-3371 registration (`extensions: [skills]`) will not go through attach(), so no hook may be public API. + test('the ext/skills declarations give subclasses no setup hook to override', () => { + // The after-setup hook is internal wiring between SkillsExtension and SkillCatalog, so it must not be public API. const graph = declarationGraphOf(join(distDir, 'ext/skills/index.d.mts')); expect(graph.some(file => /\bdeclare class SkillCatalog\b/.test(readFileSync(file, 'utf8')))).toBe(true); for (const file of graph) { expect({ file, content: readFileSync(file, 'utf8') }).not.toEqual( - expect.objectContaining({ content: expect.stringMatching(/\bafterAttach\b/) }) + expect.objectContaining({ content: expect.stringMatching(/\bafterSetup\b/) }) ); } }); diff --git a/skills/mcp-typescript-sdk-skills/SKILL.md b/skills/mcp-typescript-sdk-skills/SKILL.md index 131b572e79..4f75559e68 100644 --- a/skills/mcp-typescript-sdk-skills/SKILL.md +++ b/skills/mcp-typescript-sdk-skills/SKILL.md @@ -37,7 +37,7 @@ npm view @modelcontextprotocol/server@latest exports --json | grep -c '"./ext/sk ## Serve skills -**Pick where the skills come from.** Each source gives you an extension that you attach as shown below; only the first three have `reload()`: +**Pick where the skills come from.** Each source gives you an extension that you pass in `extensions` as shown below; only the first three have `reload()`: - **A folder on disk** (Node): `loadSkills`, which the steps below walk through. - **Skills defined in code** (Workers and other runtimes without a file system, tests): `memorySkillFiles`. See [Serve skills defined in code](references/server.md#serve-skills-defined-in-code). @@ -46,7 +46,7 @@ npm view @modelcontextprotocol/server@latest exports --json | grep -c '"./ext/sk **Lay out the folder.** Each folder holding a `SKILL.md` is one skill. Its frontmatter needs a `name` equal to the folder's name and a `description`. Other folders only group skills in the URI. -**Load the folder once and attach it before the server connects.** `attach` throws on a connected server, and each server takes one skills extension. A server connected by hand keeps its `server.connect(transport)` call, after `attach`. +**Load the folder once and pass it in `extensions` when you construct the server.** Each server takes one skills extension: a second one fails construction. A server connected by hand keeps its `server.connect(transport)` call. ```ts source="../../examples/guides/servers/skills.examples.ts#loadSkills_basic" import { McpServer } from '@modelcontextprotocol/server'; @@ -54,22 +54,17 @@ import { loadSkills } from '@modelcontextprotocol/server/ext/skills/node'; const skills = await loadSkills('./skills'); -const server = new McpServer({ name: 'team-skills', version: '1.0.0' }); -skills.attach(server); +const server = new McpServer({ name: 'team-skills', version: '1.0.0' }, { extensions: [skills] }); ``` **Resolve the folder from the module, not the working directory.** A relative path like `'./skills'` resolves against `process.cwd()`, and MCP hosts may start stdio servers from another directory (such as `/`), where the load throws and takes the whole server down with it. Build the path from the module's location instead: `fileURLToPath(new URL('../skills', import.meta.url))` from `src/index.ts`, or `path.join(__dirname, '../skills')` in CommonJS. -**Where a factory builds the server, attach inside it**, so every server it builds serves the same catalog. That covers `serveStdio(createServer)` on stdio as well as `createMcpHandler` over HTTP: +**Where a factory builds the server, pass it in `extensions` there**, so every server it builds serves the same catalog. That covers `serveStdio(createServer)` on stdio as well as `createMcpHandler` over HTTP: -```ts source="../../examples/guides/servers/skills.examples.ts#attach_perRequest" +```ts source="../../examples/guides/servers/skills.examples.ts#extensions_perRequest" import { createMcpHandler } from '@modelcontextprotocol/server'; -const handler = createMcpHandler(() => { - const mcp = new McpServer({ name: 'team-skills', version: '1.0.0' }); - skills.attach(mcp); - return mcp; -}); +const handler = createMcpHandler(() => new McpServer({ name: 'team-skills', version: '1.0.0' }, { extensions: [skills] })); ``` **Pick up edits with `reload()`** if the user expects changes to show without a restart. Call it from whatever tells the project the folder changed (a file watcher, an admin route, a deploy hook). A server that stays connected (stdio) notifies its clients itself. Behind `createMcpHandler`, publish the change on the handler, where `changes` is what `await skills.reload()` returned: diff --git a/skills/mcp-typescript-sdk-skills/references/server.md b/skills/mcp-typescript-sdk-skills/references/server.md index 762a6797eb..ee1a21a750 100644 --- a/skills/mcp-typescript-sdk-skills/references/server.md +++ b/skills/mcp-typescript-sdk-skills/references/server.md @@ -2,7 +2,7 @@ -`loadSkills` reads a folder of skills once; `attach` serves it on a server. +`loadSkills` reads a folder of skills once; passing it in `extensions` serves it on a server. ```ts source="../../../examples/guides/servers/skills.examples.ts#loadSkills_basic" import { McpServer } from '@modelcontextprotocol/server'; @@ -10,11 +10,10 @@ import { loadSkills } from '@modelcontextprotocol/server/ext/skills/node'; const skills = await loadSkills('./skills'); -const server = new McpServer({ name: 'team-skills', version: '1.0.0' }); -skills.attach(server); +const server = new McpServer({ name: 'team-skills', version: '1.0.0' }, { extensions: [skills] }); ``` -`attach` is experimental: how an extension is wired to a server may change in a minor release. +The `extensions` option and the extension shape are experimental: how an extension is wired to a server may change in a minor release. A **skill** is a folder with a `SKILL.md` and the files it references (SEP-2640). Clients list skills with `skills/list` and read each file with `resources/read`, checking it against the digest your server published. @@ -46,16 +45,12 @@ skill://git-workflow/SKILL.md ## Serve one catalog from every server -Attach the same catalog to each server your handler creates. +Pass the same catalog to each server your handler creates. -```ts source="../../../examples/guides/servers/skills.examples.ts#attach_perRequest" +```ts source="../../../examples/guides/servers/skills.examples.ts#extensions_perRequest" import { createMcpHandler } from '@modelcontextprotocol/server'; -const handler = createMcpHandler(() => { - const mcp = new McpServer({ name: 'team-skills', version: '1.0.0' }); - skills.attach(mcp); - return mcp; -}); +const handler = createMcpHandler(() => new McpServer({ name: 'team-skills', version: '1.0.0' }, { extensions: [skills] })); ``` The catalog keeps metadata only (paths, digests, sizes, frontmatter); each read goes to the folder, and the SDK serves the bytes only if they still match the published digest. It sends a file as `text` whenever its bytes are valid UTF-8 (a BOM included), whatever its MIME type, and as a base64 `blob` otherwise. @@ -189,7 +184,7 @@ Skills work on both protocol versions; the `cacheHint` option applies on 2026-07 ## Recap -- `loadSkills('./skills')` builds a catalog once; `attach(server)` serves it on any number of servers. +- `loadSkills('./skills')` builds a catalog once; `extensions: [skills]` serves it on any number of servers. - Each folder with a `SKILL.md` is a skill; other folders group skills in the URI. - `reload()` swaps in a rescanned catalog all at once; behind `createMcpHandler`, call `handler.notify.resourcesChanged()` when it reports `listChanged`. - `memorySkillFiles`, a `SkillFiles` adapter or a `SkillSource` serve skills without a folder. diff --git a/test/conformance/src/everythingServer.ts b/test/conformance/src/everythingServer.ts index 2aa3d31601..a42ed62ef8 100644 --- a/test/conformance/src/everythingServer.ts +++ b/test/conformance/src/everythingServer.ts @@ -165,13 +165,12 @@ function createMcpServer() { // request that carries requestState is verified before the handler // runs. A rejection answers a wire-level -32602 with // data.reason 'invalid_request_state'. - requestState: { verify: requestStateCodec.verify } + requestState: { verify: requestStateCodec.verify }, + // SEP-2640: skills/list, skills/get, resources/directory/read and the skill:// resource template. + extensions: [conformanceSkills] } ); - // SEP-2640: skills/list, skills/get, resources/directory/read and the skill:// resource template. - conformanceSkills.attach(mcpServer); - // Helper to send log messages using the underlying server function sendLog( level: 'debug' | 'info' | 'notice' | 'warning' | 'error' | 'critical' | 'alert' | 'emergency', diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index 519d92acfa..bbe70f59d5 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -3396,7 +3396,7 @@ export const REQUIREMENTS: Record = { 'skills:capability:declared': { source: `${SKILLS_SPEC}L35-L65`, behavior: - "A server with skills attached declares extensions['io.modelcontextprotocol/skills'] next to the resources capability: { directoryRead: true } when the source implements directory reads, {} otherwise. SkillsClient.supported and .directoryRead reflect the declaration." + "A server serving skills declares extensions['io.modelcontextprotocol/skills'] next to the resources capability: { directoryRead: true } when the source implements directory reads, {} otherwise. SkillsClient.supported and .directoryRead reflect the declaration." }, 'skills:client:requires-declaration': { source: `${SKILLS_SPEC}L67`, @@ -3415,7 +3415,7 @@ export const REQUIREMENTS: Record = { 'typescript:skills:resources:app-resource-first': { source: 'sdk', behavior: - 'A resource the application registers before attaching the extension wins at its URI in resources/read, and is listed ahead of the skills in resources/list.' + 'A resource the application registers on a server serving skills wins at its URI in resources/read, and is listed ahead of the skills in resources/list.' }, 'skills:list:entries': { source: `${SKILLS_SPEC}L139-L231`, @@ -3599,17 +3599,17 @@ export const REQUIREMENTS: Record = { behavior: 'A skills/list that repeats a URI makes SkillsClient.list() throw INVALID_RESULT naming every copy; with skipInvalid it returns the valid entries and lists the refused ones, by index, in invalid.' }, - 'typescript:skills:attach:rules': { + 'typescript:skills:extension:one-per-server': { source: 'sdk', behavior: - 'Attaching the same skills extension to a server twice does nothing; attaching a second extension throws and leaves the server serving the first; attaching to a connected server throws.', + 'A server takes one skills extension: passing two, or the same one twice, in extensions fails construction; a server with one serves its catalog.', transports: ['inMemory'], note: 'local API; the transport plays no part' }, 'typescript:skills:catalog:reload-notifies': { source: 'sdk', behavior: - "SkillCatalog.reload() that adds or removes a skill, or changes a skill's description, reports listChanged: true and sends notifications/resources/list_changed through every connected server it is attached to; a reload that changes only file content reports listChanged: false and sends nothing.", + "SkillCatalog.reload() that adds or removes a skill, or changes a skill's description, reports listChanged: true and sends notifications/resources/list_changed through every connected server it is set up on; a reload that changes only file content reports listChanged: false and sends nothing.", transports: STATEFUL_TRANSPORTS, removedInSpecVersion: '2026-07-28', supersededBy: 'typescript:skills:catalog:reload-listen', diff --git a/test/e2e/scenarios/skills.test.ts b/test/e2e/scenarios/skills.test.ts index 44edaa331f..3de037b766 100644 --- a/test/e2e/scenarios/skills.test.ts +++ b/test/e2e/scenarios/skills.test.ts @@ -1,8 +1,8 @@ /** * Self-contained test bodies for the Skills extension (SEP-2640). * - * Each body builds its own catalog (in memory) or SkillSource, attaches it - * inside the server factory, wires a real Client and asserts through + * Each body builds its own catalog (in memory) or SkillSource, passes it in + * `extensions` inside the server factory, wires a real Client and asserts through * SkillsClient or raw requests. Bodies that send skills/* or * resources/directory/read pass SKILLS_WIRE so the wire sniffer permits them. */ @@ -16,7 +16,7 @@ import { expect, vi } from 'vitest'; import * as z from 'zod/v4'; import type { Wired } from '../helpers/index'; -import { defined, tapWire, wire } from '../helpers/index'; +import { tapWire, wire } from '../helpers/index'; import { verifies } from '../helpers/verifies'; import type { TestArgs } from '../types'; @@ -41,12 +41,8 @@ function catalogOf(files: Record, options?: SkillCa return SkillsExtension.load(memorySkillFiles(files), { onWarning: quiet, ...options }); } -function serving(skills: { attach(mcp: McpServer): void }): () => McpServer { - return () => { - const s = new McpServer({ name: 's', version: '0' }); - skills.attach(s); - return s; - }; +function serving(skills: SkillsExtension): () => McpServer { + return () => new McpServer({ name: 's', version: '0' }, { extensions: [skills] }); } function raw(client: Client, method: string, params?: Record): Promise> { @@ -147,9 +143,8 @@ verifies('typescript:skills:resources:template', async ({ transport }: TestArgs) verifies('typescript:skills:resources:app-resource-first', async ({ transport }: TestArgs) => { const catalog = await catalogOf({ 'git-workflow/SKILL.md': GIT_MD, 'git-workflow/references/REVIEW.md': REVIEW_MD }); const makeServer = () => { - const s = new McpServer({ name: 's', version: '0' }); + const s = new McpServer({ name: 's', version: '0' }, { extensions: [catalog] }); s.registerResource('override', REVIEW, { mimeType: 'text/plain' }, () => ({ contents: [{ uri: REVIEW, text: 'app wins' }] })); - catalog.attach(s); return s; }; const client = newClient(); @@ -895,27 +890,18 @@ verifies('typescript:skills:client:skip-invalid', async ({ transport }: TestArgs // Lifecycle // ─────────────────────────────────────────────────────────────────────────────── -verifies('typescript:skills:attach:rules', async ({ transport }: TestArgs) => { +verifies('typescript:skills:extension:one-per-server', async ({ transport }: TestArgs) => { const a = await catalogOf({ 'alpha/SKILL.md': skillMd('alpha', 'First catalog.') }); const b = await catalogOf({ 'bravo/SKILL.md': skillMd('bravo', 'Second catalog.') }); - let server: McpServer | undefined; - const makeServer = () => { - const s = new McpServer({ name: 's', version: '0' }); - a.attach(s); - a.attach(s); - expect(() => b.attach(s)).toThrow(/another skills extension is already attached/); - server = s; - return s; - }; + const info = { name: 's', version: '0' }; + expect(() => new McpServer(info, { extensions: [a, b] })).toThrow(/listed more than once/); + expect(() => new McpServer(info, { extensions: [a, a] })).toThrow(/listed more than once/); + const client = newClient(); - await using _ = await wire(transport, makeServer, client, SKILLS_WIRE); + await using _ = await wire(transport, serving(a), client, SKILLS_WIRE); const { skills } = await new SkillsClient(client).list(); expect(skills.map(entry => entry.uri)).toEqual(['skill://alpha/SKILL.md']); - - const connected = defined(server, 'server'); - expect(() => a.attach(connected)).not.toThrow(); - expect(() => b.attach(connected)).toThrow('attach skills before connecting the server'); }); verifies('typescript:skills:catalog:reload-notifies', async ({ transport }: TestArgs) => { diff --git a/test/integration/test/__fixtures__/skillsStdioServer.ts b/test/integration/test/__fixtures__/skillsStdioServer.ts index 7a58dd2258..e0296aa278 100644 --- a/test/integration/test/__fixtures__/skillsStdioServer.ts +++ b/test/integration/test/__fixtures__/skillsStdioServer.ts @@ -1,6 +1,6 @@ /** - * A skills server over stdio: `loadSkills()` once, attached - * to every server the dual-era `serveStdio` entry creates. Spawned by + * A skills server over stdio: `loadSkills()` once, passed in + * `extensions` to every server the dual-era `serveStdio` entry creates. Spawned by * `test/skills/skillsEndToEnd.test.ts`. */ import { McpServer } from '@modelcontextprotocol/server'; @@ -11,11 +11,7 @@ const root = process.argv[2]; if (root === undefined) throw new Error('usage: skillsStdioServer.ts '); const skills = await loadSkills(root); -const handle = serveStdio(() => { - const mcp = new McpServer({ name: 'skills-stdio-fixture', version: '1.0.0' }); - skills.attach(mcp); - return mcp; -}); +const handle = serveStdio(() => new McpServer({ name: 'skills-stdio-fixture', version: '1.0.0' }, { extensions: [skills] })); const exit = async () => { await handle.close(); diff --git a/test/integration/test/server/cloudflareWorkers.test.ts b/test/integration/test/server/cloudflareWorkers.test.ts index a30831ff67..4caa12f8af 100644 --- a/test/integration/test/server/cloudflareWorkers.test.ts +++ b/test/integration/test/server/cloudflareWorkers.test.ts @@ -307,7 +307,7 @@ describe('Cloudflare Workers compatibility (no nodejs_compat)', () => { fs.writeFileSync(path.join(tempDir, 'wrangler.jsonc'), JSON.stringify(wranglerConfig, null, 2)); // Write server source. `/skills` also serves a skills catalog, loaded once at module scope - // and attached to each per-request server (the shape the server guide recommends for + // and passed in `extensions` to each per-request server (the shape the server guide recommends for // Workers); `/runtime` reports the globals the skills extension feature-detects. const serverSource = ` import { McpServer, WebStandardStreamableHTTPServerTransport } from '@modelcontextprotocol/server'; @@ -318,7 +318,7 @@ const skills = await SkillsExtension.load(memorySkillFiles({ 'release-notes/SKIL // A stateless transport serves one request: connect a new pair for each. async function connectPair(withSkills) { - const server = new McpServer({ name: "test-server", version: "${SERVER_VERSION_NONCE}" }); + const server = new McpServer({ name: "test-server", version: "${SERVER_VERSION_NONCE}" }, withSkills ? { extensions: [skills] } : {}); server.registerTool("greet", { description: "Greet someone", @@ -326,7 +326,6 @@ async function connectPair(withSkills) { }, async ({ name }) => ({ content: [{ type: "text", text: "Hello, " + name + "!" }] })); - if (withSkills) skills.attach(server); const transport = new WebStandardStreamableHTTPServerTransport(); await server.connect(transport); @@ -398,7 +397,7 @@ export default { await client.close(); }, 30_000); - it('serves a skills catalog attached per request, on a compatibility date without WeakRef', async () => { + it('serves a skills catalog per request, on a compatibility date without WeakRef', async () => { // The premise: `enable_weak_ref` is on by default only from compatibility date 2025-05-05. const response = await fetch(`http://127.0.0.1:${port}/runtime`); expect(await response.json()).toEqual({ weakRef: 'undefined' }); diff --git a/test/integration/test/skills/skillsEndToEnd.test.ts b/test/integration/test/skills/skillsEndToEnd.test.ts index 158bfed00e..635d013f97 100644 --- a/test/integration/test/skills/skillsEndToEnd.test.ts +++ b/test/integration/test/skills/skillsEndToEnd.test.ts @@ -31,9 +31,7 @@ function clientFor(revision: string): Client { async function overHttp(catalog: SkillCatalog, revision: string, onServer: () => void = () => {}): Promise { const handler = createMcpHandler(() => { onServer(); - const mcp = new McpServer({ name: 'skills-http', version: '1.0.0' }); - catalog.attach(mcp); - return mcp; + return new McpServer({ name: 'skills-http', version: '1.0.0' }, { extensions: [catalog] }); }); const client = clientFor(revision); await client.connect( @@ -166,14 +164,9 @@ describe('one catalog behind per-request HTTP servers', () => { it('tells listen streams about a listing change through handler.notify, which reload() alone cannot reach', async () => { const editable = await writeSkillTree(); const own = await loadSkills(editable, { onWarning: () => {} }); - const handler = createMcpHandler( - () => { - const mcp = new McpServer({ name: 'skills-http', version: '1.0.0' }); - own.attach(mcp); - return mcp; - }, - { keepAliveMs: 0 } - ); + const handler = createMcpHandler(() => new McpServer({ name: 'skills-http', version: '1.0.0' }, { extensions: [own] }), { + keepAliveMs: 0 + }); const client = clientFor(MODERN); await client.connect( new StreamableHTTPClientTransport(new URL('http://localhost/mcp'), { diff --git a/test/integration/test/skills/skillsWire.test.ts b/test/integration/test/skills/skillsWire.test.ts index 83aa40bc21..fee7b8168e 100644 --- a/test/integration/test/skills/skillsWire.test.ts +++ b/test/integration/test/skills/skillsWire.test.ts @@ -5,6 +5,7 @@ */ import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; import { SkillsClient } from '@modelcontextprotocol/client/ext/skills'; +import type { ServerExtension } from '@modelcontextprotocol/server'; import { createMcpHandler, McpServer } from '@modelcontextprotocol/server'; import type { SkillCatalog, SkillSource } from '@modelcontextprotocol/server/ext/skills'; import { memorySkillFiles, skillEntry, SkillsExtension } from '@modelcontextprotocol/server/ext/skills'; @@ -46,10 +47,10 @@ afterEach(async () => { await Promise.all(open.splice(0).map(client => client.close())); }); -async function connect(revision: string, build: (mcp: McpServer) => void): Promise { +async function connect(revision: string, extension: ServerExtension, configure: (mcp: McpServer) => void = () => {}): Promise { const handler = createMcpHandler(() => { - const mcp = new McpServer({ name: 'skills-wire', version: '1.0.0' }); - build(mcp); + const mcp = new McpServer({ name: 'skills-wire', version: '1.0.0' }, { extensions: [extension] }); + configure(mcp); return mcp; }); const client = new Client( @@ -83,18 +84,18 @@ describe.each([LEGACY, MODERN])('skills wire behaviour on %s', revision => { revision === MODERN ? { _meta: { [SERVER_INFO]: { name: 'skills-wire', version: '1.0.0' } }, ttlMs: 0, cacheScope: 'private' } : {}; it('declares the extension next to resources', async () => { - const client = await connect(revision, mcp => catalog.attach(mcp)); + const client = await connect(revision, catalog); expect(client.getServerCapabilities()).toMatchObject({ resources: {}, extensions: { [EXTENSION]: { directoryRead: true } } }); }); it('publishes one resource template', async () => { - const client = await connect(revision, mcp => catalog.attach(mcp)); + const client = await connect(revision, catalog); const { resourceTemplates } = await raw(client, 'resources/templates/list'); expect(resourceTemplates).toEqual([{ name: EXTENSION, uriTemplate: 'skill://{+path}' }]); }); it('lists each SKILL.md in resources/list', async () => { - const client = await connect(revision, mcp => catalog.attach(mcp)); + const client = await connect(revision, catalog); const { resources } = await raw(client, 'resources/list'); expect(resources).toEqual([ { @@ -108,13 +109,13 @@ describe.each([LEGACY, MODERN])('skills wire behaviour on %s', revision => { }); it('serves files through resources/read', async () => { - const client = await connect(revision, mcp => catalog.attach(mcp)); + const client = await connect(revision, catalog); const { contents } = await raw(client, 'resources/read', { uri: REVIEW }); expect(contents).toEqual([{ uri: REVIEW, mimeType: 'text/markdown', text: REVIEW_MD }]); }); it('answers skills/list, skills/get and resources/directory/read', async () => { - const client = await connect(revision, mcp => catalog.attach(mcp)); + const client = await connect(revision, catalog); expect(await raw(client, 'skills/list')).toEqual({ skills: catalog.entries, ...envelope }); expect(await raw(client, 'skills/get', { uri: GIT })).toEqual({ skill: catalog.entries[0], ...envelope }); expect(await raw(client, 'resources/directory/read', { uri: 'skill://git-workflow' })).toEqual({ @@ -127,7 +128,7 @@ describe.each([LEGACY, MODERN])('skills wire behaviour on %s', revision => { }); it('uses the agreed error codes', async () => { - const client = await connect(revision, mcp => catalog.attach(mcp)); + const client = await connect(revision, catalog); await expect(raw(client, 'resources/read', { uri: 'skill://nope/x.md' })).rejects.toMatchObject({ code: -32_602 }); await expect(raw(client, 'resources/read', { uri: 'skill://git-workflow' })).rejects.toMatchObject({ code: -32_602, @@ -137,10 +138,9 @@ describe.each([LEGACY, MODERN])('skills wire behaviour on %s', revision => { await expect(raw(client, 'skills/list', { cursor: 'garbage' })).rejects.toMatchObject({ code: -32_602 }); }); - it('lets an app resource registered first win at its URI (documented: do not do this)', async () => { - const client = await connect(revision, mcp => { + it('lets an app resource at a skill URI win over the skill template (do not do this)', async () => { + const client = await connect(revision, catalog, mcp => { mcp.registerResource('override', REVIEW, { mimeType: 'text/plain' }, () => ({ contents: [{ uri: REVIEW, text: 'app wins' }] })); - catalog.attach(mcp); }); const { contents } = await raw(client, 'resources/read', { uri: REVIEW }); expect(contents).toEqual([{ uri: REVIEW, text: 'app wins' }]); @@ -150,7 +150,7 @@ describe.each([LEGACY, MODERN])('skills wire behaviour on %s', revision => { it('lists nothing in resources/list for a source without listResources, and no directoryRead', async () => { const source = new SkillsExtension({ source: await notesSource(false) }); - const client = await connect(revision, mcp => source.attach(mcp)); + const client = await connect(revision, source); expect(client.getServerCapabilities()?.extensions).toEqual({ [EXTENSION]: {} }); const { resources } = await raw(client, 'resources/list'); expect(resources).toEqual([]); @@ -163,7 +163,7 @@ describe.each([LEGACY, MODERN])('skills wire behaviour on %s', revision => { it('lists what listResources returns, and the host verifies source-built skills', async () => { const source = new SkillsExtension({ source: await notesSource(true) }); - const client = await connect(revision, mcp => source.attach(mcp)); + const client = await connect(revision, source); const { resources } = await raw(client, 'resources/list'); expect(resources).toEqual([{ uri: 'skill://notes/SKILL.md', name: 'notes', mimeType: 'text/markdown' }]); const skills = new SkillsClient(client); @@ -183,7 +183,7 @@ describe.each([LEGACY, MODERN])('skills wire behaviour on %s', revision => { }, baseUri: 'github://owner/repo/skills' }); - const client = await connect(revision, mcp => source.attach(mcp)); + const client = await connect(revision, source); const { resourceTemplates } = await raw(client, 'resources/templates/list'); expect(resourceTemplates).toEqual([{ name: EXTENSION, uriTemplate: 'github://owner/repo/skills/{+path}' }]); expect(await new SkillsClient(client).read(notes)).toMatchObject({ From 8bbb9e1fbfba6e20433089b840f6c3e4d21e2a49 Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Tue, 27 Oct 2026 12:36:27 +0000 Subject: [PATCH 2/4] test(server): keep the Skills cacheHint construction check covered The move from attach to setup dropped the only test of the constructor's cacheHint validation, which is unchanged. --- packages/server/test/ext/skills/extension.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/server/test/ext/skills/extension.test.ts b/packages/server/test/ext/skills/extension.test.ts index 1e126cb57f..bad5645afd 100644 --- a/packages/server/test/ext/skills/extension.test.ts +++ b/packages/server/test/ext/skills/extension.test.ts @@ -279,6 +279,10 @@ describe('setup', () => { expect(calls).toBe(0); expect('afterSetup' in SkillsExtension.prototype).toBe(false); }); + + it('rejects an invalid cacheHint at construction', () => { + expect(() => new SkillsExtension({ source: sourceOf(), cacheHint: { ttlMs: -1 } })).toThrow(); + }); }); describe('skillEntry', () => { From 46237e47b8deb829b194874a141ce181dd33accb Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Tue, 27 Oct 2026 13:38:41 +0000 Subject: [PATCH 3/4] feat(server): keep SkillsExtension.attach as a deprecated path for one minor attach shipped in 2.4.0. It now sets the extension up through the same setup as the extensions option and keeps its rules: before connecting, idempotent per server, one skills extension per server, and a refused or failed attach leaves no capability behind. Restores its e2e requirement, and the agent skill falls back to attach on releases without the extensions option. --- .changeset/skills-as-extension.md | 2 +- packages/server/src/ext/skills/extension.ts | 30 ++++++++++++++++ packages/server/src/server/extension.ts | 31 ++++++++++++++++ .../server/test/ext/skills/extension.test.ts | 35 ++++++++++++++++++- skills/mcp-typescript-sdk-skills/SKILL.md | 8 +++++ test/e2e/requirements.ts | 7 ++++ test/e2e/scenarios/skills.test.ts | 25 ++++++++++++- 7 files changed, 135 insertions(+), 3 deletions(-) diff --git a/.changeset/skills-as-extension.md b/.changeset/skills-as-extension.md index 6eec5511fc..53e7356b79 100644 --- a/.changeset/skills-as-extension.md +++ b/.changeset/skills-as-extension.md @@ -2,4 +2,4 @@ '@modelcontextprotocol/server': minor --- -`SkillsExtension` is now a server extension: serve it with `new McpServer(info, { extensions: [skills] })`. This removes the experimental `SkillsExtension.attach(server)`; replace `skills.attach(server)` with the `extensions` option. Passing two skills extensions to one server now fails construction (a duplicate extension id) instead of throwing from the second `attach`. +`SkillsExtension` is now a server extension: serve it with `new McpServer(info, { extensions: [skills] })`. `SkillsExtension.attach(server)` still works and is deprecated; it will be removed in a later minor release. Passing two skills extensions to one server in `extensions` fails construction (a duplicate extension id). diff --git a/packages/server/src/ext/skills/extension.ts b/packages/server/src/ext/skills/extension.ts index 5cd087cfa6..d7a8a3fc02 100644 --- a/packages/server/src/ext/skills/extension.ts +++ b/packages/server/src/ext/skills/extension.ts @@ -26,6 +26,7 @@ import { } from '@modelcontextprotocol/core-internal/ext/skills'; import type { ServerExtension, ServerExtensionContext } from '../../server/extension'; +import { attachServerExtension } from '../../server/extension'; import type { McpServer } from '../../server/mcp'; import { ResourceTemplate } from '../../server/mcp'; import type { Server } from '../../server/server'; @@ -59,6 +60,10 @@ function toContents(uri: string, result: SkillReadResult): ReadResourceResult['c return 'text' in result ? { uri, ...mimeType, text: result.text } : { uri, ...mimeType, blob: encodeBase64(result.bytes) }; } +const ONE_PER_SERVER = + 'another skills extension is already attached to this server; attach one per server ' + + '(to serve several folders or sources, combine them behind one SkillFiles adapter or SkillSource)'; + // Runs after a successful `setup`, by extension (`SkillCatalog` tracks servers); off the class so no subclass hooks it. const afterSetup = new WeakMap void>(); @@ -79,6 +84,7 @@ export class SkillsExtension implements ServerExtension { readonly #cache: { ttlMs: number; cacheScope: 'public' | 'private' }; readonly #strict: boolean; readonly #warn: (warning: SkillWarning) => void; + readonly #attached = new WeakSet(); /** * Builds a catalog from storage through a {@linkcode SkillFiles} adapter @@ -139,6 +145,30 @@ export class SkillsExtension implements ServerExtension { afterSetup.get(this)?.(mcp); } + /** + * Serves these skills on `mcp`, which is already constructed. Must run + * before connecting. Calling it again for the same server does nothing. + * One skills extension per server: attaching another one throws and + * leaves the server as it was, as does attaching to a server that + * already handles one of the skills methods. + * + * @deprecated Pass the extension in `extensions` instead: + * `new McpServer(info, { extensions: [skills] })`. `attach` will be + * removed in a later minor release. + */ + attach(mcp: McpServer): void { + if (this.#attached.has(mcp)) return; + if (mcp.isConnected()) throw new Error('attach skills before connecting the server'); + const server = mcp.server; + if (server.getCapabilities().extensions?.[SKILLS_EXTENSION_ID] !== undefined) throw new Error(ONE_PER_SERVER); + // Check before changing anything, so a refused attach leaves the server as it was. + server.assertCanSetRequestHandler(SKILLS_LIST_METHOD); + server.assertCanSetRequestHandler(SKILLS_GET_METHOD); + if (this.#source.readDirectory !== undefined) server.assertCanSetRequestHandler(RESOURCES_DIRECTORY_READ_METHOD); + attachServerExtension(mcp, this); + this.#attached.add(mcp); + } + #cacheFields(server: Server): { ttlMs?: number; cacheScope?: 'public' | 'private' } { return isModernProtocolVersion(server.getNegotiatedProtocolVersion() ?? '') ? { ...this.#cache } : {}; } diff --git a/packages/server/src/server/extension.ts b/packages/server/src/server/extension.ts index b06405a86b..0931feaf5b 100644 --- a/packages/server/src/server/extension.ts +++ b/packages/server/src/server/extension.ts @@ -126,3 +126,34 @@ function contextFor(mcp: McpServer, id: string, installed: ReadonlySet): } }; } + +/** + * Sets one extension up on a server that is already constructed, for the + * deprecated `SkillsExtension.attach`. The capability entry is written only + * once `setup` has returned, so a failed setup leaves none. Middleware goes in + * the application tier. + * + * Package-internal. + */ +export function attachServerExtension(mcp: McpServer, extension: ServerExtension): void { + const server = mcp.server; + const installed = new Set([...Object.keys(server.getCapabilities().extensions ?? {}), extension.id]); + let settings: JSONObject = {}; + const result = extension.setup({ + mcp, + capability(next) { + settings = next; + }, + require(extensionId) { + requireExtension('server', installed, extension.id, extensionId); + }, + use(method, middleware) { + server.use(method, middleware); + }, + setRequestHandler(name, schemas, handler) { + setExtensionRequestHandler(server, extension.id, name, schemas, handler); + } + }); + if (result !== undefined) throw new TypeError(`Extension "${extension.id}" has an async setup; pass it in extensions instead`); + server.registerCapabilities({ extensions: { [extension.id]: settings } }); +} diff --git a/packages/server/test/ext/skills/extension.test.ts b/packages/server/test/ext/skills/extension.test.ts index bad5645afd..df1f14100c 100644 --- a/packages/server/test/ext/skills/extension.test.ts +++ b/packages/server/test/ext/skills/extension.test.ts @@ -2,6 +2,7 @@ import { createHash } from 'node:crypto'; import type { Skill } from '@modelcontextprotocol/core/ext/skills'; import { ListSkillsRequestParamsSchema, ReadResourceDirectoryRequestParamsSchema } from '@modelcontextprotocol/core/ext/skills'; +import { InMemoryTransport } from '@modelcontextprotocol/core-internal'; import { describe, expect, it } from 'vitest'; import type { SkillSource, SkillWarning } from '../../../src/ext/skills/index'; @@ -256,7 +257,7 @@ describe('setup', () => { const takesName: ServerExtension = { id: 'com.example/app', setup: ctx => - ctx.mcp.registerResource( + void ctx.mcp.registerResource( 'io.modelcontextprotocol/skills', new ResourceTemplate('app://{+path}', { list: undefined }), {}, @@ -280,6 +281,38 @@ describe('setup', () => { expect('afterSetup' in SkillsExtension.prototype).toBe(false); }); + it('still serves through the deprecated attach, with its rules', async () => { + const mcp = newServer(); + const skills = new SkillsExtension({ source: sourceOf() }); + skills.attach(mcp); + skills.attach(mcp); + expect(mcp.server.getCapabilities().extensions).toEqual({ 'io.modelcontextprotocol/skills': {} }); + expect(await resultOf(mcp, 'skills/list')).toMatchObject({ skills: [gitWorkflow] }); + expect(() => new SkillsExtension({ source: sourceOf() }).attach(mcp)).toThrow(/another skills extension is already attached/); + expect(() => new SkillsExtension({ source: sourceOf() }).attach(newServer(new SkillsExtension({ source: sourceOf() })))).toThrow( + /another skills extension is already attached/ + ); + const connected = newServer(); + const [, serverSide] = InMemoryTransport.createLinkedPair(); + await connected.connect(serverSide); + expect(() => new SkillsExtension({ source: sourceOf() }).attach(connected)).toThrow(/before connecting/); + await connected.close(); + }); + + it('leaves no capability when attach fails on a taken template name', () => { + const mcp = newServer(); + mcp.registerResource( + 'io.modelcontextprotocol/skills', + new ResourceTemplate('app://{+path}', { list: undefined }), + {}, + async uri => ({ + contents: [{ uri: uri.href, text: 'app' }] + }) + ); + expect(() => new SkillsExtension({ source: sourceOf() }).attach(mcp)).toThrow(/already registered/); + expect(mcp.server.getCapabilities().extensions?.['io.modelcontextprotocol/skills']).toBeUndefined(); + }); + it('rejects an invalid cacheHint at construction', () => { expect(() => new SkillsExtension({ source: sourceOf(), cacheHint: { ttlMs: -1 } })).toThrow(); }); diff --git a/skills/mcp-typescript-sdk-skills/SKILL.md b/skills/mcp-typescript-sdk-skills/SKILL.md index 4f75559e68..9e67462332 100644 --- a/skills/mcp-typescript-sdk-skills/SKILL.md +++ b/skills/mcp-typescript-sdk-skills/SKILL.md @@ -35,6 +35,14 @@ npm view @modelcontextprotocol/server@latest exports --json | grep -c '"./ext/sk - **The latest release has it:** upgrade (`server` and `client` to the same version, if the project uses both) and continue. - **It does not:** stop here. Tell the user that Skills over MCP is not in a released version of the SDK yet, and that this works once a release exports `@modelcontextprotocol/server/ext/skills`. Do not write `skills/list` handlers or `skill://` resources by hand as a workaround. Clients verify every file against the digests and frontmatter the extension publishes, and the extension also declares the capability, pages the catalog and handles both protocol versions, so a hand-written copy is easy to get wrong in ways those clients refuse, and drifts from the spec. +**Check how the installed release takes extensions.** Servers take the skills extension in the `extensions` option. Releases that export `./ext/skills` but predate that option attach it instead: + +```bash +grep -c 'ServerExtension' node_modules/@modelcontextprotocol/server/dist/index.d.mts +``` + +If it prints `0`, upgrade if the latest release has the option. Otherwise write `skills.attach(server)` after constructing the server and before it connects, wherever the steps below pass `{ extensions: [skills] }`. `attach` is deprecated in later releases. + ## Serve skills **Pick where the skills come from.** Each source gives you an extension that you pass in `extensions` as shown below; only the first three have `reload()`: diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index bbe70f59d5..c28e8d67fe 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -3599,6 +3599,13 @@ export const REQUIREMENTS: Record = { behavior: 'A skills/list that repeats a URI makes SkillsClient.list() throw INVALID_RESULT naming every copy; with skipInvalid it returns the valid entries and lists the refused ones, by index, in invalid.' }, + 'typescript:skills:attach:rules': { + source: 'sdk', + behavior: + 'The deprecated SkillsExtension.attach: attaching the same skills extension to a server twice does nothing; attaching a second extension throws and leaves the server serving the first; attaching to a connected server throws.', + transports: ['inMemory'], + note: 'local API; the transport plays no part' + }, 'typescript:skills:extension:one-per-server': { source: 'sdk', behavior: diff --git a/test/e2e/scenarios/skills.test.ts b/test/e2e/scenarios/skills.test.ts index 3de037b766..3f09827c9d 100644 --- a/test/e2e/scenarios/skills.test.ts +++ b/test/e2e/scenarios/skills.test.ts @@ -16,7 +16,7 @@ import { expect, vi } from 'vitest'; import * as z from 'zod/v4'; import type { Wired } from '../helpers/index'; -import { tapWire, wire } from '../helpers/index'; +import { defined, tapWire, wire } from '../helpers/index'; import { verifies } from '../helpers/verifies'; import type { TestArgs } from '../types'; @@ -890,6 +890,29 @@ verifies('typescript:skills:client:skip-invalid', async ({ transport }: TestArgs // Lifecycle // ─────────────────────────────────────────────────────────────────────────────── +verifies('typescript:skills:attach:rules', async ({ transport }: TestArgs) => { + const a = await catalogOf({ 'alpha/SKILL.md': skillMd('alpha', 'First catalog.') }); + const b = await catalogOf({ 'bravo/SKILL.md': skillMd('bravo', 'Second catalog.') }); + let server: McpServer | undefined; + const makeServer = () => { + const s = new McpServer({ name: 's', version: '0' }); + a.attach(s); + a.attach(s); + expect(() => b.attach(s)).toThrow(/another skills extension is already attached/); + server = s; + return s; + }; + const client = newClient(); + await using _ = await wire(transport, makeServer, client, SKILLS_WIRE); + + const { skills } = await new SkillsClient(client).list(); + expect(skills.map(entry => entry.uri)).toEqual(['skill://alpha/SKILL.md']); + + const connected = defined(server, 'server'); + expect(() => a.attach(connected)).not.toThrow(); + expect(() => b.attach(connected)).toThrow('attach skills before connecting the server'); +}); + verifies('typescript:skills:extension:one-per-server', async ({ transport }: TestArgs) => { const a = await catalogOf({ 'alpha/SKILL.md': skillMd('alpha', 'First catalog.') }); const b = await catalogOf({ 'bravo/SKILL.md': skillMd('bravo', 'Second catalog.') }); From 7ff034c59ffd863a1269762cf742c1c1f77a9d01 Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Tue, 27 Oct 2026 13:59:03 +0000 Subject: [PATCH 4/4] docs(skills): precise agent-skill gate, fixed-URI precedence, template order in the changeset - The agent skill tells 2.4.0 projects to use attach, and says it keeps working after an upgrade; the factory note no longer says attach. - The app-resource-first requirement holds for fixed-URI resources only: the skills template is registered at construction, before any template the application adds. The changeset now says so. - Drop the JSDoc on a test helper. --- .changeset/skills-as-extension.md | 2 +- packages/server/test/ext/skills/harness.ts | 1 - skills/mcp-typescript-sdk-skills/SKILL.md | 4 ++-- test/e2e/requirements.ts | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.changeset/skills-as-extension.md b/.changeset/skills-as-extension.md index 53e7356b79..dc8f571dbb 100644 --- a/.changeset/skills-as-extension.md +++ b/.changeset/skills-as-extension.md @@ -2,4 +2,4 @@ '@modelcontextprotocol/server': minor --- -`SkillsExtension` is now a server extension: serve it with `new McpServer(info, { extensions: [skills] })`. `SkillsExtension.attach(server)` still works and is deprecated; it will be removed in a later minor release. Passing two skills extensions to one server in `extensions` fails construction (a duplicate extension id). +`SkillsExtension` is now a server extension: serve it with `new McpServer(info, { extensions: [skills] })`. `SkillsExtension.attach(server)` still works and is deprecated; it will be removed in a later minor release. Passing two skills extensions to one server in `extensions` fails construction (a duplicate extension id). With `extensions`, the skills resource template is registered at construction, so a `ResourceTemplate` the application registers for `skill://` URIs comes after it; a fixed-URI resource still wins at its URI. diff --git a/packages/server/test/ext/skills/harness.ts b/packages/server/test/ext/skills/harness.ts index 4c46aa52a5..96c0a62eb4 100644 --- a/packages/server/test/ext/skills/harness.ts +++ b/packages/server/test/ext/skills/harness.ts @@ -20,7 +20,6 @@ export interface Reply { let nextId = 1; -/** A server with `extensions` set up on it, as `new McpServer(info, { extensions })` does. */ export function newServer(...extensions: ServerExtension[]): McpServer { return new McpServer({ name: 'skills-test', version: '1.0.0' }, { extensions }); } diff --git a/skills/mcp-typescript-sdk-skills/SKILL.md b/skills/mcp-typescript-sdk-skills/SKILL.md index 9e67462332..a747bbde07 100644 --- a/skills/mcp-typescript-sdk-skills/SKILL.md +++ b/skills/mcp-typescript-sdk-skills/SKILL.md @@ -41,7 +41,7 @@ npm view @modelcontextprotocol/server@latest exports --json | grep -c '"./ext/sk grep -c 'ServerExtension' node_modules/@modelcontextprotocol/server/dist/index.d.mts ``` -If it prints `0`, upgrade if the latest release has the option. Otherwise write `skills.attach(server)` after constructing the server and before it connects, wherever the steps below pass `{ extensions: [skills] }`. `attach` is deprecated in later releases. +If it prints `0`, the installed release predates the option: write `skills.attach(server)` after constructing the server and before it connects, wherever the steps below pass `{ extensions: [skills] }`. A later release with the option also keeps `attach`, deprecated, so this code keeps working after an upgrade; move it to `extensions` then. ## Serve skills @@ -118,7 +118,7 @@ Serving: - **Writing the protocol by hand.** Serve skills only through `SkillsExtension` (`loadSkills`, `memorySkillFiles`, a `SkillFiles` adapter or a `SkillSource`). Never `setRequestHandler('skills/list', …)`, and never register your own resources under `skill://`: a resource at a skill file's URI takes precedence over the file, and clients then fail to verify it. - **Missing the skipped skills.** `loadSkills` skips a skill whose `name` differs from its folder, or that contains a symlink, and only warns (on stderr by default) while the others serve. Read the warnings after loading and tell the user which skills were skipped. Pass `strict: true` (in CI, say) to make the load throw instead. - **Dot-named entries.** Files and folders whose names start with `.` are skipped with no warning at all, and `strict: true` does not catch them. A skill in a `.name` folder, or a dotfile its `SKILL.md` refers to, is simply missing: rename it. -- **A catalog outside the factory's servers.** `serveStdio` and `createMcpHandler` build their servers from a factory, and each server `createMcpHandler` builds lives for one request. Attach inside the factory, and behind `createMcpHandler` call `handler.notify.resourcesChanged()` when `reload()` reports `listChanged`, or connected clients never hear of the change. +- **A catalog outside the factory's servers.** `serveStdio` and `createMcpHandler` build their servers from a factory, and each server `createMcpHandler` builds lives for one request. Pass the catalog in `extensions` inside the factory, and behind `createMcpHandler` call `handler.notify.resourcesChanged()` when `reload()` reports `listChanged`, or connected clients never hear of the change. - **Expecting edits to show up on their own.** The catalog keeps the digests from load time, so a file edited on disk afterwards is refused with an error until `reload()` runs. - **`loadSkills` without a file system.** It reads the disk (`ext/skills/node`). On Workers and similar runtimes, use `memorySkillFiles` or a `SkillFiles` adapter from `@modelcontextprotocol/server/ext/skills`. diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index c28e8d67fe..4f807d2ae6 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -3415,7 +3415,7 @@ export const REQUIREMENTS: Record = { 'typescript:skills:resources:app-resource-first': { source: 'sdk', behavior: - 'A resource the application registers on a server serving skills wins at its URI in resources/read, and is listed ahead of the skills in resources/list.' + 'A fixed-URI resource the application registers on a server serving skills wins at its URI in resources/read, and is listed ahead of the skills in resources/list.' }, 'skills:list:entries': { source: `${SKILLS_SPEC}L139-L231`,