diff --git a/docs/advanced/skills.md b/docs/advanced/skills.md new file mode 100644 index 0000000000..a2527cd2e6 --- /dev/null +++ b/docs/advanced/skills.md @@ -0,0 +1,107 @@ +# 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 a directory +of files — minimally a `SKILL.md` with YAML frontmatter — exposed as ordinary MCP resources, +conventionally under a `skill://` URI. A server enumerates its skills with `skills/list`, +answers for any one of them 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`). If +[Extensions](extensions.md) are new to you, skim that page first. + +`Skills` provides 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 serve +each skill's files as ordinary resources through `MCPServer.add_resource` or an +`@mcp.resource(...)` template handler. + +## Serving a skill + +```python title="server.py" hl_lines="31-41 44-51 55" +--8<-- "docs_src/skills/tutorial001.py" +``` + +Three moves: + +* `Skill(uri=..., frontmatter=..., resources=[...])`: one entry, identical in 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`/`get_skill`: plain async callables, invoked per request. `get_skill` **must** + answer for a skill even if a real `list_skills` implementation omitted it — SEP-2640 requires + a server to answer by URI for every skill it serves, listed or not. +* `mcp.add_resource(TextResource(uri=SKILL_URI, ...))`: the skill's actual file content, served + through the SDK's ordinary resource machinery. `Skills` never reads or writes resource content + itself. + +`Skills(list_skills=..., get_skill=...)` is all a server needs; `resources/directory/read` is +optional (below). + +## Fetching a skill + +```python title="client.py" hl_lines="5" +--8<-- "docs_src/skills/tutorial001_client.py" +``` + +`list_skills` and `read_directory` follow `nextCursor` to completion, so you get every page's +skills or resources in one call; `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: a thin, discoverable alias for +`resources/read` that returns bytes and validates nothing itself (see the next paragraph). + +`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 treating 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. + +!!! 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. Both are host responsibilities the SDK cannot + discharge for you — see 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` cannot 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`. +`mcp.client.skills.read_directory` raises before sending if the connected server hasn't +advertised the setting. + +## Protocol version and caching + +In protocol version `2026-07-28` and later, `skills/list` results carry the base protocol's +list-caching fields, [`ttlMs` and `cacheScope`](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2549) — the +same freshness hint `tools/list` and `resources/list` 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..07044d2282 --- /dev/null +++ b/docs_src/skills/tutorial001.py @@ -0,0 +1,55 @@ +import hashlib +from typing import Any + +from mcp_types import INVALID_PARAMS + +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, +) + +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..7e23428f77 --- /dev/null +++ b/docs_src/skills/tutorial001_client.py @@ -0,0 +1,22 @@ +import anyio +from mcp_types import TextResourceContents + +from mcp import Client +from mcp.client.skills import get_skill, list_skills, read_skill_uri, verify_skill_resource + + +async def main() -> None: + async with Client("http://localhost:8000/mcp") as client: + for skill in await list_skills(client.session): + print(skill.uri, skill.frontmatter["description"]) + + skill = await get_skill(client.session, "skill://git-workflow/SKILL.md") + result = await read_skill_uri(client.session, 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..e0dc0a6e97 --- /dev/null +++ b/src/mcp/client/skills.py @@ -0,0 +1,126 @@ +"""Client-side convenience wrappers for the Skills extension (SEP-2640). + +SEP-2640 needs no client-side method registration: `skills/list`, `skills/get`, +and `resources/directory/read` are ordinary vendor requests sent through +`ClientSession.send_request`, exactly like [Extension verbs](../advanced/extensions.md#extension-verbs). +The functions below are the thin, named wrappers SEP-2640's "SDKs: Convenience +Wrappers" section recommends — each validates the server's advertised support +before sending, and `list_skills`/`read_directory` follow `nextCursor` to +completion so a caller sees one page's worth of ergonomics regardless of how +many requests it took. + + async with Client("http://localhost:8000/mcp") as client: + for skill in await list_skills(client.session): + print(skill.uri, skill.frontmatter["description"]) +""" + +from __future__ import annotations + +from mcp_types import ReadResourceResult, Resource + +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, + validate_list_result, + validate_skill, +) +from mcp.shared.skills import verify_skill_resource as verify_skill_resource + +__all__ = ["get_skill", "list_skills", "read_directory", "read_skill_uri", "verify_skill_resource"] + + +def _require_extension(session: ClientSession, *, directory_read: bool = False) -> None: + capabilities = 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(session: ClientSession, params: ListSkillsParams | None = None) -> list[Skill]: + """Call `skills/list`, following `nextCursor` to completion, and validate the result. + + Raises: + ValueError: If the server doesn't advertise the Skills extension, or + its response is not SEP-2640 conformant. + """ + _require_extension(session) + cursor = params.cursor if params is not None else None + skills: list[Skill] = [] + seen_cursors: set[str] = set() + while True: + page = await session.send_request(ListSkillsRequest(params=ListSkillsParams(cursor=cursor)), ListSkillsResult) + validate_list_result(page) + skills.extend(page.skills) + if page.next_cursor is None: + return skills + if page.next_cursor in seen_cursors: + raise ValueError(f"server repeated skills/list pagination cursor {page.next_cursor!r}") + seen_cursors.add(page.next_cursor) + cursor = page.next_cursor + + +async def get_skill(session: ClientSession, 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. + """ + _require_extension(session) + result = await 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}") + validate_skill(result.skill) + return result.skill + + +async def read_skill_uri(session: ClientSession, 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. + """ + return await session.read_resource(uri) + + +async def read_directory(session: ClientSession, uri: str, params: ReadDirectoryParams | None = None) -> list[Resource]: + """Call `resources/directory/read` for `uri`, following `nextCursor` to completion. + + Raises: + ValueError: If the server doesn't advertise the `directoryRead` + setting, or its response is not a valid child listing of `uri`. + """ + _require_extension(session, directory_read=True) + cursor = params.cursor if params is not None else None + resources: list[Resource] = [] + seen_cursors: set[str] = set() + while True: + page = await session.send_request( + ReadDirectoryRequest(params=ReadDirectoryParams(uri=uri, cursor=cursor)), ReadDirectoryResult + ) + validate_directory_result(uri, page) + resources.extend(page.resources) + if page.next_cursor is None: + return resources + if page.next_cursor in seen_cursors: + raise ValueError(f"server repeated resources/directory/read pagination cursor {page.next_cursor!r}") + 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..0c90f66243 --- /dev/null +++ b/src/mcp/server/skills.py @@ -0,0 +1,169 @@ +"""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 + +from collections.abc import Awaitable, Callable, Sequence +from typing import Any + +from mcp_types.jsonrpc import INVALID_PARAMS +from mcp_types.version import MODERN_PROTOCOL_VERSIONS + +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, + validate_list_result, + validate_skill, +) + +__all__ = ["Skills"] + +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. + """ + + 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: + result = await self._list_skills(ctx, params) + try: + validate_list_result(result) + except ValueError as exc: + raise MCPError( + code=INVALID_PARAMS, message=f"list_skills handler returned an invalid result: {exc}" + ) from exc + 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) + result = await self._get_skill(ctx, params) + if result.skill.uri != params.uri: + raise MCPError( + code=INVALID_PARAMS, + message=f"get_skill handler returned {result.skill.uri!r} for requested {params.uri!r}", + ) + try: + validate_skill(result.skill) + except ValueError as exc: + raise MCPError(code=INVALID_PARAMS, message=f"get_skill handler returned an invalid result: {exc}") from exc + return result + + 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 as exc: + raise MCPError( + code=INVALID_PARAMS, message=f"read_directory handler returned an invalid result: {exc}" + ) from exc + 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: ListSkillsResult, protocol_version: str) -> HandlerResult: + """Gate SEP-2549's `ttlMs`/`cacheScope` to protocol version 2026-07-28+. + + `skills/list` is an extension method, so — unlike a core spec method — the + runner's per-version surface sieve never runs on its result; 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..22bccfce77 --- /dev/null +++ b/src/mcp/shared/skills.py @@ -0,0 +1,289 @@ +"""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 Any, Literal +from urllib.parse import urlsplit + +from mcp_types import CacheableResult, PaginatedRequestParams, PaginatedResult, Request, RequestParams, Resource, Result +from pydantic import BaseModel, ConfigDict +from pydantic.alias_generators import to_camel + +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 limit, `SKILL.md` included.""" + +MAX_TOTAL_SIZE = 16 * 1024 * 1024 +"""SEP-2640 per-skill total-byte-size limit (16 MiB), summed over `resources[].size`.""" + +_NAME_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") +_DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$") + + +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}`.""" + + uri: str + digest: str + """SHA-256 digest of the file's raw bytes, formatted `sha256:{64 hex chars}`.""" + size: int + """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`.""" + + uri: str + """Resource URI of the skill's `SKILL.md`.""" + frontmatter: Frontmatter + resources: SkillResources + + +class ListSkillsParams(PaginatedRequestParams): + """Parameters for `skills/list`.""" + + +class ListSkillsResult(PaginatedResult, CacheableResult): + """Result of `skills/list`. + + `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] + + +class GetSkillParams(RequestParams): + """Parameters for `skills/get`.""" + + uri: str + """URI of the skill's `SKILL.md`.""" + + +class GetSkillResult(Result): + """Result of `skills/get`.""" + + 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 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 validate_skill(skill: Skill) -> None: + """Validate `skill` against the SEP-2640 and Agent Skills conformance rules. + + Checks the frontmatter's `name`/`description` fields, that `resources` (when + not `"dynamic"`) is complete and within the `MAX_RESOURCES_PER_SKILL`/ + `MAX_TOTAL_SIZE` limits, and that every resource entry names a file within + the skill's own directory with a well-formed digest. + + Raises: + ValueError: If `skill` violates any of the above. + """ + name = skill_name_from_uri(skill.uri) + frontmatter_name = skill.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 {skill.uri!r} frontmatter name must be 1-64 lowercase, digits, or hyphens") + if frontmatter_name != name: + raise ValueError(f"skill {skill.uri!r} frontmatter name {frontmatter_name!r} does not match URI name {name!r}") + description = skill.frontmatter.get("description") + if not isinstance(description, str) or not (1 <= len(description) <= 1024): + raise ValueError(f"skill {skill.uri!r} frontmatter description must contain 1 to 1024 characters") + + if skill.resources == "dynamic": + return + resources = skill.resources + if len(resources) > MAX_RESOURCES_PER_SKILL: + raise ValueError(f"skill {skill.uri!r} has {len(resources)} resources, exceeding {MAX_RESOURCES_PER_SKILL}") + seen: set[str] = set() + total_size = 0 + for resource in resources: + _validate_resource_uri_in_skill(skill.uri, resource.uri) + if resource.uri in seen: + raise ValueError(f"skill {skill.uri!r} lists resource {resource.uri!r} more than once") + seen.add(resource.uri) + if not _DIGEST_RE.fullmatch(resource.digest): + raise ValueError(f"skill {skill.uri!r} resource {resource.uri!r} has an invalid SHA-256 digest") + if resource.size < 0: + raise ValueError(f"skill {skill.uri!r} resource {resource.uri!r} has a negative size") + total_size += resource.size + if skill.uri not in seen: + raise ValueError(f"skill {skill.uri!r} resources does not include its own SKILL.md") + if total_size > MAX_TOTAL_SIZE: + raise ValueError(f"skill {skill.uri!r} has {total_size} bytes, exceeding {MAX_TOTAL_SIZE}") + + +def validate_list_result(result: ListSkillsResult) -> None: + """Validate every skill in `result.skills` and reject duplicate URIs. + + Raises: + ValueError: If any skill is invalid, or two entries share a `uri`. + """ + seen: set[str] = set() + for skill in result.skills: + validate_skill(skill) + if skill.uri in seen: + raise ValueError(f"skills/list result lists skill {skill.uri!r} more than once") + seen.add(skill.uri) + + +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 `result.resources` are exactly the direct children of `uri`. + + 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: + 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: + """Verify `content` (the bytes read from `uri`) against `skill`'s held manifest. + + Implements the SEP-2640 Integrity and verification requirement: a host + MUST verify a retrieved file's bytes against its manifest entry before + using them. Not applicable to a skill whose `resources` is `"dynamic"`, + which offers no digest to verify against. + + Raises: + ValueError: If `uri` is not one of `skill`'s resources, `skill.resources` + is `"dynamic"`, or `content` does not match the entry's `size`/`digest`. + """ + 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..878c9249bd --- /dev/null +++ b/tests/client/test_skills.py @@ -0,0 +1,226 @@ +"""Tests for the client-side Skills convenience wrappers (SEP-2640, `mcp.client.skills`).""" + +import hashlib +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 get_skill, list_skills, read_directory, read_skill_uri, verify_skill_resource +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 + + +async def test_list_skills_follows_next_cursor_to_completion() -> None: + async with Client(_server()) as client: + skills = await list_skills(client.session) + assert [s.uri for s in skills] == [_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 Client(server) as client: + with pytest.raises(ValueError, match="repeated"): + await list_skills(client.session) + + +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 Client(MCPServer("plain")) as client: + with pytest.raises(ValueError, match="does not advertise"): + await list_skills(client.session) + + +async def test_get_skill_returns_the_matching_entry() -> None: + async with Client(_server()) as client: + skill = await get_skill(client.session, _SKILL_URI) + assert skill.uri == _SKILL_URI + + +async def test_get_skill_propagates_the_servers_unknown_skill_error() -> None: + async with Client(_server()) as client: + with pytest.raises(MCPError) as exc_info: + await get_skill(client.session, "skill://missing/SKILL.md") + assert exc_info.value.code == INVALID_PARAMS + + +async def test_read_skill_uri_reads_the_registered_resource() -> None: + async with Client(_server()) as client: + result = await read_skill_uri(client.session, _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 Client(_server()) as client: + skill = await get_skill(client.session, _SKILL_URI) + result = await read_skill_uri(client.session, _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 Client(_server(with_directory_read=True)) as client: + resources = await read_directory(client.session, "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 Client(_server(with_directory_read=False)) as client: + with pytest.raises(ValueError, match="directoryRead"): + await read_directory(client.session, "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 Client(server) as client: + with pytest.raises(ValueError, match="repeated"): + await read_directory(client.session, "skill://git-workflow/references") + + +async def test_get_skill_rejects_a_mismatched_uri_from_a_non_conformant_server() -> None: + server = MCPServer("catalog", extensions=[_NonConformantGetSkill()]) + async with Client(server) as client: + with pytest.raises(ValueError, match="returned skill"): + await get_skill(client.session, "skill://other/SKILL.md") diff --git a/tests/docs_src/test_skills.py b/tests/docs_src/test_skills.py new file mode 100644 index 0000000000..cc68175db1 --- /dev/null +++ b/tests/docs_src/test_skills.py @@ -0,0 +1,55 @@ +"""`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 get_skill, list_skills, read_skill_uri, 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.""" + async with Client(tutorial001.mcp) as client: + skills = await list_skills(client.session) + assert [s.uri for s in skills] == [tutorial001.SKILL_URI] + + +async def test_get_skill_answers_by_uri() -> None: + """tutorial001: `get_skill` returns the same entry `list_skills` does.""" + async with Client(tutorial001.mcp) as client: + skill = await get_skill(client.session, 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.""" + async with Client(tutorial001.mcp) as client: + with pytest.raises(MCPError) as exc_info: + await get_skill(client.session, "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.""" + async with Client(tutorial001.mcp) as client: + skill = await get_skill(client.session, tutorial001.SKILL_URI) + result = await read_skill_uri(client.session, 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..88bc43b9a0 --- /dev/null +++ b/tests/server/test_skills.py @@ -0,0 +1,291 @@ +"""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 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 Invalid params error 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 == INVALID_PARAMS + + +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_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 Invalid params 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 == INVALID_PARAMS + + +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 Invalid params 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 == INVALID_PARAMS + + +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: + 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 == INVALID_PARAMS diff --git a/tests/shared/test_skills.py b/tests/shared/test_skills.py new file mode 100644 index 0000000000..f0faf72190 --- /dev/null +++ b/tests/shared/test_skills.py @@ -0,0 +1,375 @@ +"""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 + +import pytest +from mcp_types import Resource + +from mcp.shared.skills import ( + ListSkillsResult, + ReadDirectoryResult, + Skill, + SkillResource, + parse_directory_uri, + skill_name_from_uri, + validate_directory_result, + validate_list_result, + validate_skill, + 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_validate_skill_accepts_a_conformant_skill() -> None: + validate_skill(_skill()) + + +def test_validate_skill_rejects_a_non_string_frontmatter_name() -> None: + skill = Skill( + uri="skill://git-workflow/SKILL.md", + frontmatter={"name": 1, "description": "d"}, + resources=[_resource("skill://git-workflow/SKILL.md")], + ) + with pytest.raises(ValueError, match="frontmatter name"): + validate_skill(skill) + + +@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_validate_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" + skill = Skill( + uri=uri, + frontmatter={"name": name, "description": "d"}, + resources=[SkillResource(uri=uri, digest=_DIGEST, size=4)], + ) + with pytest.raises(ValueError, match="frontmatter name"): + validate_skill(skill) + + +def test_validate_skill_rejects_an_invalid_resource_uri() -> None: + """A resource URI with a query component fails the same shape check as a skill URI.""" + skill = _skill(extra_resources=[_resource("skill://git-workflow/x.md?y=1")]) + with pytest.raises(ValueError, match="is invalid"): + validate_skill(skill) + + +def test_validate_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" + skill = Skill( + uri=root, + frontmatter={"name": "refunds", "description": "d"}, + resources=[_resource(root), _resource("skill://acme/billing/other/x.md")], + ) + with pytest.raises(ValueError, match="outside the skill root"): + validate_skill(skill) + + +def test_validate_skill_rejects_a_traversal_segment_in_a_resource_uri() -> None: + skill = _skill(extra_resources=[_resource("skill://git-workflow/../evil.md")]) + with pytest.raises(ValueError, match="traversal segment"): + validate_skill(skill) + + +def test_validate_skill_rejects_a_negative_resource_size() -> None: + root = "skill://git-workflow/SKILL.md" + skill = Skill( + uri=root, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[SkillResource(uri=root, digest=_DIGEST, size=-1)], + ) + with pytest.raises(ValueError, match="negative size"): + validate_skill(skill) + + +def test_validate_skill_rejects_frontmatter_name_uri_mismatch() -> None: + """SEP-2640 Resource Mapping: `frontmatter.name` MUST equal the URI-derived name.""" + skill = Skill( + uri="skill://git-workflow/SKILL.md", + frontmatter={"name": "other-name", "description": "d"}, + resources=[_resource("skill://git-workflow/SKILL.md")], + ) + with pytest.raises(ValueError, match="does not match URI name"): + validate_skill(skill) + + +@pytest.mark.parametrize("description", ["", "x" * 1025]) +def test_validate_skill_rejects_out_of_range_description_length(description: str) -> None: + skill = Skill( + uri="skill://git-workflow/SKILL.md", + frontmatter={"name": "git-workflow", "description": description}, + resources=[_resource("skill://git-workflow/SKILL.md")], + ) + with pytest.raises(ValueError, match="description"): + validate_skill(skill) + + +def test_validate_skill_rejects_a_resource_outside_the_skill_root() -> None: + """SEP-2640 Resources: every entry MUST name a file within the skill's own directory.""" + skill = _skill(extra_resources=[_resource("skill://other-skill/file.md")]) + with pytest.raises(ValueError, match="outside the skill root"): + validate_skill(skill) + + +def test_validate_skill_rejects_missing_skill_md_entry() -> None: + """SEP-2640 Resources: `resources` MUST include an entry for the skill's own `SKILL.md`.""" + skill = Skill( + uri="skill://git-workflow/SKILL.md", + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[_resource("skill://git-workflow/references/GUIDE.md")], + ) + with pytest.raises(ValueError, match="does not include its own SKILL.md"): + validate_skill(skill) + + +def test_validate_skill_rejects_duplicate_resource_uris() -> None: + root = "skill://git-workflow/SKILL.md" + skill = Skill( + uri=root, frontmatter={"name": "git-workflow", "description": "d"}, resources=[_resource(root), _resource(root)] + ) + with pytest.raises(ValueError, match="more than once"): + validate_skill(skill) + + +@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_validate_skill_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.""" + root = "skill://git-workflow/SKILL.md" + bad = SkillResource(uri=root, digest=digest, size=1) + skill = Skill(uri=root, frontmatter={"name": "git-workflow", "description": "d"}, resources=[bad]) + with pytest.raises(ValueError, match="digest"): + validate_skill(skill) + + +def test_validate_skill_rejects_more_than_512_resources() -> None: + """SEP-2640 Limits: 512 entries per skill, `SKILL.md` included.""" + 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) + with pytest.raises(ValueError, match="exceeding 512"): + validate_skill(skill) + + +def test_validate_skill_rejects_total_size_over_16mib() -> None: + """SEP-2640 Limits: 16 MiB total per skill, summed over `resources[].size`.""" + root = "skill://git-workflow/SKILL.md" + skill = Skill( + uri=root, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[_resource(root, size=16 * 1024 * 1024 + 1)], + ) + with pytest.raises(ValueError, match="exceeding"): + validate_skill(skill) + + +def test_validate_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" + ) + validate_skill(skill) + + +def test_validate_list_result_rejects_duplicate_skill_uris_across_entries() -> None: + result = ListSkillsResult(skills=[_skill(), _skill()]) + with pytest.raises(ValueError, match="more than once"): + validate_list_result(result) + + +def test_validate_list_result_accepts_an_empty_listing() -> None: + """SEP-2640 Enumeration: `skills/list` MAY return an empty result.""" + validate_list_result(ListSkillsResult(skills=[])) + + +@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) + + +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")