Skip to content

Sep 2640 python sdk support - #3485

Open
vijaydeepsinha wants to merge 18 commits into
modelcontextprotocol:mainfrom
vijaydeepsinha:sep-2640-python-sdk-support
Open

Sep 2640 python sdk support#3485
vijaydeepsinha wants to merge 18 commits into
modelcontextprotocol:mainfrom
vijaydeepsinha:sep-2640-python-sdk-support

Conversation

@vijaydeepsinha

@vijaydeepsinha vijaydeepsinha commented Sep 9, 2026

Copy link
Copy Markdown

Fixes #3486

Summary

Adds Python SDK support for SEP-2640 (Skills Extension): skills/list, skills/get, and resources/directory/read as 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/MethodBinding mechanism (the same one backing the shipped Apps extension, 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), and verify_skill_resource (digest+size integrity check for content already read).
  • src/mcp/server/skills.py — the Skills extension: handler-based (list_skills, get_skill, optional read_directory), validates results against SEP-2640 before they hit the wire, gates the SEP-2549 ttlMs/cacheScope fields to protocol version 2026-07-28+.
  • src/mcp/client/skills.pylist_skills/get_skill/read_directory (auto-paginating, with cursor-repeat detection), read_skill_uri (a thin, discoverable resources/read alias), verify_skill_resource re-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

from mcp.server.skills import Skills
from mcp.shared.skills import ListSkillsResult, GetSkillResult

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)])
mcp.add_resource(TextResource(uri="skill://git-workflow/SKILL.md", ...))  # file content is served normally

Client usage

from mcp.client.skills import get_skill, list_skills, read_skill_uri, verify_skill_resource

skills = await list_skills(client.session)
skill = await get_skill(client.session, "skill://git-workflow/SKILL.md")
result = await read_skill_uri(client.session, skill.uri)
verify_skill_resource(skill, skill.uri, result.contents[0].text.encode())

Protocol version / compatibility notes

  • capabilities.extensions (SEP-2133) and ttlMs/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. Skills gates its own cache fields to match.
  • A JSON-RPC -32602 error 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 shared ERROR_CODE_HTTP_STATUS table; nothing Skills-specific.
  • No breaking changes to any existing public API. All new files; the only touched existing file is 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 via coverage report --fail-under=0.
  • Full suite: ./scripts/test → 6000+ passed, 0 failed, 100% total coverage, strict-no-cover clean.
  • ruff format/ruff check, pyright, markdownlint, mkdocs build --strict (Zensical), README-snippet check: all clean.
  • Cross-version: uv run --python 3.10 pytest tests/*/test_skills.py passes.

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):

Scenario Checks Result
sep-2640-skills-enumeration (skills/list + skills/get) 30 ✅ 30/30
sep-2640-skills-manifest (SKILL.md resource metadata) 6 ✅ 6/6
sep-2640-skills-directory (resources/directory/read) 7 ✅ 7/7
sep-2640-client-no-prefetch 1 ✅ PASS
sep-2640-client-verify-digest 1 ✅ PASS
sep-2640-client-verify-size 1 ✅ PASS
sep-2640-client-verify-frontmatter 1 ✅ PASS
sep-2640-client-verify-unlisted 1 ✅ PASS

43 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)

  • Filesystem discovery/indexing/caching: this SDK gives the protocol primitives only; a higher-level provider library (e.g. one that walks a directory tree and computes digests) builds on top of Skills, list_skills, and get_skill.
  • Client-side frontmatter parsing: SEP-2640 requires hosts to parse a 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_resource covers the digest/size half (no new dependency needed for that). Documented explicitly in docs/advanced/skills.md's "What this SDK doesn't do".
  • Skills types living in mcp-types instead of mcp.shared: mcp-types is 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 existing Apps extension (SEP-2133) uses.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Documentation update

Checklist

  • I am assigned to the linked issue (or it is labeled help wanted, or I'm a maintainer) — N/A while targeting my own fork; will file/link before retargeting to upstream
  • I have disclosed any AI assistance and can explain the change in my own words
  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

vijay added 9 commits September 9, 2026 20:07
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.
@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Sep 9, 2026
@github-actions github-actions Bot closed this Sep 9, 2026
@pja-ant pja-ant reopened this Sep 9, 2026
@github-actions github-actions Bot added bypass-issue-check and removed missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) labels Sep 9, 2026
@pja-ant

pja-ant commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Assigned #3486 to @vijaydeepsinha and re-opened

@vijaydeepsinha
vijaydeepsinha force-pushed the sep-2640-python-sdk-support branch from 321ab53 to b845490 Compare September 10, 2026 00:12
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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 12 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/mcp/client/skills.py Outdated
Comment thread src/mcp/client/skills.py Outdated
Comment thread docs/advanced/skills.md Outdated
Comment thread docs/advanced/skills.md Outdated
Comment thread src/mcp/server/skills.py Outdated
Comment thread src/mcp/shared/skills.py Outdated
Comment thread src/mcp/shared/skills.py
Comment thread docs_src/skills/tutorial001.py Outdated
vijay added 4 commits September 10, 2026 06:20
…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.
@vijaydeepsinha
vijaydeepsinha force-pushed the sep-2640-python-sdk-support branch from 3b09033 to f1d2920 Compare September 10, 2026 00:54
…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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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)

vijay added 3 commits September 10, 2026 06:41
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add SEP-2640 (Skills Extension) protocol support

2 participants