feat(sep-2640): skills server conformance against the Accepted SEP, enumeration + manifest + directory - #330
Open
panyam wants to merge 19 commits into
Open
feat(sep-2640): skills server conformance against the Accepted SEP, enumeration + manifest + directory#330panyam wants to merge 19 commits into
panyam wants to merge 19 commits into
Conversation
…rce-template) Re-extract sep-2640.yaml against current SEP-2640 HEAD. The SEP removed the `mcp-resource-template` index entry type in two commits on 2026-06-04 (fd50cc91 "Remove mcp-resource-template entries from skill index", 556154c0 "Remove remaining resource template mentions from SDK and rationale sections"), after this extraction was first captured on 2026-06-03 at b77fdfe8. Provenance comment bumped accordingly. Resulting changes: - sep-2640-index-entry-type-enum: drop `"mcp-resource-template"` from the enum. - sep-2640-index-name-required: drop the "is Required for ... Omitted for mcp-resource-template" conditional language; the column is now unconditional Required=Yes at SEP HEAD. - sep-2640-index-digest-required: same simplification; also align "hexadecimal" -> "hex" with the HEAD table text. - sep-2640-template-resource-template-registered: removed. The SEP no longer defines a template entry type, so the SHOULD on registering an MCP resource template is gone. - Excluded "Hosts SHOULD surface template entries..." excerpt: removed. The sentence no longer exists in the SEP. Sweep verified: zero "template" mentions in the SEP at HEAD. Security Implications section sentences referenced by the remaining excluded entries are intact. No other check rows touched.
…y/read
Adds the conformance scenario for the SEP-2640 directoryRead surface that
landed in spec commit 2e04c48d (2026-06-09). Per AGENTS.md "fewer scenarios,
more checks", a single ResourcesDirectoryReadScenario emits 6
ConformanceChecks, one per new sep-2640.yaml requirement row.
Class named for the wire method (resources/directory/read), matching the
existing ResourcesListScenario / ResourcesReadTextScenario / etc. family in
src/scenarios/server/resources.ts. The runner-facing name field stays as
'sep-2640-skills' (umbrella) so mcpkit's conformance/Makefile entry
--scenario sep-2640-skills keeps working without a cross-repo race.
src/seps/sep-2640.yaml
- File-level provenance held at 556154c (the existing PR 330 baseline).
- 6 new check rows appended for the directoryRead additions, each
carrying a verbatim, grep-F-searchable excerpt from the SEP at
2e04c48da90224000e750ffd54a3611f2824fbc0:
- sep-2640-capability-directory-read-flag
- sep-2640-directory-read-method-registered
- sep-2640-directory-read-subdir-mimetype
- sep-2640-directory-read-result-resources-shape
- sep-2640-directory-read-invalid-params
- sep-2640-directory-read-pagination
- forward_reference header comment notes the asymmetry: PR 97 schema
rewrite (360123d0, 2026-06-08) made 3 existing rows stale and drifted
~11 others' wording; full re-extraction at SEP HEAD is mcpkit#780's
lifecycle. Provenance deliberately holds at 556154c until that lands.
src/types.ts
- Adds io.modelcontextprotocol/skills to EXTENSION_IDS so the scenario's
source: { extensionId: ... } tag type-checks.
src/scenarios/server/directory.ts (new)
- Capability discovery via wire-observable signal: -32601 method-not-found
is the only definitive "server did not declare directoryRead" signal.
- Fixture assumption: server exposes skill://acme/billing/refunds/templates
with at least one subdirectory child (mcpkit examples/skills layout).
No skill:// resources at all -> every check emits SKIPPED so the
scenario stays green against upstream's everything-server fixture.
- 6 checks (1:1 with the YAML rows above):
1. directoryRead declared (derived from method registration)
2. method registered (happy-path call succeeds)
3. result.resources shape matches resources/list
4. subdir child carries mimeType: "inode/directory"
5. non-directory URI returns -32602
6. nextCursor round-trips (single-page is conformant)
src/scenarios/index.ts
- Imports + registers ResourcesDirectoryReadScenario in
allClientScenariosList, matching the registration name 'sep-2640-skills'
that mcpkit's conformance/Makefile already passes to --scenario.
Verified end-to-end against mcpkit examples/skills:
cd ~/newstack/mcpkit/main && \
MCPCONFORMANCE_SKILLS_PATH=~/newstack/mcpkit/conf-skills \
make -C conformance testconf-skills
-> 6/6 SUCCESS, 0 failed, 0 warnings.
Out of scope:
- PR 97 schema rewrite YAML refresh (separate ticket, mcpkit#780 lifecycle).
- Negative-capability fixture (server without directoryRead) - mcpkit
examples/skills has no flag for it; ext/skills/client_directory_test.go
already covers the SDK pre-call guard.
feat(sep-2640): ResourcesDirectoryReadScenario for resources/directory/read (mcpkit#784)
panyam
marked this pull request as ready for review
June 16, 2026 21:34
panyam
marked this pull request as draft
June 16, 2026 21:34
# Conflicts: # src/types.ts
commit: |
# Conflicts: # src/types.ts
…s, drift fixes
Builds on the directory-read scenario with the server-observable skills surface
and brings the whole set up to the current framework conventions.
New scenarios (src/scenarios/server/skills/):
- SkillsIndexScenario (sep-2640-skills-index): skill://index.json is exposed and
readable; every skills[].type is in the SEP enum; every entry has a non-empty
name; a present digest matches sha256:{64hex}; entry URLs use skill://.
- SkillsManifestScenario (sep-2640-skills-manifest): a skill-md SKILL.md is
markdown-typed; frontmatter name/description surface on the resource; the
final skill-path segment equals the frontmatter name; skill _meta keys use the
reverse-domain prefix.
directory.ts (moved into skills/): capability is now read from server/discover
instead of inferred from -32601; an undeclared optional capability SKIPs rather
than FAILs; the untestable.ts helpers back the not-exercisable paths; all
mcpkit-specific fixture URIs are gone — the directory to exercise is discovered
dynamically, so the scenario is brand-neutral.
Capability gating accepts directoryRead at either extensions[id].directoryRead
(SEP-2640's inline example) or extensions[id].config.directoryRead (the SEP-2133
negotiation envelope SEP-2640 defers to); the two SEPs are inconsistent on
nesting, so the check accepts either rather than privileging one reading.
All three scenarios verified green against a live SEP-2640 server (6/6, 6/6, 7/7);
typecheck, eslint, prettier, and the 504-test vitest suite pass. sep-2640.yaml
gains a header note mapping each scenario to the rows it now backs.
panyam
changed the title
panyam
marked this pull request as ready for review
August 4, 2026 04:38
The prior extraction held at SEP commit 556154c (2026-06-05) by an explicit
in-file decision to wait for the index-schema refactor. The 08-21 core-maintainer
rewrite made that pin actively wrong rather than merely behind.
Removed as no longer normative: the skill://index.json well-known resource
(absent from the current text), the entry type enum, the no-nested-skills rule
(now reversed - skills MAY nest), and the five archive rows (archives are
deferred to an appendix). Reworded the two index-field rows, the empty-listing
row and the verify-digest row.
Added the new surface: skills/list and skills/get with their pagination,
atomicity and caching attributes; the {uri, frontmatter, resources} entry
schema; resources completeness, the "dynamic" sentinel and per-file size; the
512-entry and 16 MiB limits; and the host obligations around lazy retrieval,
content-bound approval and frontmatter re-verification.
index.ts is replaced by enumeration.ts, which drives both new methods. manifest.ts
and directory.ts swap their skill://index.json discovery fallback for skills/list.
The capability helper no longer accepts a config envelope. SEP-2133 is Final and
defines the extensions field as a map of identifiers to settings objects, with no
envelope and no slot for id/specVersion/stability; SEP-2640's capability block
matches. The earlier leniency was based on a misreading.
77 rows: 65 checks (39 wire-tested by the three server scenarios) and 12 excluded
as host-internal.
…ce audit The first pass extracted the Specification sections but under-covered Security Implications, which carries 21 of the SEP's 97 RFC-2119 sentences. Sweeping every normative sentence in the source surfaced rows that were missing entirely: - origin tagging at the point content enters model context (the L179 host MUST) - allowed-tools MUST be ignored for MCP-origin skills - reads bound to the originating server, so a skill from A cannot drive a read against B, and any cross-origin read gated behind per-call approval - servers identified by a host-assigned label, never serverInfo.name - the three disk-cache rules: write-isolation or re-hash, exclusion from filesystem-skill discovery paths, and removal when a server is removed - a resources/read of a SKILL.md by another route is not a load - a nested SKILL.md read as supporting content is ordinary markdown - the registry-assembly and virtual-mount no-prefetch restatements - the SDK affordances guidance, which is the row mcpkit itself is measured by Five rows previously filed as excluded are now declared instead. They are real requirements that a host-side harness could check; calling them unobservable was a limitation of this scenario set, not of the requirement. Adds one wire check, names-should-be-unique, since a within-listing collision is observable and tells an operator the listing needs disambiguating. 96 rows: 89 checks (40 wire-tested) and 7 excluded.
Quote-style normalisation in the yaml (escaped single quotes become double-quoted scalars) and one return-type wrap in helpers.ts. The parsed yaml is byte-identical in content; verified by comparing the parse before and after.
It appears in all three result examples but is a base-protocol field on the common Result interface, not a SEP-2640 requirement; its caching-hint obligations are already owned by sep-2549.yaml.
panyam
changed the title
Member
|
I ran these scenarios against modelcontextprotocol/go-sdk#1238 plus its split filesystem-helper branch and found one harness false negative around directory pagination. Repro: configure the server directory page size to 1, with root children The run also confirmed the already-documented runner diagnostic: omitting Final pinned-wire results after fixing a YAML normalization bug caught by the suite: enumeration 29/29, manifest 4/4 with the two expected metadata SHOULD warnings, directory 7/7, no failures. |
The directory scenario issued one resources/directory/read and every check below it inspected that single page: the result-shape check, the subdirectory-mimeType check, and the target selection for the -32602 probe. SEP-2640 says directory-read pagination mirrors resources/list, so a conformant server MAY split a directory across pages, and reading only the first made "no subdirectory here" indistinguishable from "the subdirectory is on page two". Reported by Sam Bloomberg against the go-sdk implementation, which exposes DirectoryOptions.PageSize. Rebuilding that server with PageSize 1 fails sep-2640-directory-read-subdir-mimetype; with this change it passes 7/7, and both servers still pass at their default page size. Only findable against a second implementation. mcpkit hardcoded its page size to zero with no override, so it never emitted a cursor and the defect was invisible to every run against it. listAllResources already did this for resources/list. directoryReadAll applies the same shape to directory reads, with a page bound and a repeated-cursor guard.
Contributor
Author
|
Confirmed and now fixed. @sambhav - spot on with the diagnosis. directoryReadAll now aggregates pages with a page bound and a repeated-cursor guard, the same shape listAllResources already used for resources/list. The pattern was in the file and had simply never been applied to directory reads. Verified against your SUT rebuilt with DirectoryOptions{PageSize: 1}:
Agreed on the -32022 diagnostic being a runner-message problem rather than a scenario or SDK one. I would rather fix that upstream than massage over it here. I love that the suite caught a bug in the go-sdk (yaml norm) and vice versa (pagination bug in the suite). Neither was findable from one side. In mcpkit I hardcoded pagesize to 0 with no override, so it never emited a cursor and every run was against a server with exactly one page. By changing page boundary in a conforming way, this failure mode can be caught. Once go-sdk#1238 and the filesystem-helper branch settle happy to add a go-sdk row to the results table. I can run both later too. |
7 tasks
The traceability file filed retrieval policy under "host obligations a harness cannot observe", alongside 48 other rows. That was wrong for this one: "hosts MUST NOT retrieve a skill's files ahead of need" reduces to whether a request arrived, which makes it among the most wire-observable rules in the SEP. Adds a Scenario (harness stands up the server, client is the SUT) that serves a listing naming a SKILL.md and a supporting file, then fails if either is read. The client's contract, keyed on MCP_CONFORMANCE_SCENARIO, is to connect, enumerate, and stop. SKIPs rather than passes when the client never calls skills/list, since without a listing there is no window in which prefetching is possible and a pass would prove nothing. Verified both directions against mcpkit's client: SUCCESS as written, and FAILURE with a deliberate ReadFromEntry added to the driver. A check that has never been shown to fail is the same trap as the directory-pagination false negative this suite shipped for months.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
src/seps/sep-2640.yaml, the Skills Extension requirement-traceability file, together with three runnable server scenarios that exercise the server-observable half of it.SEP-2640 passed the core-maintainer vote on 2026-09-01 and is Accepted. This branch is extracted against spec branch head
a3e147ca2710(2026-08-25), which is the commit the vote ran on, so the requirements here track the voted text rather than an earlier draft. An earlier revision of this PR targeted the June draft and testedskill://index.json, thetypeenum and archive distribution. All three were removed from the SEP by the 2026-08-21 rewrite and have been removed here.Coverage, stated up front
The SEP carries 97 sentences with an RFC 2119 term. This file declares 96 rows: 89 checks and 7 excluded, each exclusion carrying its reason inline. Of the 89 checks, 40 run on the wire across the three scenarios below.
The other 49 are client and host obligations, which a harness acting as a client cannot observe. They split into two groups, and the split matters for what happens next.
A substantial subset is testable with the mechanism this repo already has. Retrieval policy (a client MUST NOT fetch a skill's files ahead of need), reads of URIs absent from
resources, digest and size mismatches, frontmatter that disagrees with the entry, a directory listing being treated as extending the manifest, and cross-origin reads are all observable by aScenariothat stands up a server and points a client implementation at it, the same shape as the existing client scenarios undersrc/scenarios/client/. We have not written those yet. We would like to, and would rather do it as a follow-up PR than bolt it onto this one.The remainder is genuinely out of reach of any harness. What a host displays, whether a user was actually prompted, disk-cache isolation, and the "acting on a skill" context window are application-internal. Those stay traceability-only by nature rather than by omission, and the 7 excluded rows below are the subset where we could not even state a check.
The 7 excluded rows are things the wire genuinely cannot show: DNS resolution of a URI authority component, what a host displays to a user, and cache hits, which are the absence of a request and indistinguishable from a host that simply did not need the file again.
What the three scenarios cover
sep-2640-skills-enumeration(enumeration.ts, 29 checks) -skills/listandskills/get, which replaced the retiredskill://index.jsonresource. Covers pagination and the atomic-entry rule, thettlMs/cacheScopeattributes on 2026-07-28 and later, the{uri, frontmatter, resources}entry schema,resourcescompleteness and the"dynamic"sentinel, per-resourcedigestandsize, the 512-entry and 16 MiB limits, URI scheme and naming rules, and the capability declaration.sep-2640-skills-manifest(manifest.ts, 5 checks) - the SKILL.md resource itself: markdown mime type, frontmatternameanddescriptionsurfacing on the resource, the final skill-path segment equalling the frontmattername, and theio.modelcontextprotocol.skills/reverse-domain prefix on_metakeys.sep-2640-skills-directory(directory.ts, 6 checks) -resources/directory/read, which survived the rewrite behind the samedirectoryReadcapability gate. Reads the capability fromserver/discoverrather than inferring it from a-32601, SKIPs whendirectoryReadis undeclared, and discovers the directory to exercise dynamically.Discovery is dynamic throughout, so the scenarios certify any conformant SEP-2640 server rather than one implementation's fixture.
Motivation and Context
Conformance tests are one of the three deliverables the WG named as gating SEP-2640's move from Accepted to Final, alongside a reference implementation in a Tier 1 SDK and spec-language docs in the extension repo. This PR is that deliverable.
The traceability YAML went up first to float the requirement model for WG review ahead of scenario design. With the model settled and the SEP voted, this lands the scenarios that assert the server-observable half.
How Has This Been Tested?
Two independent implementations (09/04), both green.
modelcontextprotocol/go-sdkext/skillsRun 2026-09-04. Per-scenario,
wire-schema-valid.The two-check difference is not a conformance gap. go-sdk serves
SKILL.mdthrough resource-template dispatch, so it does not appear inresources/listand the twoSHOULD-level metadata checks (sep-2640-skillmd-metadata-nameand-description) report untestable rather than passing. mcpkit registers it as a listed resource, so the same two are exercised. Both are conformant and this is a nice (imo) example of what a server-side harness can and cannot see.The scenarios hardcode no fixture URIs, so pointing them at a new implementation is a URL change. As other SDKs/Servers are ready happy to run the suite against them and add a row.
What the pagination checks do and do not prove.
skills/listandresources/directory/readare both followed to exhaustion, with a page bound and a repeated-cursor guard, and the round-trip is asserted whenever a server offers a cursor. A harness cannot make a server paginate, though, so against a server that returns everything in one page these checks pass without a multi-page flow ever running. Read them as "cursors are honoured correctly when present" rather than as pagination coverage.These checks do fail when they should.
sep-2640-capability-declaration-inlinecaught a server nesting its extension settings one level too deep, which burieddirectoryReadwhere a spec-following client could not see it and made sixresources/directory/readrows SKIP against a feature that was implemented correctly underneath. That bug is invisible to manual testing and was invisible to its author until a check asserted the exact nesting.Running against a server below 2026-07-28. SEP-2640 states it "has no dependencies beyond the base MCP Resources primitive", and only
ttlMs/cacheScopeare conditioned on 2026-07-28 and later, which the enumeration scenario already checks conditionally. Verified at 30/30 on both the stateful 2025-11-25 wire and the stateless 2026-07-28 one. The runner defaults to the stateless draft wire and extension scenarios do not match--spec-versionalone, so an older server needs both flags:Without them a 2025-x server refuses with
-32022and the failure reads as a missing method rather than a wire mismatch. Worth a clearer message, though that belongs in the runner rather than this PR.npm run check(typecheck, eslint, prettier) passes. The vitest suite passes. lefthook pre-push passes.Breaking Changes
None. The PR is purely additive: four new files under
src/scenarios/server/skills/, one new file undersrc/seps/, plusio.modelcontextprotocol/skillsappended toEXTENSION_IDSinsrc/types.tsand the three scenarios registered in both the pending and the all-scenarios lists insrc/scenarios/index.ts.The scenarios sit in the pending list because the everything-server does not implement the skills extension. Targeted runs point at a conformant fixture with
npm start -- server --scenario sep-2640-skills-* --url <fixture>. Each scenario SKIPs cleanly against a server that does not declare the extension.Types of changes
Checklist
Additional context
The capability-nesting question from the earlier revision is resolved, and the check no longer accepts both forms. That revision flagged a disagreement between SEP-2640's capability example, which places
directoryReaddirectly on the extension object, and an SEP-2133 reading that wrapped settings in a{specVersion, stability, config}envelope. It asked which nesting was canonical and accepted either.SEP-2133 is Final and settles it. It defines the field as a map of extension identifiers to per-extension settings objects, with no envelope, and SEP-2640's declaration block matches that exactly. There is no disagreement between the two SEPs. The envelope was an implementation's invention.
sep-2640-capability-declaration-inlinenow asserts the inline form only, which is what caught the bug described above.A note on
resultType, which appears in all three result examples in the SEP but is deliberately not declared here. It is a base-protocol field on the commonResultinterface rather than a SEP-2640 requirement, and its caching-hint obligations belong tosep-2549.yaml. Declaring it here would double-count a base-protocol rule against this extension.The
sep-2640.yamlheader carries the full provenance: what the 2026-08-21 rewrite changed, which rows were removed and why, which were reworded, and the scenario-to-row mapping.Reproducing the mcpkit row. Both servers in the table above are startable locally. mcpkit's is its skills example:
It is listed in the WG's
related-work.md, though that entry still describes the pre-rewriteskill://index.jsonshape and is due an update. The go-sdk row runs against a minimalskills.AddDirectoryserver built from PR 1238.