feat: support the Skills extension (SEP-2640) with digest verification
Adds detection, enumeration, and conformance checking for the Skills extension (`io.modelcontextprotocol/skills`) — phases 1 and 2 of #2234, which is everything its Acceptance list names. Skills is a *server*-declared extension, read off the connecting server's `capabilities.extensions`, so it is deliberately absent from `ADVERTISABLE_EXTENSIONS` — that registry is what the Inspector advertises and the user toggles, and an entry there would be a meaningless toggle. `skills/list` and `skills/get` are consumer-owned extension methods that neither era codec defines, so the SDK's era gate skips them and they go out as ordinary `client.request` calls with explicit result schemas. The raw-wire channel modern `tasks/*` needs is not used, and the Skills tab is not era-gated: a legacy-era server that declares the extension is serving it. The checks are the point, not the list view. `core/mcp/skills.ts` reports each obligation SEP-2640 states — the name/path invariant, the digest format, the 512-entry and 16 MiB limits, and `resources: "dynamic"`, which means integrity cannot be verified at all. Digest verification hashes the fetched bytes with WebCrypto and returns a mismatch with both digests attached rather than throwing, because showing a mismatch loudly is the whole value proposition. Files are fetched on demand: SEP-2640 is explicit that a `resources/read` of a `SKILL.md` is not a load and confers no standing, so none of the SEP's host machinery is implemented. The `skills-http` fixture serves four skills over two pages, three of them deliberately non-conforming — without those the verification code is untestable. Phase 3 (CLI, TUI, `resources/directory/read`, paged mode) is #2248. Closes #2234 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018kfrH3uRNUM4JAk1reKtyY Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
1c1e70820b
commit
63623b754b
5 changed files with 3684 additions and 4 deletions
140
clients/web/src/test/core/mcp/inspectorClient-skills.test.ts
Normal file
140
clients/web/src/test/core/mcp/inspectorClient-skills.test.ts
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import type { ServerCapabilities } from "@modelcontextprotocol/client";
|
||||
import { SKILLS_EXTENSION_KEY } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
|
||||
/**
|
||||
* Unit coverage for the Skills extension methods (#2234, SEP-2640).
|
||||
*
|
||||
* The SDK client is stubbed rather than connected: what these assert is the
|
||||
* shape of the outbound request and the normalization of the result, both of
|
||||
* which are decided entirely in `InspectorClient` — and the point worth pinning
|
||||
* is that `skills/*` go out through the ordinary `client.request` path with an
|
||||
* explicit result schema, NOT through the raw-wire channel modern `tasks/*`
|
||||
* needs.
|
||||
*/
|
||||
describe("InspectorClient skills methods (#2234)", () => {
|
||||
const ENTRY = {
|
||||
uri: "skill://demo/SKILL.md",
|
||||
frontmatter: { name: "demo", description: "A demo skill" },
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://demo/ref.md",
|
||||
digest: `sha256:${"a".repeat(64)}`,
|
||||
size: 3,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
interface SkillsInternals {
|
||||
client: {
|
||||
request: (
|
||||
req: { method: string; params: Record<string, unknown> },
|
||||
schema: { parse: (value: unknown) => unknown },
|
||||
) => Promise<unknown>;
|
||||
} | null;
|
||||
capabilities: ServerCapabilities | undefined;
|
||||
}
|
||||
|
||||
function makeClient(): InspectorClient {
|
||||
return new InspectorClient(
|
||||
{ type: "stdio", command: "noop", args: [] },
|
||||
// `environment.transport` is only used on connect(); these tests never
|
||||
// connect, they stub the SDK client directly.
|
||||
{ environment: { transport: () => ({}) as never } },
|
||||
);
|
||||
}
|
||||
|
||||
function internals(client: InspectorClient): SkillsInternals {
|
||||
return client as unknown as SkillsInternals;
|
||||
}
|
||||
|
||||
/** Stub the SDK client so `request` parses through the supplied schema. */
|
||||
function stubRequest(client: InspectorClient, result: unknown) {
|
||||
const request = vi.fn(
|
||||
async (
|
||||
_req: { method: string; params: Record<string, unknown> },
|
||||
schema: { parse: (value: unknown) => unknown },
|
||||
) => schema.parse(result),
|
||||
);
|
||||
internals(client).client = { request };
|
||||
return request;
|
||||
}
|
||||
|
||||
it("getSkillsExtension reads the server's declaration", () => {
|
||||
const client = makeClient();
|
||||
expect(client.getSkillsExtension()).toBeUndefined();
|
||||
internals(client).capabilities = {
|
||||
extensions: { [SKILLS_EXTENSION_KEY]: { directoryRead: true } },
|
||||
} as ServerCapabilities;
|
||||
expect(client.getSkillsExtension()).toEqual({ directoryRead: true });
|
||||
});
|
||||
|
||||
it("listSkills throws when not connected", async () => {
|
||||
await expect(makeClient().listSkills()).rejects.toThrow(/not connected/i);
|
||||
});
|
||||
|
||||
it("getSkill throws when not connected", async () => {
|
||||
await expect(makeClient().getSkill("skill://x/SKILL.md")).rejects.toThrow(
|
||||
/not connected/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("sends skills/list with no cursor on the first page", async () => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, { skills: [ENTRY] });
|
||||
const page = await client.listSkills();
|
||||
expect(request.mock.calls[0][0].method).toBe("skills/list");
|
||||
expect(request.mock.calls[0][0].params).not.toHaveProperty("cursor");
|
||||
expect(page.skills).toEqual([ENTRY]);
|
||||
expect(page.nextCursor).toBeUndefined();
|
||||
});
|
||||
|
||||
it("forwards a cursor and returns the server's nextCursor", async () => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, { skills: [], nextCursor: "4" });
|
||||
const page = await client.listSkills("2");
|
||||
expect(request.mock.calls[0][0].params.cursor).toBe("2");
|
||||
expect(page.nextCursor).toBe("4");
|
||||
});
|
||||
|
||||
it("stamps call metadata onto skills/list as _meta", async () => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, { skills: [] });
|
||||
await client.listSkills(undefined, { trace: "abc" });
|
||||
expect(request.mock.calls[0][0].params._meta).toMatchObject({
|
||||
trace: "abc",
|
||||
});
|
||||
});
|
||||
|
||||
it("sends skills/get with the requested uri", async () => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, { skill: ENTRY });
|
||||
await client.getSkill("skill://demo/SKILL.md");
|
||||
expect(request.mock.calls[0][0].method).toBe("skills/get");
|
||||
expect(request.mock.calls[0][0].params.uri).toBe("skill://demo/SKILL.md");
|
||||
});
|
||||
|
||||
it("normalizes the enveloped skills/get result to the entry", async () => {
|
||||
const client = makeClient();
|
||||
stubRequest(client, { skill: ENTRY });
|
||||
expect(await client.getSkill("skill://demo/SKILL.md")).toEqual(ENTRY);
|
||||
});
|
||||
|
||||
it("normalizes the inline skills/get result to the entry", async () => {
|
||||
// The SEP settles the entry shape but not the envelope; a server that
|
||||
// returns the entry at the top level must not fail here.
|
||||
const client = makeClient();
|
||||
stubRequest(client, ENTRY);
|
||||
expect(await client.getSkill("skill://demo/SKILL.md")).toEqual(ENTRY);
|
||||
});
|
||||
|
||||
it("rejects a skills/list result that is not a skills page", async () => {
|
||||
// The explicit result schema is the whole client-side mechanism for a
|
||||
// consumer-owned extension method, so a nonconforming result must fail
|
||||
// here rather than reaching the UI as a half-parsed shape.
|
||||
const client = makeClient();
|
||||
stubRequest(client, { notSkills: true });
|
||||
await expect(client.listSkills()).rejects.toBeDefined();
|
||||
});
|
||||
});
|
||||
333
clients/web/src/test/core/mcp/skills.test.ts
Normal file
333
clients/web/src/test/core/mcp/skills.test.ts
Normal file
|
|
@ -0,0 +1,333 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import type { ServerCapabilities } from "@modelcontextprotocol/client";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas";
|
||||
import { SKILLS_EXTENSION_KEY } from "@inspector/core/mcp/skillsSchemas";
|
||||
import {
|
||||
SKILL_MAX_RESOURCE_ENTRIES,
|
||||
SKILL_MAX_TOTAL_BYTES,
|
||||
base64ToBytes,
|
||||
checkSkillConformance,
|
||||
getSkillsExtension,
|
||||
isSkillsExtensionSupported,
|
||||
sha256Digest,
|
||||
skillDisplayName,
|
||||
skillNameFromUri,
|
||||
textToBytes,
|
||||
totalSkillBytes,
|
||||
verifySkillResource,
|
||||
} from "@inspector/core/mcp/skills";
|
||||
|
||||
/** The digest of the string "hello", precomputed so the assertion is a fact
|
||||
* about SHA-256 rather than a restatement of what the code just did. */
|
||||
const HELLO_SHA256 =
|
||||
"sha256:2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824";
|
||||
|
||||
function entry(overrides: Partial<SkillEntry> = {}): SkillEntry {
|
||||
return {
|
||||
uri: "skill://demo/SKILL.md",
|
||||
frontmatter: { name: "demo", description: "A demo skill" },
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://demo/ref.md",
|
||||
digest: `sha256:${"a".repeat(64)}`,
|
||||
size: 10,
|
||||
},
|
||||
],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function caps(extensions?: Record<string, unknown>): ServerCapabilities {
|
||||
return { ...(extensions ? { extensions } : {}) } as ServerCapabilities;
|
||||
}
|
||||
|
||||
describe("getSkillsExtension", () => {
|
||||
it("returns undefined when the server declared no extensions at all", () => {
|
||||
expect(getSkillsExtension(undefined)).toBeUndefined();
|
||||
expect(getSkillsExtension(caps())).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined when other extensions are declared but not skills", () => {
|
||||
expect(
|
||||
getSkillsExtension(caps({ "io.modelcontextprotocol/tasks": {} })),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it("reports directoryRead false for a bare declaration", () => {
|
||||
expect(getSkillsExtension(caps({ [SKILLS_EXTENSION_KEY]: {} }))).toEqual({
|
||||
directoryRead: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("reports directoryRead only for a literal true", () => {
|
||||
expect(
|
||||
getSkillsExtension(
|
||||
caps({ [SKILLS_EXTENSION_KEY]: { directoryRead: true } }),
|
||||
),
|
||||
).toEqual({ directoryRead: true });
|
||||
// A truthy non-`true` value is a non-conforming advertisement; treating it
|
||||
// as support would make the Inspector call a method the server may not
|
||||
// serve, so it reads as unsupported.
|
||||
expect(
|
||||
getSkillsExtension(
|
||||
caps({ [SKILLS_EXTENSION_KEY]: { directoryRead: "yes" } }),
|
||||
),
|
||||
).toEqual({ directoryRead: false });
|
||||
});
|
||||
|
||||
it("treats a declared-but-null value as no declaration", () => {
|
||||
expect(
|
||||
getSkillsExtension(caps({ [SKILLS_EXTENSION_KEY]: null })),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it("treats a non-object declaration as declared with no sub-options", () => {
|
||||
expect(getSkillsExtension(caps({ [SKILLS_EXTENSION_KEY]: true }))).toEqual({
|
||||
directoryRead: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("isSkillsExtensionSupported mirrors presence", () => {
|
||||
expect(isSkillsExtensionSupported(caps())).toBe(false);
|
||||
expect(
|
||||
isSkillsExtensionSupported(caps({ [SKILLS_EXTENSION_KEY]: {} })),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("skillNameFromUri", () => {
|
||||
it("returns the segment before /SKILL.md, not the filename", () => {
|
||||
expect(skillNameFromUri("skill://a/b/data-analysis/SKILL.md")).toBe(
|
||||
"data-analysis",
|
||||
);
|
||||
});
|
||||
|
||||
it("returns undefined for a URI that does not end in /SKILL.md", () => {
|
||||
expect(skillNameFromUri("skill://demo/other.md")).toBeUndefined();
|
||||
// The suffix must include the separator: a bare "SKILL.md" has no segment.
|
||||
expect(skillNameFromUri("SKILL.md")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined when the segment before the suffix is empty", () => {
|
||||
expect(skillNameFromUri("skill:///SKILL.md")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("skillDisplayName", () => {
|
||||
it("prefers the declared frontmatter name", () => {
|
||||
expect(skillDisplayName(entry())).toBe("demo");
|
||||
});
|
||||
|
||||
it("falls back to the URI segment when the name is blank", () => {
|
||||
expect(skillDisplayName(entry({ frontmatter: { name: " " } }))).toBe(
|
||||
"demo",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the raw URI when neither is available", () => {
|
||||
expect(skillDisplayName(entry({ uri: "skill://x", frontmatter: {} }))).toBe(
|
||||
"skill://x",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("checkSkillConformance", () => {
|
||||
it("reports nothing for a conforming entry", () => {
|
||||
expect(checkSkillConformance(entry())).toEqual([]);
|
||||
});
|
||||
|
||||
it("reports a missing name as an error", () => {
|
||||
const issues = checkSkillConformance(
|
||||
entry({ frontmatter: { description: "d" } }),
|
||||
);
|
||||
expect(issues.map((i) => i.code)).toEqual(["missing-name"]);
|
||||
expect(issues[0].severity).toBe("error");
|
||||
});
|
||||
|
||||
it("reports a missing description as a warning", () => {
|
||||
const issues = checkSkillConformance(
|
||||
entry({ frontmatter: { name: "demo" } }),
|
||||
);
|
||||
expect(issues.map((i) => i.code)).toEqual(["missing-description"]);
|
||||
expect(issues[0].severity).toBe("warning");
|
||||
});
|
||||
|
||||
it("reports a URI that does not carry a skill path", () => {
|
||||
const issues = checkSkillConformance(entry({ uri: "skill://demo/x.md" }));
|
||||
expect(issues.map((i) => i.code)).toContain("malformed-uri");
|
||||
// The name/path check is suppressed: there is no path segment to compare,
|
||||
// and reporting both would present one defect as two.
|
||||
expect(issues.map((i) => i.code)).not.toContain("name-path-mismatch");
|
||||
});
|
||||
|
||||
it("reports a path segment that disagrees with frontmatter.name", () => {
|
||||
const issues = checkSkillConformance(
|
||||
entry({ uri: "skill://wrong-folder/SKILL.md" }),
|
||||
);
|
||||
const mismatch = issues.find((i) => i.code === "name-path-mismatch");
|
||||
expect(mismatch?.severity).toBe("error");
|
||||
expect(mismatch?.message).toContain("wrong-folder");
|
||||
expect(mismatch?.message).toContain("demo");
|
||||
});
|
||||
|
||||
it("does not report a mismatch when the name is missing entirely", () => {
|
||||
// The missing name is already an error of its own; a second finding
|
||||
// comparing against an absent value would be noise.
|
||||
const issues = checkSkillConformance(
|
||||
entry({ uri: "skill://other/SKILL.md", frontmatter: {} }),
|
||||
);
|
||||
expect(issues.map((i) => i.code)).not.toContain("name-path-mismatch");
|
||||
expect(issues.map((i) => i.code)).toContain("missing-name");
|
||||
});
|
||||
|
||||
it("reports dynamic resources as a warning and checks nothing further", () => {
|
||||
const issues = checkSkillConformance(entry({ resources: "dynamic" }));
|
||||
expect(issues.map((i) => i.code)).toEqual(["dynamic-resources"]);
|
||||
expect(issues[0].severity).toBe("warning");
|
||||
});
|
||||
|
||||
it("reports a manifest entry with no digest as unverifiable", () => {
|
||||
const issues = checkSkillConformance(
|
||||
entry({ resources: [{ uri: "skill://demo/ref.md", size: 1 }] }),
|
||||
);
|
||||
expect(issues.map((i) => i.code)).toEqual(["missing-digest"]);
|
||||
expect(issues[0].resourceUri).toBe("skill://demo/ref.md");
|
||||
});
|
||||
|
||||
it("reports a digest that is not sha256 + 64 lowercase hex", () => {
|
||||
for (const digest of [
|
||||
"sha256:XYZ",
|
||||
`sha256:${"A".repeat(64)}`,
|
||||
`sha512:${"a".repeat(64)}`,
|
||||
`sha256:${"a".repeat(63)}`,
|
||||
]) {
|
||||
const issues = checkSkillConformance(
|
||||
entry({ resources: [{ uri: "skill://demo/ref.md", digest }] }),
|
||||
);
|
||||
expect(issues.map((i) => i.code)).toEqual(["malformed-digest"]);
|
||||
}
|
||||
});
|
||||
|
||||
it("reports a manifest over the 512-entry limit", () => {
|
||||
const resources = Array.from(
|
||||
{ length: SKILL_MAX_RESOURCE_ENTRIES + 1 },
|
||||
(_unused, i) => ({
|
||||
uri: `skill://demo/f${i}.md`,
|
||||
digest: `sha256:${"a".repeat(64)}`,
|
||||
}),
|
||||
);
|
||||
const issues = checkSkillConformance(entry({ resources }));
|
||||
expect(issues.map((i) => i.code)).toContain("resource-limit-exceeded");
|
||||
});
|
||||
|
||||
it("reports a manifest over the 16 MiB limit", () => {
|
||||
const issues = checkSkillConformance(
|
||||
entry({
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://demo/big.bin",
|
||||
digest: `sha256:${"a".repeat(64)}`,
|
||||
size: SKILL_MAX_TOTAL_BYTES + 1,
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(issues.map((i) => i.code)).toContain("size-limit-exceeded");
|
||||
});
|
||||
|
||||
it("does not report the size limit at exactly the boundary", () => {
|
||||
const issues = checkSkillConformance(
|
||||
entry({
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://demo/big.bin",
|
||||
digest: `sha256:${"a".repeat(64)}`,
|
||||
size: SKILL_MAX_TOTAL_BYTES,
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(issues).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("totalSkillBytes", () => {
|
||||
it("sums declared sizes and treats a missing size as zero", () => {
|
||||
expect(
|
||||
totalSkillBytes([
|
||||
{ uri: "a", size: 10 },
|
||||
{ uri: "b" },
|
||||
{ uri: "c", size: 5 },
|
||||
]),
|
||||
).toBe(15);
|
||||
});
|
||||
});
|
||||
|
||||
describe("byte helpers", () => {
|
||||
it("textToBytes produces UTF-8, not code units", () => {
|
||||
// "é" is two bytes in UTF-8 and one JS code unit — the digest is over the
|
||||
// former, so a naive per-char encoding would verify the wrong thing.
|
||||
expect(Array.from(textToBytes("é"))).toEqual([0xc3, 0xa9]);
|
||||
});
|
||||
|
||||
it("base64ToBytes decodes standard base64", () => {
|
||||
expect(Array.from(base64ToBytes("aGVsbG8="))).toEqual([
|
||||
104, 101, 108, 108, 111,
|
||||
]);
|
||||
});
|
||||
|
||||
it("sha256Digest matches the known digest of 'hello'", async () => {
|
||||
expect(await sha256Digest(textToBytes("hello"))).toBe(HELLO_SHA256);
|
||||
});
|
||||
|
||||
it("sha256Digest hashes only the view, not the whole backing buffer", async () => {
|
||||
// A Uint8Array can be a window into a larger ArrayBuffer. Hashing the
|
||||
// buffer instead of the view would silently digest neighbouring bytes.
|
||||
const backing = new Uint8Array([0xff, ...textToBytes("hello"), 0xff]);
|
||||
const view = backing.subarray(1, 6);
|
||||
expect(await sha256Digest(view)).toBe(HELLO_SHA256);
|
||||
});
|
||||
});
|
||||
|
||||
describe("verifySkillResource", () => {
|
||||
it("verifies matching bytes", async () => {
|
||||
const result = await verifySkillResource(
|
||||
{ uri: "skill://demo/a.md", digest: HELLO_SHA256 },
|
||||
textToBytes("hello"),
|
||||
);
|
||||
expect(result.status).toBe("verified");
|
||||
expect(result.actualDigest).toBe(HELLO_SHA256);
|
||||
});
|
||||
|
||||
it("reports a mismatch with both digests instead of throwing", async () => {
|
||||
const expected = `sha256:${"b".repeat(64)}`;
|
||||
const result = await verifySkillResource(
|
||||
{ uri: "skill://demo/a.md", digest: expected },
|
||||
textToBytes("hello"),
|
||||
);
|
||||
expect(result.status).toBe("mismatch");
|
||||
expect(result.expectedDigest).toBe(expected);
|
||||
expect(result.actualDigest).toBe(HELLO_SHA256);
|
||||
});
|
||||
|
||||
it("reports unverifiable when no digest is advertised", async () => {
|
||||
const result = await verifySkillResource(
|
||||
{ uri: "skill://demo/a.md" },
|
||||
textToBytes("hello"),
|
||||
);
|
||||
expect(result.status).toBe("unverifiable");
|
||||
expect(result.actualDigest).toBeUndefined();
|
||||
});
|
||||
|
||||
it("reports unverifiable — not a mismatch — for a malformed digest", async () => {
|
||||
// A malformed digest is already a conformance finding; calling it a
|
||||
// mismatch would accuse the file's bytes of being wrong when the manifest
|
||||
// is what is broken.
|
||||
const result = await verifySkillResource(
|
||||
{ uri: "skill://demo/a.md", digest: "sha256:nope" },
|
||||
textToBytes("hello"),
|
||||
);
|
||||
expect(result.status).toBe("unverifiable");
|
||||
expect(result.expectedDigest).toBe("sha256:nope");
|
||||
});
|
||||
});
|
||||
121
clients/web/src/test/core/mcp/skillsSchemas.test.ts
Normal file
121
clients/web/src/test/core/mcp/skillsSchemas.test.ts
Normal file
|
|
@ -0,0 +1,121 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
DYNAMIC_RESOURCES,
|
||||
GetSkillResultSchema,
|
||||
ListSkillsResultSchema,
|
||||
ReadResourceDirectoryResultSchema,
|
||||
SKILLS_EXTENSION_KEY,
|
||||
SKILLS_GET_METHOD,
|
||||
SKILLS_LIST_METHOD,
|
||||
SkillEntrySchema,
|
||||
normalizeGetSkillResult,
|
||||
} from "@inspector/core/mcp/skillsSchemas";
|
||||
|
||||
const ENTRY = {
|
||||
uri: "skill://demo/SKILL.md",
|
||||
frontmatter: { name: "demo", description: "A demo skill" },
|
||||
resources: [
|
||||
{ uri: "skill://demo/ref.md", digest: `sha256:${"a".repeat(64)}`, size: 3 },
|
||||
],
|
||||
};
|
||||
|
||||
describe("skills wire constants", () => {
|
||||
it("names the extension and its two required methods", () => {
|
||||
expect(SKILLS_EXTENSION_KEY).toBe("io.modelcontextprotocol/skills");
|
||||
expect(SKILLS_LIST_METHOD).toBe("skills/list");
|
||||
expect(SKILLS_GET_METHOD).toBe("skills/get");
|
||||
});
|
||||
});
|
||||
|
||||
describe("SkillEntrySchema", () => {
|
||||
it("parses a full manifest entry", () => {
|
||||
expect(SkillEntrySchema.parse(ENTRY)).toEqual(ENTRY);
|
||||
});
|
||||
|
||||
it("parses the dynamic form", () => {
|
||||
const dynamic = { ...ENTRY, resources: DYNAMIC_RESOURCES };
|
||||
expect(SkillEntrySchema.parse(dynamic).resources).toBe("dynamic");
|
||||
});
|
||||
|
||||
it("passes unknown frontmatter fields through untouched", () => {
|
||||
// The skill *format* versions independently of this extension, so an
|
||||
// unrecognized frontmatter field is a future Agent Skills field, not junk.
|
||||
const parsed = SkillEntrySchema.parse({
|
||||
...ENTRY,
|
||||
frontmatter: { ...ENTRY.frontmatter, license: "MIT" },
|
||||
});
|
||||
expect(parsed.frontmatter.license).toBe("MIT");
|
||||
});
|
||||
|
||||
it("accepts a malformed digest rather than rejecting the entry", () => {
|
||||
// Rejecting here would turn a reportable server bug into a parse failure,
|
||||
// and the Inspector exists to report it. See `checkSkillConformance`.
|
||||
const parsed = SkillEntrySchema.parse({
|
||||
...ENTRY,
|
||||
resources: [{ uri: "skill://demo/ref.md", digest: "nope" }],
|
||||
});
|
||||
expect(parsed.resources).toEqual([
|
||||
{ uri: "skill://demo/ref.md", digest: "nope" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("rejects an entry with no uri", () => {
|
||||
expect(() =>
|
||||
SkillEntrySchema.parse({ frontmatter: {}, resources: [] }),
|
||||
).toThrow();
|
||||
});
|
||||
|
||||
it("rejects a resources value that is neither a list nor 'dynamic'", () => {
|
||||
expect(() =>
|
||||
SkillEntrySchema.parse({ ...ENTRY, resources: "static" }),
|
||||
).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("ListSkillsResultSchema", () => {
|
||||
it("parses a page with a cursor", () => {
|
||||
const parsed = ListSkillsResultSchema.parse({
|
||||
skills: [ENTRY],
|
||||
nextCursor: "2",
|
||||
});
|
||||
expect(parsed.skills).toHaveLength(1);
|
||||
expect(parsed.nextCursor).toBe("2");
|
||||
});
|
||||
|
||||
it("parses a final page with no cursor", () => {
|
||||
expect(
|
||||
ListSkillsResultSchema.parse({ skills: [] }).nextCursor,
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("GetSkillResultSchema", () => {
|
||||
it("normalizes the enveloped form to the entry", () => {
|
||||
expect(GetSkillResultSchema.parse({ skill: ENTRY })).toEqual(ENTRY);
|
||||
});
|
||||
|
||||
it("normalizes the inline form to the entry", () => {
|
||||
expect(GetSkillResultSchema.parse(ENTRY)).toEqual(ENTRY);
|
||||
});
|
||||
|
||||
it("normalizeGetSkillResult accepts either shape directly", () => {
|
||||
expect(normalizeGetSkillResult({ skill: ENTRY })).toEqual(ENTRY);
|
||||
expect(normalizeGetSkillResult(ENTRY)).toEqual(ENTRY);
|
||||
});
|
||||
|
||||
it("rejects a result that is neither shape", () => {
|
||||
expect(() => GetSkillResultSchema.parse({ nothing: true })).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("ReadResourceDirectoryResultSchema", () => {
|
||||
it("parses directory children including the directory mime type", () => {
|
||||
const parsed = ReadResourceDirectoryResultSchema.parse({
|
||||
contents: [
|
||||
{ uri: "skill://demo/sub", mimeType: "inode/directory" },
|
||||
{ uri: "skill://demo/ref.md", mimeType: "text/markdown", size: 3 },
|
||||
],
|
||||
});
|
||||
expect(parsed.contents).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
178
clients/web/src/test/core/mcp/state/managedSkillsState.test.ts
Normal file
178
clients/web/src/test/core/mcp/state/managedSkillsState.test.ts
Normal file
|
|
@ -0,0 +1,178 @@
|
|||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas";
|
||||
import {
|
||||
ManagedSkillsState,
|
||||
REPEATED_CURSOR_MESSAGE,
|
||||
} from "@inspector/core/mcp/state/managedSkillsState";
|
||||
import { FakeInspectorClient } from "@inspector/core/mcp/__tests__/fakeInspectorClient";
|
||||
|
||||
function skill(name: string): SkillEntry {
|
||||
return {
|
||||
uri: `skill://${name}/SKILL.md`,
|
||||
frontmatter: { name, description: `${name} skill` },
|
||||
resources: [],
|
||||
};
|
||||
}
|
||||
|
||||
function waitFor<T>(
|
||||
state: ManagedSkillsState,
|
||||
event: "skillsChange" | "errorChange" | "paginationChange",
|
||||
): Promise<T> {
|
||||
return new Promise((resolve) => {
|
||||
state.addEventListener(
|
||||
event,
|
||||
// The union of detail types across the three events is wider than any one
|
||||
// caller wants, so the resolve is typed at the call site.
|
||||
(e) => resolve(e.detail as T),
|
||||
{ once: true },
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
describe("ManagedSkillsState", () => {
|
||||
let client: FakeInspectorClient;
|
||||
let state: ManagedSkillsState;
|
||||
|
||||
beforeEach(() => {
|
||||
client = new FakeInspectorClient();
|
||||
client.skillsExtension = { directoryRead: false };
|
||||
state = new ManagedSkillsState(client);
|
||||
});
|
||||
|
||||
it("starts empty and returns defensive copies", () => {
|
||||
expect(state.getSkills()).toEqual([]);
|
||||
expect(state.getSkills()).not.toBe(state.getSkills());
|
||||
expect(state.getPagination()).toEqual({ pageCount: 0 });
|
||||
expect(state.getError()).toBeNull();
|
||||
});
|
||||
|
||||
it("refresh no-ops while disconnected", async () => {
|
||||
await state.refresh();
|
||||
expect(client.listSkills).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns an empty list without calling the server when the extension is absent", async () => {
|
||||
// Calling `skills/list` against a server that never declared the extension
|
||||
// gets -32601 and spams the console for a question already answered.
|
||||
client.setStatus("connected");
|
||||
client.skillsExtension = undefined;
|
||||
await state.refresh();
|
||||
expect(client.listSkills).not.toHaveBeenCalled();
|
||||
expect(state.getSkills()).toEqual([]);
|
||||
});
|
||||
|
||||
it("walks every page and reports how many it took", async () => {
|
||||
client.setStatus("connected");
|
||||
client.skillPages = [
|
||||
{ skills: [skill("a"), skill("b")], nextCursor: "2" },
|
||||
{ skills: [skill("c")], nextCursor: undefined },
|
||||
];
|
||||
const skills = await state.refresh();
|
||||
expect(skills.map((s) => s.frontmatter.name)).toEqual(["a", "b", "c"]);
|
||||
expect(state.getPagination()).toEqual({ pageCount: 2 });
|
||||
expect(client.listSkills).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("dispatches skillsChange and paginationChange on a successful walk", async () => {
|
||||
client.setStatus("connected");
|
||||
client.skillPages = [{ skills: [skill("a")] }];
|
||||
const skillsEvent = waitFor<SkillEntry[]>(state, "skillsChange");
|
||||
const paginationEvent = waitFor<{ pageCount: number }>(
|
||||
state,
|
||||
"paginationChange",
|
||||
);
|
||||
await state.refresh();
|
||||
expect(await skillsEvent).toHaveLength(1);
|
||||
expect(await paginationEvent).toEqual({ pageCount: 1 });
|
||||
});
|
||||
|
||||
it("loads on connect", async () => {
|
||||
client.skillPages = [{ skills: [skill("a")] }];
|
||||
const skillsEvent = waitFor<SkillEntry[]>(state, "skillsChange");
|
||||
await client.connect();
|
||||
expect(await skillsEvent).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("stops and reports when the server repeats a cursor", async () => {
|
||||
client.setStatus("connected");
|
||||
// A server stuck on one cursor would otherwise walk forever, so the guard
|
||||
// is what keeps a server bug from becoming a hang.
|
||||
client.listSkills.mockResolvedValue({
|
||||
skills: [skill("a")],
|
||||
nextCursor: "same",
|
||||
});
|
||||
await expect(state.refresh()).rejects.toThrow(REPEATED_CURSOR_MESSAGE);
|
||||
expect(state.getError()?.message).toBe(REPEATED_CURSOR_MESSAGE);
|
||||
});
|
||||
|
||||
it("records a failure as observable state and re-throws it", async () => {
|
||||
client.setStatus("connected");
|
||||
const failure = new Error("boom");
|
||||
client.listSkills.mockRejectedValueOnce(failure);
|
||||
const errorEvent = waitFor<Error | null>(state, "errorChange");
|
||||
await expect(state.refresh()).rejects.toThrow("boom");
|
||||
expect(await errorEvent).toBe(failure);
|
||||
expect(state.getError()).toBe(failure);
|
||||
});
|
||||
|
||||
it("wraps a non-Error rejection", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listSkills.mockRejectedValueOnce("just a string");
|
||||
await expect(state.refresh()).rejects.toBeDefined();
|
||||
expect(state.getError()?.message).toBe("just a string");
|
||||
});
|
||||
|
||||
it("clears the error once a later walk succeeds", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listSkills.mockRejectedValueOnce(new Error("boom"));
|
||||
await expect(state.refresh()).rejects.toThrow();
|
||||
client.skillPages = [{ skills: [skill("a")] }];
|
||||
await state.refresh();
|
||||
expect(state.getError()).toBeNull();
|
||||
});
|
||||
|
||||
it("swallows the connect-time load's rejection rather than leaking it", async () => {
|
||||
// Nobody awaits the connect-time load, so an unhandled rejection would
|
||||
// fail an unrelated test file. The failure still lands on `getError`.
|
||||
client.listSkills.mockRejectedValueOnce(new Error("connect boom"));
|
||||
const errorEvent = waitFor<Error | null>(state, "errorChange");
|
||||
await client.connect();
|
||||
expect((await errorEvent)?.message).toBe("connect boom");
|
||||
});
|
||||
|
||||
it("makes a second refresh a no-op while one is in flight", async () => {
|
||||
client.setStatus("connected");
|
||||
let release: (() => void) | undefined;
|
||||
client.listSkills.mockImplementationOnce(
|
||||
async () =>
|
||||
new Promise((resolve) => {
|
||||
release = () => resolve({ skills: [skill("a")] });
|
||||
}),
|
||||
);
|
||||
const first = state.refresh();
|
||||
const second = await state.refresh();
|
||||
expect(second).toEqual([]);
|
||||
expect(client.listSkills).toHaveBeenCalledTimes(1);
|
||||
release?.();
|
||||
await first;
|
||||
expect(state.getSkills()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("clears the list and the error on disconnect", async () => {
|
||||
client.setStatus("connected");
|
||||
client.skillPages = [{ skills: [skill("a")] }];
|
||||
await state.refresh();
|
||||
await client.disconnect();
|
||||
expect(state.getSkills()).toEqual([]);
|
||||
expect(state.getPagination()).toEqual({ pageCount: 0 });
|
||||
expect(state.getError()).toBeNull();
|
||||
});
|
||||
|
||||
it("destroy unsubscribes so a later connect does not refetch", async () => {
|
||||
state.destroy();
|
||||
await client.connect();
|
||||
expect(client.listSkills).not.toHaveBeenCalled();
|
||||
// Idempotent.
|
||||
state.destroy();
|
||||
});
|
||||
});
|
||||
114
clients/web/src/test/core/react/useManagedSkills.test.tsx
Normal file
114
clients/web/src/test/core/react/useManagedSkills.test.tsx
Normal file
|
|
@ -0,0 +1,114 @@
|
|||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas";
|
||||
import { FakeInspectorClient } from "@inspector/core/mcp/__tests__/fakeInspectorClient";
|
||||
import { ManagedSkillsState } from "@inspector/core/mcp/state/managedSkillsState";
|
||||
import { useManagedSkills } from "@inspector/core/react/useManagedSkills";
|
||||
|
||||
function skill(name: string): SkillEntry {
|
||||
return {
|
||||
uri: `skill://${name}/SKILL.md`,
|
||||
frontmatter: { name, description: `${name} skill` },
|
||||
resources: [],
|
||||
};
|
||||
}
|
||||
|
||||
describe("useManagedSkills", () => {
|
||||
let client: FakeInspectorClient;
|
||||
let state: ManagedSkillsState;
|
||||
|
||||
beforeEach(() => {
|
||||
client = new FakeInspectorClient({ status: "connected" });
|
||||
client.skillsExtension = { directoryRead: false };
|
||||
state = new ManagedSkillsState(client);
|
||||
});
|
||||
|
||||
it("reports the store's current snapshot on first render", async () => {
|
||||
client.skillPages = [{ skills: [skill("a"), skill("b")] }];
|
||||
await state.refresh();
|
||||
|
||||
const { result } = renderHook(() => useManagedSkills(client, state));
|
||||
expect(result.current.skills.map((s) => s.frontmatter.name)).toEqual([
|
||||
"a",
|
||||
"b",
|
||||
]);
|
||||
expect(result.current.pageCount).toBe(1);
|
||||
expect(result.current.error).toBeNull();
|
||||
});
|
||||
|
||||
it("degrades to empty values when no store is attached", async () => {
|
||||
const { result } = renderHook(() => useManagedSkills(client, null));
|
||||
expect(result.current.skills).toEqual([]);
|
||||
expect(result.current.pageCount).toBe(0);
|
||||
expect(result.current.error).toBeNull();
|
||||
// The refresh is still callable and simply resolves to the empty list.
|
||||
await expect(result.current.refresh()).resolves.toEqual([]);
|
||||
});
|
||||
|
||||
it("updates when the store dispatches", async () => {
|
||||
const { result } = renderHook(() => useManagedSkills(client, state));
|
||||
expect(result.current.skills).toEqual([]);
|
||||
|
||||
client.skillPages = [
|
||||
{ skills: [skill("a")], nextCursor: "1" },
|
||||
{ skills: [skill("b")] },
|
||||
];
|
||||
await act(async () => {
|
||||
await state.refresh();
|
||||
});
|
||||
expect(result.current.skills).toHaveLength(2);
|
||||
expect(result.current.pageCount).toBe(2);
|
||||
});
|
||||
|
||||
it("holds the snapshot identity stable across renders with no dispatch", async () => {
|
||||
client.skillPages = [{ skills: [skill("a")] }];
|
||||
await state.refresh();
|
||||
const { result, rerender } = renderHook(() =>
|
||||
useManagedSkills(client, state),
|
||||
);
|
||||
const first = result.current.skills;
|
||||
rerender();
|
||||
// `getSkills()` returns a fresh copy per call, so an uncached snapshot
|
||||
// would hand back a new array every render and defeat every downstream memo.
|
||||
expect(result.current.skills).toBe(first);
|
||||
});
|
||||
|
||||
it("surfaces the store's error", async () => {
|
||||
client.listSkills.mockRejectedValueOnce(new Error("boom"));
|
||||
const { result } = renderHook(() => useManagedSkills(client, state));
|
||||
await act(async () => {
|
||||
await state.refresh().catch(() => {});
|
||||
});
|
||||
expect(result.current.error?.message).toBe("boom");
|
||||
});
|
||||
|
||||
it("refresh drives the store", async () => {
|
||||
const { result } = renderHook(() => useManagedSkills(client, state));
|
||||
client.skillPages = [{ skills: [skill("a")] }];
|
||||
await act(async () => {
|
||||
await result.current.refresh();
|
||||
});
|
||||
expect(result.current.skills).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("swaps to another server's store in the same render", async () => {
|
||||
const other = new FakeInspectorClient({ status: "connected" });
|
||||
other.skillsExtension = { directoryRead: false };
|
||||
const otherState = new ManagedSkillsState(other);
|
||||
other.skillPages = [{ skills: [skill("z")] }];
|
||||
await otherState.refresh();
|
||||
|
||||
client.skillPages = [{ skills: [skill("a")] }];
|
||||
await state.refresh();
|
||||
|
||||
const { result, rerender } = renderHook(
|
||||
({ s }: { s: ManagedSkillsState }) => useManagedSkills(client, s),
|
||||
{ initialProps: { s: state } },
|
||||
);
|
||||
expect(result.current.skills[0].frontmatter.name).toBe("a");
|
||||
rerender({ s: otherState });
|
||||
// Read during render, so the swap lands in the same frame — no frame of
|
||||
// the previous server's skills.
|
||||
expect(result.current.skills[0].frontmatter.name).toBe("z");
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue