diff --git a/docs/advanced/skills.md b/docs/advanced/skills.md new file mode 100644 index 0000000000..dfaab9e2f8 --- /dev/null +++ b/docs/advanced/skills.md @@ -0,0 +1,137 @@ +# Skills + +[SEP-2640](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2640) defines a +convention for serving [Agent Skills](https://agentskills.io/) over MCP. A skill is just a +directory of files — at minimum a `SKILL.md` with YAML frontmatter — that you expose as ordinary +MCP resources, conventionally under a `skill://` URI. + +A server enumerates its skills with `skills/list`, answers for any single one by URI with +`skills/get`, and — optionally — lists a directory's direct children with +`resources/directory/read`. + +The SDK ships this as the built-in `Skills` extension (`io.modelcontextprotocol/skills`). There's +one on the server side and one on the client side. If [Extensions](extensions.md) are new to you, +skim that page first. + +!!! info + `Skills` gives you the **protocol** primitives: request/response handling, capability + advertisement, and SEP-2640 conformance validation. + + It does **not** discover, read, or hash skills from a filesystem. You supply handlers that + answer from wherever your catalog actually lives — a database, a generated index, an in-memory + list, or a directory you walk yourself — and you serve each skill's files as ordinary resources + through `MCPServer.add_resource` or an `@mcp.resource(...)` template handler. + +## Serving a skill + +Here's a server that serves one skill: + +```python title="server.py" hl_lines="31-41 44-51 55" +--8<-- "docs_src/skills/tutorial001.py" +``` + +There are three moves here: + +* `Skill(uri=..., frontmatter=..., resources=[...])` is one entry. It has the same shape whether it + comes back from `skills/list` or `skills/get`. `resources` is the skill's complete file + manifest — every file, `SKILL.md` included, each with a `sha256:...` digest and byte size — or + the string `"dynamic"` for content generated on demand. +* `list_skills` and `get_skill` are plain async callables, invoked once per request. `get_skill` + **must** answer for a skill even if your `list_skills` left it out — SEP-2640 requires a server + to answer by URI for every skill it serves, listed or not. +* `mcp.add_resource(TextResource(uri=SKILL_URI, ...))` registers the skill's actual file content, + served through the SDK's ordinary resource machinery. `Skills` never reads or writes resource + content itself. + +And that's it. `Skills(list_skills=..., get_skill=...)` is all a server needs; +`resources/directory/read` is optional (more on that below). + +## Fetching a skill + +On the client side, `Skills` is a [`ClientExtension`](extensions.md). You register it the same way +you register any other one — by passing it to `Client(extensions=[...])` — and then call `bind` to +get the verbs tied to that connection: + +```python title="client.py" hl_lines="9-11" +--8<-- "docs_src/skills/tutorial001_client.py" +``` + +`skills.bind(client)` hands you a `BoundSkills`, and its methods are the SEP-2640 verbs: + +* `list_skills` and `read_directory` follow `nextCursor` to completion, so a single call gives you + every page's skills or resources. +* `get_skill` costs exactly one request. + +These three validate the server's response against the SEP-2640 conformance rules before returning +it. A name that doesn't match its URI, a digest in the wrong shape, or an incomplete manifest raises +`ValueError` rather than reaching your code. + +`read_skill_uri` is the exception. It's a thin, discoverable alias for `resources/read` that returns +a `ReadResourceResult` (text or blob contents) and validates nothing itself — that's the next step. + +!!! tip + `verify_skill_resource(skill, uri, content)` checks a file's bytes — size, then SHA-256 + digest — against the entry you hold for it. Call it after `read_skill_uri` and *before* you + treat the content as trustworthy. + + `resources/read` returns whatever bytes the server sends *right now*; verification is what ties + those bytes back to the manifest you already validated. It applies to a static manifest only — + a `"dynamic"` skill carries no digests, so calling it on one raises `ValueError`. + +!!! warning + Skill content is untrusted model input, exactly like any other server-provided text. SEP-2640 + requires a host to tag it with its originating server before it reaches the model, and to + never grant the frontmatter's `allowed-tools` field (or any other permission-widening field) + without explicit per-skill user approval. + + A digest match confirms the *bytes*, not the *frontmatter*: it doesn't prove the + `frontmatter` the server advertised in `skills/get` matches the frontmatter inside the fetched + `SKILL.md`. If you act on `skill.frontmatter` — especially `allowed-tools` — parse the fetched + file and compare its frontmatter yourself. + + These are host responsibilities the SDK cannot discharge for you — read the SEP's + [Security Implications](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2640) + section before building a host on top of this extension. + +## Directory reads + +A skill's instructions often point at a directory rather than a file ("pick the matching +template from `templates/`"). `resources/list` can't answer that — it enumerates a server's +entire resource space, not one subtree — so SEP-2640 adds `resources/directory/read`, gated +behind the `directoryRead` capability setting: + +```python +mcp = MCPServer( + "catalog", + extensions=[ + Skills( + list_skills=list_skills, + get_skill=get_skill, + read_directory=read_directory, # lists uri's direct children + ) + ], +) +``` + +Supplying `read_directory` advertises `{"directoryRead": true}` under the extension's +capabilities. Omitting it advertises neither the setting nor the method — a client calling +`resources/directory/read` against such a server gets `METHOD_NOT_FOUND`. On the client side, +`read_directory` raises before it sends anything if the connected server hasn't advertised the +setting. + +## Protocol version and caching + +In protocol version `2026-07-28` and later, `skills/list` and `skills/get` results carry the base +protocol's caching fields, [`ttlMs` and `cacheScope`](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2549) — +the same freshness hint `tools/list`, `resources/list`, and `resources/read` carry. `Skills` fills +`cacheScope` with `"public"` when your handler leaves it unset, and omits both fields entirely on an +older connection. So you don't have to branch on protocol version yourself. + +## What this SDK doesn't do + +`Skills` is a protocol adapter, not a skills provider. It has no opinion on where a skill's bytes +live, how they're indexed, or when a catalog is refreshed — that's for a higher-level library, or +your own handler, to decide. + +If you're looking for "scan this directory and serve whatever's in it," you're looking for a +provider built on top of `Skills`, not `Skills` itself. diff --git a/docs_src/skills/__init__.py b/docs_src/skills/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/docs_src/skills/tutorial001.py b/docs_src/skills/tutorial001.py new file mode 100644 index 0000000000..61cc43c2e8 --- /dev/null +++ b/docs_src/skills/tutorial001.py @@ -0,0 +1,54 @@ +import hashlib +from typing import Any + +from mcp.server.context import ServerRequestContext +from mcp.server.mcpserver import MCPServer +from mcp.server.mcpserver.resources import TextResource +from mcp.server.skills import Skills +from mcp.shared.exceptions import MCPError +from mcp.shared.skills import ( + GetSkillParams, + GetSkillResult, + ListSkillsParams, + ListSkillsResult, + Skill, + SkillResource, +) +from mcp.types import INVALID_PARAMS + +SKILL_URI = "skill://git-workflow/SKILL.md" +SKILL_MD = """\ +--- +name: git-workflow +description: Follow this team's Git conventions for branching and commits +--- + +Branch from `main` using `type/short-description`. Write commit subjects in the +imperative mood, under 72 characters. +""" + +GIT_WORKFLOW = Skill( + uri=SKILL_URI, + frontmatter={"name": "git-workflow", "description": "Follow this team's Git conventions for branching and commits"}, + resources=[ + SkillResource( + uri=SKILL_URI, + digest=f"sha256:{hashlib.sha256(SKILL_MD.encode()).hexdigest()}", + size=len(SKILL_MD.encode()), + ) + ], +) + + +async def list_skills(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + return ListSkillsResult(skills=[GIT_WORKFLOW]) + + +async def get_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + if params.uri != SKILL_URI: + raise MCPError(code=INVALID_PARAMS, message=f"unknown skill: {params.uri}") + return GetSkillResult(skill=GIT_WORKFLOW) + + +mcp = MCPServer("catalog", extensions=[Skills(list_skills=list_skills, get_skill=get_skill)]) +mcp.add_resource(TextResource(uri=SKILL_URI, name="SKILL.md", mime_type="text/markdown", text=SKILL_MD)) diff --git a/docs_src/skills/tutorial001_client.py b/docs_src/skills/tutorial001_client.py new file mode 100644 index 0000000000..4064bc9e95 --- /dev/null +++ b/docs_src/skills/tutorial001_client.py @@ -0,0 +1,24 @@ +import anyio + +from mcp import Client +from mcp.client.skills import Skills, verify_skill_resource +from mcp.types import TextResourceContents + + +async def main() -> None: + skills = Skills() + async with Client("http://localhost:8000/mcp", extensions=[skills]) as client: + catalog = skills.bind(client) + for skill in await catalog.list_skills(): + print(skill.uri, skill.frontmatter["description"]) + + skill = await catalog.get_skill("skill://git-workflow/SKILL.md") + result = await catalog.read_skill_uri(skill.uri) + content = result.contents[0] + if isinstance(content, TextResourceContents): + verify_skill_resource(skill, skill.uri, content.text.encode()) + print(content.text) + + +if __name__ == "__main__": + anyio.run(main) diff --git a/mkdocs.yml b/mkdocs.yml index a75053326f..6d2ee1d526 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -67,6 +67,7 @@ nav: - Middleware: advanced/middleware.md - Extensions: advanced/extensions.md - MCP Apps: advanced/apps.md + - Skills: advanced/skills.md - Troubleshooting: troubleshooting.md - Translations: translations.md - Migration Guide: migration.md diff --git a/src/mcp/client/skills.py b/src/mcp/client/skills.py new file mode 100644 index 0000000000..79b3cafd23 --- /dev/null +++ b/src/mcp/client/skills.py @@ -0,0 +1,198 @@ +"""Client-side Skills extension (SEP-2640). + +`Skills` is an opt-in [`ClientExtension`](../advanced/extensions.md) for talking +to a skills catalog server. Register it with `Client(extensions=[Skills()])`, +then call `bind(client)` for the SEP-2640 verbs — `list_skills`, `get_skill`, +`read_skill_uri`, and `read_directory` — tied to that connection: + + async with Client("http://localhost:8000/mcp", extensions=[skills := Skills()]) as client: + for skill in await skills.bind(client).list_skills(): + print(skill.uri, skill.frontmatter["description"]) + +`bind(client)` returns a `BoundSkills`. Its catalog verbs — `list_skills`, +`get_skill`, and `read_directory` — check that the server advertises the +extension and validate its response; `list_skills` and `read_directory` follow +`nextCursor` to completion, so one call returns every page's results. +`read_skill_uri` is a thin `resources/read` alias that does neither — verify its +result with `verify_skill_resource`. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from mcp_types import ReadResourceResult, Resource + +from mcp.client.extension import ClientExtension +from mcp.client.session import ClientSession +from mcp.shared.skills import ( + EXTENSION_ID, + GetSkillParams, + GetSkillRequest, + GetSkillResult, + ListSkillsParams, + ListSkillsRequest, + ListSkillsResult, + ReadDirectoryParams, + ReadDirectoryRequest, + ReadDirectoryResult, + Skill, + validate_directory_result, +) +from mcp.shared.skills import verify_skill_resource as verify_skill_resource + +if TYPE_CHECKING: + from mcp.client.client import Client + +__all__ = ["BoundSkills", "Skills", "verify_skill_resource"] + + +class Skills(ClientExtension): + """The client-side Skills extension: register, then `bind` for typed verbs. + + Pass an instance to `Client(extensions=[Skills()])` — this advertises + `io.modelcontextprotocol/skills` under the client's capabilities — and call + `bind(client)` once the client is connected for a `BoundSkills` handle. + """ + + identifier = EXTENSION_ID + + def bind(self, client: Client) -> BoundSkills: + """Return the SEP-2640 verbs bound to `client`'s connected session. + + Raises: + RuntimeError: If `client` has not entered its `async with` block yet. + """ + return BoundSkills(client.session) + + +class BoundSkills: + """The SEP-2640 verbs bound to one connected session. + + Obtain it from `Skills.bind(client)`. The catalog verbs — `list_skills`, + `get_skill`, and `read_directory` — check that the server advertises the + extension and validate its response against the SEP-2640 conformance rules + before returning; `list_skills` and `read_directory` also follow + `nextCursor` to completion. `read_skill_uri` is the exception: a thin + `resources/read` alias that neither checks advertisement nor validates — + pair it with `verify_skill_resource`. + """ + + def __init__(self, session: ClientSession) -> None: + self._session = session + + def _require_extension(self, *, directory_read: bool = False) -> None: + capabilities = self._session.server_capabilities + settings = (capabilities.extensions or {}).get(EXTENSION_ID) if capabilities else None + if settings is None: + raise ValueError(f"server does not advertise the {EXTENSION_ID!r} extension") + if directory_read and not settings.get("directoryRead"): + raise ValueError(f"server does not advertise {EXTENSION_ID!r}'s directoryRead setting") + + async def list_skills(self, params: ListSkillsParams | None = None) -> list[Skill]: + """Call `skills/list`, following `nextCursor` to completion, and validate the result. + + A skill a changing catalog surfaces on more than one page is returned once, in the order + first seen. + + Raises: + ValueError: If the server doesn't advertise the Skills extension, its response is not + SEP-2640 conformant, or it repeats a pagination cursor. + MCPError: If the server returns an error response. + """ + self._require_extension() + base = params if params is not None else ListSkillsParams() + cursor = base.cursor + skills: list[Skill] = [] + seen_cursors: set[str] = {cursor} if cursor is not None else set() + seen_uris: set[str] = set() + while True: + # `send_request` parses each page into `ListSkillsResult`, whose validators reject a + # non-conformant skill or a duplicate URI — no separate conformance call is needed. + page = await self._session.send_request( + ListSkillsRequest(params=base.model_copy(update={"cursor": cursor})), ListSkillsResult + ) + # A server stuck repeating its cursor is a loop; catch that before the content checks. + if page.next_cursor is not None and page.next_cursor in seen_cursors: + raise ValueError(f"server repeated skills/list pagination cursor {page.next_cursor!r}") + # A catalog that changes between page fetches can legitimately repeat a skill across + # pages; keep the first occurrence rather than treating it as an error. + for skill in page.skills: + if skill.uri not in seen_uris: + seen_uris.add(skill.uri) + skills.append(skill) + if page.next_cursor is None: + return skills + seen_cursors.add(page.next_cursor) + cursor = page.next_cursor + + async def get_skill(self, uri: str) -> Skill: + """Call `skills/get` for `uri` and validate the result. + + Unlike `list_skills`, this succeeds for a skill absent from any listing — + per SEP-2640, a server MUST answer `skills/get` for every skill it serves. + + Raises: + ValueError: If the server doesn't advertise the Skills extension, its + response names a different skill, or the skill is not conformant. + MCPError: If the server returns an error response, such as `-32602` + for a URI it does not serve. + """ + self._require_extension() + # Parsing `GetSkillResult` validates the skill's own conformance; the requested-URI match + # is the one rule the skill body can't carry, so it stays an explicit check here. + result = await self._session.send_request(GetSkillRequest(params=GetSkillParams(uri=uri)), GetSkillResult) + if result.skill.uri != uri: + raise ValueError(f"server returned skill {result.skill.uri!r} for requested {uri!r}") + return result.skill + + async def read_skill_uri(self, uri: str) -> ReadResourceResult: + """Read a skill file's content via `resources/read`. + + A thin, discoverable alias: works for any `skill://` (or other-scheme) + file regardless of whether the skill was ever enumerated. Verify the + result against a held `Skill` entry with `verify_skill_resource` before + treating it as trusted content — this call does not verify anything itself. + + Raises: + MCPError: If the server returns an error response. + RuntimeError: If the server returns an `InputRequiredResult`; this + alias does not drive the input-required loop. + """ + return await self._session.read_resource(uri) + + async def read_directory(self, uri: str, params: ReadDirectoryParams | None = None) -> list[Resource]: + """Call `resources/directory/read` for `uri`, following `nextCursor` to completion. + + A child a changing directory surfaces on more than one page is returned once, in the order + first seen. + + Raises: + ValueError: If the server doesn't advertise the `directoryRead` setting, its response + is not a valid child listing of `uri`, or it repeats a pagination cursor. + MCPError: If the server returns an error response. + """ + self._require_extension(directory_read=True) + base = params if params is not None else ReadDirectoryParams(uri=uri) + cursor = base.cursor + resources: list[Resource] = [] + seen_cursors: set[str] = {cursor} if cursor is not None else set() + seen_uris: set[str] = set() + while True: + page = await self._session.send_request( + ReadDirectoryRequest(params=base.model_copy(update={"uri": uri, "cursor": cursor})), + ReadDirectoryResult, + ) + # A server stuck repeating its cursor is a loop; catch that before the content checks. + if page.next_cursor is not None and page.next_cursor in seen_cursors: + raise ValueError(f"server repeated resources/directory/read pagination cursor {page.next_cursor!r}") + validate_directory_result(uri, page) + # A changing directory can legitimately repeat a child across pages; keep the first. + for resource in page.resources: + if resource.uri not in seen_uris: + seen_uris.add(resource.uri) + resources.append(resource) + if page.next_cursor is None: + return resources + seen_cursors.add(page.next_cursor) + cursor = page.next_cursor diff --git a/src/mcp/server/skills.py b/src/mcp/server/skills.py new file mode 100644 index 0000000000..667aa39941 --- /dev/null +++ b/src/mcp/server/skills.py @@ -0,0 +1,181 @@ +"""The Skills extension (`io.modelcontextprotocol/skills`, SEP-2640). + +SEP-2640 defines a convention for serving Agent Skills over MCP using the +Resources primitive: a skill is a directory of files, conventionally exposed +under the `skill://` scheme, and enumerated and fetched through two required +methods (`skills/list`, `skills/get`) plus one optional one +(`resources/directory/read`). See +https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2640. + +This module provides the protocol-level plumbing only: request/response +handling, SEP-2640 conformance validation, and capability advertisement. It +does not discover, read, or hash skills from a filesystem — a server author +supplies handlers that answer `skills/list`/`skills/get`/`resources/directory/read` +however their catalog is stored, and serves the underlying `skill://` file +content through the server's ordinary resource-registration APIs +(`MCPServer.add_resource`, or an `@mcp.resource(...)` template). + + async def list_skills(ctx, params): + return ListSkillsResult(skills=[...]) + + async def get_skill(ctx, params): + if params.uri != "skill://git-workflow/SKILL.md": + raise MCPError(code=INVALID_PARAMS, message="unknown skill") + return GetSkillResult(skill=...) + + mcp = MCPServer("catalog", extensions=[Skills(list_skills=list_skills, get_skill=get_skill)]) +""" + +from __future__ import annotations + +import logging +from collections.abc import Awaitable, Callable, Sequence +from typing import Any + +from mcp_types import CacheableResult +from mcp_types.jsonrpc import INTERNAL_ERROR, INVALID_PARAMS +from mcp_types.version import MODERN_PROTOCOL_VERSIONS +from pydantic import ValidationError + +from mcp.server.context import HandlerResult, ServerRequestContext +from mcp.server.extension import Extension, MethodBinding +from mcp.shared.exceptions import MCPError +from mcp.shared.skills import ( + EXTENSION_ID, + METHOD_GET, + METHOD_LIST, + METHOD_READ_DIRECTORY, + GetSkillParams, + GetSkillResult, + ListSkillsParams, + ListSkillsResult, + ReadDirectoryParams, + ReadDirectoryResult, + parse_directory_uri, + skill_name_from_uri, + validate_directory_result, +) + +__all__ = ["Skills"] + +logger = logging.getLogger(__name__) + +ListSkillsHandler = Callable[[ServerRequestContext[Any, Any], ListSkillsParams], Awaitable[ListSkillsResult]] +GetSkillHandler = Callable[[ServerRequestContext[Any, Any], GetSkillParams], Awaitable[GetSkillResult]] +ReadDirectoryHandler = Callable[[ServerRequestContext[Any, Any], ReadDirectoryParams], Awaitable[ReadDirectoryResult]] + + +class Skills(Extension): + """The Skills extension: serve `skills/list`, `skills/get`, and directory reads. + + `list_skills` and `get_skill` are required; a server MUST answer both per + SEP-2640, whether or not a skill appears in the listing. `read_directory` + is optional — supplying it advertises the `directoryRead` capability + setting and serves `resources/directory/read`; omitting it advertises + neither. Handlers run per request, so a catalog that changes over time + (or is too large to enumerate) can return a partial or empty listing. + + Handler error contract: raise `MCPError` to return a specific error to the + caller (e.g. `INVALID_PARAMS` from `get_skill` for a URI it doesn't serve). + A result that isn't SEP-2640 conformant is caught here and reported as + `INTERNAL_ERROR` — a server fault, not the caller's bad params. + """ + + identifier = EXTENSION_ID + + def __init__( + self, + *, + list_skills: ListSkillsHandler, + get_skill: GetSkillHandler, + read_directory: ReadDirectoryHandler | None = None, + ) -> None: + self._list_skills = list_skills + self._get_skill = get_skill + self._read_directory = read_directory + + def settings(self) -> dict[str, Any]: + return {"directoryRead": True} if self._read_directory is not None else {} + + def methods(self) -> Sequence[MethodBinding]: + bindings = [ + MethodBinding(METHOD_LIST, ListSkillsParams, self._handle_list), + MethodBinding(METHOD_GET, GetSkillParams, self._handle_get), + ] + if self._read_directory is not None: + bindings.append(MethodBinding(METHOD_READ_DIRECTORY, ReadDirectoryParams, self._handle_read_directory)) + return bindings + + async def _handle_list(self, ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> HandlerResult: + # `ListSkillsResult`/`Skill` self-validate on construction, so a handler that builds a + # non-conformant listing raises `ValidationError` here — a server fault, surfaced as an + # Internal error rather than the framework's default Invalid params for a bad body. The + # models stay mutable, so re-validate the outbound payload too: a handler that mutates a + # skill after building the result can't ship a non-conformant listing past this point. + try: + result = await self._list_skills(ctx, params) + ListSkillsResult.model_validate(result.model_dump()) + except ValidationError: + logger.exception("list_skills handler returned an invalid result") + raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None + return _finalize_cacheable(result, ctx.protocol_version) + + async def _handle_get(self, ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> HandlerResult: + _require_skill_md_uri(params.uri) + # `Skill` self-validates on construction, and the re-validation guards post-construction + # mutation, both as in `_handle_list`. + try: + result = await self._get_skill(ctx, params) + GetSkillResult.model_validate(result.model_dump()) + except ValidationError: + logger.exception("get_skill handler returned an invalid result") + raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None + if result.skill.uri != params.uri: + logger.error("get_skill handler returned %r for requested %r", result.skill.uri, params.uri) + raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") + return _finalize_cacheable(result, ctx.protocol_version) + + async def _handle_read_directory( + self, ctx: ServerRequestContext[Any, Any], params: ReadDirectoryParams + ) -> HandlerResult: + assert self._read_directory is not None + _require_directory_uri(params.uri) + result = await self._read_directory(ctx, params) + try: + validate_directory_result(params.uri, result) + except ValueError: + logger.exception("read_directory handler returned an invalid result") + raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None + return result + + +def _require_skill_md_uri(uri: str) -> None: + try: + skill_name_from_uri(uri) + except ValueError as exc: + raise MCPError(code=INVALID_PARAMS, message=str(exc)) from exc + + +def _require_directory_uri(uri: str) -> None: + try: + parse_directory_uri(uri) + except ValueError as exc: + raise MCPError(code=INVALID_PARAMS, message=str(exc)) from exc + + +def _finalize_cacheable(result: CacheableResult, protocol_version: str) -> HandlerResult: + """Gate SEP-2549's `ttlMs`/`cacheScope` to protocol version 2026-07-28+. + + `skills/list` and `skills/get` are extension methods, so — unlike a core spec + method — the runner's per-version surface sieve never runs on their results; + nothing else strips these fields for a legacy connection. `CacheableResult` + defaults to `cache_scope="private"`; SEP-2640 calls for `"public"` when unset. + """ + if protocol_version in MODERN_PROTOCOL_VERSIONS: + if "cache_scope" not in result.model_fields_set: + result = result.model_copy(update={"cache_scope": "public"}) + return result + dumped = result.model_dump(by_alias=True, mode="json", exclude_none=True) + dumped.pop("ttlMs", None) + dumped.pop("cacheScope", None) + return dumped diff --git a/src/mcp/shared/skills.py b/src/mcp/shared/skills.py new file mode 100644 index 0000000000..12add5e15d --- /dev/null +++ b/src/mcp/shared/skills.py @@ -0,0 +1,337 @@ +"""Wire types and SEP-2640 conformance checks for the Skills extension. + +Shared by the server (`mcp.server.skills`) and client (`mcp.client.skills`) +surfaces, mirroring how `mcp.shared.extension` hosts the identifier grammar +both tiers need. See https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2640. +""" + +from __future__ import annotations + +import hashlib +import re +from typing import Annotated, Any, Literal +from urllib.parse import urlsplit + +from mcp_types import CacheableResult, PaginatedRequestParams, PaginatedResult, Request, RequestParams, Resource +from pydantic import AfterValidator, BaseModel, ConfigDict, Field, model_validator +from pydantic.alias_generators import to_camel + +__all__ = [ + "EXTENSION_ID", + "METHOD_LIST", + "METHOD_GET", + "METHOD_READ_DIRECTORY", + "MAX_RESOURCES_PER_SKILL", + "MAX_TOTAL_SIZE", + "Frontmatter", + "SkillResource", + "SkillResources", + "Skill", + "ListSkillsParams", + "ListSkillsResult", + "GetSkillParams", + "GetSkillResult", + "ReadDirectoryParams", + "ReadDirectoryResult", + "ListSkillsRequest", + "GetSkillRequest", + "ReadDirectoryRequest", + "skill_name_from_uri", + "parse_directory_uri", + "validate_directory_result", + "verify_skill_resource", +] + +EXTENSION_ID = "io.modelcontextprotocol/skills" +"""The Skills extension identifier, advertised under `ServerCapabilities.extensions`.""" + +METHOD_LIST = "skills/list" +METHOD_GET = "skills/get" +METHOD_READ_DIRECTORY = "resources/directory/read" + +MAX_RESOURCES_PER_SKILL = 512 +"""SEP-2640 per-skill resource-count threshold (`SKILL.md` included). + +A SHOULD NOT limit, not a hard cap: the spec requires a host to support skills +*up to and including* 512 entries and permits it to support larger ones, so a +`Skill` does not reject an over-count manifest.""" + +MAX_TOTAL_SIZE = 16 * 1024 * 1024 +"""SEP-2640 per-skill total-byte-size threshold (16 MiB), summed over `resources[].size`. + +A SHOULD NOT limit, not a hard cap (see `MAX_RESOURCES_PER_SKILL`); an over-size +manifest is not rejected.""" + +_NAME_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") +_DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$") + + +def _check_digest(value: str) -> str: + if not _DIGEST_RE.fullmatch(value): + raise ValueError(f"invalid SHA-256 digest {value!r}; expected 'sha256:' + 64 lowercase hex characters") + return value + + +class _SkillModel(BaseModel): + """Base for Skills value types: matches `mcp_types`' internal `MCPModel` config. + + `MCPModel` itself isn't public; every field defined below is already a + single word, so this only matters if a future field needs camelCase. + """ + + model_config = ConfigDict(alias_generator=to_camel, populate_by_name=True) + + +class SkillResource(_SkillModel): + """One file in a skill's manifest: `{uri, digest, size}`. + + Shape rules are intrinsic: a `digest` that isn't `sha256:` + 64 lowercase + hex characters, or a negative `size`, is rejected at construction. + """ + + uri: str + digest: Annotated[str, AfterValidator(_check_digest)] + """SHA-256 digest of the file's raw bytes, formatted `sha256:{64 hex chars}`.""" + size: Annotated[int, Field(ge=0)] + """Length in bytes of the file's raw content.""" + + +Frontmatter = dict[str, Any] +"""A skill's `SKILL.md` YAML frontmatter, rendered verbatim as JSON.""" + +SkillResources = list[SkillResource] | Literal["dynamic"] +"""A skill's complete resource manifest, or the `"dynamic"` marker (SEP-2640 Resources).""" + + +class Skill(_SkillModel): + """An entry returned by `skills/list` or `skills/get`. + + SEP-2640 conformance is intrinsic: constructing (or parsing) a `Skill` + validates the frontmatter `name`/`description`, and — unless `resources` is + `"dynamic"` — that every entry names a file within the skill's own directory, + with no duplicates and `SKILL.md` present. The `MAX_RESOURCES_PER_SKILL` + (512-entry) and `MAX_TOTAL_SIZE` (16-MiB) limits are SHOULD NOT thresholds, + not MUST NOT, so an over-limit manifest is accepted. + """ + + uri: str + """Resource URI of the skill's `SKILL.md`.""" + frontmatter: Frontmatter + resources: SkillResources + + @model_validator(mode="after") + def _check_conformance(self) -> Skill: + name = skill_name_from_uri(self.uri) + frontmatter_name = self.frontmatter.get("name") + if ( + not isinstance(frontmatter_name, str) + or not _NAME_RE.fullmatch(frontmatter_name) + or len(frontmatter_name) > 64 + ): + raise ValueError(f"skill {self.uri!r} frontmatter name must be 1-64 lowercase, digits, or hyphens") + if frontmatter_name != name: + raise ValueError( + f"skill {self.uri!r} frontmatter name {frontmatter_name!r} does not match URI name {name!r}" + ) + description = self.frontmatter.get("description") + if not isinstance(description, str) or not (1 <= len(description) <= 1024): + raise ValueError(f"skill {self.uri!r} frontmatter description must contain 1 to 1024 characters") + if self.resources == "dynamic": + return self + seen: set[str] = set() + for resource in self.resources: + _validate_resource_uri_in_skill(self.uri, resource.uri) + if resource.uri in seen: + raise ValueError(f"skill {self.uri!r} lists resource {resource.uri!r} more than once") + seen.add(resource.uri) + if self.uri not in seen: + raise ValueError(f"skill {self.uri!r} resources does not include its own SKILL.md") + return self + + +class ListSkillsParams(PaginatedRequestParams): + """Parameters for `skills/list`.""" + + +class ListSkillsResult(PaginatedResult, CacheableResult): + """Result of `skills/list`. + + Each skill self-validates; on top of that, constructing (or parsing) this + result rejects two entries that share a `uri`. + + `ttl_ms`/`cache_scope` are SEP-2549 fields inherited from `CacheableResult`; + unlike a core spec method, nothing sieves them off the wire for a + pre-2026-07-28 connection automatically (see `mcp.server.skills`), so + callers constructing this directly for such a connection must omit them. + """ + + skills: list[Skill] + + @model_validator(mode="after") + def _check_unique_uris(self) -> ListSkillsResult: + seen: set[str] = set() + for skill in self.skills: + if skill.uri in seen: + raise ValueError(f"skills/list result lists skill {skill.uri!r} more than once") + seen.add(skill.uri) + return self + + +class GetSkillParams(RequestParams): + """Parameters for `skills/get`.""" + + uri: str + """URI of the skill's `SKILL.md`.""" + + +class GetSkillResult(CacheableResult): + """Result of `skills/get`. + + Like `ListSkillsResult`, this extends `CacheableResult`: the stable spec page + makes `GetSkillResult` carry SEP-2549's `ttl_ms`/`cache_scope`, the same + freshness hint `resources/read` gives. As on `skills/list`, nothing sieves + these fields off the wire for a pre-2026-07-28 connection automatically (see + `mcp.server.skills`), so a caller constructing this directly for such a + connection must omit them. + """ + + skill: Skill + + +class ReadDirectoryParams(PaginatedRequestParams): + """Parameters for `resources/directory/read`.""" + + uri: str + """URI of the directory resource whose direct children are listed.""" + + +class ReadDirectoryResult(PaginatedResult): + """Result of `resources/directory/read`.""" + + resources: list[Resource] + + +class ListSkillsRequest(Request[ListSkillsParams | None, Literal["skills/list"]]): + method: Literal["skills/list"] = "skills/list" + params: ListSkillsParams | None = None + + +class GetSkillRequest(Request[GetSkillParams, Literal["skills/get"]]): + method: Literal["skills/get"] = "skills/get" + params: GetSkillParams + + +class ReadDirectoryRequest(Request[ReadDirectoryParams, Literal["resources/directory/read"]]): + method: Literal["resources/directory/read"] = "resources/directory/read" + params: ReadDirectoryParams + + +def skill_name_from_uri(uri: str) -> str: + """Return the skill `name` encoded in a `SKILL.md` resource URI. + + Per SEP-2640 Resource Mapping, the final `` segment equals the + skill's `name`; for a bare `skill:///SKILL.md` (no organizational + prefix) that segment is the authority. + + Raises: + ValueError: If `uri` is not an absolute URI ending in `/SKILL.md`. + """ + parts = urlsplit(uri) + if not parts.scheme or parts.query or parts.fragment: + raise ValueError(f"skill URI {uri!r} is not a valid absolute resource URI") + if not parts.path.endswith("/SKILL.md"): + raise ValueError(f"skill URI {uri!r} must end in /SKILL.md") + directory = parts.path[: -len("/SKILL.md")].strip("/") + if directory: + name = directory.rsplit("/", 1)[-1] + else: + name = parts.hostname or "" + if not name: + raise ValueError(f"skill URI {uri!r} has no skill name") + return name + + +def _validate_resource_uri_in_skill(skill_uri: str, resource_uri: str) -> None: + """Raise `ValueError` unless `resource_uri` names a file within `skill_uri`'s directory.""" + skill_parts = urlsplit(skill_uri) + resource_parts = urlsplit(resource_uri) + if not resource_parts.scheme or resource_parts.query or resource_parts.fragment: + raise ValueError(f"resource URI {resource_uri!r} is invalid") + if resource_uri.endswith("/"): + raise ValueError(f"resource URI {resource_uri!r} names a directory, not a file") + if skill_parts.scheme != resource_parts.scheme or skill_parts.netloc != resource_parts.netloc: + raise ValueError(f"resource URI {resource_uri!r} is outside the skill root {skill_uri!r}") + root = skill_parts.path[: -len("/SKILL.md")] + if resource_parts.path != skill_parts.path and not resource_parts.path.startswith(root + "/"): + raise ValueError(f"resource URI {resource_uri!r} is outside the skill root {skill_uri!r}") + if any(segment in (".", "..") for segment in resource_parts.path.split("/")): + raise ValueError(f"resource URI {resource_uri!r} contains a traversal segment") + + +def parse_directory_uri(uri: str) -> tuple[str, str, str]: + """Split a directory resource URI into `(scheme, netloc, path)`. + + Raises: + ValueError: If `uri` has a trailing slash, or is otherwise not a valid + absolute resource URI. + """ + if uri.endswith("/"): + raise ValueError(f"directory URI {uri!r} must not have a trailing slash") + parts = urlsplit(uri) + if not parts.scheme or parts.query or parts.fragment: + raise ValueError(f"directory URI {uri!r} is not a valid absolute resource URI") + return parts.scheme, parts.netloc, parts.path + + +def validate_directory_result(uri: str, result: ReadDirectoryResult) -> None: + """Validate that each entry in `result.resources` is a unique direct child of `uri`. + + Checks containment and shape only — that every listed resource is a direct + child of `uri` with a unique `uri` and `name`. It cannot confirm the listing + is exhaustive, since it has no independent view of the directory's contents. + + Raises: + ValueError: If `uri` is malformed, or any entry is not a direct child, + or two entries share a `uri` or `name`. + """ + scheme, netloc, parent_path = parse_directory_uri(uri) + seen_uris: set[str] = set() + seen_names: set[str] = set() + prefix = parent_path.rstrip("/") + "/" if parent_path.rstrip("/") else "/" + for resource in result.resources: + child = urlsplit(resource.uri) + if not child.scheme or child.query or child.fragment: + raise ValueError(f"directory {uri!r} child has invalid URI {resource.uri!r}") + if child.scheme != scheme or child.netloc != netloc: + raise ValueError(f"resource {resource.uri!r} is not a child of directory {uri!r}") + relative = child.path.removeprefix(prefix) + if relative == child.path or not relative or "/" in relative or relative in (".", ".."): + raise ValueError(f"resource {resource.uri!r} is not a direct child of directory {uri!r}") + if resource.uri in seen_uris or resource.name in seen_names: + raise ValueError(f"directory {uri!r} contains a duplicate child {resource.uri!r}") + seen_uris.add(resource.uri) + seen_names.add(resource.name) + + +def verify_skill_resource(skill: Skill, uri: str, content: bytes) -> None: + """Check that `content` (the bytes read from `uri`) matches `skill`'s manifest entry. + + Recomputes the size and SHA-256 digest of `content` and compares them to the + entry `skill` holds for `uri` — the byte-integrity check SEP-2640 requires + before a host trusts a fetched file. A `"dynamic"` skill carries no digests, + so it has nothing to verify against. + + Raises: + ValueError: If `uri` is not one of `skill`'s resources, `skill.resources` + is `"dynamic"`, or `content`'s size or digest doesn't match the entry. + """ + if skill.resources == "dynamic": + raise ValueError(f"skill {skill.uri!r} has dynamic resources and cannot be integrity-verified") + entry = next((r for r in skill.resources if r.uri == uri), None) + if entry is None: + raise ValueError(f"{uri!r} is not in skill {skill.uri!r}'s held manifest") + if len(content) != entry.size: + raise ValueError(f"resource {uri!r} has size {len(content)}, expected {entry.size}") + digest = f"sha256:{hashlib.sha256(content).hexdigest()}" + if digest != entry.digest: + raise ValueError(f"resource {uri!r} has digest {digest!r}, expected {entry.digest!r}") diff --git a/tests/client/test_skills.py b/tests/client/test_skills.py new file mode 100644 index 0000000000..13a259c191 --- /dev/null +++ b/tests/client/test_skills.py @@ -0,0 +1,345 @@ +"""Tests for the client-side Skills extension (SEP-2640, `mcp.client.skills`).""" + +import hashlib +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager +from typing import Any + +import pytest +from mcp_types import INVALID_PARAMS, Resource, TextResourceContents + +from mcp.client.client import Client +from mcp.client.skills import BoundSkills, verify_skill_resource +from mcp.client.skills import Skills as ClientSkills +from mcp.server.context import ServerRequestContext +from mcp.server.extension import Extension, MethodBinding +from mcp.server.mcpserver import MCPServer +from mcp.server.mcpserver.resources import TextResource +from mcp.server.skills import Skills +from mcp.shared.exceptions import MCPError +from mcp.shared.skills import ( + EXTENSION_ID, + METHOD_GET, + GetSkillParams, + GetSkillResult, + ListSkillsParams, + ListSkillsResult, + ReadDirectoryParams, + ReadDirectoryResult, + Skill, + SkillResource, +) + +pytestmark = pytest.mark.anyio + +_SKILL_URI = "skill://git-workflow/SKILL.md" +_SKILL_CONTENT = "# git-workflow\n" +_SKILL_DIGEST = f"sha256:{hashlib.sha256(_SKILL_CONTENT.encode()).hexdigest()}" + + +def _skill() -> Skill: + return Skill( + uri=_SKILL_URI, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[SkillResource(uri=_SKILL_URI, digest=_SKILL_DIGEST, size=len(_SKILL_CONTENT))], + ) + + +async def _get_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + if params.uri != _SKILL_URI: + raise MCPError(code=INVALID_PARAMS, message="unknown skill") + return GetSkillResult(skill=_skill()) + + +def _paginated_list_handler() -> Any: + """A `list_skills` handler serving two skills across two pages, by URI order.""" + pages = { + None: ([_skill()], "page-2"), + "page-2": ( + [ + Skill( + uri="skill://other/SKILL.md", frontmatter={"name": "other", "description": "d"}, resources="dynamic" + ) + ], + None, + ), + } + + async def handler(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + skills, next_cursor = pages[params.cursor] + return ListSkillsResult(skills=skills, next_cursor=next_cursor) + + return handler + + +def _repeating_cursor_list_handler() -> Any: + async def handler(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + return ListSkillsResult(skills=[_skill()], next_cursor="same-cursor-forever") + + return handler + + +class _NonConformantGetSkill(Extension): + """A server that advertises the extension but answers `skills/get` with the wrong skill - + the case the SDK's own `Skills` extension already rules out server-side, so this + exercises `mcp.client.skills.get_skill`'s own defense-in-depth check.""" + + identifier = EXTENSION_ID + + def methods(self) -> Any: + async def handler(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + return GetSkillResult(skill=_skill()) + + return [MethodBinding(METHOD_GET, GetSkillParams, handler)] + + +def _repeating_cursor_directory_handler() -> Any: + async def handler(ctx: ServerRequestContext[Any, Any], params: ReadDirectoryParams) -> ReadDirectoryResult: + return ReadDirectoryResult( + resources=[Resource(uri="skill://git-workflow/references/A.md", name="A.md")], + next_cursor="same-cursor-forever", + ) + + return handler + + +def _paginated_directory_handler() -> Any: + pages = { + None: ( + [Resource(uri="skill://git-workflow/references/A.md", name="A.md")], + "page-2", + ), + "page-2": ( + [Resource(uri="skill://git-workflow/references/B.md", name="B.md")], + None, + ), + } + + async def handler(ctx: ServerRequestContext[Any, Any], params: ReadDirectoryParams) -> ReadDirectoryResult: + resources, next_cursor = pages[params.cursor] + return ReadDirectoryResult(resources=resources, next_cursor=next_cursor) + + return handler + + +def _server(*, with_directory_read: bool = False) -> MCPServer: + server = MCPServer( + "catalog", + extensions=[ + Skills( + list_skills=_paginated_list_handler(), + get_skill=_get_skill, + read_directory=_paginated_directory_handler() if with_directory_read else None, + ) + ], + ) + server.add_resource(TextResource(uri=_SKILL_URI, name="SKILL.md", text=_SKILL_CONTENT)) + return server + + +@asynccontextmanager +async def _skills(server: MCPServer) -> AsyncIterator[BoundSkills]: + """Register the client `Skills` extension, connect, and yield the bound verbs.""" + extension = ClientSkills() + async with Client(server, extensions=[extension]) as client: + yield extension.bind(client) + + +async def test_list_skills_follows_next_cursor_to_completion() -> None: + async with _skills(_server()) as skills: + result = await skills.list_skills() + assert [s.uri for s in result] == [_SKILL_URI, "skill://other/SKILL.md"] + + +async def test_list_skills_raises_on_a_server_that_repeats_its_cursor() -> None: + server = MCPServer( + "catalog", extensions=[Skills(list_skills=_repeating_cursor_list_handler(), get_skill=_get_skill)] + ) + async with _skills(server) as skills: + with pytest.raises(ValueError, match="repeated"): + await skills.list_skills() + + +async def test_list_skills_collapses_a_skill_repeated_across_pages() -> None: + """A catalog that changes between page fetches can surface the same skill on two pages; the + client returns it once (first occurrence) rather than failing or duplicating it.""" + + async def handler(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + if params.cursor is None: + return ListSkillsResult(skills=[_skill()], next_cursor="page-2") + return ListSkillsResult(skills=[_skill()], next_cursor=None) + + server = MCPServer("catalog", extensions=[Skills(list_skills=handler, get_skill=_get_skill)]) + async with _skills(server) as skills: + result = await skills.list_skills() + assert [s.uri for s in result] == [_SKILL_URI] + + +async def test_list_skills_requires_the_extension_to_be_advertised() -> None: + """No `Skills` extension at all: the server never advertises `io.modelcontextprotocol/skills`.""" + async with _skills(MCPServer("plain")) as skills: + with pytest.raises(ValueError, match="does not advertise"): + await skills.list_skills() + + +async def test_get_skill_returns_the_matching_entry() -> None: + async with _skills(_server()) as skills: + skill = await skills.get_skill(_SKILL_URI) + assert skill.uri == _SKILL_URI + + +async def test_get_skill_propagates_the_servers_unknown_skill_error() -> None: + async with _skills(_server()) as skills: + with pytest.raises(MCPError) as exc_info: + await skills.get_skill("skill://missing/SKILL.md") + assert exc_info.value.code == INVALID_PARAMS + + +async def test_read_skill_uri_reads_the_registered_resource() -> None: + async with _skills(_server()) as skills: + result = await skills.read_skill_uri(_SKILL_URI) + contents = result.contents[0] + assert isinstance(contents, TextResourceContents) + assert contents.text == _SKILL_CONTENT + + +async def test_read_skill_uri_content_verifies_against_the_held_skill() -> None: + """End-to-end: `skills/get`'s digest and `resources/read`'s bytes agree.""" + async with _skills(_server()) as skills: + skill = await skills.get_skill(_SKILL_URI) + result = await skills.read_skill_uri(_SKILL_URI) + contents = result.contents[0] + assert isinstance(contents, TextResourceContents) + verify_skill_resource(skill, _SKILL_URI, contents.text.encode()) + + +async def test_read_directory_follows_next_cursor_to_completion() -> None: + async with _skills(_server(with_directory_read=True)) as skills: + resources = await skills.read_directory("skill://git-workflow/references") + assert [r.uri for r in resources] == [ + "skill://git-workflow/references/A.md", + "skill://git-workflow/references/B.md", + ] + + +async def test_read_directory_requires_the_directory_read_setting() -> None: + """The extension is advertised, but without `directoryRead: true`.""" + async with _skills(_server(with_directory_read=False)) as skills: + with pytest.raises(ValueError, match="directoryRead"): + await skills.read_directory("skill://git-workflow/references") + + +async def test_read_directory_raises_on_a_server_that_repeats_its_cursor() -> None: + server = MCPServer( + "catalog", + extensions=[ + Skills( + list_skills=_paginated_list_handler(), + get_skill=_get_skill, + read_directory=_repeating_cursor_directory_handler(), + ) + ], + ) + async with _skills(server) as skills: + with pytest.raises(ValueError, match="repeated"): + await skills.read_directory("skill://git-workflow/references") + + +async def test_read_directory_collapses_a_child_repeated_across_pages() -> None: + """A changing directory can surface the same child on two pages; the client returns it once.""" + child = Resource(uri="skill://git-workflow/references/A.md", name="A.md") + + async def handler(ctx: ServerRequestContext[Any, Any], params: ReadDirectoryParams) -> ReadDirectoryResult: + if params.cursor is None: + return ReadDirectoryResult(resources=[child], next_cursor="page-2") + return ReadDirectoryResult(resources=[child], next_cursor=None) + + server = MCPServer( + "catalog", + extensions=[Skills(list_skills=_paginated_list_handler(), get_skill=_get_skill, read_directory=handler)], + ) + async with _skills(server) as skills: + resources = await skills.read_directory("skill://git-workflow/references") + assert [r.uri for r in resources] == ["skill://git-workflow/references/A.md"] + + +async def test_get_skill_rejects_a_mismatched_uri_from_a_non_conformant_server() -> None: + server = MCPServer("catalog", extensions=[_NonConformantGetSkill()]) + async with _skills(server) as skills: + with pytest.raises(ValueError, match="returned skill"): + await skills.get_skill("skill://other/SKILL.md") + + +async def test_get_skill_round_trips_a_dynamic_skill() -> None: + """SEP-2640 Resources: the `"dynamic"` marker survives the full server -> wire -> client + path — validated on both ends — with the union type intact, not coerced to a list or null.""" + dynamic = Skill( + uri="skill://generated/SKILL.md", + frontmatter={"name": "generated", "description": "instructions generated on demand"}, + resources="dynamic", + ) + + async def get_dynamic(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + # The server's own _handle_get already enforces the requested-uri match; this test only + # ever asks for `dynamic.uri`, so the handler just returns it. + return GetSkillResult(skill=dynamic) + + server = MCPServer("catalog", extensions=[Skills(list_skills=_paginated_list_handler(), get_skill=get_dynamic)]) + async with _skills(server) as skills: + skill = await skills.get_skill(dynamic.uri) + assert skill.resources == "dynamic" + assert skill.frontmatter["name"] == "generated" + + +async def test_list_skills_starts_from_a_caller_supplied_cursor() -> None: + """A host resuming from a saved cursor: `list_skills` begins at that cursor rather than the + top, so only the pages after it come back (here, page 1's skill is skipped).""" + async with _skills(_server()) as skills: + result = await skills.list_skills(ListSkillsParams(cursor="page-2")) + assert [s.uri for s in result] == ["skill://other/SKILL.md"] + + +async def test_list_skills_detects_a_server_repeating_the_caller_supplied_cursor() -> None: + """The starting cursor seeds the seen-cursor set: a server that hands back the very cursor + the caller resumed from is caught as a repeat on the first page, not chased a second time.""" + calls = 0 + + async def handler(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + nonlocal calls + calls += 1 + return ListSkillsResult(skills=[_skill()], next_cursor="resume-here") + + server = MCPServer("catalog", extensions=[Skills(list_skills=handler, get_skill=_get_skill)]) + async with _skills(server) as skills: + with pytest.raises(ValueError, match="repeated"): + await skills.list_skills(ListSkillsParams(cursor="resume-here")) + assert calls == 1 + + +async def test_list_skills_threads_request_meta_onto_every_page() -> None: + """A caller-supplied `_meta` rides along with each page request, not just the first.""" + seen_meta: list[Any] = [] + + async def handler(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + seen_meta.append(params.meta) + if params.cursor is None: + return ListSkillsResult(skills=[_skill()], next_cursor="page-2") + return ListSkillsResult( + skills=[ + Skill( + uri="skill://other/SKILL.md", frontmatter={"name": "other", "description": "d"}, resources="dynamic" + ) + ], + next_cursor=None, + ) + + server = MCPServer("catalog", extensions=[Skills(list_skills=handler, get_skill=_get_skill)]) + async with _skills(server) as skills: + params = ListSkillsParams.model_validate({"_meta": {"progressToken": "t"}}) + await skills.list_skills(params) + # `_meta` crosses the wire camelCase (`progressToken`), but the server deserializes it back + # through the meta model, which exposes the known field snake_case as `progress_token`; the + # transport also enriches `_meta` with its own keys. What matters is the caller's token + # reaching the server on both the first page and the cursor-following second one. + assert len(seen_meta) == 2 + assert all(m is not None and m.get("progress_token") == "t" for m in seen_meta) diff --git a/tests/docs_src/test_skills.py b/tests/docs_src/test_skills.py new file mode 100644 index 0000000000..d5eecd9e57 --- /dev/null +++ b/tests/docs_src/test_skills.py @@ -0,0 +1,60 @@ +"""`docs/advanced/skills.md`: every claim the page makes, proved against the real SDK.""" + +import pytest +from mcp_types import INVALID_PARAMS, TextResourceContents + +from docs_src.skills import tutorial001 +from mcp import Client +from mcp.client.skills import Skills, verify_skill_resource +from mcp.shared.exceptions import MCPError + +pytestmark = pytest.mark.anyio + + +async def test_list_skills_returns_the_registered_skill() -> None: + """tutorial001: `list_skills` returns the one skill the server declared.""" + skills = Skills() + async with Client(tutorial001.mcp, extensions=[skills]) as client: + result = await skills.bind(client).list_skills() + assert [s.uri for s in result] == [tutorial001.SKILL_URI] + + +async def test_get_skill_answers_by_uri() -> None: + """tutorial001: `get_skill` returns the same entry `list_skills` does.""" + skills = Skills() + async with Client(tutorial001.mcp, extensions=[skills]) as client: + skill = await skills.bind(client).get_skill(tutorial001.SKILL_URI) + assert skill.frontmatter["name"] == "git-workflow" + + +async def test_get_skill_rejects_an_unknown_uri() -> None: + """tutorial001: `get_skill` raises `-32602` (Invalid params) for an unknown skill.""" + skills = Skills() + async with Client(tutorial001.mcp, extensions=[skills]) as client: + with pytest.raises(MCPError) as exc_info: + await skills.bind(client).get_skill("skill://unknown/SKILL.md") + assert exc_info.value.code == INVALID_PARAMS + + +async def test_the_skill_file_is_served_as_an_ordinary_resource() -> None: + """tutorial001: `Skills` never reads or serves content itself — the file is registered + through `mcp.add_resource`, the SDK's ordinary resource machinery.""" + async with Client(tutorial001.mcp) as client: + result = await client.read_resource(tutorial001.SKILL_URI) + contents = result.contents[0] + assert isinstance(contents, TextResourceContents) + assert contents.mime_type == "text/markdown" + assert contents.text == tutorial001.SKILL_MD + + +async def test_read_skill_uri_content_verifies_against_the_held_skill() -> None: + """tutorial001_client: fetch the entry, read the file, verify the bytes against it - + the digest/size check `verify_skill_resource` performs.""" + skills = Skills() + async with Client(tutorial001.mcp, extensions=[skills]) as client: + catalog = skills.bind(client) + skill = await catalog.get_skill(tutorial001.SKILL_URI) + result = await catalog.read_skill_uri(skill.uri) + contents = result.contents[0] + assert isinstance(contents, TextResourceContents) + verify_skill_resource(skill, skill.uri, contents.text.encode()) diff --git a/tests/server/test_skills.py b/tests/server/test_skills.py new file mode 100644 index 0000000000..2cc9d5e04b --- /dev/null +++ b/tests/server/test_skills.py @@ -0,0 +1,350 @@ +"""Tests for the Skills extension (`io.modelcontextprotocol/skills`, SEP-2640). + +`mcp.shared.skills`'s validators are unit-tested in `tests/shared/test_skills.py`; this +file covers this module's own wiring: request dispatch, capability advertisement, the +`skills/get`/`resources/directory/read` URI checks the extension performs itself before +calling the handler, and — the one piece of SEP-2549 behavior a hand-rolled extension +method must implement itself — gating `ttlMs`/`cacheScope` to protocol version 2026-07-28+. +""" + +from typing import Any + +import pytest +from inline_snapshot import snapshot +from mcp_types import INTERNAL_ERROR, INVALID_PARAMS, METHOD_NOT_FOUND, Resource +from pydantic import BaseModel, ConfigDict + +from mcp.client.client import Client +from mcp.server.context import ServerRequestContext +from mcp.server.mcpserver import MCPServer +from mcp.server.skills import Skills +from mcp.shared.exceptions import MCPError +from mcp.shared.skills import ( + GetSkillParams, + GetSkillRequest, + GetSkillResult, + ListSkillsParams, + ListSkillsRequest, + ListSkillsResult, + ReadDirectoryParams, + ReadDirectoryRequest, + ReadDirectoryResult, + Skill, + SkillResource, +) + +pytestmark = pytest.mark.anyio + + +class _RawResult(BaseModel): + """Captures every wire field, typed, so assertions can inspect fields our own + `ListSkillsResult` model doesn't declare being absent (there are none) or present.""" + + model_config = ConfigDict(extra="allow") + + +_DIGEST = "sha256:" + "a" * 64 +_SKILL_URI = "skill://git-workflow/SKILL.md" + + +def _git_workflow_skill() -> Skill: + return Skill( + uri=_SKILL_URI, + frontmatter={"name": "git-workflow", "description": "Follow this team's Git conventions"}, + resources=[SkillResource(uri=_SKILL_URI, digest=_DIGEST, size=10)], + ) + + +async def _list_skills(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + return ListSkillsResult(skills=[_git_workflow_skill()]) + + +async def _get_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + if params.uri != _SKILL_URI: + raise MCPError(code=INVALID_PARAMS, message="unknown skill") + return GetSkillResult(skill=_git_workflow_skill()) + + +async def _read_directory(ctx: ServerRequestContext[Any, Any], params: ReadDirectoryParams) -> ReadDirectoryResult: + return ReadDirectoryResult( + resources=[Resource(uri="skill://git-workflow/references", name="references", mime_type="inode/directory")] + ) + + +def _server(*, with_directory_read: bool = False) -> MCPServer: + return MCPServer( + "catalog", + extensions=[ + Skills( + list_skills=_list_skills, + get_skill=_get_skill, + read_directory=_read_directory if with_directory_read else None, + ) + ], + ) + + +async def test_skills_list_returns_the_handlers_skills() -> None: + async with Client(_server(), mode="2026-07-28") as client: + result = await client.session.send_request(ListSkillsRequest(), ListSkillsResult) + assert [s.uri for s in result.skills] == [_SKILL_URI] + + +async def test_skills_list_may_return_an_empty_result() -> None: + """SEP-2640 Enumeration: the result MAY be empty.""" + + async def empty(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + return ListSkillsResult(skills=[]) + + server = MCPServer("catalog", extensions=[Skills(list_skills=empty, get_skill=_get_skill)]) + async with Client(server, mode="2026-07-28") as client: + result = await client.session.send_request(ListSkillsRequest(), ListSkillsResult) + assert result.skills == [] + + +async def test_skills_list_carries_cache_fields_on_the_2026_07_28_wire() -> None: + """SEP-2640 Dependencies: on 2026-07-28+, the result carries the SEP-2549 cache fields, + defaulting `cacheScope` to `"public"` when the handler left it unset.""" + async with Client(_server(), mode="2026-07-28") as client: + raw = await client.session.send_request(ListSkillsRequest(), _RawResult) + extra = raw.model_extra or {} + assert extra["cacheScope"] == "public" + assert extra["ttlMs"] == 0 + + +async def test_skills_list_omits_cache_fields_on_a_legacy_wire() -> None: + """The runner's per-version sieve only applies to core spec methods, so this extension + method must strip SEP-2549 fields itself for a pre-2026-07-28 connection.""" + async with Client(_server(), mode="legacy") as client: + raw = await client.session.send_request(ListSkillsRequest(), _RawResult) + extra = raw.model_extra or {} + assert "cacheScope" not in extra + assert "ttlMs" not in extra + + +async def test_skills_list_handler_setting_cache_scope_explicitly_is_not_overridden() -> None: + async def private_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + return ListSkillsResult(skills=[_git_workflow_skill()], cache_scope="private") + + server = MCPServer("catalog", extensions=[Skills(list_skills=private_list, get_skill=_get_skill)]) + async with Client(server, mode="2026-07-28") as client: + raw = await client.session.send_request(ListSkillsRequest(), _RawResult) + extra = raw.model_extra or {} + assert extra["cacheScope"] == "private" + + +async def test_skills_list_rejects_a_handler_result_with_an_invalid_skill() -> None: + """SDK-defined: a `list_skills` handler bug (a non-conformant skill entry) is caught + before it reaches the client, as an Internal error — the fault is the server's, not the + caller's params — rather than a silently non-conformant listing.""" + + async def bad_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + return ListSkillsResult( + skills=[Skill(uri=_SKILL_URI, frontmatter={"name": "git-workflow", "description": "d"}, resources=[])] + ) + + server = MCPServer("catalog", extensions=[Skills(list_skills=bad_list, get_skill=_get_skill)]) + async with Client(server) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request(ListSkillsRequest(), ListSkillsResult) + assert exc_info.value.code == INTERNAL_ERROR + + +async def test_skills_list_rejects_a_skill_mutated_after_the_result_is_built() -> None: + """SDK-defined: the value models validate on construction but stay mutable. A handler that + builds a valid result and then mutates a skill in place must not slip a non-conformant + listing past the server — the outbound payload is re-validated, so this is an Internal error.""" + + async def mutating_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + result = ListSkillsResult(skills=[_git_workflow_skill()]) + result.skills[0].frontmatter["name"] = "not the uri name" + return result + + server = MCPServer("catalog", extensions=[Skills(list_skills=mutating_list, get_skill=_get_skill)]) + async with Client(server) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request(ListSkillsRequest(), ListSkillsResult) + assert exc_info.value.code == INTERNAL_ERROR + + +async def test_skills_get_returns_the_matching_skill() -> None: + async with Client(_server()) as client: + result = await client.session.send_request( + GetSkillRequest(params=GetSkillParams(uri=_SKILL_URI)), GetSkillResult + ) + assert result.skill.uri == _SKILL_URI + + +async def test_skills_get_carries_cache_fields_on_the_2026_07_28_wire() -> None: + """SEP-2640 Retrieval: on 2026-07-28+, `GetSkillResult` extends `CacheableResult`, so the + result carries the SEP-2549 cache fields, defaulting `cacheScope` to `"public"` when unset — + the same treatment `skills/list` gets.""" + async with Client(_server(), mode="2026-07-28") as client: + raw = await client.session.send_request(GetSkillRequest(params=GetSkillParams(uri=_SKILL_URI)), _RawResult) + extra = raw.model_extra or {} + assert extra["cacheScope"] == "public" + assert extra["ttlMs"] == 0 + + +async def test_skills_get_omits_cache_fields_on_a_legacy_wire() -> None: + """As with `skills/list`, this extension method strips the SEP-2549 fields itself for a + pre-2026-07-28 connection, since the runner's per-version sieve only applies to core methods.""" + async with Client(_server(), mode="legacy") as client: + raw = await client.session.send_request(GetSkillRequest(params=GetSkillParams(uri=_SKILL_URI)), _RawResult) + extra = raw.model_extra or {} + assert "cacheScope" not in extra + assert "ttlMs" not in extra + + +async def test_skills_get_answers_for_a_skill_absent_from_the_listing() -> None: + """SEP-2640 Retrieval: a server MUST answer for every skill it serves, whether or not + that skill appears in `skills/list`.""" + + async def list_without_it(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + return ListSkillsResult(skills=[]) + + server = MCPServer("catalog", extensions=[Skills(list_skills=list_without_it, get_skill=_get_skill)]) + async with Client(server) as client: + listing = await client.session.send_request(ListSkillsRequest(), ListSkillsResult) + result = await client.session.send_request( + GetSkillRequest(params=GetSkillParams(uri=_SKILL_URI)), GetSkillResult + ) + assert listing.skills == [] + assert result.skill.uri == _SKILL_URI + + +async def test_skills_get_rejects_an_unknown_uri_with_invalid_params() -> None: + """SEP-2640 Retrieval: an unknown skill URI MUST be rejected with -32602 (Invalid params).""" + async with Client(_server()) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request( + GetSkillRequest(params=GetSkillParams(uri="skill://other/SKILL.md")), GetSkillResult + ) + assert exc_info.value.code == INVALID_PARAMS + + +async def test_skills_get_rejects_a_uri_that_does_not_end_in_skill_md() -> None: + async with Client(_server()) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request( + GetSkillRequest(params=GetSkillParams(uri="skill://git-workflow/README.md")), GetSkillResult + ) + assert exc_info.value.code == INVALID_PARAMS + + +async def test_skills_get_rejects_a_handler_that_returns_a_mismatched_uri() -> None: + """SDK-defined: a `get_skill` handler bug (returning the wrong skill) is caught before + it reaches the client, as an Internal error rather than a silently wrong answer.""" + + async def wrong_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + return GetSkillResult(skill=_git_workflow_skill()) + + server = MCPServer("catalog", extensions=[Skills(list_skills=_list_skills, get_skill=wrong_skill)]) + async with Client(server) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request( + GetSkillRequest(params=GetSkillParams(uri="skill://other/SKILL.md")), GetSkillResult + ) + assert exc_info.value.code == INTERNAL_ERROR + + +async def test_skills_get_rejects_a_matching_but_non_conformant_skill() -> None: + """SDK-defined: a `get_skill` handler bug (a non-conformant skill body, distinct from a + URI mismatch) is caught the same way, as an Internal error.""" + + async def bad_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + return GetSkillResult( + skill=Skill(uri=params.uri, frontmatter={"name": "git-workflow", "description": "d"}, resources=[]) + ) + + server = MCPServer("catalog", extensions=[Skills(list_skills=_list_skills, get_skill=bad_skill)]) + async with Client(server) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request(GetSkillRequest(params=GetSkillParams(uri=_SKILL_URI)), GetSkillResult) + assert exc_info.value.code == INTERNAL_ERROR + + +async def test_skills_get_rejects_a_skill_mutated_after_the_result_is_built() -> None: + """SDK-defined: as on `skills/list`, a handler that mutates the skill in place after building + the result can't ship a non-conformant body — the re-validated outbound payload fails it as an + Internal error.""" + + async def mutating_get(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + result = GetSkillResult(skill=_git_workflow_skill()) + assert isinstance(result.skill.resources, list) + result.skill.resources.clear() + return result + + server = MCPServer("catalog", extensions=[Skills(list_skills=_list_skills, get_skill=mutating_get)]) + async with Client(server) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request(GetSkillRequest(params=GetSkillParams(uri=_SKILL_URI)), GetSkillResult) + assert exc_info.value.code == INTERNAL_ERROR + + +async def test_missing_uri_param_is_rejected_before_the_handler_runs() -> None: + """SDK-defined: `uri` is a required field on `GetSkillParams`, so an omitted param is + rejected by params validation - the handler never sees it.""" + async with Client(_server()) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request(GetSkillRequest.model_construct(params=None), GetSkillResult) + assert exc_info.value.code == INVALID_PARAMS + + +async def test_directory_read_is_not_advertised_without_a_handler() -> None: + async with Client(_server(with_directory_read=False)) as client: + assert client.server_capabilities.extensions == {"io.modelcontextprotocol/skills": {}} + + +async def test_directory_read_is_advertised_when_a_handler_is_supplied() -> None: + async with Client(_server(with_directory_read=True)) as client: + assert client.server_capabilities.extensions == snapshot( + {"io.modelcontextprotocol/skills": {"directoryRead": True}} + ) + + +async def test_directory_read_returns_the_handlers_children() -> None: + async with Client(_server(with_directory_read=True)) as client: + result = await client.session.send_request( + ReadDirectoryRequest(params=ReadDirectoryParams(uri="skill://git-workflow")), ReadDirectoryResult + ) + assert [r.uri for r in result.resources] == ["skill://git-workflow/references"] + + +async def test_directory_read_is_not_registered_without_a_handler() -> None: + """SDK-defined: omitting `read_directory` doesn't just skip the capability ad - the + method itself isn't registered, so calling it anyway is Method not found.""" + async with Client(_server(with_directory_read=False)) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request( + ReadDirectoryRequest(params=ReadDirectoryParams(uri="skill://git-workflow")), ReadDirectoryResult + ) + assert exc_info.value.code == METHOD_NOT_FOUND + + +async def test_directory_read_rejects_a_trailing_slash_uri() -> None: + """SEP-2640 Directory resources: directory URIs are written without a trailing slash.""" + async with Client(_server(with_directory_read=True)) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request( + ReadDirectoryRequest(params=ReadDirectoryParams(uri="skill://git-workflow/")), ReadDirectoryResult + ) + assert exc_info.value.code == INVALID_PARAMS + + +async def test_directory_read_rejects_a_handler_result_with_a_grandchild() -> None: + """SDK-defined: a `read_directory` handler bug (a non-direct child) is a server fault, + surfaced as an Internal error, not the caller's Invalid params.""" + + async def bad_directory(ctx: ServerRequestContext[Any, Any], params: ReadDirectoryParams) -> ReadDirectoryResult: + return ReadDirectoryResult(resources=[Resource(uri="skill://git-workflow/a/b/c.md", name="c.md")]) + + server = MCPServer( + "catalog", extensions=[Skills(list_skills=_list_skills, get_skill=_get_skill, read_directory=bad_directory)] + ) + async with Client(server) as client: + with pytest.raises(MCPError) as exc_info: + await client.session.send_request( + ReadDirectoryRequest(params=ReadDirectoryParams(uri="skill://git-workflow")), ReadDirectoryResult + ) + assert exc_info.value.code == INTERNAL_ERROR diff --git a/tests/shared/test_skills.py b/tests/shared/test_skills.py new file mode 100644 index 0000000000..b564de032b --- /dev/null +++ b/tests/shared/test_skills.py @@ -0,0 +1,406 @@ +"""SEP-2640 conformance checks in `mcp.shared.skills`: types, validation, and verification. + +Server- and client-side end-to-end wiring live in `tests/server/test_skills.py` and +`tests/client/test_skills.py`; this file pins the pure functions both depend on. +""" + +import hashlib +from typing import Any + +import pytest +from mcp_types import Resource +from pydantic import ValidationError + +from mcp.shared.skills import ( + ListSkillsParams, + ListSkillsResult, + ReadDirectoryParams, + ReadDirectoryResult, + Skill, + SkillResource, + parse_directory_uri, + skill_name_from_uri, + validate_directory_result, + verify_skill_resource, +) + +_DIGEST = "sha256:" + "a" * 64 + + +def _resource(uri: str, *, size: int = 4) -> SkillResource: + return SkillResource(uri=uri, digest=_DIGEST, size=size) + + +def _skill(name: str = "git-workflow", *, extra_resources: list[SkillResource] | None = None) -> Skill: + uri = f"skill://{name}/SKILL.md" + return Skill( + uri=uri, + frontmatter={"name": name, "description": "d"}, + resources=[_resource(uri), *(extra_resources or [])], + ) + + +@pytest.mark.parametrize( + ("uri", "name"), + [ + ("skill://git-workflow/SKILL.md", "git-workflow"), + ("skill://acme/billing/refunds/SKILL.md", "refunds"), + ("https://example.com/skills/pdf/SKILL.md", "pdf"), + ], +) +def test_skill_name_from_uri_reads_the_final_path_segment(uri: str, name: str) -> None: + """SEP-2640 Resource Mapping: the final `` segment is the skill name.""" + assert skill_name_from_uri(uri) == name + + +@pytest.mark.parametrize( + "uri", + [ + "skill://git-workflow/README.md", # doesn't end in /SKILL.md + "not-a-uri", # no scheme + "skill://git-workflow/SKILL.md?x=1", # query component + ], +) +def test_skill_name_from_uri_rejects_malformed_uris(uri: str) -> None: + with pytest.raises(ValueError, match="SKILL.md|absolute resource URI"): + skill_name_from_uri(uri) + + +def test_skill_name_from_uri_rejects_a_uri_with_no_recoverable_name() -> None: + with pytest.raises(ValueError, match="has no skill name"): + skill_name_from_uri("skill:///SKILL.md") + + +def test_skill_with_a_static_resources_array_round_trips_through_json() -> None: + """SEP-2640 Resources: `resources` MUST serialize as a JSON array of `{uri, digest, size}` + triples - proves the union type doesn't collapse or mistag on the wire.""" + original = _skill() + dumped = original.model_dump(mode="json", by_alias=True) + assert isinstance(dumped["resources"], list) + restored = Skill.model_validate(dumped) + assert restored == original + + +def test_skill_with_dynamic_resources_round_trips_through_json_as_the_literal_string() -> None: + """SEP-2640 Resources: a dynamically generated skill MUST carry the literal string + `"dynamic"` in place of an array - not `null`, not `{}`, not omitted.""" + original = Skill( + uri="skill://generated/SKILL.md", frontmatter={"name": "generated", "description": "d"}, resources="dynamic" + ) + dumped = original.model_dump(mode="json", by_alias=True) + assert dumped["resources"] == "dynamic" + restored = Skill.model_validate(dumped) + assert restored == original + assert restored.resources == "dynamic" + + +def test_skill_accepts_a_conformant_manifest() -> None: + skill = _skill() + assert isinstance(skill.resources, list) + + +def test_skill_rejects_a_non_string_frontmatter_name() -> None: + with pytest.raises(ValidationError, match="frontmatter name"): + Skill( + uri="skill://git-workflow/SKILL.md", + frontmatter={"name": 1, "description": "d"}, + resources=[_resource("skill://git-workflow/SKILL.md")], + ) + + +@pytest.mark.parametrize( + "name", + [ + "foo--bar", # consecutive hyphens + "-foo", # leading hyphen + "foo-", # trailing hyphen + "UPPER", # uppercase not allowed + "with_underscore", # underscore not allowed + "a" * 65, # exceeds the 64-char limit + ], +) +def test_skill_rejects_names_violating_the_agent_skills_grammar(name: str) -> None: + """SEP-2640 defers naming to the Agent Skills spec: 1-64 chars, lowercase alphanumeric and + hyphens, no leading/trailing/consecutive hyphens. A URI whose final path segment carries the + bad name (so `frontmatter.name` can match it) still fails the name-grammar check first.""" + uri = f"skill://acme/{name}/SKILL.md" + with pytest.raises(ValidationError, match="frontmatter name"): + Skill( + uri=uri, + frontmatter={"name": name, "description": "d"}, + resources=[SkillResource(uri=uri, digest=_DIGEST, size=4)], + ) + + +def test_skill_rejects_an_invalid_resource_uri() -> None: + """A resource URI with a query component fails the same shape check as a skill URI.""" + with pytest.raises(ValidationError, match="is invalid"): + _skill(extra_resources=[_resource("skill://git-workflow/x.md?y=1")]) + + +def test_skill_rejects_a_resource_with_the_same_authority_but_a_sibling_path() -> None: + """Same scheme+authority as the skill (so the URI passes the authority check) but a path + outside the skill's own directory, per SEP-2640 Resources ('a file within the skill's + directory').""" + root = "skill://acme/billing/refunds/SKILL.md" + with pytest.raises(ValidationError, match="outside the skill root"): + Skill( + uri=root, + frontmatter={"name": "refunds", "description": "d"}, + resources=[_resource(root), _resource("skill://acme/billing/other/x.md")], + ) + + +def test_skill_rejects_a_traversal_segment_in_a_resource_uri() -> None: + with pytest.raises(ValidationError, match="traversal segment"): + _skill(extra_resources=[_resource("skill://git-workflow/../evil.md")]) + + +def test_skill_rejects_a_resource_uri_with_a_trailing_slash() -> None: + """A manifest entry names a file, not a directory: a trailing slash is rejected.""" + with pytest.raises(ValidationError, match="names a directory"): + _skill(extra_resources=[_resource("skill://git-workflow/references/")]) + + +def test_skill_resource_rejects_a_negative_size() -> None: + with pytest.raises(ValidationError, match="greater than or equal to 0"): + SkillResource(uri="skill://git-workflow/SKILL.md", digest=_DIGEST, size=-1) + + +def test_skill_rejects_frontmatter_name_uri_mismatch() -> None: + """SEP-2640 Resource Mapping: `frontmatter.name` MUST equal the URI-derived name.""" + with pytest.raises(ValidationError, match="does not match URI name"): + Skill( + uri="skill://git-workflow/SKILL.md", + frontmatter={"name": "other-name", "description": "d"}, + resources=[_resource("skill://git-workflow/SKILL.md")], + ) + + +@pytest.mark.parametrize("description", ["", "x" * 1025]) +def test_skill_rejects_out_of_range_description_length(description: str) -> None: + with pytest.raises(ValidationError, match="description"): + Skill( + uri="skill://git-workflow/SKILL.md", + frontmatter={"name": "git-workflow", "description": description}, + resources=[_resource("skill://git-workflow/SKILL.md")], + ) + + +def test_skill_rejects_a_resource_outside_the_skill_root() -> None: + """SEP-2640 Resources: every entry MUST name a file within the skill's own directory.""" + with pytest.raises(ValidationError, match="outside the skill root"): + _skill(extra_resources=[_resource("skill://other-skill/file.md")]) + + +def test_skill_rejects_missing_skill_md_entry() -> None: + """SEP-2640 Resources: `resources` MUST include an entry for the skill's own `SKILL.md`.""" + with pytest.raises(ValidationError, match="does not include its own SKILL.md"): + Skill( + uri="skill://git-workflow/SKILL.md", + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[_resource("skill://git-workflow/references/GUIDE.md")], + ) + + +def test_skill_rejects_duplicate_resource_uris() -> None: + root = "skill://git-workflow/SKILL.md" + with pytest.raises(ValidationError, match="more than once"): + Skill( + uri=root, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[_resource(root), _resource(root)], + ) + + +@pytest.mark.parametrize( + "digest", + [ + "not-a-digest", # no sha256: prefix at all + "sha256:" + "A" * 64, # uppercase hex - spec requires lowercase + "sha256:" + "a" * 63, # one hex char short + "sha256:" + "a" * 65, # one hex char long + "sha1:" + "a" * 40, # wrong algorithm prefix + "sha256:" + "g" * 64, # non-hex characters + ], +) +def test_skill_resource_rejects_malformed_digest_formats(digest: str) -> None: + """SEP-2640 Integrity and verification: `sha256:{hex}` where `{hex}` is exactly 64 + lowercase hexadecimal characters - each of these near-misses must still be rejected.""" + with pytest.raises(ValidationError, match="digest"): + SkillResource(uri="skill://git-workflow/SKILL.md", digest=digest, size=1) + + +def test_skill_accepts_exactly_512_resources() -> None: + """SEP-2640 Limits: a host MUST support skills up to and including 512 entries + (`SKILL.md` counted), so a 512-entry manifest validates.""" + root = "skill://git-workflow/SKILL.md" + resources = [_resource(root)] + [_resource(f"skill://git-workflow/f{i}.md") for i in range(511)] + skill = Skill(uri=root, frontmatter={"name": "git-workflow", "description": "d"}, resources=resources) + assert isinstance(skill.resources, list) + assert len(skill.resources) == 512 + + +def test_skill_accepts_more_than_512_resources() -> None: + """SEP-2640 Limits: 512 is a SHOULD NOT threshold, not a hard cap — a host MAY support + larger skills, so an over-count manifest is not rejected.""" + root = "skill://git-workflow/SKILL.md" + resources = [_resource(root)] + [_resource(f"skill://git-workflow/f{i}.md") for i in range(512)] + skill = Skill(uri=root, frontmatter={"name": "git-workflow", "description": "d"}, resources=resources) + assert isinstance(skill.resources, list) + assert len(skill.resources) == 513 + + +def test_skill_accepts_total_size_over_16mib() -> None: + """SEP-2640 Limits: 16 MiB total is a SHOULD NOT threshold, not a hard cap — an over-size + manifest is not rejected.""" + root = "skill://git-workflow/SKILL.md" + skill = Skill( + uri=root, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[_resource(root, size=16 * 1024 * 1024 + 1)], + ) + assert isinstance(skill.resources, list) + assert skill.resources[0].size == 16 * 1024 * 1024 + 1 + + +def test_skill_accepts_dynamic_resources_without_further_checks() -> None: + """SEP-2640 Resources: `"dynamic"` offers no manifest to check against limits.""" + skill = Skill( + uri="skill://generated/SKILL.md", frontmatter={"name": "generated", "description": "d"}, resources="dynamic" + ) + assert skill.resources == "dynamic" + + +def test_list_result_rejects_duplicate_skill_uris_across_entries() -> None: + with pytest.raises(ValidationError, match="more than once"): + ListSkillsResult(skills=[_skill(), _skill()]) + + +def test_list_result_accepts_an_empty_listing() -> None: + """SEP-2640 Enumeration: `skills/list` MAY return an empty result.""" + assert ListSkillsResult(skills=[]).skills == [] + + +@pytest.mark.parametrize( + ("params_type", "payload"), + [ + (ListSkillsParams, {"_meta": {"progressToken": "t"}}), + (ReadDirectoryParams, {"uri": "skill://pdf/templates", "_meta": {"progressToken": "t"}}), + ], +) +def test_request_meta_is_camelcase_on_the_wire_but_snakecase_to_a_handler( + params_type: type[ListSkillsParams] | type[ReadDirectoryParams], payload: dict[str, Any] +) -> None: + """A caller's `_meta.progressToken` rides the wire camelCase (its JSON alias) yet is read back + snake_case as `progress_token` once deserialized - two spellings, one value. This is what lets + `list_skills`/`read_directory` forward a caller's `_meta` to every page while a server handler + still finds `progress_token` in `params.meta`; both spellings working is the point.""" + params = params_type.model_validate(payload) + wire = params.model_dump(by_alias=True, mode="json", exclude_none=True) + assert wire["_meta"] == {"progressToken": "t"} # camelCase over the wire + assert params.meta is not None and params.meta.get("progress_token") == "t" # snake_case to a handler + + +@pytest.mark.parametrize("uri", ["skill://pdf/templates/", "not-a-uri"]) +def test_parse_directory_uri_rejects_malformed_uris(uri: str) -> None: + with pytest.raises(ValueError, match="directory URI"): + parse_directory_uri(uri) + + +def test_validate_directory_result_rejects_a_child_with_a_query_component() -> None: + result = ReadDirectoryResult(resources=[Resource(uri="skill://pdf/templates/x.md?y=1", name="x.md")]) + with pytest.raises(ValueError, match="invalid URI"): + validate_directory_result("skill://pdf/templates", result) + + +def test_validate_directory_result_rejects_a_duplicate_child_name() -> None: + result = ReadDirectoryResult( + resources=[ + Resource(uri="skill://pdf/templates/a.md", name="dup"), + Resource(uri="skill://pdf/templates/b.md", name="dup"), + ] + ) + with pytest.raises(ValueError, match="duplicate child"): + validate_directory_result("skill://pdf/templates", result) + + +def test_validate_directory_result_accepts_direct_children() -> None: + result = ReadDirectoryResult( + resources=[ + Resource(uri="skill://pdf/templates/invoice.md", name="invoice.md", mime_type="text/markdown"), + Resource(uri="skill://pdf/templates/regional", name="regional", mime_type="inode/directory"), + ] + ) + validate_directory_result("skill://pdf/templates", result) + + +def test_validate_directory_result_rejects_a_grandchild() -> None: + """SEP-2640 Directory Listing: the listing is not recursive.""" + result = ReadDirectoryResult( + resources=[Resource(uri="skill://pdf/templates/regional/eu.md", name="eu.md", mime_type="text/markdown")] + ) + with pytest.raises(ValueError, match="direct child"): + validate_directory_result("skill://pdf/templates", result) + + +@pytest.mark.parametrize("child_uri", ["skill://pdf/templates/.", "skill://pdf/templates/.."]) +def test_validate_directory_result_rejects_a_dot_segment_child(child_uri: str) -> None: + """A `.`/`..` child is a traversal segment, not a real direct child of the directory.""" + result = ReadDirectoryResult(resources=[Resource(uri=child_uri, name="x")]) + with pytest.raises(ValueError, match="direct child"): + validate_directory_result("skill://pdf/templates", result) + + +def test_validate_directory_result_rejects_a_uri_outside_the_directory() -> None: + result = ReadDirectoryResult(resources=[Resource(uri="skill://other/file.md", name="file.md")]) + with pytest.raises(ValueError, match="not a child"): + validate_directory_result("skill://pdf/templates", result) + + +def test_verify_skill_resource_accepts_matching_content() -> None: + content = b"hello skill" + digest = f"sha256:{hashlib.sha256(content).hexdigest()}" + uri = "skill://git-workflow/SKILL.md" + skill = Skill( + uri=uri, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[SkillResource(uri=uri, digest=digest, size=len(content))], + ) + verify_skill_resource(skill, uri, content) + + +def test_verify_skill_resource_rejects_a_digest_mismatch() -> None: + """SEP-2640 Integrity and verification: a mismatch MUST be treated as a verification failure.""" + uri = "skill://git-workflow/SKILL.md" + skill = Skill( + uri=uri, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[SkillResource(uri=uri, digest=_DIGEST, size=5)], + ) + with pytest.raises(ValueError, match="digest"): + verify_skill_resource(skill, uri, b"wrong") + + +def test_verify_skill_resource_rejects_a_size_mismatch_before_hashing() -> None: + uri = "skill://git-workflow/SKILL.md" + skill = Skill( + uri=uri, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[SkillResource(uri=uri, digest=_DIGEST, size=999)], + ) + with pytest.raises(ValueError, match="size"): + verify_skill_resource(skill, uri, b"short") + + +def test_verify_skill_resource_rejects_dynamic_resources() -> None: + uri = "skill://generated/SKILL.md" + skill = Skill(uri=uri, frontmatter={"name": "generated", "description": "d"}, resources="dynamic") + with pytest.raises(ValueError, match="dynamic"): + verify_skill_resource(skill, uri, b"anything") + + +def test_verify_skill_resource_rejects_a_uri_not_in_the_manifest() -> None: + skill = _skill() + with pytest.raises(ValueError, match="not in skill"): + verify_skill_resource(skill, "skill://git-workflow/unlisted.md", b"x")