diff --git a/docs/advanced/skills.md b/docs/advanced/skills.md index 56f95cfa40..18d3ae7825 100644 --- a/docs/advanced/skills.md +++ b/docs/advanced/skills.md @@ -35,7 +35,8 @@ 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. + the string `"dynamic"` for content generated on demand. An optional `_meta` dict rides along + unchanged on both methods. * `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. @@ -117,7 +118,8 @@ setting. ## Protocol version and caching -In protocol version `2026-07-28` and later, `skills/list` and `skills/get` results carry the base +In protocol version `2026-07-28` and later, `skills/list`, `skills/get`, and +`resources/directory/read` 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 `"private"` when your handler leaves it unset, and omits both fields entirely on an diff --git a/src/mcp/server/skills.py b/src/mcp/server/skills.py index 0c0fac05c4..36eee9fa5c 100644 --- a/src/mcp/server/skills.py +++ b/src/mcp/server/skills.py @@ -139,7 +139,7 @@ async def _handle_read_directory( validate_directory_result(params.uri, result) except ValueError: raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None - return result + return _finalize_cacheable(result, ctx.protocol_version) def _require_skill_md_uri(uri: str) -> None: @@ -159,7 +159,7 @@ def _require_directory_uri(uri: str) -> None: 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 + `skills/list`, `skills/get` and `resources/directory/read` 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"`, keeping user-specific entries out of shared caches. diff --git a/src/mcp/shared/skills.py b/src/mcp/shared/skills.py index eb6dbe51ca..0a0d1f1f85 100644 --- a/src/mcp/shared/skills.py +++ b/src/mcp/shared/skills.py @@ -104,6 +104,8 @@ class Skill(_SkillModel): """Resource URI of the skill's `SKILL.md`.""" frontmatter: Frontmatter resources: SkillResources + meta: dict[str, Any] | None = Field(alias="_meta", default=None) + """Opaque `_meta` the server attaches to this entry.""" @model_validator(mode="after") def _check_conformance(self) -> Skill: @@ -191,8 +193,12 @@ class ReadDirectoryParams(PaginatedRequestParams): """URI of the directory resource whose direct children are listed.""" -class ReadDirectoryResult(PaginatedResult): - """Result of `resources/directory/read`.""" +class ReadDirectoryResult(PaginatedResult, CacheableResult): + """Result of `resources/directory/read`. + + Like `ListSkillsResult`, this carries SEP-2549's `ttl_ms`/`cache_scope`, which + `mcp.server.skills` strips for a pre-2026-07-28 connection. + """ resources: list[Resource] diff --git a/tests/client/extensions/test_skills.py b/tests/client/extensions/test_skills.py index c6d044ea09..08aad0c1d3 100644 --- a/tests/client/extensions/test_skills.py +++ b/tests/client/extensions/test_skills.py @@ -188,6 +188,30 @@ async def test_get_skill_returns_the_matching_entry() -> None: assert skill.uri == _SKILL_URI +async def test_list_and_get_skill_preserve_skill_meta() -> None: + """ext-skills#157: a skill's optional `_meta` survives server -> wire -> client on both methods.""" + meta = {"example.com/owner": "platform"} + skill_with_meta = Skill( + uri=_SKILL_URI, + frontmatter={"name": "git-workflow", "description": "d"}, + resources=[SkillResource(uri=_SKILL_URI, digest=_SKILL_DIGEST, size=len(_SKILL_CONTENT))], + _meta=meta, + ) + + async def list_skills(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + return ListSkillsResult(skills=[skill_with_meta]) + + async def get_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult: + return GetSkillResult(skill=skill_with_meta) + + server = MCPServer("catalog", extensions=[Skills(list_skills=list_skills, get_skill=get_skill)]) + async with _skills(server) as skills: + listed = await skills.list_skills() + fetched = await skills.get_skill(_SKILL_URI) + assert [s.meta for s in listed] == [meta] + assert fetched.meta == meta + + 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: diff --git a/tests/server/test_skills.py b/tests/server/test_skills.py index 3f430838b4..8c74860333 100644 --- a/tests/server/test_skills.py +++ b/tests/server/test_skills.py @@ -179,6 +179,20 @@ async def mutating_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsP assert exc_info.value.code == INTERNAL_ERROR +async def test_skills_list_sends_skill_meta_as_underscore_meta() -> None: + """ext-skills#157: a skill's optional metadata crosses the wire as `_meta`, not `meta`.""" + + async def with_meta(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult: + skill = _git_workflow_skill().model_copy(update={"meta": {"example.com/owner": "platform"}}) + return ListSkillsResult(skills=[skill]) + + server = MCPServer("catalog", extensions=[Skills(list_skills=with_meta, 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["skills"][0]["_meta"] == {"example.com/owner": "platform"} + + async def test_skills_get_returns_the_matching_skill() -> None: async with Client(_server()) as client: result = await client.session.send_request( @@ -321,6 +335,28 @@ async def test_directory_read_returns_the_handlers_children() -> None: assert [r.uri for r in result.resources] == ["skill://git-workflow/references"] +async def test_directory_read_defaults_to_private_cache_scope_on_the_2026_07_28_wire() -> None: + """ext-skills#157: `resources/directory/read` results carry SEP-2549's cache fields too.""" + async with Client(_server(with_directory_read=True), mode="2026-07-28") as client: + raw = await client.session.send_request( + ReadDirectoryRequest(params=ReadDirectoryParams(uri="skill://git-workflow")), _RawResult + ) + extra = raw.model_extra or {} + assert (extra["ttlMs"], extra["cacheScope"]) == (0, "private") + + +async def test_directory_read_omits_cache_fields_on_a_legacy_wire() -> None: + """SEP-2549 fields are 2026-07-28+ only, and the runner's sieve skips extension methods, + so `Skills` strips them itself for an older connection.""" + async with Client(_server(with_directory_read=True), mode="legacy") as client: + raw = await client.session.send_request( + ReadDirectoryRequest(params=ReadDirectoryParams(uri="skill://git-workflow")), _RawResult + ) + extra = raw.model_extra or {} + assert "cacheScope" not in extra + assert "ttlMs" not in extra + + 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."""