Merge pull request #2374 from modelcontextprotocol/v2/fix/2373-skills-client-extension
Declare the skills extension in the Inspector's client capabilities
This commit is contained in:
commit
6b97708226
18 changed files with 578 additions and 174 deletions
|
|
@ -7,12 +7,21 @@ import {
|
|||
buildClientExtensions,
|
||||
} from "@inspector/core/mcp/extensions.js";
|
||||
import { TASKS_EXTENSION_KEY } from "@inspector/core/mcp/modernTaskSchemas.js";
|
||||
import { SKILLS_EXTENSION_KEY } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
|
||||
// The `ui` extension carries a non-empty advertisement value; the others are
|
||||
// declared with `{}`. Spelled out here so the map assertions stay readable.
|
||||
const UI_ADVERTISEMENT = { mimeTypes: [MCP_APP_MIME_TYPE] };
|
||||
|
||||
describe("extensions (#1738, #1740)", () => {
|
||||
// Every registry entry switched off, so a test can re-enable exactly the ones
|
||||
// it is about without restating the whole registry.
|
||||
const ALL_REGISTRY_OFF = {
|
||||
[TASKS_EXTENSION_KEY]: false,
|
||||
[UI_EXTENSION_KEY]: false,
|
||||
[SKILLS_EXTENSION_KEY]: false,
|
||||
};
|
||||
|
||||
describe("extensions (#1738, #1740, #2373)", () => {
|
||||
describe("ADVERTISABLE_EXTENSIONS registry", () => {
|
||||
it("lists the Tasks extension, advertised by default", () => {
|
||||
const tasks = ADVERTISABLE_EXTENSIONS.find(
|
||||
|
|
@ -37,6 +46,19 @@ describe("extensions (#1738, #1740)", () => {
|
|||
expect(typeof MCP_APP_MIME_TYPE).toBe("string");
|
||||
});
|
||||
|
||||
it("lists the Skills extension, advertised by default with no settings (#2373)", () => {
|
||||
// SEP-2133 negotiates an extension from both sides, so a server may
|
||||
// refuse `skills/*` to a client that did not declare it. The Inspector
|
||||
// calls those methods, so it must declare the extension by default.
|
||||
const skills = ADVERTISABLE_EXTENSIONS.find(
|
||||
(e) => e.key === SKILLS_EXTENSION_KEY,
|
||||
);
|
||||
expect(skills).toBeDefined();
|
||||
expect(skills?.defaultAdvertised).toBe(true);
|
||||
expect(skills?.advertisement).toBeUndefined();
|
||||
expect(skills?.label).toContain("Skills");
|
||||
});
|
||||
|
||||
it("does not list EMA (it follows the auth mode, not a toggle)", () => {
|
||||
expect(
|
||||
ADVERTISABLE_EXTENSIONS.some((e) => e.key === EMA_EXTENSION_KEY),
|
||||
|
|
@ -53,11 +75,12 @@ describe("extensions (#1738, #1740)", () => {
|
|||
});
|
||||
|
||||
describe("buildClientExtensions", () => {
|
||||
it("advertises registry defaults with no overrides (tasks + ui)", () => {
|
||||
it("advertises registry defaults with no overrides (tasks + ui + skills)", () => {
|
||||
const map = buildClientExtensions({ enterpriseManaged: false });
|
||||
expect(map).toEqual({
|
||||
[TASKS_EXTENSION_KEY]: {},
|
||||
[UI_EXTENSION_KEY]: UI_ADVERTISEMENT,
|
||||
[SKILLS_EXTENSION_KEY]: {},
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -82,6 +105,7 @@ describe("extensions (#1738, #1740)", () => {
|
|||
expect(map).toEqual({
|
||||
[TASKS_EXTENSION_KEY]: {},
|
||||
[UI_EXTENSION_KEY]: UI_ADVERTISEMENT,
|
||||
[SKILLS_EXTENSION_KEY]: {},
|
||||
[EMA_EXTENSION_KEY]: {},
|
||||
});
|
||||
});
|
||||
|
|
@ -94,23 +118,39 @@ describe("extensions (#1738, #1740)", () => {
|
|||
it("honors a user override that disables a default-on extension", () => {
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: false,
|
||||
advertised: { [TASKS_EXTENSION_KEY]: false, [UI_EXTENSION_KEY]: false },
|
||||
advertised: ALL_REGISTRY_OFF,
|
||||
});
|
||||
expect(map).toEqual({});
|
||||
});
|
||||
|
||||
it("can disable just the UI extension, keeping Tasks (#1740)", () => {
|
||||
it("can disable just the UI extension, keeping the others (#1740)", () => {
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: false,
|
||||
advertised: { [UI_EXTENSION_KEY]: false },
|
||||
});
|
||||
expect(map).toEqual({ [TASKS_EXTENSION_KEY]: {} });
|
||||
expect(map).toEqual({
|
||||
[TASKS_EXTENSION_KEY]: {},
|
||||
[SKILLS_EXTENSION_KEY]: {},
|
||||
});
|
||||
});
|
||||
|
||||
it("can disable just the Skills extension, keeping the others (#2373)", () => {
|
||||
// The Server Settings toggle for skills is how a user checks that a
|
||||
// server refuses `skills/*` to a client that did not declare it.
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: false,
|
||||
advertised: { [SKILLS_EXTENSION_KEY]: false },
|
||||
});
|
||||
expect(map).toEqual({
|
||||
[TASKS_EXTENSION_KEY]: {},
|
||||
[UI_EXTENSION_KEY]: UI_ADVERTISEMENT,
|
||||
});
|
||||
});
|
||||
|
||||
it("honors a user override that keeps a default-on extension enabled", () => {
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: false,
|
||||
advertised: { [TASKS_EXTENSION_KEY]: true, [UI_EXTENSION_KEY]: false },
|
||||
advertised: { ...ALL_REGISTRY_OFF, [TASKS_EXTENSION_KEY]: true },
|
||||
});
|
||||
expect(map).toEqual({ [TASKS_EXTENSION_KEY]: {} });
|
||||
});
|
||||
|
|
@ -134,13 +174,14 @@ describe("extensions (#1738, #1740)", () => {
|
|||
expect(map).toEqual({
|
||||
[TASKS_EXTENSION_KEY]: {},
|
||||
[UI_EXTENSION_KEY]: UI_ADVERTISEMENT,
|
||||
[SKILLS_EXTENSION_KEY]: {},
|
||||
});
|
||||
});
|
||||
|
||||
it("layers EMA on even when all registry entries are disabled", () => {
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: true,
|
||||
advertised: { [TASKS_EXTENSION_KEY]: false, [UI_EXTENSION_KEY]: false },
|
||||
advertised: ALL_REGISTRY_OFF,
|
||||
});
|
||||
expect(map).toEqual({ [EMA_EXTENSION_KEY]: {} });
|
||||
});
|
||||
|
|
@ -163,7 +204,11 @@ describe("extensions (#1738, #1740)", () => {
|
|||
});
|
||||
// A nested setting, NOT a second extension — the contract is explicit
|
||||
// that no new extension id is introduced.
|
||||
expect(Object.keys(map)).toEqual([TASKS_EXTENSION_KEY, UI_EXTENSION_KEY]);
|
||||
expect(Object.keys(map)).toEqual([
|
||||
TASKS_EXTENSION_KEY,
|
||||
UI_EXTENSION_KEY,
|
||||
SKILLS_EXTENSION_KEY,
|
||||
]);
|
||||
});
|
||||
|
||||
it("advertises nothing when the UI extension itself is turned off", () => {
|
||||
|
|
|
|||
|
|
@ -150,8 +150,9 @@ describe("InspectorClient skills methods (#2234)", () => {
|
|||
|
||||
it("requires the modern list envelope on a modern connection", async () => {
|
||||
// SEP-2640: "In protocol versions 2026-07-28 and later, the result also
|
||||
// carries … `ttlMs` and `cacheScope`." Nothing else validates it —
|
||||
// `skills/*` is consumer-owned, so the SDK codec never sees it.
|
||||
// carries … `ttlMs` and `cacheScope`." The SDK codec checks and removes
|
||||
// `resultType` on every modern result, but for a consumer-owned method it
|
||||
// checks neither caching attribute — so this schema is what does.
|
||||
const client = makeClient();
|
||||
internals(client).protocolEra = "modern";
|
||||
stubRequest(client, { skills: [] });
|
||||
|
|
@ -159,10 +160,12 @@ describe("InspectorClient skills methods (#2234)", () => {
|
|||
});
|
||||
|
||||
it("accepts a modern result that carries the envelope", async () => {
|
||||
// No `resultType` in the stub: the SDK codec lifts it off before the
|
||||
// client's schema runs, so this is the shape a real modern page arrives
|
||||
// in (#2373).
|
||||
const client = makeClient();
|
||||
internals(client).protocolEra = "modern";
|
||||
stubRequest(client, {
|
||||
resultType: "complete",
|
||||
ttlMs: 0,
|
||||
cacheScope: "public",
|
||||
skills: [ENTRY],
|
||||
|
|
@ -317,14 +320,18 @@ describe("InspectorClient skills methods (#2234)", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("requires resultType on a modern connection", async () => {
|
||||
it("accepts a modern result as the SDK codec delivers it, without resultType (#2373)", async () => {
|
||||
// The codec checks `resultType` and lifts it off before this schema
|
||||
// runs, so a stub returning the lifted shape is what a real modern
|
||||
// connection hands the client. Requiring it here failed every conforming
|
||||
// server; the live check is in the integration suite.
|
||||
const client = makeClient();
|
||||
internals(client).protocolEra = "modern";
|
||||
stubRequest(client, { resources: [] });
|
||||
stubRequest(client, { resources: [CHILD] });
|
||||
declareSkills(client, true);
|
||||
await expect(
|
||||
client.readResourceDirectory("skill://demo"),
|
||||
).rejects.toBeDefined();
|
||||
).resolves.toMatchObject({ resources: [CHILD] });
|
||||
});
|
||||
|
||||
it("accepts a legacy result without resultType", async () => {
|
||||
|
|
@ -388,21 +395,14 @@ describe("InspectorClient skills methods (#2234)", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("requires resultType on a modern skills/get", async () => {
|
||||
// Base-protocol (SEP-2322) and present in SEP-2640's own example, unlike
|
||||
// the caching attributes the SEP leaves open.
|
||||
it("accepts a modern skills/get as the SDK codec delivers it, without resultType (#2373)", async () => {
|
||||
// `resultType` is base-protocol (SEP-2322), so the codec enforces it and
|
||||
// lifts it off before this schema runs — requiring it here rejected every
|
||||
// conforming modern server. The caching attributes SEP-2640 leaves open
|
||||
// stay optional too.
|
||||
const client = makeClient();
|
||||
internals(client).protocolEra = "modern";
|
||||
stubRequest(client, { skill: ENTRY });
|
||||
await expect(
|
||||
client.getSkill("skill://demo/SKILL.md"),
|
||||
).rejects.toBeDefined();
|
||||
});
|
||||
|
||||
it("accepts a modern skills/get without the caching attributes", async () => {
|
||||
const client = makeClient();
|
||||
internals(client).protocolEra = "modern";
|
||||
stubRequest(client, { skill: ENTRY, resultType: "complete" });
|
||||
await expect(client.getSkill("skill://demo/SKILL.md")).resolves.toEqual(
|
||||
ENTRY,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -2,12 +2,10 @@ import { describe, it, expect } from "vitest";
|
|||
import {
|
||||
DIRECTORY_MIME_TYPE,
|
||||
DirectoryReadResultSchema,
|
||||
ModernDirectoryReadResultSchema,
|
||||
RESOURCES_DIRECTORY_READ_METHOD,
|
||||
DYNAMIC_RESOURCES,
|
||||
GetSkillEnvelopeSchema,
|
||||
GetSkillResultSchema,
|
||||
ModernGetSkillEnvelopeSchema,
|
||||
ListSkillsResultSchema,
|
||||
ModernListSkillsResultSchema,
|
||||
SKILLS_EXTENSION_KEY,
|
||||
|
|
@ -95,9 +93,11 @@ describe("ListSkillsResultSchema", () => {
|
|||
});
|
||||
|
||||
describe("ModernListSkillsResultSchema", () => {
|
||||
const envelope = { resultType: "complete", ttlMs: 0, cacheScope: "public" };
|
||||
// What the schema actually receives on a modern connection: the SDK codec
|
||||
// has already checked `resultType` and lifted it off the result (#2373).
|
||||
const envelope = { ttlMs: 0, cacheScope: "public" };
|
||||
|
||||
it("accepts a modern page carrying the base list envelope", () => {
|
||||
it("accepts a modern page carrying the caching attributes", () => {
|
||||
const parsed = ModernListSkillsResultSchema.parse({
|
||||
...envelope,
|
||||
skills: [ENTRY],
|
||||
|
|
@ -105,10 +105,20 @@ describe("ModernListSkillsResultSchema", () => {
|
|||
expect(parsed.skills).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("does not require resultType, which the SDK codec lifts before this schema runs (#2373)", () => {
|
||||
// Requiring it rejected every conforming modern page: the codec checks
|
||||
// `resultType` itself, then deletes it. An integration test proves the
|
||||
// lift against a real connection; this pins the schema's side of it.
|
||||
expect(
|
||||
ModernListSkillsResultSchema.safeParse({ ...envelope, skills: [] })
|
||||
.success,
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects a modern page that omits the caching attributes", () => {
|
||||
// The whole reason for the era split: `skills/*` is consumer-owned, so the
|
||||
// SDK codec validates none of it, and `{ skills: [] }` would otherwise
|
||||
// reach the conformance UI as a clean list.
|
||||
// SDK codec checks none of the caching attributes, and `{ skills: [] }`
|
||||
// would otherwise reach the conformance UI as a clean list.
|
||||
expect(() => ModernListSkillsResultSchema.parse({ skills: [] })).toThrow();
|
||||
expect(() =>
|
||||
ModernListSkillsResultSchema.parse({
|
||||
|
|
@ -195,9 +205,6 @@ describe("directory read schemas (#2248)", () => {
|
|||
],
|
||||
};
|
||||
expect(DirectoryReadResultSchema.safeParse(example).success).toBe(true);
|
||||
expect(ModernDirectoryReadResultSchema.safeParse(example).success).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it("accepts an empty directory", () => {
|
||||
|
|
@ -230,39 +237,24 @@ describe("directory read schemas (#2248)", () => {
|
|||
).toBe(false);
|
||||
});
|
||||
|
||||
it("requires resultType on the modern variant only", () => {
|
||||
const legacyShape = { resources: [CHILD] };
|
||||
expect(DirectoryReadResultSchema.safeParse(legacyShape).success).toBe(true);
|
||||
expect(ModernDirectoryReadResultSchema.safeParse(legacyShape).success).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("does NOT require the caching attributes on the modern variant", () => {
|
||||
it("does NOT require the caching attributes, unlike a modern skills/list page", () => {
|
||||
// The deliberate asymmetry with `ModernListSkillsResultSchema`: SEP-2640
|
||||
// states `ttlMs`/`cacheScope` for a modern `skills/list` and says nothing
|
||||
// of the kind for this method, whose only worked example omits them.
|
||||
// Requiring them would fail a server that matched the spec's own example.
|
||||
expect(
|
||||
ModernDirectoryReadResultSchema.safeParse({
|
||||
resultType: "complete",
|
||||
resources: [],
|
||||
}).success,
|
||||
).toBe(true);
|
||||
expect(
|
||||
ModernListSkillsResultSchema.safeParse({
|
||||
resultType: "complete",
|
||||
skills: [],
|
||||
}).success,
|
||||
).toBe(false);
|
||||
expect(DirectoryReadResultSchema.safeParse({ resources: [] }).success).toBe(
|
||||
true,
|
||||
);
|
||||
expect(ModernListSkillsResultSchema.safeParse({ skills: [] }).success).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("still accepts the caching attributes when a server sends them", () => {
|
||||
// Permitted, not mandated — a schema is not the place to reject an extra
|
||||
// member the spec leaves open.
|
||||
expect(
|
||||
ModernDirectoryReadResultSchema.safeParse({
|
||||
resultType: "complete",
|
||||
DirectoryReadResultSchema.safeParse({
|
||||
resources: [],
|
||||
ttlMs: 60,
|
||||
cacheScope: "public",
|
||||
|
|
@ -286,25 +278,10 @@ describe("GetSkillResultSchema caching attributes (#2248)", () => {
|
|||
).toBe(true);
|
||||
});
|
||||
|
||||
it("requires resultType on the modern envelope, but still not the caching fields", () => {
|
||||
// "Left open" covers `ttlMs` / `cacheScope` and only those. `resultType` is
|
||||
// base-protocol (SEP-2322) and appears in SEP-2640's own `skills/get`
|
||||
// example, so leaving it optional here while requiring it of
|
||||
// `resources/directory/read` was an inconsistency in this module rather
|
||||
// than a distinction the spec draws (Copilot).
|
||||
expect(
|
||||
ModernGetSkillEnvelopeSchema.safeParse({ skill: ENTRY }).success,
|
||||
).toBe(false);
|
||||
expect(
|
||||
ModernGetSkillEnvelopeSchema.safeParse({
|
||||
skill: ENTRY,
|
||||
resultType: "complete",
|
||||
}).success,
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps the legacy envelope permissive about resultType", () => {
|
||||
// A 2026-era member a legacy server has no business sending.
|
||||
it("serves both eras with one envelope schema (#2373)", () => {
|
||||
// The modern variant used to add a required `resultType`, which the SDK
|
||||
// codec lifts before any caller schema runs — so it could never pass on a
|
||||
// real modern connection. With that gone the two eras want the same shape.
|
||||
expect(GetSkillEnvelopeSchema.safeParse({ skill: ENTRY }).success).toBe(
|
||||
true,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -117,8 +117,9 @@ describe("advertised-extension gating (#1739)", () => {
|
|||
|
||||
it("omits the gated tool when the extension is not advertised", async () => {
|
||||
const started = await startGatedServer();
|
||||
// Disable the only registry extension → the client sends no extensions,
|
||||
// so the server never enables the gated tool.
|
||||
// Disable the Tasks extension → the client still sends the other registry
|
||||
// extensions (MCP Apps UI, Skills), but not the one this tool is gated on,
|
||||
// so the server never enables it.
|
||||
const connected = await connect(started.url, {
|
||||
[TASKS_EXTENSION_KEY]: false,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -880,10 +880,11 @@ describe("InspectorClient coverage backfill", () => {
|
|||
});
|
||||
|
||||
describe("constructor capability branches and createReceiverTask options", () => {
|
||||
it("advertises only the tasks extension when no other capabilities are set", async () => {
|
||||
it("still advertises the registry extensions when no other capabilities are set", async () => {
|
||||
// sample:false + elicit:false + no roots + no receiverTasks → the only
|
||||
// advertised capability is the always-on Tasks extension (#1631), so
|
||||
// `capabilities` is non-empty and is always attached.
|
||||
// advertised capabilities are the default-on registry extensions (Tasks
|
||||
// since #1631, Skills since #2373), so `capabilities` is non-empty and
|
||||
// is always attached.
|
||||
client = new InspectorClient(
|
||||
{
|
||||
type: "stdio",
|
||||
|
|
@ -901,6 +902,7 @@ describe("InspectorClient coverage backfill", () => {
|
|||
expect(caps.elicitation).toBeUndefined();
|
||||
expect(caps.roots).toBeUndefined();
|
||||
expect(caps.extensions?.["io.modelcontextprotocol/tasks"]).toBeDefined();
|
||||
expect(caps.extensions?.["io.modelcontextprotocol/skills"]).toBeDefined();
|
||||
await client.connect();
|
||||
expect(client.getStatus()).toBe("connected");
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { describe, it, expect, afterEach } from "vitest";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import { createTransportNode } from "@inspector/core/mcp/node/transport.js";
|
||||
import { eraToVersionNegotiation } from "@inspector/core/mcp/types.js";
|
||||
import { getSkillsExtension } from "@inspector/core/mcp/skills.js";
|
||||
import { ManagedSkillsState } from "@inspector/core/mcp/state/managedSkillsState.js";
|
||||
import {
|
||||
|
|
@ -55,7 +56,10 @@ describe("Skills extension over a real transport (#2234)", () => {
|
|||
}
|
||||
});
|
||||
|
||||
async function startSkillsServer(modern: boolean): Promise<TestServerHttp> {
|
||||
async function startSkillsServer(
|
||||
modern: boolean,
|
||||
requireClientExtension = false,
|
||||
): Promise<TestServerHttp> {
|
||||
const started = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo("skills-integration", "1.0.0"),
|
||||
// An ordinary resource alongside the skills, so the fixture's
|
||||
|
|
@ -69,6 +73,7 @@ describe("Skills extension over a real transport (#2234)", () => {
|
|||
},
|
||||
],
|
||||
skills: true,
|
||||
...(requireClientExtension && { skillsRequireClientExtension: true }),
|
||||
...(modern && { modern: {} }),
|
||||
});
|
||||
await started.start();
|
||||
|
|
@ -79,20 +84,107 @@ describe("Skills extension over a real transport (#2234)", () => {
|
|||
async function connect(
|
||||
url: string,
|
||||
modern: boolean,
|
||||
advertisedExtensions?: Record<string, boolean>,
|
||||
): Promise<InspectorClient> {
|
||||
// The era is chosen by `versionNegotiation`. This helper used to set
|
||||
// `protocolEra` on the transport config instead, which `InspectorClient`
|
||||
// does not read — so every "modern" case below connected on legacy and
|
||||
// passed without exercising the modern leg at all (#2373). The assertion
|
||||
// after connect is what keeps that from recurring silently.
|
||||
const connected = new InspectorClient(
|
||||
{ type: "streamable-http", url },
|
||||
{
|
||||
type: "streamable-http",
|
||||
url,
|
||||
...(modern && { protocolEra: "modern" as const }),
|
||||
environment: { transport: createTransportNode },
|
||||
versionNegotiation: eraToVersionNegotiation(
|
||||
modern ? "modern" : "legacy",
|
||||
),
|
||||
advertisedExtensions,
|
||||
},
|
||||
{ environment: { transport: createTransportNode } },
|
||||
);
|
||||
await connected.connect();
|
||||
client = connected;
|
||||
expect(connected.getProtocolEra()).toBe(modern ? "modern" : "legacy");
|
||||
return connected;
|
||||
}
|
||||
|
||||
/**
|
||||
* #2373: SEP-2133 negotiates an extension from both sides, and a strict
|
||||
* server refuses `skills/*` to a client that did not declare
|
||||
* `io.modelcontextprotocol/skills` itself. The Inspector never declared it,
|
||||
* and every test above passed anyway because the default fixture serves any
|
||||
* client. These run against the strict fixture on both eras — the legacy leg
|
||||
* reads the declaration from `initialize`, the modern leg from each
|
||||
* request's `_meta` envelope, so each is a separate path to prove.
|
||||
*/
|
||||
for (const modern of [false, true]) {
|
||||
const era = modern ? "modern" : "legacy";
|
||||
|
||||
describe(`against a server that requires the client's declaration (${era})`, () => {
|
||||
it("is served, because the Inspector declares the extension by default", async () => {
|
||||
const started = await startSkillsServer(modern, true);
|
||||
const connected = await connect(started.url, modern);
|
||||
const first = await connected.listSkills();
|
||||
expect(first.skills).toHaveLength(2);
|
||||
const entry = await connected.getSkill(
|
||||
"skill://data-analysis/SKILL.md",
|
||||
);
|
||||
expect(entry.frontmatter.name).toBe("data-analysis");
|
||||
const page = await connected.readResourceDirectory(
|
||||
"skill://data-analysis",
|
||||
);
|
||||
expect(page.resources).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("is refused once the declaration is turned off", async () => {
|
||||
// The negative control: without it the test above could pass against
|
||||
// a fixture that stopped checking. It is also the Server Settings
|
||||
// toggle's whole purpose — reproducing a strict server's refusal.
|
||||
const started = await startSkillsServer(modern, true);
|
||||
const connected = await connect(started.url, modern, {
|
||||
"io.modelcontextprotocol/skills": false,
|
||||
});
|
||||
// The wire code is the assertion, not just the message: the two eras
|
||||
// name this refusal differently, and a fixture sliding back to a
|
||||
// semantically different error must fail here. Modern is SEP-2575's
|
||||
// `-32021` MissingRequiredClientCapability listing the missing
|
||||
// extension; legacy has no such code and stays `-32601`.
|
||||
const expected = modern
|
||||
? {
|
||||
code: -32021,
|
||||
data: {
|
||||
requiredCapabilities: {
|
||||
extensions: { "io.modelcontextprotocol/skills": {} },
|
||||
},
|
||||
},
|
||||
}
|
||||
: { code: -32601 };
|
||||
// Thunks, not promises: starting all three up front leaves the later
|
||||
// rejections unhandled while the first is awaited, which fails the
|
||||
// run even though every assertion passes.
|
||||
const refusals = [
|
||||
() => connected.listSkills(),
|
||||
() => connected.getSkill("skill://data-analysis/SKILL.md"),
|
||||
() => connected.readResourceDirectory("skill://data-analysis"),
|
||||
];
|
||||
for (const refusal of refusals) {
|
||||
await expect(refusal()).rejects.toMatchObject({
|
||||
...expected,
|
||||
message: expect.stringMatching(
|
||||
/requires the client to declare io\.modelcontextprotocol\/skills/,
|
||||
),
|
||||
});
|
||||
}
|
||||
// A skill file is an ordinary resource and stays readable.
|
||||
const read = await connected.readResource(
|
||||
"skill://data-analysis/reference.md",
|
||||
);
|
||||
expect(read.result.contents[0].uri).toBe(
|
||||
"skill://data-analysis/reference.md",
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
for (const modern of [false, true]) {
|
||||
const era = modern ? "modern" : "legacy";
|
||||
|
||||
|
|
@ -114,12 +206,13 @@ describe("Skills extension over a real transport (#2234)", () => {
|
|||
|
||||
const first = await connected.listSkills();
|
||||
// On the modern leg this call resolving is itself the envelope
|
||||
// assertion: `listSkills` selects `ModernListSkillsResultSchema` from
|
||||
// the negotiated era, and that schema rejects a page without
|
||||
// `resultType` / `ttlMs` / `cacheScope`. It cannot be asserted on the
|
||||
// returned value — `listSkills` narrows its result to the two fields
|
||||
// below — so a modern page missing the envelope surfaces here as a
|
||||
// rejection rather than as a missing property.
|
||||
// assertion, in two halves: the SDK codec rejects a result without
|
||||
// `resultType` (and lifts it off), and `listSkills` then selects
|
||||
// `ModernListSkillsResultSchema`, which rejects a page without
|
||||
// `ttlMs` / `cacheScope`. It cannot be asserted on the returned value
|
||||
// — `listSkills` narrows its result to the two fields below — so a
|
||||
// modern page missing the envelope surfaces here as a rejection
|
||||
// rather than as a missing property.
|
||||
//
|
||||
// The fixture pages at two over eight skills, so a client that stops
|
||||
// here sees a quarter of the catalog.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,100 @@
|
|||
import { describe, it, expect, afterEach } from "vitest";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import { createTransportNode } from "@inspector/core/mcp/node/transport.js";
|
||||
import { eraToVersionNegotiation } from "@inspector/core/mcp/types.js";
|
||||
import {
|
||||
createTestServerHttp,
|
||||
type TestServerHttp,
|
||||
loadConfig,
|
||||
resolveConfig,
|
||||
} from "@modelcontextprotocol/inspector-test-server";
|
||||
|
||||
/**
|
||||
* Live coverage of the two checked-in strict Skills configs (#2373) — the
|
||||
* documented manual reproduction of a server that enforces the client's half
|
||||
* of SEP-2133 negotiation.
|
||||
*
|
||||
* `inspectorClient-skills.test.ts` exercises the fixture behavior by passing
|
||||
* `skillsRequireClientExtension` straight to `createTestServerHttp`, so it
|
||||
* never touches the JSON-to-`ServerConfig` mapping. A misspelled key in either
|
||||
* file, or a `resolveConfig` that stopped threading the flag, would leave the
|
||||
* manual fixtures quietly permissive while that suite stayed green. These
|
||||
* tests **resolve the checked-in files** and assert the refusal they exist
|
||||
* for, so that class fails here instead.
|
||||
*/
|
||||
const configsDir = path.resolve(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
"../../../../../../test-servers/configs",
|
||||
);
|
||||
|
||||
const CONFIGS = [
|
||||
{ file: "skills-strict-legacy-http.json", era: "legacy" as const },
|
||||
{ file: "skills-strict-modern-http.json", era: "modern" as const },
|
||||
];
|
||||
|
||||
describe("strict Skills showcase configs (#2373)", () => {
|
||||
let client: InspectorClient | null = null;
|
||||
let server: TestServerHttp | null = null;
|
||||
|
||||
afterEach(async () => {
|
||||
if (client) {
|
||||
try {
|
||||
await client.disconnect();
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
client = null;
|
||||
}
|
||||
if (server) {
|
||||
try {
|
||||
await server.stop();
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
server = null;
|
||||
}
|
||||
});
|
||||
|
||||
for (const { file, era } of CONFIGS) {
|
||||
describe(file, () => {
|
||||
const configPath = path.join(configsDir, file);
|
||||
|
||||
it("resolves with the strict flag, skills, and its era intact", () => {
|
||||
const resolved = resolveConfig(loadConfig(configPath));
|
||||
expect(resolved.skills).toBe(true);
|
||||
expect(resolved.skillsRequireClientExtension).toBe(true);
|
||||
// One file per era is the documented contract: a legacy client
|
||||
// reaching a modern server is served statelessly and cannot be
|
||||
// checked, so the legacy file must not turn the modern leg on.
|
||||
expect(resolved.modern !== undefined).toBe(era === "modern");
|
||||
});
|
||||
|
||||
it("refuses skills/list to a client that did not declare the extension", async () => {
|
||||
// The harness picks the port rather than the config's fixed one, so
|
||||
// this cannot collide with a showcase server someone runs by hand.
|
||||
const resolved = resolveConfig(loadConfig(configPath));
|
||||
const started = createTestServerHttp({ ...resolved, port: undefined });
|
||||
await started.start();
|
||||
server = started;
|
||||
|
||||
const connected = new InspectorClient(
|
||||
{ type: "streamable-http", url: started.url },
|
||||
{
|
||||
environment: { transport: createTransportNode },
|
||||
versionNegotiation: eraToVersionNegotiation(era),
|
||||
advertisedExtensions: { "io.modelcontextprotocol/skills": false },
|
||||
},
|
||||
);
|
||||
await connected.connect();
|
||||
client = connected;
|
||||
expect(connected.getProtocolEra()).toBe(era);
|
||||
|
||||
await expect(connected.listSkills()).rejects.toMatchObject({
|
||||
code: era === "modern" ? -32021 : -32601,
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
|
|
@ -1,6 +1,7 @@
|
|||
import type { ClientCapabilities } from "@modelcontextprotocol/client";
|
||||
import { RESOURCE_MIME_TYPE } from "@modelcontextprotocol/ext-apps/app-bridge";
|
||||
import { TASKS_EXTENSION_KEY } from "./modernTaskSchemas.js";
|
||||
import { SKILLS_EXTENSION_KEY } from "./skillsSchemas.js";
|
||||
|
||||
/**
|
||||
* Extension identifier for SEP-2350 enterprise-managed authorization. Advertised
|
||||
|
|
@ -91,6 +92,18 @@ export const ADVERTISABLE_EXTENSIONS: readonly AdvertisableExtension[] = [
|
|||
defaultAdvertised: true,
|
||||
advertisement: { mimeTypes: [MCP_APP_MIME_TYPE] },
|
||||
},
|
||||
{
|
||||
key: SKILLS_EXTENSION_KEY,
|
||||
label: "Skills (io.modelcontextprotocol/skills)",
|
||||
// The Skills extension (SEP-2640). SEP-2133 negotiates an extension from
|
||||
// both sides, so a server may refuse `skills/list`, `skills/get` and
|
||||
// `resources/directory/read` to a client that did not declare it — and the
|
||||
// Inspector calls all three once the server declares its half. Advertised
|
||||
// by default for that reason; turning it off is how to check a server's
|
||||
// refusal path. Declared with no settings: SEP-2640 defines none for the
|
||||
// client side. (#2373)
|
||||
defaultAdvertised: true,
|
||||
},
|
||||
];
|
||||
|
||||
export interface BuildClientExtensionsInput {
|
||||
|
|
|
|||
|
|
@ -150,8 +150,6 @@ import {
|
|||
DirectoryReadResultSchema,
|
||||
GetSkillEnvelopeSchema,
|
||||
ListSkillsResultSchema,
|
||||
ModernGetSkillEnvelopeSchema,
|
||||
ModernDirectoryReadResultSchema,
|
||||
ModernListSkillsResultSchema,
|
||||
RESOURCES_DIRECTORY_READ_METHOD,
|
||||
SKILLS_EXTENSION_KEY,
|
||||
|
|
@ -5741,11 +5739,13 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
...(cursor !== undefined ? { cursor } : {}),
|
||||
};
|
||||
// Era-aware: a modern (2026-07-28+) `skills/list` result also carries the
|
||||
// base list envelope (`resultType` / `ttlMs` / `cacheScope`). `skills/*` is
|
||||
// consumer-owned, so the SDK codec validates none of it — without picking
|
||||
// the schema here a modern server could answer `{ skills: [] }` and the
|
||||
// conformance UI would show a clean list. Legacy stays permissive: those
|
||||
// are 2026-era attributes.
|
||||
// base list envelope (`resultType` / `ttlMs` / `cacheScope`). The two halves
|
||||
// are checked in different places: the SDK codec checks `resultType` on
|
||||
// every modern result and removes it before this schema runs (#2373), but
|
||||
// `skills/*` is consumer-owned, so it checks neither caching attribute —
|
||||
// without picking the schema here a modern server could answer
|
||||
// `{ skills: [] }` and the conformance UI would show a clean list. Legacy
|
||||
// stays permissive: those are 2026-era attributes.
|
||||
const resultSchema = this.isModernEra()
|
||||
? ModernListSkillsResultSchema
|
||||
: ListSkillsResultSchema;
|
||||
|
|
@ -5775,8 +5775,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
}
|
||||
|
||||
/**
|
||||
* `skills/get` as the server sent it — the `{ skill }` envelope **and any
|
||||
* other members it carried**.
|
||||
* `skills/get` as the SDK decoded it — the `{ skill }` envelope **and any
|
||||
* other members the server sent**, such as the caching attributes. On a
|
||||
* modern connection `resultType` is not among them: the codec checks it and
|
||||
* removes it before the result gets here (#2373).
|
||||
*
|
||||
* Separate from {@link getSkill} because the callers differ: the UIs want the
|
||||
* entry, while the CLI prints the result and must not reshape it. SEP-2640
|
||||
|
|
@ -5796,21 +5798,16 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
uri,
|
||||
...(effectiveMeta ? { _meta: effectiveMeta } : {}),
|
||||
};
|
||||
// Era-aware for the same reason `skills/list` is: the method is
|
||||
// consumer-owned, so no SDK codec stamps or checks its envelope. The modern
|
||||
// variant requires `resultType` — a base-protocol member SEP-2322 puts on
|
||||
// every modern result — and still not the caching attributes, which
|
||||
// SEP-2640 leaves open. The envelope is returned whole; `getSkill`
|
||||
// unwraps.
|
||||
const resultSchema = this.isModernEra()
|
||||
? ModernGetSkillEnvelopeSchema
|
||||
: GetSkillEnvelopeSchema;
|
||||
// One schema for both eras (#2373): on a modern connection the SDK codec
|
||||
// has already enforced `resultType` and lifted it off, and the caching
|
||||
// attributes are left open by SEP-2640, so there is nothing era-specific
|
||||
// left to require. The envelope is returned whole; `getSkill` unwraps.
|
||||
try {
|
||||
return await this.invokeMcpClient(
|
||||
() =>
|
||||
this.client!.request(
|
||||
{ method: SKILLS_GET_METHOD, params },
|
||||
resultSchema,
|
||||
GetSkillEnvelopeSchema,
|
||||
this.getRequestOptions(this.progressTokenOf(metadata)),
|
||||
),
|
||||
{ method: SKILLS_GET_METHOD },
|
||||
|
|
@ -5872,19 +5869,16 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
// for page one.
|
||||
...(cursor !== undefined ? { cursor } : {}),
|
||||
};
|
||||
// Era-aware for the same reason `skills/list` is — the method is
|
||||
// consumer-owned, so no SDK codec stamps or checks its envelope. The modern
|
||||
// variant requires only `resultType`; see the schema for why it stops
|
||||
// short of the caching attributes that `skills/list` requires.
|
||||
const resultSchema = this.isModernEra()
|
||||
? ModernDirectoryReadResultSchema
|
||||
: DirectoryReadResultSchema;
|
||||
// One schema for both eras (#2373): the SDK codec enforces and lifts
|
||||
// `resultType` on a modern connection, and SEP-2640 requires no caching
|
||||
// attributes of this method — see `skillsSchemas.ts` for why that differs
|
||||
// from `skills/list`.
|
||||
try {
|
||||
return await this.invokeMcpClient(
|
||||
() =>
|
||||
this.client!.request(
|
||||
{ method: RESOURCES_DIRECTORY_READ_METHOD, params },
|
||||
resultSchema,
|
||||
DirectoryReadResultSchema,
|
||||
this.getRequestOptions(this.progressTokenOf(metadata)),
|
||||
),
|
||||
{ method: RESOURCES_DIRECTORY_READ_METHOD },
|
||||
|
|
|
|||
|
|
@ -11,12 +11,15 @@
|
|||
* shared by every client, and produce a structured finding list rather than a
|
||||
* boolean — a report is useful, "invalid" is not.
|
||||
*
|
||||
* ⚠️ Skills is a **server-declared** extension, read off the connecting server's
|
||||
* `capabilities.extensions`. It deliberately does NOT belong in
|
||||
* `ADVERTISABLE_EXTENSIONS` (`core/mcp/extensions.ts`), which is the catalog of
|
||||
* extensions the *Inspector* advertises and the user toggles in Server Settings.
|
||||
* The precedent is `appElicitation.ts`, which reads the server side the same
|
||||
* way; getting it backwards would put a meaningless toggle in Server Settings.
|
||||
* ⚠️ Skills is negotiated from **both** sides (SEP-2133), and the two halves
|
||||
* live in different places. This module reads the *server's* declaration off
|
||||
* the connecting server's `capabilities.extensions` — that is what gates the
|
||||
* Skills screen. The *Inspector's* own declaration is a separate entry in
|
||||
* `ADVERTISABLE_EXTENSIONS` (`core/mcp/extensions.ts`): a strict server refuses
|
||||
* `skills/*` to a client that did not declare it, and turning that toggle off
|
||||
* in Server Settings is how to check a server's refusal path (#2373). An
|
||||
* earlier revision of this comment said Skills must never be in that registry;
|
||||
* that was the missing client declaration #2373 fixed.
|
||||
*
|
||||
* The Inspector is an inspector, not a host: a `resources/read` of a `SKILL.md`
|
||||
* is explicitly not a load and confers no standing, so none of the SEP's host
|
||||
|
|
|
|||
|
|
@ -131,9 +131,18 @@ export type ListSkillsResult = z.infer<typeof ListSkillsResultSchema>;
|
|||
*
|
||||
* ⚠️ Picked by `InspectorClient.listSkills` from the negotiated era — a schema
|
||||
* cannot know it on its own.
|
||||
*
|
||||
* ⚠️ **No `resultType` member, and that is not an omission (#2373).** The
|
||||
* SDK's 2026-07-28 codec checks `resultType` on every result — missing is
|
||||
* `InvalidResult`, anything but `"complete"` is `UnsupportedResultType` — and
|
||||
* then **deletes it** before the result reaches the caller's schema. Requiring
|
||||
* it here therefore rejected every conforming modern page, and did so unseen
|
||||
* only because the integration suite's "modern" cases were connecting on
|
||||
* legacy. The caching attributes are different: the codec neither checks nor
|
||||
* lifts them for a consumer-owned method, so this schema is still the only
|
||||
* thing that does.
|
||||
*/
|
||||
export const ModernListSkillsResultSchema = ListSkillsResultSchema.extend({
|
||||
resultType: z.literal("complete"),
|
||||
// A non-negative INTEGER, matching the codec: `ttlMs: -1` or `0.5` is an
|
||||
// envelope violation, and accepting it here would hide exactly the kind of
|
||||
// defect this schema exists to surface.
|
||||
|
|
@ -172,7 +181,10 @@ export const GetSkillEnvelopeSchema = z.looseObject({
|
|||
});
|
||||
|
||||
/**
|
||||
* The `skills/get` result as the server sent it, envelope and all.
|
||||
* The `skills/get` result envelope as the SDK decoded it: the `skill` wrapper
|
||||
* plus any caching attributes the server sent. On a modern connection
|
||||
* `resultType` is not in it — the codec checks it and removes it before this
|
||||
* schema runs (#2373).
|
||||
*
|
||||
* Exported alongside the unwrapping schema below because the two callers want
|
||||
* different things: the UIs want the entry, while the CLI's job is to print
|
||||
|
|
@ -183,21 +195,6 @@ export const GetSkillEnvelopeSchema = z.looseObject({
|
|||
*/
|
||||
export type GetSkillEnvelope = z.infer<typeof GetSkillEnvelopeSchema>;
|
||||
|
||||
/**
|
||||
* `skills/get` on a **modern** (2026-07-28+) connection: the envelope plus
|
||||
* `resultType`, and deliberately still not the caching attributes.
|
||||
*
|
||||
* The "left open" quote above covers `ttlMs` / `cacheScope` and **only** those.
|
||||
* `resultType` is a different thing: SEP-2322 makes it a member of every modern
|
||||
* result, and SEP-2640's own `skills/get` example carries
|
||||
* `"resultType": "complete"`. Leaving it optional here while requiring it of
|
||||
* `resources/directory/read` was an inconsistency in this module rather than a
|
||||
* distinction the spec draws (Copilot).
|
||||
*/
|
||||
export const ModernGetSkillEnvelopeSchema = GetSkillEnvelopeSchema.extend({
|
||||
resultType: z.literal("complete"),
|
||||
});
|
||||
|
||||
/**
|
||||
* `skills/get` result, unwrapped to the entry it carries.
|
||||
*
|
||||
|
|
@ -238,8 +235,10 @@ export type GetSkillResult = SkillEntry;
|
|||
export const DirectoryChildSchema = ResourceSchema;
|
||||
|
||||
/**
|
||||
* `resources/directory/read` result on a **legacy** connection: the directory's
|
||||
* direct children plus the opaque cursor.
|
||||
* `resources/directory/read` result on **either** era: the directory's direct
|
||||
* children plus the opaque cursor. On a modern connection this is the shape
|
||||
* after the SDK codec has checked and removed `resultType`, which is why no
|
||||
* modern variant exists — see the note below (#2373).
|
||||
*
|
||||
* `looseObject`, matching every other result schema here: a server that also
|
||||
* sends the caching attributes is not wrong for doing so, and a schema is not
|
||||
|
|
@ -252,29 +251,28 @@ export const DirectoryReadResultSchema = z.looseObject({
|
|||
|
||||
export type DirectoryReadResult = z.infer<typeof DirectoryReadResultSchema>;
|
||||
|
||||
/**
|
||||
* `resources/directory/read` result on a **modern** (2026-07-28+) connection:
|
||||
* the page plus `resultType`, and deliberately **not** `ttlMs` / `cacheScope`.
|
||||
/*
|
||||
* There is deliberately **no modern variant** of the `resources/directory/read`
|
||||
* or `skills/get` schemas, though `skills/list` has one (#2373).
|
||||
*
|
||||
* That asymmetry with {@link ModernListSkillsResultSchema} is the one judgement
|
||||
* call in this module, so it is written down rather than left to be re-derived:
|
||||
* On a modern (2026-07-28+) connection the SDK codec already enforces
|
||||
* `resultType` — the base-protocol member SEP-2322 puts on every modern result
|
||||
* — and lifts it off before any caller schema runs. What remains is the
|
||||
* caching attributes, and only `skills/list` is required to carry those:
|
||||
*
|
||||
* - For `skills/list` the SEP states the requirement outright — *"In protocol
|
||||
* versions 2026-07-28 and later, the result also carries … `ttlMs` and
|
||||
* `cacheScope`"* — so requiring them is quoting the spec.
|
||||
* - For `skills/list` the SEP states it outright — *"In protocol versions
|
||||
* 2026-07-28 and later, the result also carries … `ttlMs` and
|
||||
* `cacheScope`"* — so {@link ModernListSkillsResultSchema} requiring them is
|
||||
* quoting the spec.
|
||||
* - For `resources/directory/read` it states **nothing of the kind**, and its
|
||||
* one worked example of the result carries `resultType: "complete"` and no
|
||||
* caching attributes at all. Requiring them here would fail a server that
|
||||
* matched the SEP's own example, which is the failure direction this module
|
||||
* works hardest to avoid.
|
||||
* one worked example carries `resultType: "complete"` and no caching
|
||||
* attributes at all. Requiring them would fail a server that matched the
|
||||
* SEP's own example, the failure direction this module works hardest to
|
||||
* avoid.
|
||||
* - For `skills/get` the SEP leaves the question open in as many words (see
|
||||
* {@link GetSkillEnvelopeSchema}).
|
||||
*
|
||||
* `resultType` is required because it is the base protocol's, not this
|
||||
* extension's: SEP-2322 makes it a member of every modern result, the SEP's
|
||||
* example carries it, and `skills/*` being consumer-owned means the SDK codec
|
||||
* validates none of it — so if this schema does not, nothing does.
|
||||
*
|
||||
* ⚠️ Picked by `InspectorClient.readResourceDirectory` from the negotiated era.
|
||||
* So both eras parse those two results with one schema each. An earlier
|
||||
* revision required `resultType` on "modern" variants of both; since the codec
|
||||
* lifts it, they could never pass on a real modern connection.
|
||||
*/
|
||||
export const ModernDirectoryReadResultSchema = DirectoryReadResultSchema.extend(
|
||||
{ resultType: z.literal("complete") },
|
||||
);
|
||||
|
|
|
|||
|
|
@ -62,6 +62,7 @@ as a missing capability rather than an error.
|
|||
| `tasks-{legacy,modern}-http.json` **(era per file)** | Tasks, both eras | [#1631](https://github.com/modelcontextprotocol/inspector/issues/1631) |
|
||||
| `cancellation-modern-http.json` **(modern era)** | Cancelling a call by closing its response stream | [#2140](https://github.com/modelcontextprotocol/inspector/issues/2140) |
|
||||
| `skills-http.json` **(either era)** | Skills tab: `skills/list`, `resources/directory/read`, digest verification, the frontmatter cross-check, and the non-conforming cases | [#2234](https://github.com/modelcontextprotocol/inspector/issues/2234), [#2248](https://github.com/modelcontextprotocol/inspector/issues/2248) |
|
||||
| `skills-strict-{legacy,modern}-http.json` **(era per file)** | The same skills, served only to a client that **declared** `io.modelcontextprotocol/skills` itself (SEP-2133) | [#2373](https://github.com/modelcontextprotocol/inspector/issues/2373) |
|
||||
|
||||
## Skills (SEP-2640)
|
||||
|
||||
|
|
@ -77,6 +78,37 @@ handler cannot reach it. To exercise the *undeclared* case, connect to any
|
|||
config **without** `"skills"`, where the Inspector must refuse to send the call
|
||||
locally rather than letting the server answer it.
|
||||
|
||||
⚠️ **`skills-http.json` serves any client, including one that never declared the
|
||||
extension.** SEP-2133 negotiates an extension from both sides, and a strict
|
||||
server refuses `skills/*` to a client that did not declare
|
||||
`io.modelcontextprotocol/skills` itself. The Inspector shipped without that
|
||||
declaration and passed against this fixture regardless
|
||||
([#2373](https://github.com/modelcontextprotocol/inspector/issues/2373)).
|
||||
`skills-strict-legacy-http.json` (port 3232) and
|
||||
`skills-strict-modern-http.json` (port 3233) add
|
||||
`"skillsRequireClientExtension": true`, which refuses `skills/list`,
|
||||
`skills/get` and `resources/directory/read` unless the client declared it
|
||||
(skill files still come through ordinary `resources/read`, which needs no
|
||||
extension). The refusal differs by era: a **modern** request gets SEP-2575's
|
||||
`-32021` MissingRequiredClientCapability (HTTP 400) with the Skills extension
|
||||
under `data.requiredCapabilities`; a **legacy** one, whose era has no such code,
|
||||
gets `-32601`. ⚠️ **Connect each with its own era.** A modern request
|
||||
carries the declaration in its `_meta` envelope; a legacy one relies on what
|
||||
`initialize` declared, which only a stateful legacy server keeps. A legacy
|
||||
client reaching the modern file is served statelessly, holds no declaration,
|
||||
and is refused however it is configured — so there is one file per era rather
|
||||
than one for both:
|
||||
|
||||
1. Connect — the Inspector declares the Skills extension by default, so the
|
||||
Skills tab lists the same eight skills. The client's declaration is visible
|
||||
in the `initialize` request (Legacy) or in each request's
|
||||
`io.modelcontextprotocol/clientCapabilities` `_meta` entry (Modern).
|
||||
2. Open **Server Settings → Advertised Extensions**, uncheck
|
||||
**Skills (io.modelcontextprotocol/skills)**, and reconnect.
|
||||
3. `skills/list` now fails — with `-32021` MissingRequiredClientCapability on
|
||||
the modern file and `-32601` on the legacy one, the refusal a strict server
|
||||
sends in each era.
|
||||
|
||||
**Both `skills/*` results carry the full modern base envelope** (`resultType` /
|
||||
`ttlMs` / `cacheScope`). They are consumer-owned methods, so the SDK stamps
|
||||
nothing for them; without it a 2026-era connection would receive a result
|
||||
|
|
@ -91,8 +123,10 @@ words and says nothing of the kind for this method, whose one worked example
|
|||
carries `resultType` and nothing else. A fixture sending more than the SEP shows
|
||||
would make a client that wrongly *required* them look correct, which is the
|
||||
opposite of what a conformance fixture is for — so `readDirectoryPage` stops
|
||||
where the spec does, and `ModernDirectoryReadResultSchema` requires exactly as
|
||||
much.
|
||||
where the spec does. On the client, the SDK's modern codec enforces
|
||||
`resultType` and lifts it off before the Inspector's own schema runs, so
|
||||
`DirectoryReadResultSchema` serves both eras and requires no caching attributes
|
||||
([#2373](https://github.com/modelcontextprotocol/inspector/issues/2373)).
|
||||
|
||||
It works on **either era**: `skills/list`, `skills/get` and
|
||||
`resources/directory/read` are consumer-owned extension methods that neither
|
||||
|
|
@ -496,7 +530,7 @@ Requiredness is a property of the **expression**, not the variable: RFC 6570 dro
|
|||
|
||||
1. Connect — the Inspector advertises the Tasks extension by default, so the Tools list shows both `echo` and `get_weather`.
|
||||
2. Open **Server Settings → Advertised Extensions**, uncheck **Tasks (io.modelcontextprotocol/tasks)**, and reconnect.
|
||||
3. The client now advertises no extensions, the server never enables `get_weather`, and the Tools list shows only `echo`.
|
||||
3. The client no longer advertises the Tasks extension (it still advertises the others, such as MCP Apps UI and Skills), the server never enables `get_weather`, and the Tools list shows only `echo`.
|
||||
|
||||
This is the debugging knob for a server legitimately changing tool registration based on what the client advertises. Legacy stateful leg only — the modern per-request leg has no persistent `oninitialized`.
|
||||
|
||||
|
|
|
|||
18
test-servers/configs/skills-strict-legacy-http.json
Normal file
18
test-servers/configs/skills-strict-legacy-http.json
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
{
|
||||
"serverInfo": {
|
||||
"name": "skills-strict-legacy",
|
||||
"version": "1.0.0"
|
||||
},
|
||||
"tools": [
|
||||
{
|
||||
"preset": "echo"
|
||||
}
|
||||
],
|
||||
"resources": [],
|
||||
"skills": true,
|
||||
"skillsRequireClientExtension": true,
|
||||
"transport": {
|
||||
"type": "streamable-http",
|
||||
"port": 3232
|
||||
}
|
||||
}
|
||||
19
test-servers/configs/skills-strict-modern-http.json
Normal file
19
test-servers/configs/skills-strict-modern-http.json
Normal file
|
|
@ -0,0 +1,19 @@
|
|||
{
|
||||
"serverInfo": {
|
||||
"name": "skills-strict-modern",
|
||||
"version": "1.0.0"
|
||||
},
|
||||
"tools": [
|
||||
{
|
||||
"preset": "echo"
|
||||
}
|
||||
],
|
||||
"resources": [],
|
||||
"skills": true,
|
||||
"skillsRequireClientExtension": true,
|
||||
"transport": {
|
||||
"type": "streamable-http",
|
||||
"port": 3233,
|
||||
"modern": true
|
||||
}
|
||||
}
|
||||
|
|
@ -614,6 +614,17 @@ export interface ServerConfig {
|
|||
* refuse to send the call locally.
|
||||
*/
|
||||
skills?: boolean;
|
||||
/**
|
||||
* With {@link ServerConfig.skills}, refuse `skills/list`, `skills/get` and
|
||||
* `resources/directory/read` unless the client declared
|
||||
* `io.modelcontextprotocol/skills` in its own capabilities — a strict
|
||||
* SEP-2133 server (#2373). A modern request is refused with `-32021`
|
||||
* MissingRequiredClientCapability carrying `data.requiredCapabilities`; a
|
||||
* legacy one, whose era has no such code, with `-32601`. Off by default so
|
||||
* the existing skills fixtures keep serving any client. Ignored without
|
||||
* `skills`.
|
||||
*/
|
||||
skillsRequireClientExtension?: boolean;
|
||||
/**
|
||||
* Advertise the MCP Apps `io.modelcontextprotocol/ui` extension with the
|
||||
* nested `elicitation` setting — the server-side half of the app-rendered
|
||||
|
|
@ -1675,7 +1686,9 @@ export function createMcpServer(config: ServerConfig): McpServer {
|
|||
// `skill://` half of resources/read. Wired after the SDK's own handlers so
|
||||
// the resources/read wrapper can delegate non-skill URIs to them.
|
||||
if (config.skills) {
|
||||
wireSkillsHandlers(mcpServer);
|
||||
wireSkillsHandlers(mcpServer, {
|
||||
requireClientExtension: config.skillsRequireClientExtension,
|
||||
});
|
||||
}
|
||||
|
||||
// Extension-gated tools (#1739): start each gated tool disabled, then enable
|
||||
|
|
|
|||
|
|
@ -87,6 +87,9 @@ export interface ConfigFile {
|
|||
/** Advertise the Skills extension (SEP-2640) and serve its fixture skills,
|
||||
* including `directoryRead` — see {@link ServerConfig.skills}. */
|
||||
skills?: boolean;
|
||||
/** With `skills`, refuse the skills methods to a client that did not declare
|
||||
* the extension — see {@link ServerConfig.skillsRequireClientExtension}. */
|
||||
skillsRequireClientExtension?: boolean;
|
||||
/** Advertise the MCP Apps `io.modelcontextprotocol/ui` extension with the nested
|
||||
* `elicitation` setting — the server half of app-rendered form elicitation
|
||||
* (#1854). Pair with the `app_choose_option` tool + `choose_option_app` resource. */
|
||||
|
|
|
|||
|
|
@ -91,6 +91,7 @@ export function resolveConfig(config: ConfigFile): ServerConfig {
|
|||
tasks: config.tasks,
|
||||
tasksExtension: config.tasksExtension,
|
||||
skills: config.skills,
|
||||
skillsRequireClientExtension: config.skillsRequireClientExtension,
|
||||
appElicitation: config.appElicitation,
|
||||
maxPageSize: config.maxPageSize,
|
||||
emptyStringCursor: config.emptyStringCursor,
|
||||
|
|
|
|||
|
|
@ -59,6 +59,8 @@
|
|||
import { createHash } from "node:crypto";
|
||||
import * as z from "zod/v4";
|
||||
import {
|
||||
CLIENT_CAPABILITIES_META_KEY,
|
||||
MissingRequiredClientCapabilityError,
|
||||
ProtocolError,
|
||||
ProtocolErrorCode,
|
||||
type McpServer,
|
||||
|
|
@ -698,29 +700,117 @@ const DirectoryReadResultShape = z.object({
|
|||
nextCursor: z.string().optional(),
|
||||
});
|
||||
|
||||
export interface WireSkillsOptions {
|
||||
/**
|
||||
* Refuse the extension's own methods to a client that did not declare
|
||||
* `io.modelcontextprotocol/skills` in its capabilities (#2373). SEP-2133
|
||||
* negotiates an extension from both sides, and a strict server enforces the
|
||||
* client's half; without this the fixture served every client, which is how
|
||||
* an Inspector that never declared the extension passed against it.
|
||||
*/
|
||||
requireClientExtension?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* The request's `_meta` envelope as a handler receives it. The SDK types the
|
||||
* envelope's keys as an empty object, so it is read as a plain record and each
|
||||
* value is narrowed where it is used.
|
||||
*/
|
||||
type ReceivedEnvelope = Record<string, unknown> | undefined;
|
||||
|
||||
function hasSkillsExtension(capabilities: unknown): boolean {
|
||||
if (typeof capabilities !== "object" || capabilities === null) return false;
|
||||
const extensions = (capabilities as { extensions?: unknown }).extensions;
|
||||
return (
|
||||
typeof extensions === "object" &&
|
||||
extensions !== null &&
|
||||
SKILLS_EXTENSION_KEY in extensions
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Refuse the request when the client did not declare the Skills extension:
|
||||
* `-32021` MissingRequiredClientCapability for a modern request, `-32601` for
|
||||
* a legacy one.
|
||||
*
|
||||
* The two eras keep the declaration in different places. A modern
|
||||
* (2026-07-28) request carries it in its own `_meta` envelope, which the SDK
|
||||
* hands the handler as `ctx.mcpReq.envelope` — read first, because that is the
|
||||
* per-request source of truth the SDK points handlers at. A legacy connection
|
||||
* has no envelope, so the value `initialize` declared is the fallback.
|
||||
*
|
||||
* ⚠️ That fallback only exists on a **stateful** legacy connection. A legacy
|
||||
* client reaching a server started with `modern` is served statelessly — a
|
||||
* fresh instance per request that never saw `initialize` — so it has no
|
||||
* declaration to read and is refused. That is why the strict showcase configs
|
||||
* come one per era.
|
||||
*/
|
||||
function assertClientDeclaredSkills(
|
||||
mcpServer: McpServer,
|
||||
method: string,
|
||||
envelope: ReceivedEnvelope,
|
||||
): void {
|
||||
const modernCapabilities = envelope?.[CLIENT_CAPABILITIES_META_KEY];
|
||||
const capabilities =
|
||||
modernCapabilities ?? mcpServer.server.getClientCapabilities();
|
||||
if (hasSkillsExtension(capabilities)) return;
|
||||
const message = `${method} requires the client to declare ${SKILLS_EXTENSION_KEY} in its capabilities`;
|
||||
// The two eras name this refusal differently. SEP-2575 gives a modern
|
||||
// request that needs an undeclared capability its own code, `-32021`
|
||||
// MissingRequiredClientCapability (HTTP 400), carrying the missing
|
||||
// capabilities in `data.requiredCapabilities` so the client can see what to
|
||||
// declare. `-32601` there would claim the method does not exist. The legacy
|
||||
// era has no such code, so a legacy refusal stays `-32601`.
|
||||
if (modernCapabilities !== undefined) {
|
||||
throw new MissingRequiredClientCapabilityError(
|
||||
{ requiredCapabilities: { extensions: { [SKILLS_EXTENSION_KEY]: {} } } },
|
||||
message,
|
||||
);
|
||||
}
|
||||
throw new ProtocolError(ProtocolErrorCode.MethodNotFound, message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Wire `skills/list`, `skills/get` and the `skill://` half of `resources/read`
|
||||
* onto an `McpServer`.
|
||||
*/
|
||||
export function wireSkillsHandlers(mcpServer: McpServer): void {
|
||||
export function wireSkillsHandlers(
|
||||
mcpServer: McpServer,
|
||||
options: WireSkillsOptions = {},
|
||||
): void {
|
||||
const lowLevel = mcpServer.server;
|
||||
// Only the extension's own methods are gated. A `skill://` file is read
|
||||
// through ordinary `resources/read`, a core method the client needs no
|
||||
// extension to call.
|
||||
const gate = (method: string, envelope: ReceivedEnvelope): void => {
|
||||
if (options.requireClientExtension) {
|
||||
assertClientDeclaredSkills(mcpServer, method, envelope);
|
||||
}
|
||||
};
|
||||
|
||||
lowLevel.setRequestHandler(
|
||||
"skills/list",
|
||||
{ params: ListSkillsParamsSchema, result: ListSkillsResultShape },
|
||||
async (params) => listSkillsPage(params.cursor),
|
||||
async (params, ctx) => {
|
||||
gate("skills/list", ctx.mcpReq.envelope);
|
||||
return listSkillsPage(params.cursor);
|
||||
},
|
||||
);
|
||||
|
||||
lowLevel.setRequestHandler(
|
||||
"skills/get",
|
||||
{ params: GetSkillParamsSchema, result: GetSkillResultShape },
|
||||
async (params) => getSkillEntry(params.uri),
|
||||
async (params, ctx) => {
|
||||
gate("skills/get", ctx.mcpReq.envelope);
|
||||
return getSkillEntry(params.uri);
|
||||
},
|
||||
);
|
||||
|
||||
lowLevel.setRequestHandler(
|
||||
"resources/directory/read",
|
||||
{ params: DirectoryReadParamsSchema, result: DirectoryReadResultShape },
|
||||
async (params) => {
|
||||
async (params, ctx) => {
|
||||
gate("resources/directory/read", ctx.mcpReq.envelope);
|
||||
const page = readDirectoryPage(params.uri, params.cursor);
|
||||
// `-32602` for both "no such URI" and "exists but is not a directory",
|
||||
// which is what SEP-2640 specifies — the same code `resources/read` uses
|
||||
|
|
|
|||
Loading…
Reference in a new issue