Sep 2640 python sdk support - #3485
Conversation
Wire types, request/result models, and SEP-2640 conformance validation (name/URI/frontmatter rules, resource-manifest completeness, digest and size verification) for the Skills extension, shared by the server and client surfaces.
Skills extension (io.modelcontextprotocol/skills): serves skills/list, skills/get, and the optional resources/directory/read behind the directoryRead capability setting. Handlers are supplied by the server author; the extension validates results against SEP-2640 before they reach the wire and gates the SEP-2549 ttlMs/cacheScope fields to protocol version 2026-07-28+.
Thin client wrappers for skills/list, skills/get, resources/directory/read, and resources/read: list_skills and read_directory follow nextCursor to completion, all four validate the server's response before returning it, and verify_skill_resource checks a read's bytes against a held skill's manifest entry.
Adds the Skills page under Advanced, with a runnable server/client example, and tests proving every claim the page makes against the real SDK.
Parametrize the digest-format rejection test over near-miss cases (uppercase, wrong length, missing/wrong prefix), and add explicit JSON round-trip tests for both shapes of the resources union type (a static array and the "dynamic" marker) to prove neither collapses or mistags on the wire.
_resource_uri_in_skill reads like a boolean predicate but returns None and raises; rename to _validate_resource_uri_in_skill to match its sibling validators (validate_skill, validate_list_result, validate_directory_result) and signal that it asserts.
_handle_read_directory inlined the same "validate incoming URI, convert ValueError to MCPError" pattern that _handle_get had already extracted into a helper. Add a parallel _require_directory_uri so both handlers open with a symmetric one-line precondition check, matching the _require_ui_scheme helper idiom from the Apps extension.
validate_skill already rejects names that violate the Agent Skills grammar (SEP-2640 defers to it), but nothing pinned the edge cases. Add a parametrized test covering consecutive, leading, and trailing hyphens, uppercase, underscores, and the 64-character ceiling.
- Correct the server/client snippet hl_lines, which highlighted blank and unrelated lines after the example imports were expanded. - Fix "all four validate": read_skill_uri is a thin resources/read pass-through that validates nothing, contradicting the same section's own next paragraph. Only list_skills/get_skill/read_directory validate. - Replace the phantom add_resource_template API (no such method) with the @mcp.resource(...) template decorator, in both the guide and the mcp.server.skills module docstring.
|
Assigned #3486 to @vijaydeepsinha and re-opened |
321ab53 to
b845490
Compare
Two behavioral cases the existing suite left unpinned: - A "dynamic" skill now round-trips through the real server extension, the wire, and the client wrapper (validated on both ends), proving the resources union survives intact rather than only in an isolated model round-trip. - list_skills honours a caller-supplied starting cursor, skipping the pages before it — the resume-from-a-saved-cursor contract.
There was a problem hiding this comment.
All reported issues were addressed across 12 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…RAMS A non-conformant result from a `list_skills`/`get_skill`/`read_directory` handler (or a `get_skill` URI mismatch) is a server-side bug, not a bad caller request, so -32603 is the correct code rather than -32602. Log the real cause server-side and return a generic message, mirroring the runner's existing handling of invalid handler results. Input validation (`_require_skill_md_uri`, `_require_directory_uri`, params) stays -32602.
… children `_validate_resource_uri_in_skill` now rejects a resource URI ending in `/`, which names a directory rather than a file, and `validate_directory_result` now rejects a `.`/`..` child, which is a traversal segment rather than a real direct child. Both slipped past the prior checks.
…ages `list_skills`/`read_directory` now seed the seen-cursor set with a caller-supplied starting cursor, so a server echoing that cursor is caught on the first page instead of being chased a second time. Rebuilding each page request from the caller's own params (via model_copy) also carries `_meta` forward to every page rather than dropping it after the first.
…ic skills `read_skill_uri` returns a `ReadResourceResult`, not bytes, and `verify_skill_resource` raises for a `"dynamic"` skill (no digests to check). Import the tutorials' symbols from `mcp.types` rather than the internal `mcp_types` package.
3b09033 to
f1d2920
Compare
…ent test The keyword form `ListSkillsParams(meta=...)` fails pyright: the field's alias is `_meta`, so the synthesized constructor only accepts the alias. Build the params through `model_validate` instead, matching how the field is populated off the wire.
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/client/test_skills.py">
<violation number="1" location="tests/client/test_skills.py:299">
P2: The assertion checks `m.get("progress_token")`, but the wrapper only ever sets the camelCase key: `ListSkillsParams.model_validate({"_meta": {"progressToken": "t"}})`. Pydantic does not alias keys inside the `_meta` dict, and `send_request` dumps it with `model_dump(by_alias=True)`, so the server handler's `params.meta` contains `"progressToken"`, not `"progress_token"` — `m.get("progress_token")` is `None`, making `m.get(...) == "t"` always False and the test fail (or, if a transport layer renames keys, the check no longer verifies that the caller's token reached the server, contradicting the docstring). Assert on `"progressToken"` instead.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| # The transport 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) |
There was a problem hiding this comment.
P2: The assertion checks m.get("progress_token"), but the wrapper only ever sets the camelCase key: ListSkillsParams.model_validate({"_meta": {"progressToken": "t"}}). Pydantic does not alias keys inside the _meta dict, and send_request dumps it with model_dump(by_alias=True), so the server handler's params.meta contains "progressToken", not "progress_token" — m.get("progress_token") is None, making m.get(...) == "t" always False and the test fail (or, if a transport layer renames keys, the check no longer verifies that the caller's token reached the server, contradicting the docstring). Assert on "progressToken" instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/client/test_skills.py, line 299:
<comment>The assertion checks `m.get("progress_token")`, but the wrapper only ever sets the camelCase key: `ListSkillsParams.model_validate({"_meta": {"progressToken": "t"}})`. Pydantic does not alias keys inside the `_meta` dict, and `send_request` dumps it with `model_dump(by_alias=True)`, so the server handler's `params.meta` contains `"progressToken"`, not `"progress_token"` — `m.get("progress_token")` is `None`, making `m.get(...) == "t"` always False and the test fail (or, if a transport layer renames keys, the check no longer verifies that the caller's token reached the server, contradicting the docstring). Assert on `"progressToken"` instead.</comment>
<file context>
@@ -253,3 +253,47 @@ async def test_list_skills_starts_from_a_caller_supplied_cursor() -> None:
+ # The transport 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)
</file context>
| assert all(m is not None and m.get("progress_token") == "t" for m in seen_meta) | |
| assert all(m is not None and m.get("progressToken") == "t" for m in seen_meta) |
…handler A caller's `_meta.progressToken` is carried over the wire under its camelCase JSON alias but deserialized back to the snake_case field `progress_token`, so a server handler reading `params.meta` finds `progress_token`. Pin both forms of the same params object for `skills/list` and `resources/directory/read`, and expand the client round-trip test's comment to spell out the distinction.
Fixes #3486
Summary
Adds Python SDK support for SEP-2640 (Skills Extension):
skills/list,skills/get, andresources/directory/readas protocol primitives, SEP-2640 conformance validation, and capability negotiation. This SDK does not provide filesystem discovery, catalog indexing, or caching/refresh policy — those belong to a higher-level provider built on top of this.Motivation and Context
SEP-2640 defines a convention for serving Agent Skills over MCP using the Resources primitive. The Python SDK has no support for it today. This PR adds the extension using the SDK's existing
Extension/MethodBindingmechanism (the same one backing the shippedAppsextension, SEP-2133) — no schema or codegen changes, no new required dependencies.What's included
src/mcp/shared/skills.py— wire types (Skill,SkillResource, params/results), SEP-2640 conformance validation (name/URI/frontmatter consistency, digest format, resource-manifest completeness, the 512-entry/16 MiB limits), andverify_skill_resource(digest+size integrity check for content already read).src/mcp/server/skills.py— theSkillsextension: handler-based (list_skills,get_skill, optionalread_directory), validates results against SEP-2640 before they hit the wire, gates the SEP-2549ttlMs/cacheScopefields to protocol version 2026-07-28+.src/mcp/client/skills.py—list_skills/get_skill/read_directory(auto-paginating, with cursor-repeat detection),read_skill_uri(a thin, discoverableresources/readalias),verify_skill_resourcere-exported for client use.docs/advanced/skills.md+docs_src/skills/— a new doc page with a runnable example, explicitly scoping what the SDK does and doesn't do.Server usage
Client usage
Protocol version / compatibility notes
capabilities.extensions(SEP-2133) andttlMs/cacheScope(SEP-2549) are 2026-07-28+-only wire fields in this SDK's existing type surface — this is pre-existing, documented SDK behavior (docs/advanced/extensions.md), not something this PR changes.Skillsgates its own cache fields to match.-32602error returns HTTP 200 on the classic (pre-2026-07-28) wire and HTTP 400 on the modern (2026-07-28+) wire. This is existing, spec-mandated (SEP-2575) SDK-wide transport behavior — every handler in the SDK gets it automatically via the sharedERROR_CODE_HTTP_STATUStable; nothing Skills-specific.mkdocs.yml(one nav entry).How Has This Been Tested?
tests/{shared,server,client,docs_src}/test_skills.py— 100% line+branch coverage on all three new modules (shared/skills.py,server/skills.py,client/skills.py), verified viacoverage report --fail-under=0../scripts/test→ 6000+ passed, 0 failed, 100% total coverage,strict-no-coverclean.ruff format/ruff check,pyright,markdownlint,mkdocs build --strict(Zensical), README-snippet check: all clean.uv run --python 3.10 pytest tests/*/test_skills.pypasses.Conformance
Ran the modelcontextprotocol/conformance PR #330 SEP-2640 scenarios end-to-end against a real server and client built on this implementation (server scenarios exercise this PR's server; client scenarios exercise this PR's client against a hostile server the harness stands up):
sep-2640-skills-enumeration(skills/list+skills/get)sep-2640-skills-manifest(SKILL.mdresource metadata)sep-2640-skills-directory(resources/directory/read)sep-2640-client-no-prefetchsep-2640-client-verify-digestsep-2640-client-verify-sizesep-2640-client-verify-frontmattersep-2640-client-verify-unlisted43 wire checks + 5 client checks, 0 failures, 0 warnings.
Also manually verified via a live server against a Postman collection covering both the session-based (2025-11-25) and stateless (2026-07-28) wires.
Breaking Changes
None. New files only; no existing public API is modified.
Deliberate scope exclusions (and why)
Skills,list_skills, andget_skill.SKILL.md's YAML frontmatter and compare it field-by-field against the held entry. This SDK does not ship that comparison, to avoid adding a new required YAML dependency to the core SDK for a check any host already has the means to do with whatever YAML library it uses elsewhere.verify_skill_resourcecovers the digest/size half (no new dependency needed for that). Documented explicitly indocs/advanced/skills.md's "What this SDK doesn't do".mcp-typesinstead ofmcp.shared:mcp-typesis generated from the official, versioned MCP JSON Schema; SEP-2640 is an extension, not core spec vocabulary, so its types are hand-written and live alongside the extension code — the same placement the existingAppsextension (SEP-2133) uses.Types of changes
Checklist
help wanted, or I'm a maintainer) — N/A while targeting my own fork; will file/link before retargeting to upstream