From a27067952a8e7a408901f5dfc015d9ed007a9f54 Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Thu, 22 Oct 2026 12:17:13 +0100 Subject: [PATCH] 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-extension.md | 2 +- docs/servers/skills.md | 17 +- 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 | 81 ++++----- 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 +- test/conformance/src/everythingServer.ts | 7 +- .../test/__fixtures__/skillsStdioServer.ts | 10 +- .../test/server/cloudflareWorkers.test.ts | 7 +- .../test/skills/skillsEndToEnd.test.ts | 15 +- .../test/skills/skillsWire.test.ts | 30 ++-- 16 files changed, 168 insertions(+), 229 deletions(-) diff --git a/.changeset/skills-extension.md b/.changeset/skills-extension.md index 71fd138eba..d475139195 100644 --- a/.changeset/skills-extension.md +++ b/.changeset/skills-extension.md @@ -10,7 +10,7 @@ author: @mattzcarey Add the Skills extension (SEP-2640), reachable through new `ext/skills` subpaths; package root exports are unchanged. -- `@modelcontextprotocol/server/ext/skills`: `SkillsExtension` (`attach(server)` serves `skills/list`, `skills/get`, `resources/directory/read` and a `skill://{+path}` resource template), `SkillsExtension.load(files)` returning a `SkillCatalog` with `entries` and `reload()`, the `SkillFiles` storage adapter with `memorySkillFiles`, `SkillSource` for generated catalogs, `skillEntry`, and `SkillCatalogError`. Invalid skills are skipped with a warning (`onWarning`), or fail under `strict`. A file whose bytes are valid UTF-8 is served as `text`, whatever its MIME type; any other file as a base64 `blob`. +- `@modelcontextprotocol/server/ext/skills`: `SkillsExtension`, a server extension (`new McpServer(info, { extensions: [skills] })` serves `skills/list`, `skills/get`, `resources/directory/read` and a `skill://{+path}` resource template; the `extensions` option is experimental), `SkillsExtension.load(files)` returning a `SkillCatalog` with `entries` and `reload()`, the `SkillFiles` storage adapter with `memorySkillFiles`, `SkillSource` for generated catalogs, `skillEntry`, and `SkillCatalogError`. Invalid skills are skipped with a warning (`onWarning`), or fail under `strict`. A file whose bytes are valid UTF-8 is served as `text`, whatever its MIME type; any other file as a base64 `blob`. - `@modelcontextprotocol/server/ext/skills/node`: `loadSkills(folder)` and `nodeSkillFiles(folder)`. - `@modelcontextprotocol/client/ext/skills`: `SkillsClient` with `list()` (throws on an invalid entry; `skipInvalid: true` keeps the valid ones and returns the rest in `invalid`), `get()`, `read(skill, path?)` (size, digest and `SKILL.md` frontmatter verified, `SkillVerificationError` on a mismatch; a copy from the response cache that fails a check is fetched once more with `cacheMode: 'refresh'` first) and `readDirectory()`. - `@modelcontextprotocol/core/ext/skills`: schemas, types and constants, including `SKILL_LIMITS`. The result schemas follow the stable spec, so `ttlMs` and `cacheScope` are required. diff --git a/docs/servers/skills.md b/docs/servers/skills.md index b3388d42c9..05087ac1ba 100644 --- a/docs/servers/skills.md +++ b/docs/servers/skills.md @@ -4,7 +4,7 @@ shape: how-to # Skills -`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. The `extensions` option and the extension shape are [experimental](../advanced/extensions.md). ```ts source="../../examples/guides/servers/skills.examples.ts#loadSkills_basic" import { McpServer } from '@modelcontextprotocol/server'; @@ -12,8 +12,7 @@ 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] }); ``` 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/examples/guides/clients/skills.examples.ts b/examples/guides/clients/skills.examples.ts index d4412df94e..58731c5418 100644 --- a/examples/guides/clients/skills.examples.ts +++ b/examples/guides/clients/skills.examples.ts @@ -37,8 +37,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 ecef49c604..737443ccfd 100644 --- a/examples/guides/servers/skills.examples.ts +++ b/examples/guides/servers/skills.examples.ts @@ -22,23 +22,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[] = []; @@ -143,6 +138,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 123607f045..b4fbaae18b 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,45 +104,41 @@ 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: the extension shape may change in a minor release. + * + * @experimental */ - 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' } { @@ -234,7 +225,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 } { @@ -278,7 +269,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 { @@ -316,12 +307,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/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/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({