[KKonstantinov]

Adds the Skills extension (SEP-2640, #3047) to the e2e manifest: 41 requirements in test/e2e/requirements.ts, each proved in the new test/e2e/scenarios/skills.test.ts across the transport and spec-version matrix. Follow-up to this comment on #3047.

Motivation and Context

#3047 shipped with unit and integration tests, but none of its behaviours were in the e2e manifest, so the coverage gate did not track them and they never ran across the transports. This covers the list from the review comment and the rest of the extension surface: discovery, listing, get, reading and verification, directories, cache fields, custom sources, and attach()/reload().

• skills: ids cite the line of the stable spec page they come from; typescript:skills: ids are this SDK's own API.
• 2026-07-28-only behaviour (cache fields, resultType) runs on entryModern. The absence of cache fields on 2025-11-25 is the entry it supersedes.
• Loading from disk, the stdio child process and handler.notify stay in test/integration.

How Has This Been Tested?

• vitest run scenarios/skills.test.ts coverage.test.ts: 431 passed, no known failures.
• Each assertion was checked to fail against a deliberately broken SDK (10 mutations).
• The full e2e suite passes apart from protocol:timeout:max-total [sse *]. That test also fails locally on main under full-suite load, and passes in isolation.

Breaking Changes

None. Test-only, no changeset.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• 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

Additional context

The spec says servers SHOULD NOT serve a skill over the per-skill limits. The SDK serves it with a warning, and fails the load under strict. typescript:skills:catalog:limits pins that choice rather than recording it as a known failure.

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@3065

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@3065

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@3065

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@3065

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@3065

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@3065

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@3065

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@3065

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@3065

commit: 511a704

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline comments, I also checked whether skills:cache-fields:present needs a raw wire assertion for ttlMs and cacheScope. It does not: on 2026-07-28 the client parses these results with both fields required and no defaults (packages/client/src/ext/skills/pageSchemas.ts), so the values SkillsClient returns can only have come from the server's response.

Extended reasoning...

Test-only change (three files under test/e2e, no SDK source), touching no security-sensitive surface. This note records one concern beyond the inline comments that was examined and ruled out; it is informational, and the 1183-line addition under a CODEOWNERS-covered path still merits a human look.

3 optional notes from this repository's REVIEW.md or CLAUDE.md checks were not posted as comments, over this review's limit for such notes; they are on this commit's check card.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 511a704

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[KKonstantinov]

Two of the notes on the review's check card are left as they are:

• serving() helper (skills.test.ts:44). It's an in-file factory, like echoFactory in hosting-entry-http.test.ts and cacheConfiguredFactory in hosting-entry-stamping.test.ts. Each body still builds its own catalog, server and client; "no shared fixture files" in test/e2e/CLAUDE.md is about fixtures shared across files.
• JSDoc in a test-only file (skills.test.ts:8). Every e2e scenario file opens with a /** … */ header, and hosting-entry-stamping.test.ts documents its helpers the same way, so this follows the suite's existing style.

[claude]

I re-reviewed this after the follow-up commit and found no bugs; that commit changes each of the places my earlier comments pointed at. Since this adds about 1,200 lines that fix 40-odd requirements and their spec citations into the e2e manifest, a maintainer look is still worthwhile.

What was reviewed:

• test/e2e/requirements.ts: both supersedes/supersededBy pairs are symmetric with matching addedInSpecVersion/removedInSpecVersion; the entryModern-only entries and the skills:directory:undeclared exclusion follow the conventions in test/e2e/CLAUDE.md.
• test/e2e/scenarios/skills.test.ts: read each body against its behavior string, including the new typescript:skills:catalog:reload-listen body, which follows the same self-hosted createMcpHandler pattern as resources:listen:list-changed.
• test/e2e/tsconfig.json: both new path aliases point at src/ext/skills/index.ts files that exist in the client and server packages.
• Not checked: the L<line> anchors against the pinned ext-skills page (no network access in this run), and the suite was not executed.

Extended reasoning...

Test-only change across three files in test/e2e (manifest entries, a new 980-line scenario file, two tsconfig path aliases); no SDK source is touched and it has no security-sensitive surface. The second commit drops the relaxed wire-sniffer option from the bodies named earlier, adds a 2026-07-28 sibling for the reload requirement, and cites the spec line on the limits entry. This run found no bugs. It is not approved automatically because of its size, because CODEOWNERS assigns every path to the SDK maintainers, and because the spec line citations and the one dismissed human review could not be assessed from here.