From 9382794e0f6f3655842a251e0264d11835ac87ac Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Sun, 13 Sep 2026 20:37:10 +0200 Subject: [PATCH 1/3] [Schema][Server] Rework Skills extension for the redesigned SEP-2640 The ext-skills spec moved past PR #372 before it was published as specification/stable/skills.mdx: discovery is no longer a skill://index.json resource but two mandatory RPC methods, skills/list and skills/get, each returning a complete {uri, digest, size} manifest of every file in the skill. This rewrites the extension to match, built directly against current main. McpSkills is now a full ExtensionInterface implementation contributing ListSkillsRequest/Result and GetSkillRequest/Result plus their handlers, backed by a SkillRegistry that SkillProvider populates while it registers skill:// resources. The old discovery-index and archive layer is gone along with SkillDiscoveryIndex/Entry and SkillArchive/Archiver, since no listing ever pointed at them once the index disappeared from the spec. SkillMetadata now requires description, matching the spec's Skill.frontmatter shape. The SKILL.md resource's name comes directly from the frontmatter name instead of a generated slug, since names don't need to be registry-unique. Extra frontmatter fields land under _meta as one io.modelcontextprotocol.skills/ prefixed key each, not one key holding the whole map. SkillProvider also enforces the extension's fixed per-skill limits (512 resources, 16 MiB) and records each file's byte size on its resource definition. Current main already carries the ServerCapabilities empty-extension-payload fix this PR originally introduced, independently. --- composer.json | 3 +- docs/advanced/extensions.md | 89 ++++++ docs/examples.md | 1 + examples/server/skills/README.md | 45 +++ examples/server/skills/server.php | 30 ++ .../skills/acme/billing/refunds/SKILL.md | 25 ++ .../server/skills/skills/code-review/SKILL.md | 38 +++ .../skills/code-review/references/SECURITY.md | 10 + .../Extension/Skills/GetSkillRequest.php | 54 ++++ .../Extension/Skills/GetSkillResult.php | 61 ++++ .../Extension/Skills/ListSkillsRequest.php | 53 ++++ .../Extension/Skills/ListSkillsResult.php | 71 +++++ src/Schema/Extension/Skills/McpSkills.php | 85 ++++++ src/Schema/Extension/Skills/Skill.php | 89 ++++++ src/Schema/Extension/Skills/SkillMetadata.php | 71 +++++ src/Schema/Extension/Skills/SkillResource.php | 73 +++++ src/Server/Builder.php | 31 ++ .../Request/Skills/GetSkillHandler.php | 57 ++++ .../Request/Skills/ListSkillsHandler.php | 70 +++++ src/Server/Skill/FrontmatterParser.php | 64 ++++ src/Server/Skill/SkillProvider.php | 279 ++++++++++++++++++ src/Server/Skill/SkillRegistry.php | 50 ++++ src/Server/Wire/Rev2026Codec.php | 2 + tests/Inspector/Stdio/StdioSkillsTest.php | 43 +++ .../StdioSkillsTest-prompts_list.json | 4 + .../StdioSkillsTest-resources_list.json | 39 +++ ...illsTest-resources_read-read_skill_md.json | 17 ++ ...t-resources_read-read_supporting_file.json | 10 + ...ioSkillsTest-resources_templates_list.json | 4 + .../snapshots/StdioSkillsTest-tools_list.json | 4 + .../Schema/Extension/Skills/McpSkillsTest.php | 61 ++++ .../Extension/Skills/SkillMetadataTest.php | 58 ++++ .../Extension/Skills/SkillResourceTest.php | 66 +++++ .../Schema/Extension/Skills/SkillTest.php | 97 ++++++ .../Request/Skills/GetSkillHandlerTest.php | 71 +++++ .../Request/Skills/ListSkillsHandlerTest.php | 122 ++++++++ .../Fixtures/mismatch/wrong-name/SKILL.md | 6 + .../skills/acme/billing/refunds/SKILL.md | 8 + .../Fixtures/skills/code-review/SKILL.md | 11 + .../skills/code-review/references/SECURITY.md | 3 + .../Server/Skill/FrontmatterParserTest.php | 89 ++++++ tests/Unit/Server/Skill/SkillProviderTest.php | 256 ++++++++++++++++ tests/Unit/Server/Skill/SkillRegistryTest.php | 62 ++++ 43 files changed, 2381 insertions(+), 1 deletion(-) create mode 100644 examples/server/skills/README.md create mode 100644 examples/server/skills/server.php create mode 100644 examples/server/skills/skills/acme/billing/refunds/SKILL.md create mode 100644 examples/server/skills/skills/code-review/SKILL.md create mode 100644 examples/server/skills/skills/code-review/references/SECURITY.md create mode 100644 src/Schema/Extension/Skills/GetSkillRequest.php create mode 100644 src/Schema/Extension/Skills/GetSkillResult.php create mode 100644 src/Schema/Extension/Skills/ListSkillsRequest.php create mode 100644 src/Schema/Extension/Skills/ListSkillsResult.php create mode 100644 src/Schema/Extension/Skills/McpSkills.php create mode 100644 src/Schema/Extension/Skills/Skill.php create mode 100644 src/Schema/Extension/Skills/SkillMetadata.php create mode 100644 src/Schema/Extension/Skills/SkillResource.php create mode 100644 src/Server/Handler/Request/Skills/GetSkillHandler.php create mode 100644 src/Server/Handler/Request/Skills/ListSkillsHandler.php create mode 100644 src/Server/Skill/FrontmatterParser.php create mode 100644 src/Server/Skill/SkillProvider.php create mode 100644 src/Server/Skill/SkillRegistry.php create mode 100644 tests/Inspector/Stdio/StdioSkillsTest.php create mode 100644 tests/Inspector/Stdio/snapshots/StdioSkillsTest-prompts_list.json create mode 100644 tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_list.json create mode 100644 tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_read-read_skill_md.json create mode 100644 tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_read-read_supporting_file.json create mode 100644 tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_templates_list.json create mode 100644 tests/Inspector/Stdio/snapshots/StdioSkillsTest-tools_list.json create mode 100644 tests/Unit/Schema/Extension/Skills/McpSkillsTest.php create mode 100644 tests/Unit/Schema/Extension/Skills/SkillMetadataTest.php create mode 100644 tests/Unit/Schema/Extension/Skills/SkillResourceTest.php create mode 100644 tests/Unit/Schema/Extension/Skills/SkillTest.php create mode 100644 tests/Unit/Server/Handler/Request/Skills/GetSkillHandlerTest.php create mode 100644 tests/Unit/Server/Handler/Request/Skills/ListSkillsHandlerTest.php create mode 100644 tests/Unit/Server/Skill/Fixtures/mismatch/wrong-name/SKILL.md create mode 100644 tests/Unit/Server/Skill/Fixtures/skills/acme/billing/refunds/SKILL.md create mode 100644 tests/Unit/Server/Skill/Fixtures/skills/code-review/SKILL.md create mode 100644 tests/Unit/Server/Skill/Fixtures/skills/code-review/references/SECURITY.md create mode 100644 tests/Unit/Server/Skill/FrontmatterParserTest.php create mode 100644 tests/Unit/Server/Skill/SkillProviderTest.php create mode 100644 tests/Unit/Server/Skill/SkillRegistryTest.php diff --git a/composer.json b/composer.json index 5c572b02..415bc10d 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,8 @@ "psr/http-server-middleware": "^1.0", "psr/log": "^1.0 || ^2.0 || ^3.0", "symfony/deprecation-contracts": "^2.5 || ^3.0", - "symfony/uid": "^5.4 || ^6.4 || ^7.3 || ^8.0" + "symfony/uid": "^5.4 || ^6.4 || ^7.3 || ^8.0", + "symfony/yaml": "^5.4 || ^6.4 || ^7.3 || ^8.0" }, "suggest": { "symfony/finder": "Required for file-based discovery." diff --git a/docs/advanced/extensions.md b/docs/advanced/extensions.md index 041f9862..0e5c86f8 100644 --- a/docs/advanced/extensions.md +++ b/docs/advanced/extensions.md @@ -157,4 +157,93 @@ TypeScript SDK (`@modelcontextprotocol/ext-apps`), and view-side examples. A working minimal view is included in [`examples/server/mcp-apps/weather-app.html`](https://github.com/modelcontextprotocol/php-sdk/blob/main/examples/server/mcp-apps/weather-app.html). +## Skills (`io.modelcontextprotocol/skills`) + +The [Skills extension][ext-skills] (SEP-2640) lets servers ship **skills** — +multi-step workflow instructions that tell an agent *how to orchestrate* tools to +reach a goal. Each skill file is served through the existing **Resources** +primitive (`skill:///SKILL.md` plus any supporting files), and the +extension adds two mandatory RPC methods: + +- `skills/list` — enumerates the skills a server serves, paginated like + `resources/list`. +- `skills/get` — returns the entry for a single skill by its `SKILL.md` URI. + +Both return a `Skill` entry: the skill's frontmatter verbatim, and a complete, +`{uri, digest, size}` manifest of every file the skill comprises (`SKILL.md` +included), so a host can build its registry, present the skill for approval, and +verify every later read without fetching anything first. + +The simplest way to expose a directory of skills is `addSkillsFromDirectory()`, +which auto-enables the extension and registers every skill it finds: + +```php +use Mcp\Server; + +$server = Server::builder() + ->setServerInfo('My Server', '1.0.0') + ->addSkillsFromDirectory(__DIR__.'/skills') + ->build(); +``` + +Given this layout, the following `skill://` resources are registered, and a +matching `Skill` entry is added to the `skills/list`/`skills/get` catalog: + +``` +skills/ +├── code-review/ +│ ├── SKILL.md → skill://code-review/SKILL.md +│ └── references/SECURITY.md → skill://code-review/references/SECURITY.md +└── acme/billing/refunds/ + └── SKILL.md → skill://acme/billing/refunds/SKILL.md +``` + +Each `SKILL.md` is served as `text/markdown`. Its YAML frontmatter's `name` and +`description` become the resource `name`/`description`; any remaining frontmatter +keys are exposed under the `io.modelcontextprotocol.skills/` `_meta` namespace on +the resource, and pass through verbatim in the `skills/list`/`skills/get` entry's +`frontmatter`. Supporting files are served with a MIME type guessed from their +extension/content. + +```yaml +--- +name: code-review +description: Review a pull request for correctness, security, and style. +version: 1.0.0 +tags: [review, quality] +--- + +# Code Review +... +``` + +> The frontmatter `name` **must** equal the final segment of the skill's directory +> path (`code-review/` → `name: code-review`), and `description` is required; a +> violation throws an `InvalidArgumentException`. + +The extension fixes two per-skill limits so every conforming host knows what it +must accept: 512 resources and 16 MiB total content. `addSkillsFromDirectory()` +throws if a skill exceeds either. + +Parsing `SKILL.md` frontmatter requires the [`symfony/yaml`][symfony-yaml] +component, which is a dependency of this SDK. + +### Server-side classes + +| Class | Purpose | +| --- | --- | +| `McpSkills` | Extension; provides `EXTENSION_ID`, `MIME_TYPE`, `URI_SCHEME`, `ENTRY_POINT`, `META_PREFIX` constants and the `skills/list`/`skills/get` handlers. | +| `SkillProvider` | Walks a directory and registers each skill (and its files) as `skill://` resources, recording each skill's manifest in a `SkillRegistry`. | +| `SkillRegistry` | The skills a server serves, keyed by `SKILL.md` URI; backs `skills/list`/`skills/get`. | +| `FrontmatterParser` | Splits a `SKILL.md` into its YAML frontmatter and markdown body. | +| `SkillMetadata` | Value object for parsed frontmatter: `name`, `description`, `extra`. | +| `Skill` | One `skills/list`/`skills/get` entry: `uri`, `frontmatter`, `resources`. | +| `SkillResource` | One file of a skill's manifest: `uri`, `digest`, `size`. | + +A complete example lives in +[`examples/server/skills/`](https://github.com/modelcontextprotocol/php-sdk/blob/main/examples/server/skills/). + +[ext-skills]: https://github.com/modelcontextprotocol/ext-skills +[symfony-yaml]: https://github.com/symfony/yaml + [ext-apps]: https://github.com/modelcontextprotocol/ext-apps diff --git a/docs/examples.md b/docs/examples.md index 901b567d..7fd1a281 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -33,6 +33,7 @@ npx @modelcontextprotocol/inspector php examples/server/discovery-calculator/ser | [`elicitation`](https://github.com/modelcontextprotocol/php-sdk/tree/main/examples/server/elicitation) | Asking the user for input mid-call with `ClientGateway::elicit()` and typed elicitation schemas, on either protocol era | [Asking for input](handlers/input-required.md) | | [`custom-method-handlers`](https://github.com/modelcontextprotocol/php-sdk/tree/main/examples/server/custom-method-handlers) | Registering handlers for custom JSON-RPC methods | [Custom message handlers](advanced/custom-handlers.md) | | [`mcp-apps`](https://github.com/modelcontextprotocol/php-sdk/tree/main/examples/server/mcp-apps) | The MCP Apps extension: a tool that ships an interactive HTML view | [Protocol extensions](advanced/extensions.md) | +| [`skills`](https://github.com/modelcontextprotocol/php-sdk/tree/main/examples/server/skills) | The Skills extension: `skills/list`/`skills/get` over a directory of `SKILL.md` files | [Protocol extensions](advanced/extensions.md) | | [`stateless-lifecycle`](https://github.com/modelcontextprotocol/php-sdk/tree/main/examples/server/stateless-lifecycle) | Revision `2026-07-28`: cache policy, request state, notification bus | [Serving both eras](run/protocol-eras.md), [Caching](run/caching.md), [Subscriptions](run/subscriptions.md) | | [`oauth-keycloak`](https://github.com/modelcontextprotocol/php-sdk/tree/main/examples/server/oauth-keycloak) | OAuth authorization against a Keycloak instance (own README) | [Authorization](run/authorization.md) | | [`oauth-microsoft`](https://github.com/modelcontextprotocol/php-sdk/tree/main/examples/server/oauth-microsoft) | OAuth authorization against Microsoft Entra ID (own README) | [Authorization](run/authorization.md) | diff --git a/examples/server/skills/README.md b/examples/server/skills/README.md new file mode 100644 index 00000000..9c2b5dc3 --- /dev/null +++ b/examples/server/skills/README.md @@ -0,0 +1,45 @@ +# MCP Skills Example + +Demonstrates the **Skills extension** (`io.modelcontextprotocol/skills`, SEP-2640): serving +multi-step workflow instructions ("skills") to clients. Each skill file is served through the +existing MCP **Resources** primitive, and the extension adds two RPC methods, `skills/list` and +`skills/get`, that return a complete, digest-and-size manifest of a skill's files. + +## Running + +```bash +php examples/server/skills/server.php +``` + +A single call exposes the whole `skills/` directory: + +```php +Server::builder() + ->setServerInfo('MCP Skills Example', '1.0.0') + ->addSkillsFromDirectory(__DIR__.'/skills') + ->build(); +``` + +This auto-enables the `McpSkills` extension and registers every `SKILL.md` (plus supporting +files) as a `skill://` resource, and its manifest as a `skills/list`/`skills/get` entry. + +## Layout & URIs + +``` +skills/ +├── code-review/ +│ ├── SKILL.md → skill://code-review/SKILL.md +│ └── references/SECURITY.md → skill://code-review/references/SECURITY.md +└── acme/billing/refunds/ + └── SKILL.md → skill://acme/billing/refunds/SKILL.md +``` + +## Conventions + +- A skill is any folder containing a `SKILL.md`. Its frontmatter `name` **must** equal the final + segment of the folder path (e.g. `code-review` → `name: code-review`). +- `name`/`description` come from the SKILL.md YAML frontmatter and are always present in the + `skills/list`/`skills/get` entry; any extra frontmatter passes through verbatim, and is also + exposed on the SKILL.md resource under the `io.modelcontextprotocol.skills/` `_meta` namespace. +- Supporting files are served with a MIME type guessed from their extension/content. +- Skills are plain files — no PHP handler class is required. diff --git a/examples/server/skills/server.php b/examples/server/skills/server.php new file mode 100644 index 00000000..5ff340f0 --- /dev/null +++ b/examples/server/skills/server.php @@ -0,0 +1,30 @@ +#!/usr/bin/env php +info('Starting MCP Skills Example Server...'); + +$server = Server::builder() + ->setServerInfo('MCP Skills Example', '1.0.0') + ->setLogger(logger()) + ->addSkillsFromDirectory(__DIR__.'/skills') + ->build(); + +$result = $server->run(transport()); + +logger()->info('Server stopped gracefully.', ['result' => $result]); + +shutdown($result); diff --git a/examples/server/skills/skills/acme/billing/refunds/SKILL.md b/examples/server/skills/skills/acme/billing/refunds/SKILL.md new file mode 100644 index 00000000..5e457933 --- /dev/null +++ b/examples/server/skills/skills/acme/billing/refunds/SKILL.md @@ -0,0 +1,25 @@ +--- +name: refunds +description: Process a customer refund following Acme's billing policy and approval thresholds. +version: 1.0.0 +tags: + - billing + - support +--- + +# Processing Refunds + +A nested skill demonstrating multi-segment skill paths (`skill://acme/billing/refunds/SKILL.md`). + +## Policy + +1. Verify the charge exists and has not already been refunded. +2. Refunds up to $100 may be issued directly. +3. Refunds above $100 require a team lead's approval before issuing. + +## Steps + +1. Look up the original charge by order ID. +2. Confirm the refund amount does not exceed the charged amount. +3. Issue the refund and record the reason code. +4. Notify the customer with the expected settlement window. diff --git a/examples/server/skills/skills/code-review/SKILL.md b/examples/server/skills/skills/code-review/SKILL.md new file mode 100644 index 00000000..b0112098 --- /dev/null +++ b/examples/server/skills/skills/code-review/SKILL.md @@ -0,0 +1,38 @@ +--- +name: code-review +description: Review a pull request for correctness, security, and style following this team's conventions. +version: 1.0.0 +tags: + - review + - quality +--- + +# Code Review + +Follow these steps to review a pull request thoroughly and consistently. + +## 1. Understand the change + +- Read the PR description and linked issue to understand the intended behavior. +- Skim the diff top to bottom before commenting to build a mental model. + +## 2. Correctness + +- Check edge cases: empty input, nulls, boundary values, concurrency. +- Verify error handling fails fast and preserves context. +- Confirm tests cover the new behavior and actually assert on it. + +## 3. Security + +- See `references/SECURITY.md` for the security checklist that MUST be applied to + every change touching authentication, input parsing, or external I/O. + +## 4. Style & maintainability + +- Match the surrounding code's naming, structure, and comment density. +- Prefer the simplest implementation that satisfies the requirement. + +## 5. Wrap up + +- Summarize findings grouped by severity (blocking, suggestion, nit). +- Approve only when blocking issues are resolved and CI is green. diff --git a/examples/server/skills/skills/code-review/references/SECURITY.md b/examples/server/skills/skills/code-review/references/SECURITY.md new file mode 100644 index 00000000..de9ef343 --- /dev/null +++ b/examples/server/skills/skills/code-review/references/SECURITY.md @@ -0,0 +1,10 @@ +# Security Review Checklist + +Apply this checklist to every change that touches authentication, input parsing, or external I/O. + +- **Input validation**: All external input is validated and normalized before use. +- **Injection**: Queries, shell commands, and templates use parameterization — never string concatenation. +- **AuthZ**: Every privileged action re-checks the caller's authorization server-side. +- **Secrets**: No credentials, tokens, or keys are logged or committed. +- **Output encoding**: Data rendered into HTML, URLs, or headers is contextually encoded. +- **Dependencies**: New dependencies are pinned and free of known advisories. diff --git a/src/Schema/Extension/Skills/GetSkillRequest.php b/src/Schema/Extension/Skills/GetSkillRequest.php new file mode 100644 index 00000000..e2c81d86 --- /dev/null +++ b/src/Schema/Extension/Skills/GetSkillRequest.php @@ -0,0 +1,54 @@ + + */ +final class GetSkillRequest extends Request +{ + /** + * @param non-empty-string $uri URI of the skill's SKILL.md + */ + public function __construct( + public readonly string $uri, + ) { + } + + public static function getMethod(): string + { + return 'skills/get'; + } + + protected static function fromParams(?array $params): static + { + if (!isset($params['uri']) || !\is_string($params['uri']) || '' === $params['uri']) { + throw new InvalidArgumentException('Missing or invalid "uri" parameter for skills/get.'); + } + + return new self($params['uri']); + } + + /** + * @return array{uri: non-empty-string} + */ + protected function getParams(): array + { + return ['uri' => $this->uri]; + } +} diff --git a/src/Schema/Extension/Skills/GetSkillResult.php b/src/Schema/Extension/Skills/GetSkillResult.php new file mode 100644 index 00000000..d780056b --- /dev/null +++ b/src/Schema/Extension/Skills/GetSkillResult.php @@ -0,0 +1,61 @@ + + */ +final class GetSkillResult implements ResultInterface +{ + /** + * @param ?int $ttlMs how long a client may consider this fresh, in milliseconds. Null + * leaves it to the server's configured {@see \Mcp\Server\Wire\CachePolicy}. + * @param ?CacheScope $cacheScope who may cache it. Null defers to the policy. + */ + public function __construct( + public readonly Skill $skill, + public readonly ?int $ttlMs = null, + public readonly ?CacheScope $cacheScope = null, + ) { + if (null !== $this->ttlMs && $this->ttlMs < 0) { + throw new InvalidArgumentException(\sprintf('A skills/get "ttlMs" must be zero or more, got %d.', $this->ttlMs)); + } + } + + /** + * @return array{ + * skill: Skill, + * ttlMs?: int, + * cacheScope?: string, + * } + */ + public function jsonSerialize(): array + { + $data = ['skill' => $this->skill]; + + if (null !== $this->ttlMs) { + $data['ttlMs'] = $this->ttlMs; + } + + if (null !== $this->cacheScope) { + $data['cacheScope'] = $this->cacheScope->value; + } + + return $data; + } +} diff --git a/src/Schema/Extension/Skills/ListSkillsRequest.php b/src/Schema/Extension/Skills/ListSkillsRequest.php new file mode 100644 index 00000000..ad57ddc3 --- /dev/null +++ b/src/Schema/Extension/Skills/ListSkillsRequest.php @@ -0,0 +1,53 @@ + + */ +final class ListSkillsRequest extends Request +{ + /** + * @param string|null $cursor an opaque token representing the current pagination position + */ + public function __construct( + public readonly ?string $cursor = null, + ) { + } + + public static function getMethod(): string + { + return 'skills/list'; + } + + protected static function fromParams(?array $params): static + { + if (isset($params['cursor']) && !\is_string($params['cursor'])) { + throw new InvalidArgumentException('Invalid "cursor" parameter for skills/list.'); + } + + return new self($params['cursor'] ?? null); + } + + /** + * @return array{cursor: string}|null + */ + protected function getParams(): ?array + { + return null !== $this->cursor ? ['cursor' => $this->cursor] : null; + } +} diff --git a/src/Schema/Extension/Skills/ListSkillsResult.php b/src/Schema/Extension/Skills/ListSkillsResult.php new file mode 100644 index 00000000..cdf68a64 --- /dev/null +++ b/src/Schema/Extension/Skills/ListSkillsResult.php @@ -0,0 +1,71 @@ + + */ +final class ListSkillsResult implements ResultInterface +{ + /** + * @param Skill[] $skills + * @param string|null $nextCursor an opaque token for the next page, present when more results follow + * @param ?int $ttlMs how long a client may consider this fresh, in milliseconds. Null + * leaves it to the server's configured {@see \Mcp\Server\Wire\CachePolicy}. + * @param ?CacheScope $cacheScope who may cache it. Null defers to the policy. + */ + public function __construct( + public readonly array $skills, + public readonly ?string $nextCursor = null, + public readonly ?int $ttlMs = null, + public readonly ?CacheScope $cacheScope = null, + ) { + if (null !== $this->ttlMs && $this->ttlMs < 0) { + throw new InvalidArgumentException(\sprintf('A skills/list "ttlMs" must be zero or more, got %d.', $this->ttlMs)); + } + } + + /** + * @return array{ + * skills: array, + * nextCursor?: string, + * ttlMs?: int, + * cacheScope?: string, + * } + */ + public function jsonSerialize(): array + { + $data = ['skills' => array_values($this->skills)]; + + if (null !== $this->nextCursor) { + $data['nextCursor'] = $this->nextCursor; + } + + // Only what this result actually decided; the wire codec fills the rest + // from policy, and an absent member is the signal for it to do so. + if (null !== $this->ttlMs) { + $data['ttlMs'] = $this->ttlMs; + } + + if (null !== $this->cacheScope) { + $data['cacheScope'] = $this->cacheScope->value; + } + + return $data; + } +} diff --git a/src/Schema/Extension/Skills/McpSkills.php b/src/Schema/Extension/Skills/McpSkills.php new file mode 100644 index 00000000..adc7c4eb --- /dev/null +++ b/src/Schema/Extension/Skills/McpSkills.php @@ -0,0 +1,85 @@ +/SKILL.md` (plus supporting files). The extension adds two + * mandatory RPC methods — `skills/list` and `skills/get` — that return a complete, digest-and-size + * manifest of a skill's files, so a host can build its registry, present a skill for approval, and + * verify every later read without first fetching the files. + * + * Enable on the server via {@see \Mcp\Server\Builder::enableExtension()}, or use the + * {@see \Mcp\Server\Builder::addSkillsFromDirectory()} convenience to expose a directory of skills. + * + * `resources/directory/read`, gated behind the `directoryRead` capability setting, is not yet + * implemented and is not declared. + * + * @see https://github.com/modelcontextprotocol/ext-skills + * + * @author Johannes Wachter + */ +final class McpSkills implements ExtensionInterface +{ + public const EXTENSION_ID = 'io.modelcontextprotocol/skills'; + public const MIME_TYPE = 'text/markdown'; + public const URI_SCHEME = 'skill'; + public const ENTRY_POINT = 'SKILL.md'; + + /** + * The `_meta` namespace prefix reserved by this extension under which extra SKILL.md + * frontmatter fields MAY be exposed on a skill resource descriptor. + */ + public const META_PREFIX = 'io.modelcontextprotocol.skills/'; + + public function __construct( + private readonly SkillRegistry $registry, + private readonly int $pageSize = 20, + ) { + } + + public function getId(): ExtensionIdentifier + { + return new ExtensionIdentifier(self::EXTENSION_ID); + } + + /** + * The Skills extension advertises an empty capability payload (`{}`): `directoryRead` + * is not yet implemented. + * + * @return array + */ + public function getCapabilities(): array + { + return []; + } + + public function getMessages(): array + { + return [ListSkillsRequest::class, GetSkillRequest::class]; + } + + public function getRequestHandlers(): iterable + { + yield new ListSkillsHandler($this->registry, $this->pageSize); + yield new GetSkillHandler($this->registry); + } +} diff --git a/src/Schema/Extension/Skills/Skill.php b/src/Schema/Extension/Skills/Skill.php new file mode 100644 index 00000000..37508529 --- /dev/null +++ b/src/Schema/Extension/Skills/Skill.php @@ -0,0 +1,89 @@ + + */ +final class Skill implements \JsonSerializable +{ + /** + * @param string $uri resource URI of the skill's SKILL.md, readable via resources/read + * @param SkillMetadata $frontmatter the skill's SKILL.md YAML frontmatter, rendered verbatim + * @param SkillResource[]|'dynamic' $resources a complete enumeration of SKILL.md and every supporting + * file, or "dynamic" when stable digests cannot be published + */ + public function __construct( + public readonly string $uri, + public readonly SkillMetadata $frontmatter, + public readonly array|string $resources, + ) { + if ('dynamic' !== $this->resources && !array_is_list($this->resources)) { + throw new InvalidArgumentException('A skill\'s "resources" must be a list of SkillResource or the string "dynamic".'); + } + } + + /** + * @param SkillData $data + */ + public static function fromArray(array $data): self + { + if (empty($data['uri']) || !\is_string($data['uri'])) { + throw new InvalidArgumentException('Invalid or missing "uri" in skill entry.'); + } + if (!isset($data['frontmatter']) || !\is_array($data['frontmatter'])) { + throw new InvalidArgumentException('Invalid or missing "frontmatter" in skill entry.'); + } + + $resources = $data['resources'] ?? null; + if ('dynamic' === $resources) { + $parsedResources = 'dynamic'; + } elseif (\is_array($resources)) { + $parsedResources = array_map(SkillResource::fromArray(...), $resources); + } else { + throw new InvalidArgumentException('A skill entry\'s "resources" must be an array or the string "dynamic".'); + } + + return new self( + uri: $data['uri'], + frontmatter: SkillMetadata::fromArray($data['frontmatter']), + resources: $parsedResources, + ); + } + + /** + * @return array{uri: string, frontmatter: SkillMetadata, resources: array|'dynamic'} + */ + public function jsonSerialize(): array + { + return [ + 'uri' => $this->uri, + 'frontmatter' => $this->frontmatter, + 'resources' => 'dynamic' === $this->resources ? 'dynamic' : $this->resources, + ]; + } +} diff --git a/src/Schema/Extension/Skills/SkillMetadata.php b/src/Schema/Extension/Skills/SkillMetadata.php new file mode 100644 index 00000000..ad4aa4c0 --- /dev/null +++ b/src/Schema/Extension/Skills/SkillMetadata.php @@ -0,0 +1,71 @@ + + */ +final class SkillMetadata implements \JsonSerializable +{ + /** + * @param array $extra additional frontmatter fields (everything but name/description) + */ + public function __construct( + public readonly string $name, + public readonly string $description, + public readonly array $extra = [], + ) { + } + + /** + * @param array $data the raw frontmatter mapping + */ + public static function fromArray(array $data): self + { + if (empty($data['name']) || !\is_string($data['name'])) { + throw new InvalidArgumentException('SKILL.md frontmatter must contain a non-empty string "name".'); + } + + if (empty($data['description']) || !\is_string($data['description'])) { + throw new InvalidArgumentException('SKILL.md frontmatter must contain a non-empty string "description".'); + } + + $extra = $data; + unset($extra['name'], $extra['description']); + + return new self( + name: $data['name'], + description: $data['description'], + extra: $extra, + ); + } + + /** + * @return array + */ + public function jsonSerialize(): array + { + return [ + 'name' => $this->name, + 'description' => $this->description, + ...$this->extra, + ]; + } +} diff --git a/src/Schema/Extension/Skills/SkillResource.php b/src/Schema/Extension/Skills/SkillResource.php new file mode 100644 index 00000000..c9e11576 --- /dev/null +++ b/src/Schema/Extension/Skills/SkillResource.php @@ -0,0 +1,73 @@ + + */ +final class SkillResource implements \JsonSerializable +{ + /** + * @param string $uri resource URI of the file + * @param string $digest SHA-256 digest of the file's raw bytes, formatted as `sha256:{hex}` + * @param int $size length in bytes of the file's raw content (the same bytes `digest` covers) + */ + public function __construct( + public readonly string $uri, + public readonly string $digest, + public readonly int $size, + ) { + if (1 !== preg_match('/^sha256:[0-9a-f]{64}$/', $digest)) { + throw new InvalidArgumentException(\sprintf('A skill resource digest must be "sha256:" followed by 64 lowercase hex characters, got "%s".', $digest)); + } + + if ($size < 0) { + throw new InvalidArgumentException(\sprintf('A skill resource "size" must be zero or more, got %d.', $size)); + } + } + + /** + * @param SkillResourceData $data + */ + public static function fromArray(array $data): self + { + if (empty($data['uri']) || !\is_string($data['uri'])) { + throw new InvalidArgumentException('Invalid or missing "uri" in skill resource.'); + } + if (empty($data['digest']) || !\is_string($data['digest'])) { + throw new InvalidArgumentException('Invalid or missing "digest" in skill resource.'); + } + if (!isset($data['size']) || !\is_int($data['size'])) { + throw new InvalidArgumentException('Invalid or missing "size" in skill resource.'); + } + + return new self($data['uri'], $data['digest'], $data['size']); + } + + /** + * @return SkillResourceData + */ + public function jsonSerialize(): array + { + return [ + 'uri' => $this->uri, + 'digest' => $this->digest, + 'size' => $this->size, + ]; + } +} diff --git a/src/Server/Builder.php b/src/Server/Builder.php index 1a5dd73b..14ff72e6 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -34,6 +34,7 @@ use Mcp\Schema\Enum\ProtocolVersion; use Mcp\Schema\Extension\AbstractExtension; use Mcp\Schema\Extension\ExtensionInterface; +use Mcp\Schema\Extension\Skills\McpSkills; use Mcp\Schema\Icon; use Mcp\Schema\Implementation; use Mcp\Schema\Prompt; @@ -56,6 +57,8 @@ use Mcp\Server\Session\SessionManager; use Mcp\Server\Session\SessionManagerInterface; use Mcp\Server\Session\SessionStoreInterface; +use Mcp\Server\Skill\SkillProvider; +use Mcp\Server\Skill\SkillRegistry; use Mcp\Server\Stateless\RequestStateCodec; use Mcp\Server\Stateless\StandardHeaderValidator; use Mcp\Server\Stateless\StatelessProtocol; @@ -250,6 +253,12 @@ final class Builder */ private array $extensions = []; + /** + * The registry backing {@see McpSkills} once {@see self::addSkillsFromDirectory()} has been + * called at least once, shared across calls so multiple directories accumulate into one extension. + */ + private ?SkillRegistry $skillRegistry = null; + /** * @var LoaderInterface[] */ @@ -459,6 +468,28 @@ public function enableExtension(ExtensionInterface ...$extensions): self return $this; } + /** + * Expose a directory of skills (SEP-2640) as `skill://` resources. + * + * Enables the {@see McpSkills} extension (unless already enabled) and registers every + * `SKILL.md` found under $directory — together with its supporting files — as resources, + * making them servable through `skills/list` and `skills/get`. Calling this more than once + * accumulates every directory's skills into the same extension. + * + * @see SkillProvider + */ + public function addSkillsFromDirectory(string $directory, ?SkillProvider $provider = null): self + { + if (null === $this->skillRegistry) { + $this->skillRegistry = new SkillRegistry(); + $this->enableExtension(new McpSkills($this->skillRegistry)); + } + + ($provider ?? new SkillProvider())->registerInto($this, $this->skillRegistry, $directory); + + return $this; + } + /** * Register a single custom method handler. * diff --git a/src/Server/Handler/Request/Skills/GetSkillHandler.php b/src/Server/Handler/Request/Skills/GetSkillHandler.php new file mode 100644 index 00000000..9b99bffb --- /dev/null +++ b/src/Server/Handler/Request/Skills/GetSkillHandler.php @@ -0,0 +1,57 @@ + + * + * @author Johannes Wachter + */ +final class GetSkillHandler implements RequestHandlerInterface +{ + public function __construct( + private readonly SkillRegistry $registry, + ) { + } + + public function supports(Request $request): bool + { + return $request instanceof GetSkillRequest; + } + + /** + * @throws InvalidArgumentException if the URI does not identify a skill the server serves; + * rendered as -32602 (Invalid params), per the extension's + * error handling + */ + public function handle(Request $request, SessionInterface $session): Response + { + \assert($request instanceof GetSkillRequest); + + $skill = $this->registry->get($request->uri); + + if (null === $skill) { + throw new InvalidArgumentException(\sprintf('No skill is served at %s', $request->uri)); + } + + return new Response($request->getId(), new GetSkillResult($skill)); + } +} diff --git a/src/Server/Handler/Request/Skills/ListSkillsHandler.php b/src/Server/Handler/Request/Skills/ListSkillsHandler.php new file mode 100644 index 00000000..df661801 --- /dev/null +++ b/src/Server/Handler/Request/Skills/ListSkillsHandler.php @@ -0,0 +1,70 @@ + + * + * @author Johannes Wachter + */ +final class ListSkillsHandler implements RequestHandlerInterface +{ + public function __construct( + private readonly SkillRegistry $registry, + private readonly int $pageSize = 20, + ) { + } + + public function supports(Request $request): bool + { + return $request instanceof ListSkillsRequest; + } + + /** + * @throws InvalidCursorException + */ + public function handle(Request $request, SessionInterface $session): Response + { + \assert($request instanceof ListSkillsRequest); + + $skills = $this->registry->all(); + + $offset = 0; + if (null !== $request->cursor) { + $decoded = base64_decode($request->cursor, true); + if (false === $decoded || !is_numeric($decoded)) { + throw new InvalidCursorException($request->cursor); + } + + $offset = (int) $decoded; + if ($offset < 0 || $offset > \count($skills)) { + throw new InvalidCursorException($request->cursor); + } + } + + $page = \array_slice($skills, $offset, $this->pageSize); + + $nextOffset = $offset + $this->pageSize; + $nextCursor = $nextOffset < \count($skills) ? base64_encode((string) $nextOffset) : null; + + return new Response($request->getId(), new ListSkillsResult($page, $nextCursor)); + } +} diff --git a/src/Server/Skill/FrontmatterParser.php b/src/Server/Skill/FrontmatterParser.php new file mode 100644 index 00000000..a3b499df --- /dev/null +++ b/src/Server/Skill/FrontmatterParser.php @@ -0,0 +1,64 @@ + + */ +final class FrontmatterParser +{ + /** + * Splits a `SKILL.md` document into its frontmatter mapping and the remaining markdown body. + * + * A document without a leading `---` delimited block is treated as having empty frontmatter. + * + * @return array{0: array, 1: string} the [frontmatter, body] pair + * + * @throws RuntimeException if symfony/yaml is not installed + * @throws InvalidArgumentException if the frontmatter is present but is not a YAML mapping + */ + public function parse(string $content): array + { + if (!preg_match('/^(?:\xEF\xBB\xBF)?---\R(.*?)\R---\R?(.*)$/s', $content, $matches)) { + return [[], $content]; + } + + if (!class_exists(Yaml::class)) { + throw new RuntimeException('Parsing SKILL.md frontmatter requires the "symfony/yaml" component. Run: composer require symfony/yaml'); + } + + $data = Yaml::parse($matches[1]) ?? []; + if (!\is_array($data) || ([] !== $data && array_is_list($data))) { + throw new InvalidArgumentException('SKILL.md frontmatter must be a YAML mapping.'); + } + + /* @var array $data */ + return [$data, $matches[2]]; + } + + /** + * Parses the frontmatter of a `SKILL.md` document into a {@see SkillMetadata} value object. + */ + public function parseMetadata(string $content): SkillMetadata + { + [$frontmatter] = $this->parse($content); + + return SkillMetadata::fromArray($frontmatter); + } +} diff --git a/src/Server/Skill/SkillProvider.php b/src/Server/Skill/SkillProvider.php new file mode 100644 index 00000000..80e07903 --- /dev/null +++ b/src/Server/Skill/SkillProvider.php @@ -0,0 +1,279 @@ + + */ +final class SkillProvider +{ + /** Resources per skill this extension fixes as the limit every conforming host must accept. */ + private const MAX_RESOURCES_PER_SKILL = 512; + + /** Total file size per skill this extension fixes as the limit every conforming host must accept. */ + private const MAX_TOTAL_SIZE_PER_SKILL = 16_777_216; + + public function __construct( + private readonly FrontmatterParser $frontmatter = new FrontmatterParser(), + ) { + } + + /** + * Walks $baseDirectory, registers every discovered skill (and its supporting files) as + * `skill://` resources on $builder, and records each skill's manifest in $registry. + * + * @return Skill[] the discovered skills + * + * @throws InvalidArgumentException if the directory is missing, a skill violates the spec, or + * a skill exceeds the extension's per-skill resource/size limits + */ + public function registerInto(Builder $builder, SkillRegistry $registry, string $baseDirectory): array + { + $base = realpath($baseDirectory); + if (false === $base || !is_dir($base)) { + throw new InvalidArgumentException(\sprintf('Skills directory "%s" does not exist or is not a directory.', $baseDirectory)); + } + + $skills = []; + + foreach ($this->findSkillManifests($base) as $manifestPath) { + $skill = $this->registerSkill($builder, $base, $manifestPath); + $registry->add($skill); + $skills[] = $skill; + } + + return $skills; + } + + private function registerSkill(Builder $builder, string $base, string $manifestPath): Skill + { + $skillDir = \dirname($manifestPath); + $skillPath = $this->relativePath($base, $skillDir); + + $content = (string) file_get_contents($manifestPath); + $metadata = $this->frontmatter->parseMetadata($content); + + $lastSegment = basename($skillPath); + if ($lastSegment !== $metadata->name) { + throw new InvalidArgumentException(\sprintf('Skill at "%s": frontmatter name "%s" must match the final path segment "%s".', $skillPath, $metadata->name, $lastSegment)); + } + + $entryUri = \sprintf('%s://%s/%s', McpSkills::URI_SCHEME, $skillPath, McpSkills::ENTRY_POINT); + $entrySize = \strlen($content); + $entryDigest = 'sha256:'.hash('sha256', $content); + + $this->registerFile( + $builder, + $base, + $manifestPath, + $entryUri, + name: $metadata->name, + mimeType: McpSkills::MIME_TYPE, + description: $metadata->description, + size: $entrySize, + meta: $this->metaFor($metadata), + ); + + $resources = [new SkillResource($entryUri, $entryDigest, $entrySize)]; + + foreach ($this->findSupportingFiles($skillDir, $manifestPath) as $filePath) { + $relative = $this->relativePath($skillDir, $filePath); + $uri = \sprintf('%s://%s/%s', McpSkills::URI_SCHEME, $skillPath, $relative); + $size = (int) filesize($filePath); + $digest = 'sha256:'.hash_file('sha256', $filePath); + + $this->registerFile( + $builder, + $base, + $filePath, + $uri, + name: basename($filePath), + mimeType: $this->guessMimeType($filePath), + description: null, + size: $size, + meta: null, + ); + + $resources[] = new SkillResource($uri, $digest, $size); + } + + $this->checkLimits($skillPath, $resources); + + return new Skill($entryUri, $metadata, $resources); + } + + /** + * @param SkillResource[] $resources + */ + private function checkLimits(string $skillPath, array $resources): void + { + if (\count($resources) > self::MAX_RESOURCES_PER_SKILL) { + throw new InvalidArgumentException(\sprintf('Skill "%s" has %d resources, exceeding the %d this extension fixes as the per-skill limit.', $skillPath, \count($resources), self::MAX_RESOURCES_PER_SKILL)); + } + + $totalSize = array_sum(array_map(static fn (SkillResource $r): int => $r->size, $resources)); + if ($totalSize > self::MAX_TOTAL_SIZE_PER_SKILL) { + throw new InvalidArgumentException(\sprintf('Skill "%s" totals %d bytes, exceeding the %d bytes this extension fixes as the per-skill limit.', $skillPath, $totalSize, self::MAX_TOTAL_SIZE_PER_SKILL)); + } + } + + /** + * @return array|null the `_meta` map for the skill's SKILL.md resource, each + * extra frontmatter field under its own {@see McpSkills::META_PREFIX}-prefixed key + */ + private function metaFor(SkillMetadata $metadata): ?array + { + if ([] === $metadata->extra) { + return null; + } + + $meta = []; + foreach ($metadata->extra as $key => $value) { + $meta[McpSkills::META_PREFIX.$key] = $value; + } + + return $meta; + } + + /** + * @param array|null $meta + */ + private function registerFile(Builder $builder, string $base, string $filePath, string $uri, string $name, string $mimeType, ?string $description, int $size, ?array $meta): void + { + $absolute = realpath($filePath); + if (false === $absolute || !str_starts_with($absolute, $base.\DIRECTORY_SEPARATOR)) { + throw new InvalidArgumentException(\sprintf('Skill file "%s" resolves outside the skills directory.', $filePath)); + } + + $builder->addResource( + static fn (): \SplFileInfo => new \SplFileInfo($absolute), + $uri, + name: $name, + description: $description, + mimeType: $mimeType, + size: $size, + meta: $meta, + ); + } + + /** + * @return iterable absolute paths to every SKILL.md under $base + */ + private function findSkillManifests(string $base): iterable + { + if (class_exists(Finder::class)) { + $finder = (new Finder())->files()->in($base)->name(McpSkills::ENTRY_POINT)->sortByName(); + foreach ($finder as $file) { + yield $file->getPathname(); + } + + return; + } + + yield from $this->iterateFiles($base, static fn (string $path): bool => McpSkills::ENTRY_POINT === basename($path)); + } + + /** + * @return iterable absolute paths to all files in $skillDir except the manifest + */ + private function findSupportingFiles(string $skillDir, string $manifestPath): iterable + { + if (class_exists(Finder::class)) { + $finder = (new Finder())->files()->in($skillDir)->sortByName(); + foreach ($finder as $file) { + if ($file->getPathname() !== $manifestPath) { + yield $file->getPathname(); + } + } + + return; + } + + yield from $this->iterateFiles($skillDir, static fn (string $path): bool => $path !== $manifestPath); + } + + /** + * @param callable(string): bool $accept + * + * @return iterable + */ + private function iterateFiles(string $directory, callable $accept): iterable + { + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($directory, \FilesystemIterator::SKIP_DOTS), + ); + + $paths = []; + foreach ($iterator as $file) { + if ($file instanceof \SplFileInfo && $file->isFile() && $accept($file->getPathname())) { + $paths[] = $file->getPathname(); + } + } + + sort($paths); + + yield from $paths; + } + + /** + * Returns $path relative to $base, using forward slashes. + */ + private function relativePath(string $base, string $path): string + { + $relative = ltrim(substr($path, \strlen($base)), \DIRECTORY_SEPARATOR); + + return str_replace(\DIRECTORY_SEPARATOR, '/', $relative); + } + + private function guessMimeType(string $path): string + { + $byExtension = [ + 'md' => 'text/markdown', + 'markdown' => 'text/markdown', + 'json' => 'application/json', + 'txt' => 'text/plain', + 'csv' => 'text/csv', + 'yaml' => 'application/yaml', + 'yml' => 'application/yaml', + ]; + + $extension = strtolower(pathinfo($path, \PATHINFO_EXTENSION)); + if (isset($byExtension[$extension])) { + return $byExtension[$extension]; + } + + $finfo = new \finfo(\FILEINFO_MIME_TYPE); + $detected = $finfo->file($path); + + return \is_string($detected) && '' !== $detected ? $detected : 'application/octet-stream'; + } +} diff --git a/src/Server/Skill/SkillRegistry.php b/src/Server/Skill/SkillRegistry.php new file mode 100644 index 00000000..8fd254b7 --- /dev/null +++ b/src/Server/Skill/SkillRegistry.php @@ -0,0 +1,50 @@ + + */ +final class SkillRegistry +{ + /** + * @var array + */ + private array $skills = []; + + public function add(Skill $skill): void + { + $this->skills[$skill->uri] = $skill; + } + + /** + * @return list in registration order + */ + public function all(): array + { + return array_values($this->skills); + } + + public function get(string $uri): ?Skill + { + return $this->skills[$uri] ?? null; + } +} diff --git a/src/Server/Wire/Rev2026Codec.php b/src/Server/Wire/Rev2026Codec.php index d3b21eff..d8df038e 100644 --- a/src/Server/Wire/Rev2026Codec.php +++ b/src/Server/Wire/Rev2026Codec.php @@ -42,6 +42,8 @@ final class Rev2026Codec implements WireCodecInterface 'resources/list', 'resources/templates/list', 'resources/read', + 'skills/list', + 'skills/get', ]; private readonly CachePolicy $cachePolicy; diff --git a/tests/Inspector/Stdio/StdioSkillsTest.php b/tests/Inspector/Stdio/StdioSkillsTest.php new file mode 100644 index 00000000..a310c416 --- /dev/null +++ b/tests/Inspector/Stdio/StdioSkillsTest.php @@ -0,0 +1,43 @@ + [ + 'method' => 'resources/read', + 'options' => [ + 'uri' => 'skill://code-review/SKILL.md', + ], + 'testName' => 'read_skill_md', + ], + 'Read Skill Supporting File' => [ + 'method' => 'resources/read', + 'options' => [ + 'uri' => 'skill://code-review/references/SECURITY.md', + ], + 'testName' => 'read_supporting_file', + ], + ]; + } + + protected function getServerScript(): string + { + return \dirname(__DIR__, 3).'/examples/server/skills/server.php'; + } +} diff --git a/tests/Inspector/Stdio/snapshots/StdioSkillsTest-prompts_list.json b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-prompts_list.json new file mode 100644 index 00000000..911451f2 --- /dev/null +++ b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-prompts_list.json @@ -0,0 +1,4 @@ +{ + "prompts": [] +} + diff --git a/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_list.json b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_list.json new file mode 100644 index 00000000..18123459 --- /dev/null +++ b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_list.json @@ -0,0 +1,39 @@ +{ + "resources": [ + { + "name": "refunds", + "uri": "skill://acme/billing/refunds/SKILL.md", + "description": "Process a customer refund following Acme's billing policy and approval thresholds.", + "mimeType": "text/markdown", + "size": 702, + "_meta": { + "io.modelcontextprotocol.skills/version": "1.0.0", + "io.modelcontextprotocol.skills/tags": [ + "billing", + "support" + ] + } + }, + { + "name": "code-review", + "uri": "skill://code-review/SKILL.md", + "description": "Review a pull request for correctness, security, and style following this team's conventions.", + "mimeType": "text/markdown", + "size": 1158, + "_meta": { + "io.modelcontextprotocol.skills/version": "1.0.0", + "io.modelcontextprotocol.skills/tags": [ + "review", + "quality" + ] + } + }, + { + "name": "SECURITY.md", + "uri": "skill://code-review/references/SECURITY.md", + "mimeType": "text/markdown", + "size": 647 + } + ] +} + diff --git a/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_read-read_skill_md.json b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_read-read_skill_md.json new file mode 100644 index 00000000..292d0542 --- /dev/null +++ b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_read-read_skill_md.json @@ -0,0 +1,17 @@ +{ + "contents": [ + { + "uri": "skill://code-review/SKILL.md", + "mimeType": "text/markdown", + "_meta": { + "io.modelcontextprotocol.skills/version": "1.0.0", + "io.modelcontextprotocol.skills/tags": [ + "review", + "quality" + ] + }, + "text": "---\nname: code-review\ndescription: Review a pull request for correctness, security, and style following this team's conventions.\nversion: 1.0.0\ntags:\n - review\n - quality\n---\n\n# Code Review\n\nFollow these steps to review a pull request thoroughly and consistently.\n\n## 1. Understand the change\n\n- Read the PR description and linked issue to understand the intended behavior.\n- Skim the diff top to bottom before commenting to build a mental model.\n\n## 2. Correctness\n\n- Check edge cases: empty input, nulls, boundary values, concurrency.\n- Verify error handling fails fast and preserves context.\n- Confirm tests cover the new behavior and actually assert on it.\n\n## 3. Security\n\n- See `references/SECURITY.md` for the security checklist that MUST be applied to\n every change touching authentication, input parsing, or external I/O.\n\n## 4. Style & maintainability\n\n- Match the surrounding code's naming, structure, and comment density.\n- Prefer the simplest implementation that satisfies the requirement.\n\n## 5. Wrap up\n\n- Summarize findings grouped by severity (blocking, suggestion, nit).\n- Approve only when blocking issues are resolved and CI is green.\n" + } + ] +} + diff --git a/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_read-read_supporting_file.json b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_read-read_supporting_file.json new file mode 100644 index 00000000..66bf4097 --- /dev/null +++ b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_read-read_supporting_file.json @@ -0,0 +1,10 @@ +{ + "contents": [ + { + "uri": "skill://code-review/references/SECURITY.md", + "mimeType": "text/markdown", + "text": "# Security Review Checklist\n\nApply this checklist to every change that touches authentication, input parsing, or external I/O.\n\n- **Input validation**: All external input is validated and normalized before use.\n- **Injection**: Queries, shell commands, and templates use parameterization — never string concatenation.\n- **AuthZ**: Every privileged action re-checks the caller's authorization server-side.\n- **Secrets**: No credentials, tokens, or keys are logged or committed.\n- **Output encoding**: Data rendered into HTML, URLs, or headers is contextually encoded.\n- **Dependencies**: New dependencies are pinned and free of known advisories.\n" + } + ] +} + diff --git a/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_templates_list.json b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_templates_list.json new file mode 100644 index 00000000..b921e802 --- /dev/null +++ b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-resources_templates_list.json @@ -0,0 +1,4 @@ +{ + "resourceTemplates": [] +} + diff --git a/tests/Inspector/Stdio/snapshots/StdioSkillsTest-tools_list.json b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-tools_list.json new file mode 100644 index 00000000..87b6cf12 --- /dev/null +++ b/tests/Inspector/Stdio/snapshots/StdioSkillsTest-tools_list.json @@ -0,0 +1,4 @@ +{ + "tools": [] +} + diff --git a/tests/Unit/Schema/Extension/Skills/McpSkillsTest.php b/tests/Unit/Schema/Extension/Skills/McpSkillsTest.php new file mode 100644 index 00000000..f5b63580 --- /dev/null +++ b/tests/Unit/Schema/Extension/Skills/McpSkillsTest.php @@ -0,0 +1,61 @@ +assertSame('io.modelcontextprotocol/skills', (string) $extension->getId()); + $this->assertSame([], $extension->getCapabilities()); + } + + public function testCapabilitiesSerializeAsEmptyObject(): void + { + $capabilities = new ServerCapabilities(extensions: [McpSkills::EXTENSION_ID => (new McpSkills(new SkillRegistry()))->getCapabilities()]); + + $json = json_encode($capabilities, \JSON_UNESCAPED_SLASHES); + + // The empty extension payload MUST serialize to `{}`, not `[]`. + $this->assertStringContainsString('"io.modelcontextprotocol/skills":{}', $json); + $this->assertStringNotContainsString('"io.modelcontextprotocol/skills":[]', $json); + } + + public function testDeclaresListAndGetMessages(): void + { + $extension = new McpSkills(new SkillRegistry()); + + $this->assertSame([ListSkillsRequest::class, GetSkillRequest::class], $extension->getMessages()); + } + + public function testServesListAndGetHandlers(): void + { + $extension = new McpSkills(new SkillRegistry()); + + $handlers = iterator_to_array($extension->getRequestHandlers()); + + $this->assertCount(2, $handlers); + $this->assertInstanceOf(ListSkillsHandler::class, $handlers[0]); + $this->assertInstanceOf(GetSkillHandler::class, $handlers[1]); + } +} diff --git a/tests/Unit/Schema/Extension/Skills/SkillMetadataTest.php b/tests/Unit/Schema/Extension/Skills/SkillMetadataTest.php new file mode 100644 index 00000000..03d7dd16 --- /dev/null +++ b/tests/Unit/Schema/Extension/Skills/SkillMetadataTest.php @@ -0,0 +1,58 @@ + 'code-review', + 'description' => 'Review a pull request.', + 'version' => '1.0.0', + 'tags' => ['review', 'quality'], + ]); + + $this->assertSame('code-review', $metadata->name); + $this->assertSame('Review a pull request.', $metadata->description); + $this->assertSame(['version' => '1.0.0', 'tags' => ['review', 'quality']], $metadata->extra); + } + + public function testFromArrayRequiresName(): void + { + $this->expectException(InvalidArgumentException::class); + + SkillMetadata::fromArray(['description' => 'no name here']); + } + + public function testFromArrayRequiresDescription(): void + { + $this->expectException(InvalidArgumentException::class); + + SkillMetadata::fromArray(['name' => 'refunds']); + } + + public function testSerializationMergesExtra(): void + { + $metadata = new SkillMetadata('refunds', 'Process refunds.', ['version' => '2.0.0']); + + $this->assertSame([ + 'name' => 'refunds', + 'description' => 'Process refunds.', + 'version' => '2.0.0', + ], $metadata->jsonSerialize()); + } +} diff --git a/tests/Unit/Schema/Extension/Skills/SkillResourceTest.php b/tests/Unit/Schema/Extension/Skills/SkillResourceTest.php new file mode 100644 index 00000000..964243bd --- /dev/null +++ b/tests/Unit/Schema/Extension/Skills/SkillResourceTest.php @@ -0,0 +1,66 @@ +assertSame([ + 'uri' => 'skill://code-review/SKILL.md', + 'digest' => 'sha256:'.hash('sha256', 'x'), + 'size' => 1, + ], $resource->jsonSerialize()); + } + + public function testFromArrayRoundTrip(): void + { + $digest = 'sha256:'.hash('sha256', 'x'); + + $resource = SkillResource::fromArray([ + 'uri' => 'skill://code-review/SKILL.md', + 'digest' => $digest, + 'size' => 1, + ]); + + $this->assertSame('skill://code-review/SKILL.md', $resource->uri); + $this->assertSame($digest, $resource->digest); + $this->assertSame(1, $resource->size); + } + + public function testRejectsMalformedDigest(): void + { + $this->expectException(InvalidArgumentException::class); + + new SkillResource('skill://code-review/SKILL.md', 'not-a-digest', 1); + } + + public function testRejectsUppercaseHexDigest(): void + { + $this->expectException(InvalidArgumentException::class); + + new SkillResource('skill://code-review/SKILL.md', 'sha256:'.strtoupper(hash('sha256', 'x')), 1); + } + + public function testRejectsNegativeSize(): void + { + $this->expectException(InvalidArgumentException::class); + + new SkillResource('skill://code-review/SKILL.md', 'sha256:'.hash('sha256', 'x'), -1); + } +} diff --git a/tests/Unit/Schema/Extension/Skills/SkillTest.php b/tests/Unit/Schema/Extension/Skills/SkillTest.php new file mode 100644 index 00000000..ef2ddc14 --- /dev/null +++ b/tests/Unit/Schema/Extension/Skills/SkillTest.php @@ -0,0 +1,97 @@ +jsonSerialize(); + + $this->assertSame('skill://code-review/SKILL.md', $serialized['uri']); + $this->assertInstanceOf(SkillMetadata::class, $serialized['frontmatter']); + $this->assertCount(1, $serialized['resources']); + $this->assertInstanceOf(SkillResource::class, $serialized['resources'][0]); + } + + public function testSerializationWithDynamicResources(): void + { + $skill = new Skill( + 'skill://reports/daily/SKILL.md', + new SkillMetadata('daily', 'Assemble a report.'), + 'dynamic', + ); + + $this->assertSame('dynamic', $skill->jsonSerialize()['resources']); + } + + public function testRejectsNonListResourcesArray(): void + { + $this->expectException(InvalidArgumentException::class); + + new Skill( + 'skill://code-review/SKILL.md', + new SkillMetadata('code-review', 'Review a pull request.'), + ['not-a-list' => new SkillResource('skill://code-review/SKILL.md', 'sha256:'.hash('sha256', 'x'), 1)], + ); + } + + public function testFromArrayRoundTrip(): void + { + $skill = Skill::fromArray([ + 'uri' => 'skill://acme/billing/refunds/SKILL.md', + 'frontmatter' => ['name' => 'refunds', 'description' => 'Process refunds.'], + 'resources' => [ + ['uri' => 'skill://acme/billing/refunds/SKILL.md', 'digest' => 'sha256:'.hash('sha256', 'x'), 'size' => 1], + ], + ]); + + $this->assertSame('refunds', $skill->frontmatter->name); + $this->assertIsArray($skill->resources); + $this->assertCount(1, $skill->resources); + } + + public function testFromArrayAcceptsDynamicResources(): void + { + $skill = Skill::fromArray([ + 'uri' => 'skill://reports/daily/SKILL.md', + 'frontmatter' => ['name' => 'daily', 'description' => 'Assemble a report.'], + 'resources' => 'dynamic', + ]); + + $this->assertSame('dynamic', $skill->resources); + } + + public function testFromArrayRejectsInvalidResources(): void + { + $this->expectException(InvalidArgumentException::class); + + /* @phpstan-ignore argument.type (deliberately invalid: neither an array nor "dynamic" must be rejected) */ + Skill::fromArray([ + 'uri' => 'skill://reports/daily/SKILL.md', + 'frontmatter' => ['name' => 'daily', 'description' => 'Assemble a report.'], + 'resources' => 'not-dynamic', + ]); + } +} diff --git a/tests/Unit/Server/Handler/Request/Skills/GetSkillHandlerTest.php b/tests/Unit/Server/Handler/Request/Skills/GetSkillHandlerTest.php new file mode 100644 index 00000000..d7a92c68 --- /dev/null +++ b/tests/Unit/Server/Handler/Request/Skills/GetSkillHandlerTest.php @@ -0,0 +1,71 @@ +registry = new SkillRegistry(); + $this->session = new Session(new InMemorySessionStore()); + } + + public function testSupportsGetSkillRequest(): void + { + $handler = new GetSkillHandler($this->registry); + + $this->assertTrue($handler->supports($this->request('skill://code-review/SKILL.md'))); + } + + public function testReturnsTheMatchingSkill(): void + { + $skill = new Skill('skill://code-review/SKILL.md', new SkillMetadata('code-review', 'Review a PR.'), 'dynamic'); + $this->registry->add($skill); + $handler = new GetSkillHandler($this->registry); + + $response = $handler->handle($this->request('skill://code-review/SKILL.md'), $this->session); + + /** @var GetSkillResult $result */ + $result = $response->result; + $this->assertSame($skill, $result->skill); + } + + public function testThrowsInvalidArgumentForUnknownSkill(): void + { + $handler = new GetSkillHandler($this->registry); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('No skill is served at skill://unknown/SKILL.md'); + + $handler->handle($this->request('skill://unknown/SKILL.md'), $this->session); + } + + private function request(string $uri): GetSkillRequest + { + return (new GetSkillRequest($uri))->withId('test-request-id'); + } +} diff --git a/tests/Unit/Server/Handler/Request/Skills/ListSkillsHandlerTest.php b/tests/Unit/Server/Handler/Request/Skills/ListSkillsHandlerTest.php new file mode 100644 index 00000000..43fe3be7 --- /dev/null +++ b/tests/Unit/Server/Handler/Request/Skills/ListSkillsHandlerTest.php @@ -0,0 +1,122 @@ +registry = new SkillRegistry(); + $this->session = new Session(new InMemorySessionStore()); + } + + public function testSupportsListSkillsRequest(): void + { + $handler = new ListSkillsHandler($this->registry); + + $this->assertTrue($handler->supports($this->request())); + } + + public function testReturnsFirstPage(): void + { + $this->addSkills(5); + $handler = new ListSkillsHandler($this->registry, pageSize: 3); + + $response = $handler->handle($this->request(), $this->session); + + /** @var ListSkillsResult $result */ + $result = $response->result; + $this->assertCount(3, $result->skills); + $this->assertNotNull($result->nextCursor); + $this->assertSame('skill://skill-0/SKILL.md', $result->skills[0]->uri); + $this->assertSame('skill://skill-2/SKILL.md', $result->skills[2]->uri); + } + + public function testReturnsSecondPageWithCursor(): void + { + $this->addSkills(5); + $handler = new ListSkillsHandler($this->registry, pageSize: 3); + + $firstPage = $handler->handle($this->request(), $this->session)->result; + \assert($firstPage instanceof ListSkillsResult); + + $secondPage = $handler->handle($this->request($firstPage->nextCursor), $this->session)->result; + \assert($secondPage instanceof ListSkillsResult); + + $this->assertCount(2, $secondPage->skills); + $this->assertNull($secondPage->nextCursor); + $this->assertSame('skill://skill-3/SKILL.md', $secondPage->skills[0]->uri); + $this->assertSame('skill://skill-4/SKILL.md', $secondPage->skills[1]->uri); + } + + public function testHandlesEmptyRegistry(): void + { + $handler = new ListSkillsHandler($this->registry); + + $result = $handler->handle($this->request(), $this->session)->result; + \assert($result instanceof ListSkillsResult); + + $this->assertSame([], $result->skills); + $this->assertNull($result->nextCursor); + } + + public function testThrowsForInvalidCursor(): void + { + $this->addSkills(5); + $handler = new ListSkillsHandler($this->registry); + + $this->expectException(InvalidCursorException::class); + + $handler->handle($this->request('not-base64!!'), $this->session); + } + + public function testThrowsForCursorBeyondBounds(): void + { + $this->addSkills(5); + $handler = new ListSkillsHandler($this->registry); + + $this->expectException(InvalidCursorException::class); + + $handler->handle($this->request(base64_encode('100')), $this->session); + } + + private function request(?string $cursor = null): ListSkillsRequest + { + return (new ListSkillsRequest($cursor))->withId('test-request-id'); + } + + private function addSkills(int $count): void + { + for ($i = 0; $i < $count; ++$i) { + $this->registry->add(new Skill( + "skill://skill-$i/SKILL.md", + new SkillMetadata("skill-$i", "Skill number $i."), + 'dynamic', + )); + } + } +} diff --git a/tests/Unit/Server/Skill/Fixtures/mismatch/wrong-name/SKILL.md b/tests/Unit/Server/Skill/Fixtures/mismatch/wrong-name/SKILL.md new file mode 100644 index 00000000..a325c28e --- /dev/null +++ b/tests/Unit/Server/Skill/Fixtures/mismatch/wrong-name/SKILL.md @@ -0,0 +1,6 @@ +--- +name: not-the-folder +description: The frontmatter name does not match the folder name. +--- + +# Mismatch diff --git a/tests/Unit/Server/Skill/Fixtures/skills/acme/billing/refunds/SKILL.md b/tests/Unit/Server/Skill/Fixtures/skills/acme/billing/refunds/SKILL.md new file mode 100644 index 00000000..629f21c0 --- /dev/null +++ b/tests/Unit/Server/Skill/Fixtures/skills/acme/billing/refunds/SKILL.md @@ -0,0 +1,8 @@ +--- +name: refunds +description: Process refunds. +--- + +# Refunds + +Body. diff --git a/tests/Unit/Server/Skill/Fixtures/skills/code-review/SKILL.md b/tests/Unit/Server/Skill/Fixtures/skills/code-review/SKILL.md new file mode 100644 index 00000000..210a9927 --- /dev/null +++ b/tests/Unit/Server/Skill/Fixtures/skills/code-review/SKILL.md @@ -0,0 +1,11 @@ +--- +name: code-review +description: Review a pull request. +version: 1.0.0 +tags: + - review +--- + +# Code Review + +Body. diff --git a/tests/Unit/Server/Skill/Fixtures/skills/code-review/references/SECURITY.md b/tests/Unit/Server/Skill/Fixtures/skills/code-review/references/SECURITY.md new file mode 100644 index 00000000..305f56ac --- /dev/null +++ b/tests/Unit/Server/Skill/Fixtures/skills/code-review/references/SECURITY.md @@ -0,0 +1,3 @@ +# Security Checklist + +Supporting file. diff --git a/tests/Unit/Server/Skill/FrontmatterParserTest.php b/tests/Unit/Server/Skill/FrontmatterParserTest.php new file mode 100644 index 00000000..54cbbab7 --- /dev/null +++ b/tests/Unit/Server/Skill/FrontmatterParserTest.php @@ -0,0 +1,89 @@ +parse($content); + + $this->assertSame(['name' => 'code-review', 'description' => 'Review a PR.'], $frontmatter); + $this->assertSame("\n# Heading\n\nBody text.", $body); + } + + public function testDocumentWithoutFrontmatterHasEmptyFrontmatter(): void + { + $content = "# Just markdown\n\nNo frontmatter here."; + + [$frontmatter, $body] = (new FrontmatterParser())->parse($content); + + $this->assertSame([], $frontmatter); + $this->assertSame($content, $body); + } + + public function testHandlesCrlfLineEndings(): void + { + $content = "---\r\nname: refunds\r\n---\r\n\r\n# Refunds\r\n"; + + [$frontmatter] = (new FrontmatterParser())->parse($content); + + $this->assertSame('refunds', $frontmatter['name']); + } + + public function testHandlesByteOrderMark(): void + { + $content = "\xEF\xBB\xBF---\nname: refunds\n---\n\nBody."; + + [$frontmatter] = (new FrontmatterParser())->parse($content); + + $this->assertSame('refunds', $frontmatter['name']); + } + + public function testParsesListsAndMultilineValues(): void + { + $content = "---\nname: code-review\ntags:\n - review\n - quality\n---\nbody"; + + [$frontmatter] = (new FrontmatterParser())->parse($content); + + $this->assertSame(['review', 'quality'], $frontmatter['tags']); + } + + public function testThrowsWhenFrontmatterIsNotAMapping(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('YAML mapping'); + + (new FrontmatterParser())->parse("---\n- just\n- a\n- list\n---\nbody"); + } + + public function testParseMetadataReturnsValueObject(): void + { + $metadata = (new FrontmatterParser())->parseMetadata("---\nname: refunds\ndescription: Process refunds.\n---\nbody"); + + $this->assertSame('refunds', $metadata->name); + $this->assertSame('Process refunds.', $metadata->description); + } + + public function testParseMetadataThrowsWhenNameMissing(): void + { + $this->expectException(InvalidArgumentException::class); + + (new FrontmatterParser())->parseMetadata("---\ndescription: no name\n---\nbody"); + } +} diff --git a/tests/Unit/Server/Skill/SkillProviderTest.php b/tests/Unit/Server/Skill/SkillProviderTest.php new file mode 100644 index 00000000..0f20874a --- /dev/null +++ b/tests/Unit/Server/Skill/SkillProviderTest.php @@ -0,0 +1,256 @@ +registerInto($builder, new SkillRegistry(), self::FIXTURES); + + $uris = array_column($this->registeredResources($builder), 'uri'); + + $this->assertContains('skill://code-review/SKILL.md', $uris); + $this->assertContains('skill://code-review/references/SECURITY.md', $uris); + $this->assertContains('skill://acme/billing/refunds/SKILL.md', $uris); + } + + public function testSkillManifestResourceUsesFrontmatterNameAndDescription(): void + { + $builder = Server::builder(); + + (new SkillProvider())->registerInto($builder, new SkillRegistry(), self::FIXTURES); + + $resource = $this->resourceByUri($builder, 'skill://code-review/SKILL.md'); + + $this->assertSame(McpSkills::MIME_TYPE, $resource['mimeType']); + $this->assertSame('code-review', $resource['name']); + $this->assertSame('Review a pull request.', $resource['description']); + $this->assertSame( + ['io.modelcontextprotocol.skills/version' => '1.0.0', 'io.modelcontextprotocol.skills/tags' => ['review']], + $resource['meta'], + ); + } + + public function testSupportingFileHasNoExtraMeta(): void + { + $builder = Server::builder(); + + (new SkillProvider())->registerInto($builder, new SkillRegistry(), self::FIXTURES); + + $resource = $this->resourceByUri($builder, 'skill://code-review/references/SECURITY.md'); + + $this->assertSame('text/markdown', $resource['mimeType']); + $this->assertNull($resource['meta']); + } + + public function testReturnsSkillsWithCompleteResourceManifest(): void + { + $builder = Server::builder(); + + $skills = (new SkillProvider())->registerInto($builder, new SkillRegistry(), self::FIXTURES); + + $this->assertCount(2, $skills); + + $codeReview = current(array_filter($skills, static fn ($s) => 'code-review' === $s->frontmatter->name)); + $this->assertNotFalse($codeReview); + $this->assertIsArray($codeReview->resources); + $this->assertCount(2, $codeReview->resources); // SKILL.md + references/SECURITY.md + + $manifestEntry = current(array_filter($codeReview->resources, static fn (SkillResource $r) => $r->uri === $codeReview->uri)); + $this->assertNotFalse($manifestEntry); + $this->assertMatchesRegularExpression('/^sha256:[0-9a-f]{64}$/', $manifestEntry->digest); + $this->assertGreaterThan(0, $manifestEntry->size); + } + + public function testDigestMatchesServedManifestBytes(): void + { + $builder = Server::builder(); + + $skills = (new SkillProvider())->registerInto($builder, new SkillRegistry(), self::FIXTURES); + $codeReview = current(array_filter($skills, static fn ($s) => 'code-review' === $s->frontmatter->name)); + + $resource = $this->resourceByUri($builder, $codeReview->uri); + /** @var \SplFileInfo $file */ + $file = ($resource['handler'])(); + $served = (string) file_get_contents($file->getPathname()); + + $manifestEntry = current(array_filter($codeReview->resources, static fn (SkillResource $r) => $r->uri === $codeReview->uri)); + $this->assertSame('sha256:'.hash('sha256', $served), $manifestEntry->digest); + $this->assertSame(\strlen($served), $manifestEntry->size); + } + + public function testRegistersEntriesIntoRegistry(): void + { + $builder = Server::builder(); + $registry = new SkillRegistry(); + + (new SkillProvider())->registerInto($builder, $registry, self::FIXTURES); + + $this->assertNotNull($registry->get('skill://code-review/SKILL.md')); + $this->assertNotNull($registry->get('skill://acme/billing/refunds/SKILL.md')); + } + + public function testThrowsWhenFrontmatterNameDoesNotMatchFolder(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('must match the final path segment'); + + (new SkillProvider())->registerInto(Server::builder(), new SkillRegistry(), __DIR__.'/Fixtures/mismatch'); + } + + public function testThrowsWhenDirectoryDoesNotExist(): void + { + $this->expectException(InvalidArgumentException::class); + + (new SkillProvider())->registerInto(Server::builder(), new SkillRegistry(), __DIR__.'/Fixtures/does-not-exist'); + } + + public function testBuilderHelperAutoEnablesExtension(): void + { + $builder = Server::builder()->addSkillsFromDirectory(self::FIXTURES); + + $extensions = $this->readPrivate($builder, 'extensions'); + $this->assertArrayHasKey(McpSkills::EXTENSION_ID, $extensions); + } + + public function testBuilderHelperAccumulatesAcrossCalls(): void + { + $dir1 = $this->makeTempDir(); + $dir2 = $this->makeTempDir(); + mkdir($dir1.'/skill-a', 0777, true); + file_put_contents($dir1.'/skill-a/SKILL.md', "---\nname: skill-a\ndescription: First skill.\n---\nbody"); + mkdir($dir2.'/skill-b', 0777, true); + file_put_contents($dir2.'/skill-b/SKILL.md', "---\nname: skill-b\ndescription: Second skill.\n---\nbody"); + + try { + $builder = Server::builder() + ->addSkillsFromDirectory($dir1) + ->addSkillsFromDirectory($dir2); + + $registry = $this->readPrivate($builder, 'skillRegistry'); + $this->assertNotNull($registry->get('skill://skill-a/SKILL.md')); + $this->assertNotNull($registry->get('skill://skill-b/SKILL.md')); + + $extensions = $this->readPrivate($builder, 'extensions'); + $this->assertCount(1, $extensions); + } finally { + $this->removeDir($dir1); + $this->removeDir($dir2); + } + } + + public function testThrowsWhenSkillExceedsResourceCountLimit(): void + { + $dir = $this->makeTempDir(); + $skillDir = $dir.'/many/many'; + mkdir($skillDir, 0777, true); + file_put_contents($skillDir.'/SKILL.md', "---\nname: many\ndescription: Too many files.\n---\nbody"); + for ($i = 0; $i < 512; ++$i) { + file_put_contents($skillDir.\sprintf('/file-%03d.txt', $i), 'x'); + } + + try { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('exceeding the 512'); + + (new SkillProvider())->registerInto(Server::builder(), new SkillRegistry(), $dir); + } finally { + $this->removeDir($dir); + } + } + + public function testThrowsWhenSkillExceedsTotalSizeLimit(): void + { + $dir = $this->makeTempDir(); + $skillDir = $dir.'/big/big'; + mkdir($skillDir, 0777, true); + file_put_contents($skillDir.'/SKILL.md', "---\nname: big\ndescription: Too big.\n---\nbody"); + + $handle = fopen($skillDir.'/blob.bin', 'w'); + \assert(false !== $handle); + fseek($handle, 17_000_000 - 1); + fwrite($handle, "\0"); + fclose($handle); + + try { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('exceeding the 16777216 bytes'); + + (new SkillProvider())->registerInto(Server::builder(), new SkillRegistry(), $dir); + } finally { + $this->removeDir($dir); + } + } + + /** + * @return array> + */ + private function registeredResources(Builder $builder): array + { + return $this->readPrivate($builder, 'resources'); + } + + /** + * @return array + */ + private function resourceByUri(Builder $builder, string $uri): array + { + foreach ($this->registeredResources($builder) as $resource) { + if ($resource['uri'] === $uri) { + return $resource; + } + } + + $this->fail(\sprintf('No resource registered for URI "%s".', $uri)); + } + + private function readPrivate(Builder $builder, string $property): mixed + { + $reflection = new \ReflectionProperty(Builder::class, $property); + + return $reflection->getValue($builder); + } + + private function makeTempDir(): string + { + $dir = sys_get_temp_dir().'/mcp-skill-provider-test-'.bin2hex(random_bytes(8)); + mkdir($dir, 0777, true); + + return $dir; + } + + private function removeDir(string $dir): void + { + $items = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($dir, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST, + ); + foreach ($items as $item) { + $item->isDir() ? rmdir($item->getPathname()) : unlink($item->getPathname()); + } + rmdir($dir); + } +} diff --git a/tests/Unit/Server/Skill/SkillRegistryTest.php b/tests/Unit/Server/Skill/SkillRegistryTest.php new file mode 100644 index 00000000..a41da5ba --- /dev/null +++ b/tests/Unit/Server/Skill/SkillRegistryTest.php @@ -0,0 +1,62 @@ +add($skill); + + $this->assertSame($skill, $registry->get('skill://code-review/SKILL.md')); + } + + public function testGetReturnsNullForUnknownUri(): void + { + $registry = new SkillRegistry(); + + $this->assertNull($registry->get('skill://unknown/SKILL.md')); + } + + public function testAllReturnsInRegistrationOrder(): void + { + $registry = new SkillRegistry(); + $first = new Skill('skill://a/SKILL.md', new SkillMetadata('a', 'A.'), 'dynamic'); + $second = new Skill('skill://b/SKILL.md', new SkillMetadata('b', 'B.'), 'dynamic'); + + $registry->add($first); + $registry->add($second); + + $this->assertSame([$first, $second], $registry->all()); + } + + public function testAddOverwritesSameUri(): void + { + $registry = new SkillRegistry(); + $original = new Skill('skill://a/SKILL.md', new SkillMetadata('a', 'Original.'), 'dynamic'); + $updated = new Skill('skill://a/SKILL.md', new SkillMetadata('a', 'Updated.'), 'dynamic'); + + $registry->add($original); + $registry->add($updated); + + $this->assertCount(1, $registry->all()); + $this->assertSame($updated, $registry->get('skill://a/SKILL.md')); + } +} From bb7b491e1c461bc6163e779a1f346fa74b47a190 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Sun, 13 Sep 2026 21:11:17 +0200 Subject: [PATCH 2/3] [Schema][Server] Close review gaps in the Skills extension: full-dispatch coverage, deterministic nested-skill registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit skills/list and skills/get were only tested by calling the handlers directly, so nothing exercised the actual MessageFactory/StatelessProtocol/Rev2026Codec dispatch path or asserted the mandatory resultType/ttlMs/cacheScope envelope. Added a dispatch test that builds a real server and sends JSON-RPC requests over StatelessProtocol, covering both methods, the -32602 case, and the capability advertisement. SkillProvider also registered a nested skill's own SKILL.md twice as a servable resource: once as an anonymous supporting file of the enclosing skill (name from basename, no description) and once under its own frontmatter as its own top-level skill. Since the registry keeps the last registration for a given URI, which metadata won depended on filesystem sort order between the two directory names. The nested manifest is now registered exactly once, by its own entry, while still being listed in the enclosing skill's resources manifest as the spec requires. Added a nested-skill fixture pinning this. Clarified the McpSkills docblock: addSkillsFromDirectory() and manually enabling the extension are alternatives, not composable — combining them throws because the convenience method always tries to enable its own instance. --- src/Schema/Extension/Skills/McpSkills.php | 8 +- src/Server/Skill/SkillProvider.php | 29 +++-- .../Fixtures/nested/parent-skill/SKILL.md | 5 + .../nested/parent-skill/nested-skill/SKILL.md | 5 + tests/Unit/Server/Skill/SkillProviderTest.php | 37 ++++++ .../Unit/Server/Skill/SkillsDispatchTest.php | 109 ++++++++++++++++++ 6 files changed, 180 insertions(+), 13 deletions(-) create mode 100644 tests/Unit/Server/Skill/Fixtures/nested/parent-skill/SKILL.md create mode 100644 tests/Unit/Server/Skill/Fixtures/nested/parent-skill/nested-skill/SKILL.md create mode 100644 tests/Unit/Server/Skill/SkillsDispatchTest.php diff --git a/src/Schema/Extension/Skills/McpSkills.php b/src/Schema/Extension/Skills/McpSkills.php index adc7c4eb..cf413d43 100644 --- a/src/Schema/Extension/Skills/McpSkills.php +++ b/src/Schema/Extension/Skills/McpSkills.php @@ -27,8 +27,12 @@ * manifest of a skill's files, so a host can build its registry, present a skill for approval, and * verify every later read without first fetching the files. * - * Enable on the server via {@see \Mcp\Server\Builder::enableExtension()}, or use the - * {@see \Mcp\Server\Builder::addSkillsFromDirectory()} convenience to expose a directory of skills. + * Enable on the server either via {@see \Mcp\Server\Builder::addSkillsFromDirectory()}, which + * builds and owns the {@see SkillRegistry} for you, or by constructing a `SkillRegistry` yourself, + * registering it with {@see \Mcp\Server\Builder::enableExtension()}, and feeding it via + * {@see \Mcp\Server\Skill\SkillProvider::registerInto()} for full control. Pick one: calling + * `addSkillsFromDirectory()` after the extension is already enabled throws, since it always tries + * to enable its own instance. * * `resources/directory/read`, gated behind the `directoryRead` capability setting, is not yet * implemented and is not declared. diff --git a/src/Server/Skill/SkillProvider.php b/src/Server/Skill/SkillProvider.php index 80e07903..ce510a1c 100644 --- a/src/Server/Skill/SkillProvider.php +++ b/src/Server/Skill/SkillProvider.php @@ -111,17 +111,24 @@ private function registerSkill(Builder $builder, string $base, string $manifestP $size = (int) filesize($filePath); $digest = 'sha256:'.hash_file('sha256', $filePath); - $this->registerFile( - $builder, - $base, - $filePath, - $uri, - name: basename($filePath), - mimeType: $this->guessMimeType($filePath), - description: null, - size: $size, - meta: null, - ); + // A nested skill's own SKILL.md is a supporting file of this skill's manifest too + // (the spec allows the same file in both entries), but it must be registered as a + // resource exactly once, by its own registerSkill() call below with its own + // frontmatter — registering it again here, generically, would make which metadata + // wins depend on filesystem walk order instead of always being the nested skill's own. + if (McpSkills::ENTRY_POINT !== basename($filePath)) { + $this->registerFile( + $builder, + $base, + $filePath, + $uri, + name: basename($filePath), + mimeType: $this->guessMimeType($filePath), + description: null, + size: $size, + meta: null, + ); + } $resources[] = new SkillResource($uri, $digest, $size); } diff --git a/tests/Unit/Server/Skill/Fixtures/nested/parent-skill/SKILL.md b/tests/Unit/Server/Skill/Fixtures/nested/parent-skill/SKILL.md new file mode 100644 index 00000000..dc4e5f80 --- /dev/null +++ b/tests/Unit/Server/Skill/Fixtures/nested/parent-skill/SKILL.md @@ -0,0 +1,5 @@ +--- +name: parent-skill +description: A skill that has a nested skill inside it. +--- +Do the outer thing. diff --git a/tests/Unit/Server/Skill/Fixtures/nested/parent-skill/nested-skill/SKILL.md b/tests/Unit/Server/Skill/Fixtures/nested/parent-skill/nested-skill/SKILL.md new file mode 100644 index 00000000..f8cf9488 --- /dev/null +++ b/tests/Unit/Server/Skill/Fixtures/nested/parent-skill/nested-skill/SKILL.md @@ -0,0 +1,5 @@ +--- +name: nested-skill +description: A skill nested inside another skill's directory. +--- +Do the inner thing. diff --git a/tests/Unit/Server/Skill/SkillProviderTest.php b/tests/Unit/Server/Skill/SkillProviderTest.php index 0f20874a..36d77779 100644 --- a/tests/Unit/Server/Skill/SkillProviderTest.php +++ b/tests/Unit/Server/Skill/SkillProviderTest.php @@ -113,6 +113,43 @@ public function testRegistersEntriesIntoRegistry(): void $this->assertNotNull($registry->get('skill://acme/billing/refunds/SKILL.md')); } + public function testNestedSkillIsRegisteredOnceWithItsOwnMetadata(): void + { + $builder = Server::builder(); + + $skills = (new SkillProvider())->registerInto($builder, new SkillRegistry(), __DIR__.'/Fixtures/nested'); + + $this->assertCount(2, $skills); + + // Registered as a resource exactly once, under its own frontmatter — not the generic + // "supporting file" metadata a naive walk of the parent skill's directory would produce. + $resource = $this->resourceByUri($builder, 'skill://parent-skill/nested-skill/SKILL.md'); + $this->assertSame('nested-skill', $resource['name']); + $this->assertSame('A skill nested inside another skill\'s directory.', $resource['description']); + + $uris = array_column($this->registeredResources($builder), 'uri'); + $this->assertCount(1, array_filter($uris, static fn ($uri) => 'skill://parent-skill/nested-skill/SKILL.md' === $uri)); + } + + public function testNestedSkillManifestListsItInBothEntries(): void + { + $builder = Server::builder(); + + $skills = (new SkillProvider())->registerInto($builder, new SkillRegistry(), __DIR__.'/Fixtures/nested'); + + $parent = current(array_filter($skills, static fn ($s) => 'parent-skill' === $s->frontmatter->name)); + $nested = current(array_filter($skills, static fn ($s) => 'nested-skill' === $s->frontmatter->name)); + $this->assertNotFalse($parent); + $this->assertNotFalse($nested); + + $this->assertIsArray($parent->resources); + $this->assertCount(2, $parent->resources); // its own SKILL.md + the nested one + + $this->assertIsArray($nested->resources); + $this->assertCount(1, $nested->resources); // just its own SKILL.md + $this->assertSame('skill://parent-skill/nested-skill/SKILL.md', $nested->resources[0]->uri); + } + public function testThrowsWhenFrontmatterNameDoesNotMatchFolder(): void { $this->expectException(InvalidArgumentException::class); diff --git a/tests/Unit/Server/Skill/SkillsDispatchTest.php b/tests/Unit/Server/Skill/SkillsDispatchTest.php new file mode 100644 index 00000000..faf6344f --- /dev/null +++ b/tests/Unit/Server/Skill/SkillsDispatchTest.php @@ -0,0 +1,109 @@ + MessageFactory -> StatelessProtocol -> Rev2026Codec), rather + * than calling the handlers directly, so a wiring mistake in + * {@see \Mcp\Schema\Extension\Skills\McpSkills::getMessages()} or in + * {@see Server\Wire\Rev2026Codec::CACHEABLE_METHODS} would fail a test. + */ +class SkillsDispatchTest extends TestCase +{ + private const FIXTURES = __DIR__.'/Fixtures/skills'; + + #[TestDox('skills/list is served with a cacheable envelope')] + public function testSkillsListIsServed(): void + { + $answer = self::call(self::protocol(), 'skills/list'); + + $this->assertSame(200, $answer['status']); + $this->assertSame('complete', $answer['body']['result']['resultType']); + $this->assertArrayHasKey('ttlMs', $answer['body']['result']); + $this->assertArrayHasKey('cacheScope', $answer['body']['result']); + $uris = array_column($answer['body']['result']['skills'], 'uri'); + $this->assertContains('skill://code-review/SKILL.md', $uris); + $this->assertContains('skill://acme/billing/refunds/SKILL.md', $uris); + } + + #[TestDox('skills/get returns the matching skill with a cacheable envelope')] + public function testSkillsGetIsServed(): void + { + $answer = self::call(self::protocol(), 'skills/get', ['uri' => 'skill://code-review/SKILL.md']); + + $this->assertSame(200, $answer['status']); + $this->assertSame('complete', $answer['body']['result']['resultType']); + $this->assertArrayHasKey('ttlMs', $answer['body']['result']); + $this->assertArrayHasKey('cacheScope', $answer['body']['result']); + $this->assertSame('skill://code-review/SKILL.md', $answer['body']['result']['skill']['uri']); + $this->assertSame('code-review', $answer['body']['result']['skill']['frontmatter']['name']); + } + + #[TestDox('skills/get on an unknown uri answers Invalid params')] + public function testSkillsGetUnknownUriIsInvalidParams(): void + { + $answer = self::call(self::protocol(), 'skills/get', ['uri' => 'skill://unknown/SKILL.md']); + + $this->assertSame(400, $answer['status']); + $this->assertSame(-32602, $answer['body']['error']['code']); + } + + #[TestDox('the extension is advertised under capabilities.extensions')] + public function testSkillsExtensionIsAdvertised(): void + { + $answer = self::call(self::protocol(), 'server/discover'); + + $this->assertArrayHasKey('io.modelcontextprotocol/skills', (array) $answer['body']['result']['capabilities']['extensions']); + } + + private static function protocol(): StatelessProtocol + { + return Server::builder() + ->setServerInfo('test-server', '1.0.0') + ->addSkillsFromDirectory(self::FIXTURES) + ->buildStateless([ProtocolVersion::V2026_07_28]); + } + + /** + * @param array $params + * + * @return array{status: int, body: array} + */ + private static function call(StatelessProtocol $protocol, string $method, array $params = []): array + { + $params['_meta'] = [ + RequestMeta::PROTOCOL_VERSION => ProtocolVersion::V2026_07_28->value, + RequestMeta::CLIENT_CAPABILITIES => new \stdClass(), + ]; + + $result = $protocol->handle( + json_encode(['jsonrpc' => '2.0', 'id' => 1, 'method' => $method, 'params' => $params], \JSON_THROW_ON_ERROR), + [ + 'MCP-Protocol-Version' => ProtocolVersion::V2026_07_28->value, + 'Mcp-Method' => $method, + ], + ); + + return [ + 'status' => $result->httpStatus, + 'body' => json_decode($result->toJson(), true, flags: \JSON_THROW_ON_ERROR), + ]; + } +} From 95a4e200afaf9361ef1deb0a5aeb7587ef6af06e Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Tue, 15 Sep 2026 19:40:04 +0200 Subject: [PATCH 3/3] [Docs] Add Skills extension entry to CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e77f0621..3f9977a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * [BC Break] Remove the `providerClass` argument of `#[CompletionProvider]`. Use `provider:`, which takes the same class-string and is now the first positional argument. * Add `HttpTransport::getSessionId()` to read the server-minted `Mcp-Session-Id`: a request-scoped caller can persist it and pass it back through the constructor's `$headers` on a later transport. Always `null` on `2026-07-28`, which removed protocol-level sessions. * Fix OIDC discovery rejecting issuers with a trailing slash (e.g. Authentik, Auth0). +* Add the Skills extension (`io.modelcontextprotocol/skills`, SEP-2640). `Builder::addSkillsFromDirectory()` serves a directory of `SKILL.md` skills as `skill://` resources, and `skills/list`/`skills/get` answer with a complete, digest-and-size manifest per skill. 0.8.0 -----