diff --git a/.github/workflows/conformance-weekly.yaml b/.github/workflows/conformance-weekly.yaml index d301237..9b255b3 100644 --- a/.github/workflows/conformance-weekly.yaml +++ b/.github/workflows/conformance-weekly.yaml @@ -125,10 +125,11 @@ jobs: name: Open issue on failure runs-on: ubuntu-latest needs: [server, client] - if: failure() && github.event_name == 'schedule' + if: failure() env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} steps: - name: File or comment tracking issue @@ -140,12 +141,13 @@ jobs: gh issue create \ --title '[conformance] Weekly conformance run failed' \ --label conformance-weekly \ - --body "Weekly conformance against \`@modelcontextprotocol/conformance@latest\` failed. + --body "Conformance against the newest published \`@modelcontextprotocol/conformance\` failed. - Run: $RUN_URL - - Triggered: $(date -u +%FT%TZ) + - Trigger: $EVENT_NAME + - Date: $(date -u +%FT%TZ) - Upstream likely published a release whose scenarios the SDK does not satisfy. Either fix the SDK, update the conformance fixtures, or add the new failure to \`tests/Conformance/conformance-baseline-2025-11-25.yml\`." + Upstream likely published a release whose scenarios the SDK does not satisfy. Either fix the SDK, update the conformance fixtures, or add the new failure to the matching \`tests/Conformance/conformance-baseline-.yml\`." fi publish: diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 20b6e08..7a399f1 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -35,7 +35,7 @@ jobs: uses: "ramsey/composer-install@v4" - name: Install uv - uses: astral-sh/setup-uv@v9.0.0 + uses: astral-sh/setup-uv@v10.1.0 with: enable-cache: true diff --git a/CHANGELOG.md b/CHANGELOG.md index f085872..e77f062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to `mcp/sdk` will be documented in this file. +0.9.0 +----- + +* [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). + 0.8.0 ----- diff --git a/README.md b/README.md index 04965f8..231c4fe 100644 --- a/README.md +++ b/README.md @@ -1,19 +1,10 @@ # MCP PHP SDK -
- [![Latest Version](https://img.shields.io/packagist/v/mcp/sdk.svg)](https://packagist.org/packages/mcp/sdk) [![CI](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/pipeline.yaml/badge.svg)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/pipeline.yaml) [![PHP Version](https://img.shields.io/packagist/php-v/mcp/sdk.svg)](https://packagist.org/packages/mcp/sdk) [![License](https://img.shields.io/packagist/l/mcp/sdk.svg)](LICENSE) -[![Server Conformance 2025-11-25](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/modelcontextprotocol/php-sdk/badges/server-conformance-2025-11-25.json)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/conformance-weekly.yaml) -[![Client Conformance 2025-11-25](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/modelcontextprotocol/php-sdk/badges/client-conformance-2025-11-25.json)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/conformance-weekly.yaml) -[![Server Conformance 2026-07-28](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/modelcontextprotocol/php-sdk/badges/server-conformance-2026-07-28.json)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/conformance-weekly.yaml) -[![Client Conformance 2026-07-28](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/modelcontextprotocol/php-sdk/badges/client-conformance-2026-07-28.json)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/conformance-weekly.yaml) - -
- The official PHP SDK for the Model Context Protocol (MCP). It provides a framework-agnostic API for implementing MCP servers and clients in PHP — tools, resources, prompts, STDIO and HTTP transports, sessions, authorization, and both protocol eras (the `initialize` handshake and the stateless `2026-07-28` revision). @@ -25,6 +16,17 @@ development practices and standards from the Symfony project, including [Coding Until the first major release, this SDK is considered [experimental](https://symfony.com/doc/current/contributing/code/experimental.html), please see the [roadmap](./ROADMAP.md) for planned next steps and features. +## Protocol Conformance + +Measured weekly against the [MCP Conformance Test Framework](https://github.com/modelcontextprotocol/conformance), +as passing checks out of total. The `2026-07-28` scores run against the framework's `alpha` releases, so they move as +upstream publishes new scenarios. + +| Revision | Server | Client | +|:--------:|:------:|:------:| +| `2026-07-28` | [![Server conformance 2026-07-28](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/modelcontextprotocol/php-sdk/badges/server-conformance-2026-07-28.json)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/conformance-weekly.yaml) | [![Client conformance 2026-07-28](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/modelcontextprotocol/php-sdk/badges/client-conformance-2026-07-28.json)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/conformance-weekly.yaml) | +| `2025-11-25` | [![Server conformance 2025-11-25](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/modelcontextprotocol/php-sdk/badges/server-conformance-2025-11-25.json)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/conformance-weekly.yaml) | [![Client conformance 2025-11-25](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/modelcontextprotocol/php-sdk/badges/client-conformance-2025-11-25.json)](https://github.com/modelcontextprotocol/php-sdk/actions/workflows/conformance-weekly.yaml) | + ## Installation ```bash diff --git a/docs/advanced/events.md b/docs/advanced/events.md index 590ad6e..d6d7b44 100644 --- a/docs/advanced/events.md +++ b/docs/advanced/events.md @@ -35,6 +35,7 @@ The SDK dispatches 4 broad event types at the protocol level, allowing you to ob **Dispatched**: When any request is received from the client, before it's processed by handlers. **Properties**: + - `getRequest(): Request` - The incoming request - `setRequest(Request $request): void` - Modify the request before processing - `getSession(): SessionInterface` - The current session @@ -45,6 +46,7 @@ The SDK dispatches 4 broad event types at the protocol level, allowing you to ob **Dispatched**: When a successful response is ready to be sent to the client, after handler execution. **Properties**: + - `getResponse(): Response` - The response being sent - `setResponse(Response $response): void` - Modify the response before sending - `getRequest(): Request` - The original request @@ -56,6 +58,7 @@ The SDK dispatches 4 broad event types at the protocol level, allowing you to ob **Dispatched**: When an error occurs during request processing. **Properties**: + - `getError(): Error` - The error being sent - `setError(Error $error): void` - Modify the error before sending - `getRequest(): Request` - The original request. Messages that fail to parse are rejected before this event, so a listener never sees them. @@ -67,6 +70,7 @@ The SDK dispatches 4 broad event types at the protocol level, allowing you to ob **Dispatched**: When a notification is received from the client, before it's processed by handlers. **Properties**: + - `getNotification(): Notification` - The incoming notification - `setNotification(Notification $notification): void` - Modify the notification before processing - `getSession(): SessionInterface` - The current session diff --git a/docs/client/connecting.md b/docs/client/connecting.md index b060916..1fe01e1 100644 --- a/docs/client/connecting.md +++ b/docs/client/connecting.md @@ -169,6 +169,7 @@ $client->connect($transport); ``` The `connect()` method performs the MCP initialization handshake: + 1. Opens the transport connection 2. Sends InitializeRequest with client capabilities 3. Waits for InitializeResult from server diff --git a/docs/client/transports.md b/docs/client/transports.md index dd27166..84e1500 100644 --- a/docs/client/transports.md +++ b/docs/client/transports.md @@ -18,6 +18,7 @@ $transport = new StdioTransport( ``` **Parameters:** + - `command` (string): The command to execute - `args` (array): Command arguments - `cwd` (string|null): Working directory for the process @@ -39,6 +40,7 @@ $transport = new HttpTransport( ``` **Parameters:** + - `endpoint` (string): The MCP server URL - `headers` (array): Additional HTTP headers - `httpClient` (ClientInterface|null): PSR-18 HTTP client (auto-discovered) @@ -50,6 +52,7 @@ $transport = new HttpTransport( **PSR-18 Auto-Discovery:** The transport automatically discovers PSR-18 HTTP clients from: + - `php-http/guzzle7-adapter` - `php-http/curl-client` - `symfony/http-client` diff --git a/docs/run/server-builder.md b/docs/run/server-builder.md index 52d27e7..017d4a8 100644 --- a/docs/run/server-builder.md +++ b/docs/run/server-builder.md @@ -54,6 +54,7 @@ $server = Server::builder() ``` **Parameters:** + - `$name` (string): The server name - `$version` (string): Version string (semantic versioning recommended) - `$description` (string|null): Optional description @@ -152,6 +153,7 @@ $server = Server::builder() ``` **Parameters:** + - `$basePath` (string): Base directory for discovery (typically `__DIR__`) - `$scanDirs` (array): Directories to recursively scan for `#[McpTool]`, `#[McpResource]`, etc. All subdirectories are included. (default: `['.', 'src']`) - `$excludeDirs` (array): Directory names to exclude **within** the scanned directories during recursive scanning @@ -189,6 +191,7 @@ $server = Server::builder() ``` **How `excludeDirs` works:** + - If scanning `src/` and there's `src/vendor/`, it will be excluded - If scanning `lib/` and there's `lib/tests/`, it will be excluded - But if `vendor/` and `tests/` are at the same level as `src/`, they're not scanned anyway (not in `scanDirs`) @@ -217,6 +220,7 @@ $server = Server::builder() ``` **Basic Container Features:** + - Supports constructor auto-wiring for classes with parameterless constructors - Resolves dependencies where all parameters are type-hinted classes/interfaces known to the container - Supports parameters with default values diff --git a/docs/run/sessions.md b/docs/run/sessions.md index abdac60..93f01c7 100644 --- a/docs/run/sessions.md +++ b/docs/run/sessions.md @@ -74,6 +74,7 @@ $server = Server::builder() ``` **Parameters:** + - `$gcProbability` (int): The numerator of the GC probability fraction (default: `1`). Set to `0` to disable GC. - `$gcDivisor` (int): The denominator of the GC probability fraction (default: `100`). Must be >= 1. @@ -81,6 +82,7 @@ $server = Server::builder() > the `gcProbability` and `gcDivisor` settings are ignored — you control GC behavior in your own implementation. **Available Session Stores:** + - `InMemorySessionStore`: Fast in-memory storage (default) - `FileSessionStore`: Persistent file-based storage - `Psr16SessionStore`: PSR-16 compliant cache-based storage diff --git a/docs/servers/completions.md b/docs/servers/completions.md index c422620..e1f6086 100644 --- a/docs/servers/completions.md +++ b/docs/servers/completions.md @@ -87,6 +87,7 @@ public function getUserProfile( ``` **Provider Resolution:** + - **Class strings** (`Provider::class`) → Resolved from PSR-11 container - **Instances** (`new Provider()`) → Used directly - **Values** (`['a', 'b']`) → Wrapped in `ListCompletionProvider` diff --git a/docs/servers/prompts.md b/docs/servers/prompts.md index 33da3c7..bb46ea5 100644 --- a/docs/servers/prompts.md +++ b/docs/servers/prompts.md @@ -100,6 +100,7 @@ makes the prompt handler throw. ### Error Handling Prompt handlers can throw any exception, but the type determines how it's handled: + - **`PromptGetException`**: Converted to JSON-RPC error response with the actual exception message - **Any other exception**: Converted to JSON-RPC error response, but with a generic error message diff --git a/docs/servers/registration.md b/docs/servers/registration.md index e2dc563..896333b 100644 --- a/docs/servers/registration.md +++ b/docs/servers/registration.md @@ -6,6 +6,7 @@ registry somehow. There are three ways to get it there, and they mix freely. ## Attribute-Based Discovery **Advantages:** + - Declarative and readable - Automatic parameter inference - DocBlock integration @@ -25,6 +26,7 @@ Register MCP elements programmatically without using attributes. The handler is any PHP callable. **Advantages:** + - Fine-grained control - Runtime configuration - Conditional registration diff --git a/src/Capability/Attribute/CompletionProvider.php b/src/Capability/Attribute/CompletionProvider.php index 9e8dc80..a21963c 100644 --- a/src/Capability/Attribute/CompletionProvider.php +++ b/src/Capability/Attribute/CompletionProvider.php @@ -24,9 +24,9 @@ class CompletionProvider * @param class-string|ProviderInterface|null $provider if a class-string, it will be resolved * from the container at the point of use * @param ?array $values a list of values to use for completion + * @param class-string|null $enum an enum whose cases are the completions */ public function __construct( - public ?string $providerClass = null, public string|ProviderInterface|null $provider = null, public ?array $values = null, public ?string $enum = null, diff --git a/src/Capability/Discovery/Discoverer.php b/src/Capability/Discovery/Discoverer.php index 5b7e476..c70559f 100644 --- a/src/Capability/Discovery/Discoverer.php +++ b/src/Capability/Discovery/Discoverer.php @@ -325,8 +325,6 @@ final class Discoverer implements DiscovererInterface if ($attributeInstance->provider) { $completionProviders[$param->getName()] = $attributeInstance->provider; - } elseif ($attributeInstance->providerClass) { - $completionProviders[$param->getName()] = $attributeInstance->provider; } elseif ($attributeInstance->values) { $completionProviders[$param->getName()] = new ListCompletionProvider($attributeInstance->values); } elseif ($attributeInstance->enum) { diff --git a/src/Capability/Registry.php b/src/Capability/Registry.php index 794193f..9784043 100644 --- a/src/Capability/Registry.php +++ b/src/Capability/Registry.php @@ -88,10 +88,27 @@ final class Registry implements RegistryInterface return; } + $this->loadFrom($this->loader); + + $this->loaded = true; + } + + /** + * Runs $loader with the change events its registrations would dispatch suppressed, since they + * describe the registry filling up rather than changing. + * + * Re-entrant-safe, and failure propagates. Does not mark the registry loaded: $loader is the + * caller's, and the one the constructor took is still owed its run. + */ + public function loadFrom(LoaderInterface $loader): void + { + if ($this->loading) { + return; + } + $this->loading = true; try { - $this->loader->load($this); - $this->loaded = true; + $loader->load($this); } finally { $this->loading = false; } diff --git a/src/Capability/Registry/Loader/ReflectedElementLoader.php b/src/Capability/Registry/Loader/ReflectedElementLoader.php index d1e849d..209644c 100644 --- a/src/Capability/Registry/Loader/ReflectedElementLoader.php +++ b/src/Capability/Registry/Loader/ReflectedElementLoader.php @@ -287,7 +287,7 @@ final class ReflectedElementLoader implements LoaderInterface } /** - * @return array + * @return array|ProviderInterface> */ private function getCompletionProviders(\ReflectionMethod|\ReflectionFunction $reflection): array { @@ -307,8 +307,6 @@ final class ReflectedElementLoader implements LoaderInterface if ($attributeInstance->provider) { $completionProviders[$param->getName()] = $attributeInstance->provider; - } elseif ($attributeInstance->providerClass) { - $completionProviders[$param->getName()] = $attributeInstance->providerClass; } elseif ($attributeInstance->values) { $completionProviders[$param->getName()] = new ListCompletionProvider($attributeInstance->values); } elseif ($attributeInstance->enum) { diff --git a/src/Capability/Registry/ReferenceHandler.php b/src/Capability/Registry/ReferenceHandler.php index ba5a18b..c0084cb 100644 --- a/src/Capability/Registry/ReferenceHandler.php +++ b/src/Capability/Registry/ReferenceHandler.php @@ -97,10 +97,32 @@ final class ReferenceHandler implements ReferenceHandlerInterface $finalArgs = []; foreach ($reflection->getParameters() as $parameter) { - // TODO: Handle variadic parameters. $paramName = $parameter->getName(); $paramPosition = $parameter->getPosition(); + if ($parameter->isVariadic()) { + // Each element is cast individually below; a non-array value is + // wrapped as a single element, since prompt arguments arrive as + // plain strings (Record per the protocol) even + // though tool arguments follow the advertised "array" schema. + $values = $arguments[$paramName] ?? []; + if (!\is_array($values)) { + $values = [$values]; + } elseif (!array_is_list($values)) { + throw RegistryException::invalidParams(\sprintf('Parameter `%s` must be a list of values, not an object.', $paramName)); + } + foreach ($values as $index => $value) { + try { + $finalArgs[] = $this->castArgumentType($value, $parameter); + } catch (InvalidArgumentException $e) { + throw RegistryException::invalidParams(\sprintf('Parameter `%s[%d]`: %s', $paramName, $index, $e->getMessage()), $e); + } catch (\Throwable $e) { + throw RegistryException::internalError(\sprintf('Error processing parameter `%s[%d]`: %s', $paramName, $index, $e->getMessage()), $e); + } + } + continue; + } + // Check if parameter is a special injectable type $type = $parameter->getType(); if ($type instanceof \ReflectionNamedType && !$type->isBuiltin()) { diff --git a/src/Client/Transport/HttpTransport.php b/src/Client/Transport/HttpTransport.php index b5499e8..3c12ea7 100644 --- a/src/Client/Transport/HttpTransport.php +++ b/src/Client/Transport/HttpTransport.php @@ -121,6 +121,22 @@ class HttpTransport extends BaseTransport implements HeaderAwareTransportInterfa $this->headerCallback = $callback; } + /** + * The session ID minted by the server for this connection, if any. + * + * Captured from the Mcp-Session-Id response header of a handshake-era + * server; always null on 2026-07-28, which removed protocol-level sessions + * (SEP-2567). A caller that cannot keep the transport alive between + * interactions can persist it and pass it back through the constructor's + * $headers on a later transport, where it wins over the tracked value. + * + * @return string|null null until the server mints a session, and again after close() + */ + public function getSessionId(): ?string + { + return $this->sessionId; + } + public function send(string $data): void { $request = $this->requestFactory->createRequest('POST', $this->endpoint) diff --git a/src/Schema/JsonRpc/Notification.php b/src/Schema/JsonRpc/Notification.php index bfd37f1..3ec0883 100644 --- a/src/Schema/JsonRpc/Notification.php +++ b/src/Schema/JsonRpc/Notification.php @@ -74,7 +74,7 @@ abstract class Notification implements HasMethodInterface, MessageInterface $array['params'] = $params; } - if (null !== $this->meta && !isset($params['meta'])) { + if (null !== $this->meta && !isset($params['_meta'])) { $array['params']['_meta'] = $this->meta; } diff --git a/src/Schema/JsonRpc/Request.php b/src/Schema/JsonRpc/Request.php index cb8ed83..afdf9e9 100644 --- a/src/Schema/JsonRpc/Request.php +++ b/src/Schema/JsonRpc/Request.php @@ -122,7 +122,7 @@ abstract class Request implements HasMethodInterface, MessageInterface $array['params'] = $params; } - if (null !== $this->meta && !isset($params['meta'])) { + if (null !== $this->meta && !isset($params['_meta'])) { $array['params']['_meta'] = $this->meta; } diff --git a/src/Server/Builder.php b/src/Server/Builder.php index 90272ef..1a5dd73 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -522,7 +522,8 @@ final class Builder * * Lazy (the default) defers loading to the first registry read so a persistent runtime does not * freeze the registry to a source not yet ready at build time. Disable to load eagerly at build. - * A registry supplied via setRegistry() is always loaded eagerly. + * A registry supplied via setRegistry() is always loaded eagerly; its own constructor loader, + * if it has one, still runs on the first read. */ public function setLazyLoading(bool $lazyLoading = true): self { @@ -1045,8 +1046,13 @@ final class Builder if ($this->hasCustomRegistry) { // Builder can't inject the loader into an already-constructed instance, so load it eagerly. + // Via loadFrom(), which suppresses the change events the load would otherwise dispatch. $registry = $this->registry; - $chainLoader->load($registry); + if ($registry instanceof Registry) { + $registry->loadFrom($chainLoader); + } else { + $chainLoader->load($registry); + } $eagerlyLoaded = true; } else { $registry = new Registry($eventDispatcher, $logger, loader: $chainLoader); diff --git a/src/Server/Protocol.php b/src/Server/Protocol.php index 4d6e5f5..f460c3c 100644 --- a/src/Server/Protocol.php +++ b/src/Server/Protocol.php @@ -303,9 +303,11 @@ class Protocol $notification = $result['notification']; $this->sendNotification($notification, $session); } elseif ('request' === $result['type']) { - $request = $result['request']; + // Keep $request untouched: it is the inbound request the catch + // blocks below answer under, not this outbound one. + $outboundRequest = $result['request']; $timeout = $result['timeout'] ?? 120; - $this->sendRequest($request, $timeout, $session); + $this->sendRequest($outboundRequest, $timeout, $session); } } diff --git a/src/Server/Session/FileSessionStore.php b/src/Server/Session/FileSessionStore.php index e218e75..8d08bae 100644 --- a/src/Server/Session/FileSessionStore.php +++ b/src/Server/Session/FileSessionStore.php @@ -14,6 +14,8 @@ namespace Mcp\Server\Session; use Mcp\Exception\RuntimeException; use Mcp\Server\NativeClock; use Psr\Clock\ClockInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Symfony\Component\Uid\Uuid; /** @@ -26,9 +28,13 @@ class FileSessionStore implements SessionStoreInterface private readonly string $directory, private readonly int $ttl = 3600, private readonly ClockInterface $clock = new NativeClock(), + private readonly LoggerInterface $logger = new NullLogger(), ) { - if (!is_dir($this->directory)) { - @mkdir($this->directory, 0775, true); + if (!is_dir($this->directory) && !@mkdir($this->directory, 0775, true) && !is_dir($this->directory)) { + $this->logger->warning('Failed to create session directory.', [ + 'directory' => $this->directory, + 'error' => error_get_last()['message'] ?? 'unknown', + ]); } if (!is_dir($this->directory) || !is_writable($this->directory)) { @@ -59,13 +65,23 @@ class FileSessionStore implements SessionStoreInterface $mtime = @filemtime($path) ?: 0; if (($this->clock->now()->getTimestamp() - $mtime) > $this->ttl) { - @unlink($path); + if (!@unlink($path) && is_file($path)) { + $this->logger->warning('Failed to delete expired session file.', [ + 'path' => $path, + 'error' => error_get_last()['message'] ?? 'unknown', + ]); + } return false; } $data = @file_get_contents($path); if (false === $data) { + $this->logger->warning('Failed to read session file.', [ + 'path' => $path, + 'error' => error_get_last()['message'] ?? 'unknown', + ]); + return false; } @@ -78,6 +94,11 @@ class FileSessionStore implements SessionStoreInterface $tmp = $path.'.tmp'; if (false === @file_put_contents($tmp, $data, \LOCK_EX)) { + $this->logger->warning('Failed to write session file.', [ + 'path' => $tmp, + 'error' => error_get_last()['message'] ?? 'unknown', + ]); + return false; } @@ -85,6 +106,10 @@ class FileSessionStore implements SessionStoreInterface if (!@rename($tmp, $path)) { // Fallback if rename fails cross-device if (false === @copy($tmp, $path)) { + $this->logger->warning('Failed to move session file into place.', [ + 'path' => $path, + 'error' => error_get_last()['message'] ?? 'unknown', + ]); @unlink($tmp); return false; @@ -101,8 +126,11 @@ class FileSessionStore implements SessionStoreInterface { $path = $this->pathFor($id); - if (is_file($path)) { - @unlink($path); + if (is_file($path) && !@unlink($path) && is_file($path)) { + $this->logger->warning('Failed to delete session file.', [ + 'path' => $path, + 'error' => error_get_last()['message'] ?? 'unknown', + ]); } return true; @@ -119,6 +147,11 @@ class FileSessionStore implements SessionStoreInterface $dir = @opendir($this->directory); if (false === $dir) { + $this->logger->warning('Failed to open session directory for garbage collection.', [ + 'directory' => $this->directory, + 'error' => error_get_last()['message'] ?? 'unknown', + ]); + return $deleted; } @@ -140,7 +173,15 @@ class FileSessionStore implements SessionStoreInterface $mtime = @filemtime($path) ?: 0; if (($now - $mtime) > $this->ttl) { - @unlink($path); + if (!@unlink($path) && is_file($path)) { + $this->logger->warning('Failed to delete expired session file.', [ + 'path' => $path, + 'error' => error_get_last()['message'] ?? 'unknown', + ]); + + continue; + } + $deleted[] = Uuid::fromString($entry); } } diff --git a/src/Server/Session/Session.php b/src/Server/Session/Session.php index 6eee72d..4db11f7 100644 --- a/src/Server/Session/Session.php +++ b/src/Server/Session/Session.php @@ -110,15 +110,12 @@ class Session implements SessionInterface while (\count($segments) > 1) { $segment = array_shift($segments); if (!isset($data[$segment]) || !\is_array($data[$segment])) { - $data[$segment] = []; + return; } $data = &$data[$segment]; } - $lastKey = array_shift($segments); - if (isset($data[$lastKey])) { - unset($data[$lastKey]); - } + unset($data[array_shift($segments)]); } public function clear(): void diff --git a/src/Server/Transport/Http/OAuth/OidcDiscovery.php b/src/Server/Transport/Http/OAuth/OidcDiscovery.php index 2f24685..a0c0cd6 100644 --- a/src/Server/Transport/Http/OAuth/OidcDiscovery.php +++ b/src/Server/Transport/Http/OAuth/OidcDiscovery.php @@ -146,8 +146,10 @@ class OidcDiscovery implements OidcDiscoveryInterface */ private function fetchMetadata(string $issuer): array { - $issuer = rtrim($issuer, '/'); - $parsed = parse_url($issuer); + // The trailing slash is dropped to build discovery URLs (RFC 8414 §3.1), + // but the issuer is matched verbatim (RFC 8414 §3.3, OIDC Discovery §4.3). + $discoveryIssuer = rtrim($issuer, '/'); + $parsed = parse_url($discoveryIssuer); if (false === $parsed || !isset($parsed['scheme'], $parsed['host'])) { throw new RuntimeException(\sprintf('Invalid issuer URL: %s', $issuer)); @@ -170,7 +172,7 @@ class OidcDiscovery implements OidcDiscoveryInterface // 2. OIDC path insertion $discoveryUrls[] = $baseUrl.'/.well-known/openid-configuration'.$path; // 3. OIDC path appending - $discoveryUrls[] = $issuer.'/.well-known/openid-configuration'; + $discoveryUrls[] = $discoveryIssuer.'/.well-known/openid-configuration'; } else { // For issuer URLs without path components $discoveryUrls[] = $baseUrl.'/.well-known/oauth-authorization-server'; diff --git a/src/Server/Transport/StreamableHttpTransport.php b/src/Server/Transport/StreamableHttpTransport.php index 2bf088b..c01503e 100644 --- a/src/Server/Transport/StreamableHttpTransport.php +++ b/src/Server/Transport/StreamableHttpTransport.php @@ -15,6 +15,7 @@ use Http\Discovery\Psr17FactoryDiscovery; use Mcp\Exception\InvalidArgumentException; use Mcp\Schema\Enum\ProtocolVersion; use Mcp\Schema\JsonRpc\Error; +use Mcp\Server\NativeClock; use Mcp\Server\Stateless\StatelessProtocol; use Mcp\Server\Transport\Http\Middleware\CorsMiddleware; use Mcp\Server\Transport\Http\Middleware\DnsRebindingProtectionMiddleware; @@ -22,6 +23,7 @@ use Mcp\Server\Transport\Http\Middleware\ProtocolVersionMiddleware; use Mcp\Server\Transport\Http\MiddlewareRequestHandler; use Mcp\Server\Transport\Http\StatelessResponder; use Mcp\Server\Wire\InboundClassifier; +use Psr\Clock\ClockInterface; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; @@ -79,12 +81,13 @@ class StreamableHttpTransport extends BaseTransport implements StatelessAwareTra * @param iterable|null $middleware `null` installs {@see self::defaultMiddleware()}; `[]` disables all middleware */ public function __construct( - private ServerRequestInterface $request, + private readonly ServerRequestInterface $request, ?ResponseFactoryInterface $responseFactory = null, ?StreamFactoryInterface $streamFactory = null, ?LoggerInterface $logger = null, ?iterable $middleware = null, private readonly int $maxBodyBytes = self::DEFAULT_MAX_BODY_BYTES, + private readonly ClockInterface $clock = new NativeClock(), ) { parent::__construct($logger); @@ -185,12 +188,17 @@ class StreamableHttpTransport extends BaseTransport implements StatelessAwareTra { $this->handleMessage($body, $this->sessionId); - if (null !== $this->immediateResponse) { - $response = $this->responseFactory->createResponse($this->immediateStatusCode ?? 200) - ->withHeader('Content-Type', 'application/json') - ->withBody($this->streamFactory->createStream($this->immediateResponse)); + // Consume the immediate response exactly once, so a transport instance + // reused for a later POST does not replay it. + $immediateResponse = $this->immediateResponse; + $immediateStatusCode = $this->immediateStatusCode; + $this->immediateResponse = null; + $this->immediateStatusCode = null; - return $response; + if (null !== $immediateResponse) { + return $this->responseFactory->createResponse($immediateStatusCode ?? 200) + ->withHeader('Content-Type', 'application/json') + ->withBody($this->streamFactory->createStream($immediateResponse)); } if (null !== $this->sessionFiber) { @@ -268,7 +276,7 @@ class StreamableHttpTransport extends BaseTransport implements StatelessAwareTra break; } - if (time() - $timestamp >= $timeout) { + if ($this->clock->now()->getTimestamp() - $timestamp >= $timeout) { $error = Error::forInternalError('Request timed out', $requestId); $yielded = $this->sessionFiber->resume($error); $this->handleFiberYield($yielded, $this->sessionId); @@ -376,8 +384,6 @@ class StreamableHttpTransport extends BaseTransport implements StatelessAwareTra private function handleRequest(ServerRequestInterface $request): ResponseInterface { - $this->request = $request; - if ('OPTIONS' === $request->getMethod()) { return $this->handleOptionsRequest(); } @@ -405,7 +411,7 @@ class StreamableHttpTransport extends BaseTransport implements StatelessAwareTra } if ($classification->modern) { - return $this->handleModernRequest($body ?? '', $classification->claimedVersion ?? ''); + return $this->handleModernRequest($request, $body ?? '', $classification->claimedVersion ?? ''); } // The version-header rule only reaches the traffic it is about. Running @@ -443,7 +449,7 @@ class StreamableHttpTransport extends BaseTransport implements StatelessAwareTra /** * Answers a request that claimed the modern era's per-request envelope. */ - private function handleModernRequest(string $body, string $claimedVersion): ResponseInterface + private function handleModernRequest(ServerRequestInterface $request, string $body, string $claimedVersion): ResponseInterface { if (null === $this->stateless) { return $this->responder->error( @@ -452,7 +458,7 @@ class StreamableHttpTransport extends BaseTransport implements StatelessAwareTra ); } - return $this->responder->respond($this->stateless->handle($body, self::headers($this->request))); + return $this->responder->respond($this->stateless->handle($body, self::headers($request))); } /** diff --git a/tests/Conformance/score.php b/tests/Conformance/score.php index 06c3409..37ebcab 100644 --- a/tests/Conformance/score.php +++ b/tests/Conformance/score.php @@ -16,11 +16,12 @@ * php score.php [results-dir] * * The conformance CLI writes one `checks.json` per scenario into - * `[results-dir]` (default `results`), relative to this file. A scenario - * counts as passing when none of its checks has a FAILURE status; the badge - * message is "/ (%)" and is written to + * `[results-dir]` (default `results`), relative to this file. Every check with + * a SUCCESS or FAILURE status counts towards the score; the badge message is + * "/ (%)" and is written to * `-conformance-.json`, so each revision gets its own - * badge. + * badge. The badge is labelled "checks" only: the README renders the badges in + * a table whose row and column headers already name the revision and suite. */ use Symfony\Component\Console\Command\Command; @@ -92,7 +93,7 @@ require_once dirname(__DIR__, 2).'/vendor/autoload.php'; $badge = [ 'schemaVersion' => 1, - 'label' => sprintf('%s %s', $suite, $specVersion), + 'label' => 'checks', 'message' => $total > 0 ? sprintf('%d/%d (%d%%)', $passed, $total, $pct) : 'no data', 'color' => match (true) { 0 === $total => 'lightgrey', @@ -116,7 +117,7 @@ require_once dirname(__DIR__, 2).'/vendor/autoload.php'; $io->listing($failures); } - $io->success(sprintf('%s: %s', $badge['label'], $badge['message'])); + $io->success(sprintf('%s %s: %s', $suite, $specVersion, $badge['message'])); return Command::SUCCESS; }) diff --git a/tests/Integration/CompletionTest.php b/tests/Integration/CompletionTest.php new file mode 100644 index 0000000..0da021a --- /dev/null +++ b/tests/Integration/CompletionTest.php @@ -0,0 +1,57 @@ +connect('completion'); + + $result = $client->complete(new PromptReference('book_seat'), ['name' => 'seat', 'value' => '12']); + + $this->assertSame(['12A', '12B'], $result->values); + } + + #[TestDox('an empty value offers every completion the provider knows')] + public function testEmptyValueOffersEveryCompletion(): void + { + $client = $this->connect('completion'); + + $result = $client->complete(new PromptReference('book_seat'), ['name' => 'seat', 'value' => '']); + + $this->assertSame(['12A', '12B', '14C'], $result->values); + } + + #[TestDox('an argument the prompt does not declare completes to nothing')] + public function testUnknownArgumentCompletesToNothing(): void + { + $client = $this->connect('completion'); + + $result = $client->complete(new PromptReference('book_seat'), ['name' => 'unknown', 'value' => '1']); + + $this->assertSame([], $result->values); + } +} diff --git a/tests/Integration/Fixture/Completion/BookingElements.php b/tests/Integration/Fixture/Completion/BookingElements.php new file mode 100644 index 0000000..2a64e5d --- /dev/null +++ b/tests/Integration/Fixture/Completion/BookingElements.php @@ -0,0 +1,38 @@ + + */ +final class BookingElements +{ + /** + * Confirms a seat booking. + * + * @param string $seat the seat to book + * + * @return array the prompt messages + */ + #[McpPrompt(name: 'book_seat')] + public function bookSeat( + #[CompletionProvider(provider: SeatCompletionProvider::class)] + string $seat, + ): array { + return [ + ['role' => 'user', 'content' => \sprintf('Book seat %s for me.', $seat)], + ]; + } +} diff --git a/tests/Integration/Fixture/Completion/SeatCompletionProvider.php b/tests/Integration/Fixture/Completion/SeatCompletionProvider.php new file mode 100644 index 0000000..a749fcf --- /dev/null +++ b/tests/Integration/Fixture/Completion/SeatCompletionProvider.php @@ -0,0 +1,42 @@ + + */ +final class SeatCompletionProvider implements ProviderInterface +{ + /** + * @param list $seats + */ + public function __construct( + private readonly array $seats, + ) { + } + + public function getCompletions(string $currentValue): array + { + return array_values(array_filter( + $this->seats, + static fn (string $seat): bool => str_starts_with($seat, $currentValue), + )); + } +} diff --git a/tests/Integration/Fixture/completion.php b/tests/Integration/Fixture/completion.php new file mode 100644 index 0000000..f33e951 --- /dev/null +++ b/tests/Integration/Fixture/completion.php @@ -0,0 +1,31 @@ +set(SeatCompletionProvider::class, new SeatCompletionProvider(['12A', '12B', '14C'])); + +Server::builder() + ->setServerInfo('integration-server', '1.0.0') + ->setContainer($container) + ->setDiscovery(__DIR__, ['Completion']) + ->build() + ->run(new StdioTransport()); diff --git a/tests/Unit/Capability/Attribute/CompletionProviderTest.php b/tests/Unit/Capability/Attribute/CompletionProviderTest.php index 19a7875..4b5269d 100644 --- a/tests/Unit/Capability/Attribute/CompletionProviderTest.php +++ b/tests/Unit/Capability/Attribute/CompletionProviderTest.php @@ -26,6 +26,15 @@ class CompletionProviderTest extends TestCase $this->assertNull($attribute->enum); } + public function testCanBeConstructedWithAPositionalProviderClass(): void + { + $attribute = new CompletionProvider(CompletionProviderFixture::class); + + $this->assertSame(CompletionProviderFixture::class, $attribute->provider); + $this->assertNull($attribute->values); + $this->assertNull($attribute->enum); + } + public function testCanBeConstructedWithProviderInstance(): void { $instance = new CompletionProviderFixture(); diff --git a/tests/Unit/Capability/Discovery/DiscoveryTest.php b/tests/Unit/Capability/Discovery/DiscoveryTest.php index 5c716d7..628aaa6 100644 --- a/tests/Unit/Capability/Discovery/DiscoveryTest.php +++ b/tests/Unit/Capability/Discovery/DiscoveryTest.php @@ -94,7 +94,7 @@ class DiscoveryTest extends TestCase $this->assertEquals([InvocablePromptFixture::class, '__invoke'], $prompts['InvokableGreeterPrompt']->handler); $this->assertArrayHasKey('content_creator', $prompts); - $this->assertCount(3, $prompts['content_creator']->completionProviders); + $this->assertCount(4, $prompts['content_creator']->completionProviders); $templates = $discovery->getResourceTemplates(); $this->assertCount(4, $templates); @@ -165,7 +165,7 @@ class DiscoveryTest extends TestCase $discovery = $this->discoverer->discover(__DIR__, ['Fixtures']); $this->assertArrayHasKey('content_creator', $prompts = $discovery->getPrompts()); - $this->assertCount(3, $prompts['content_creator']->completionProviders); + $this->assertCount(4, $prompts['content_creator']->completionProviders); $typeProvider = $prompts['content_creator']->completionProviders['type']; $this->assertInstanceOf(ListCompletionProvider::class, $typeProvider); @@ -182,4 +182,14 @@ class DiscoveryTest extends TestCase $categoryProvider = $templates['content://{category}/{slug}']->completionProviders['category']; $this->assertInstanceOf(ListCompletionProvider::class, $categoryProvider); } + + public function testDiscoversPositionalCompletionProviderAsClassString(): void + { + $discovery = $this->discoverer->discover(__DIR__, ['Fixtures']); + + $this->assertArrayHasKey('content_creator', $prompts = $discovery->getPrompts()); + + // Kept as a class-string so the container resolves it at the point of use. + $this->assertEquals(CompletionProviderFixture::class, $prompts['content_creator']->completionProviders['author']); + } } diff --git a/tests/Unit/Capability/Discovery/Fixtures/EnhancedCompletionHandler.php b/tests/Unit/Capability/Discovery/Fixtures/EnhancedCompletionHandler.php index 5ac67dd..9975f9a 100644 --- a/tests/Unit/Capability/Discovery/Fixtures/EnhancedCompletionHandler.php +++ b/tests/Unit/Capability/Discovery/Fixtures/EnhancedCompletionHandler.php @@ -14,13 +14,14 @@ namespace Mcp\Tests\Unit\Capability\Discovery\Fixtures; use Mcp\Capability\Attribute\CompletionProvider; use Mcp\Capability\Attribute\McpPrompt; use Mcp\Capability\Attribute\McpResourceTemplate; +use Mcp\Tests\Unit\Capability\Attribute\CompletionProviderFixture; use Mcp\Tests\Unit\Fixtures\Enum\PriorityEnum; use Mcp\Tests\Unit\Fixtures\Enum\StatusEnum; class EnhancedCompletionHandler { /** - * Create content with list and enum completion providers. + * Create content with list, enum and positional provider completion providers. */ #[McpPrompt(name: 'content_creator')] public function createContent( @@ -30,9 +31,11 @@ class EnhancedCompletionHandler string $status, #[CompletionProvider(enum: PriorityEnum::class)] string $priority, + #[CompletionProvider(CompletionProviderFixture::class)] + string $author, ): array { return [ - ['role' => 'user', 'content' => "Create a {$type} with status {$status} and priority {$priority}"], + ['role' => 'user', 'content' => "Create a {$type} with status {$status} and priority {$priority} for {$author}"], ]; } diff --git a/tests/Unit/Capability/Registry/ReferenceHandlerTest.php b/tests/Unit/Capability/Registry/ReferenceHandlerTest.php index f18196d..2952c81 100644 --- a/tests/Unit/Capability/Registry/ReferenceHandlerTest.php +++ b/tests/Unit/Capability/Registry/ReferenceHandlerTest.php @@ -14,6 +14,7 @@ namespace Mcp\Tests\Unit\Capability\Registry; use Mcp\Capability\Registry\ElementReference; use Mcp\Capability\Registry\ReferenceHandler; use Mcp\Exception\InvalidArgumentException; +use Mcp\Exception\RegistryException; use Mcp\Server\ClientGateway; use Mcp\Server\Handler\ResourceHandlerInterface; use Mcp\Server\Handler\ToolHandlerInterface; @@ -167,4 +168,99 @@ final class ReferenceHandlerTest extends TestCase (new ReferenceHandler())->handle($reference, ['_session' => $session]); } + + public function testHandleCastsEachElementOfAnArrayArgumentForAVariadicParameter(): void + { + // SchemaGenerator advertises variadic parameters as a JSON "array" schema, + // so the array arrives here as a single named argument (not spread across + // multiple keys) and must be cast element-by-element to the variadic's type. + $closure = static fn (string $name, int ...$scores): string => \sprintf('%s:%d', $name, array_sum($scores)); + $reference = new ElementReference($closure); + + $result = (new ReferenceHandler())->handle($reference, [ + '_session' => $this->createMock(SessionInterface::class), + 'name' => 'total', + 'scores' => ['1', '2', '3'], + ]); + + $this->assertSame('total:6', $result); + } + + public function testHandleTreatsOmittedVariadicArgumentAsZeroElements(): void + { + $closure = static fn (string $name, int ...$scores): int => \count($scores); + $reference = new ElementReference($closure); + + $result = (new ReferenceHandler())->handle($reference, [ + '_session' => $this->createMock(SessionInterface::class), + 'name' => 'empty', + ]); + + $this->assertSame(0, $result); + } + + public function testHandleWrapsANonArrayVariadicArgumentAsASingleElement(): void + { + // Tool arguments follow the advertised "array" schema, but MCP prompt + // arguments are always Record per the protocol - a + // client sends a plain string for a variadic prompt parameter, and + // that must still work rather than being rejected. + $closure = static fn (string $name, string ...$topics): array => $topics; + $reference = new ElementReference($closure); + + $result = (new ReferenceHandler())->handle($reference, [ + '_session' => $this->createMock(SessionInterface::class), + 'name' => 'single', + 'topics' => 'php', + ]); + + $this->assertSame(['php'], $result); + } + + public function testHandleThrowsRegistryExceptionWhenVariadicArgumentIsAnObjectNotAList(): void + { + $closure = static fn (string $name, int ...$scores): int => \count($scores); + $reference = new ElementReference($closure); + + $this->expectException(RegistryException::class); + $this->expectExceptionMessage('Parameter `scores` must be a list of values, not an object.'); + + (new ReferenceHandler())->handle($reference, [ + '_session' => $this->createMock(SessionInterface::class), + 'name' => 'bad', + 'scores' => ['a' => 1, 'b' => 2], + ]); + } + + public function testHandleKeepsArgumentOrderWhenAnInjectableParameterPrecedesAVariadic(): void + { + // Injectable and regular parameters are assigned by position while variadic + // elements are appended, so the final order only holds because a variadic is + // always the last parameter. + $closure = static fn (ClientGateway $gateway, string $sep = ',', string ...$parts): string => implode($sep, $parts); + $reference = new ElementReference($closure); + + $result = (new ReferenceHandler())->handle($reference, [ + '_session' => $this->createMock(SessionInterface::class), + '_request' => new \stdClass(), + 'parts' => ['a', 'b'], + ]); + + $this->assertSame('a,b', $result); + } + + public function testHandleIncludesParameterNameAndIndexWhenAVariadicElementFailsToCast(): void + { + $closure = static fn (string $name, int ...$scores): int => \count($scores); + $reference = new ElementReference($closure); + + $this->expectException(RegistryException::class); + $this->expectExceptionMessage('Parameter `scores[1]`: Cannot cast value to integer. Expected integer representation.'); + + (new ReferenceHandler())->handle($reference, [ + '_session' => $this->createMock(SessionInterface::class), + 'name' => 'bad', + 'scores' => ['1', 'not-a-number', '3'], + ]); + } } diff --git a/tests/Unit/Capability/RegistryTest.php b/tests/Unit/Capability/RegistryTest.php index cb32556..92782ab 100644 --- a/tests/Unit/Capability/RegistryTest.php +++ b/tests/Unit/Capability/RegistryTest.php @@ -753,6 +753,74 @@ class RegistryTest extends TestCase $this->assertTrue($registry->hasPrompts()); } + public function testListChangedEventsAreSuppressedWhenTheLoaderIsSuppliedFromOutside(): void + { + // As quiet as the deferred load above: these are the initial contents, not a change. + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->never())->method('dispatch'); + + $registry = new Registry($eventDispatcher, $this->logger); + $registry->loadFrom($this->toolLoader($this->createValidTool('loaded'))); + + $this->assertTrue($registry->hasTool('loaded')); + } + + public function testARuntimeRegistrationAfterAnExternalLoadIsStillDispatched(): void + { + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->once()) + ->method('dispatch') + ->with($this->isInstanceOf(ToolListChangedEvent::class)) + ->willReturnArgument(0); + + $registry = new Registry($eventDispatcher, $this->logger); + $registry->loadFrom($this->toolLoader($this->createValidTool('loaded'))); + + $registry->registerTool($this->createValidTool('runtime'), 'handler'); + } + + public function testAnExternalLoadDoesNotConsumeTheRegistrysOwnLoader(): void + { + $registry = new Registry($this->createMock(EventDispatcherInterface::class), $this->logger, loader: $this->toolLoader($this->createValidTool('own'))); + $registry->loadFrom($this->toolLoader($this->createValidTool('external'))); + + $this->assertTrue($registry->hasTool('external')); + $this->assertTrue($registry->hasTool('own')); + } + + public function testAnExternalLoadCanRunMoreThanOnce(): void + { + $registry = new Registry($this->createMock(EventDispatcherInterface::class), $this->logger); + $registry->loadFrom($this->toolLoader($this->createValidTool('first'))); + $registry->loadFrom($this->toolLoader($this->createValidTool('second'))); + + $this->assertTrue($registry->hasTool('first')); + $this->assertTrue($registry->hasTool('second')); + } + + public function testAFailedExternalLoadLeavesTheRegistryRetryable(): void + { + $registry = new Registry($this->createMock(EventDispatcherInterface::class), $this->logger); + $failing = new class implements LoaderInterface { + public function load(RegistryInterface $registry): void + { + // Reads during its own run, as discovery's identity check does. + $registry->hasTool('anything'); + + throw new \RuntimeException('data source not ready'); + } + }; + + foreach ([1, 2] as $attempt) { + try { + $registry->loadFrom($failing); + $this->fail('The loader was expected to fail.'); + } catch (\RuntimeException $e) { + $this->assertSame('data source not ready', $e->getMessage()); + } + } + } + public function testListChangedEventsAreStillDispatchedForRuntimeRegistrations(): void { $eventDispatcher = $this->createMock(EventDispatcherInterface::class); diff --git a/tests/Unit/Client/Transport/HttpTransportTest.php b/tests/Unit/Client/Transport/HttpTransportTest.php index 6c6119e..83a77e0 100644 --- a/tests/Unit/Client/Transport/HttpTransportTest.php +++ b/tests/Unit/Client/Transport/HttpTransportTest.php @@ -94,6 +94,53 @@ final class HttpTransportTest extends TestCase $this->assertSame('test-server', $client->getServerInfo()?->name); } + #[TestDox('the server-minted session ID is exposed while connected and cleared on close')] + public function testSessionIdIsExposedAndClearedOnClose(): void + { + $httpClient = new class implements ClientInterface { + public function sendRequest(RequestInterface $request): ResponseInterface + { + $decoded = json_decode((string) $request->getBody(), true); + + if ('initialize' !== ($decoded['method'] ?? null)) { + return new Response(202); + } + + $payload = json_encode([ + 'jsonrpc' => '2.0', + 'id' => $decoded['id'], + 'result' => [ + 'protocolVersion' => '2025-11-25', + 'capabilities' => ['tools' => ['listChanged' => false]], + 'serverInfo' => ['name' => 'test-server', 'version' => '1.0.0'], + ], + ]); + + return new Response(200, [ + 'Content-Type' => 'application/json', + 'Mcp-Session-Id' => 'session-abc123', + ], $payload); + } + }; + + $transport = new HttpTransport('http://localhost/mcp', [], $httpClient, $this->factory, $this->factory); + + $this->assertNull($transport->getSessionId()); + + $client = Client::builder() + ->setClientInfo('test-client', '1.0.0') + ->setInitTimeout(1) + ->build(); + + $client->connect($transport); + + $this->assertSame('session-abc123', $transport->getSessionId()); + + $client->disconnect(); + + $this->assertNull($transport->getSessionId()); + } + #[TestDox('SSE stream is aborted before the buffer can exceed the configured cap')] public function testSseBufferIsBoundedByConfiguredCap(): void { diff --git a/tests/Unit/Schema/JsonRpc/NotificationTest.php b/tests/Unit/Schema/JsonRpc/NotificationTest.php index 0d7d7c4..b2580de 100644 --- a/tests/Unit/Schema/JsonRpc/NotificationTest.php +++ b/tests/Unit/Schema/JsonRpc/NotificationTest.php @@ -53,4 +53,44 @@ final class NotificationTest extends TestCase $this->assertSame($expectedMeta, $notification->jsonSerialize()); } + + public function testMetaDoesNotOverrideMetaFromParams(): void + { + $notificationImplementation = new class extends Notification { + public static function getMethod(): string + { + return 'notifications/dummy'; + } + + public static function fromParams(?array $params): self + { + return new self(); + } + + protected function getParams(): array + { + return [ + '_meta' => ['key' => 'from-params'], + ]; + } + }; + + $notification = $notificationImplementation::fromArray([ + 'jsonrpc' => '2.0', + 'method' => 'notifications/dummy', + 'params' => [ + '_meta' => ['key' => 'from-meta'], + ], + ]); + + $expected = [ + 'jsonrpc' => '2.0', + 'method' => 'notifications/dummy', + 'params' => [ + '_meta' => ['key' => 'from-params'], + ], + ]; + + $this->assertSame($expected, $notification->jsonSerialize()); + } } diff --git a/tests/Unit/Schema/JsonRpc/RequestTest.php b/tests/Unit/Schema/JsonRpc/RequestTest.php index f400a90..d23b6a9 100644 --- a/tests/Unit/Schema/JsonRpc/RequestTest.php +++ b/tests/Unit/Schema/JsonRpc/RequestTest.php @@ -55,4 +55,41 @@ final class RequestTest extends TestCase $this->assertSame($expectedMeta, $notification->jsonSerialize()); } + + public function testMetaDoesNotOverrideMetaFromParams(): void + { + $requestImplementation = new class extends Request { + public static function getMethod(): string + { + return 'foo/bar'; + } + + public static function fromParams(?array $params): static + { + return new self(); + } + + protected function getParams(): array + { + return [ + '_meta' => ['key' => 'from-params'], + ]; + } + }; + + $request = $requestImplementation::fromParams(null) + ->withId('12345') + ->withMeta(['key' => 'from-meta']); + + $expected = [ + 'jsonrpc' => '2.0', + 'id' => '12345', + 'method' => 'foo/bar', + 'params' => [ + '_meta' => ['key' => 'from-params'], + ], + ]; + + $this->assertSame($expected, $request->jsonSerialize()); + } } diff --git a/tests/Unit/Server/BuilderTest.php b/tests/Unit/Server/BuilderTest.php index 9cdf329..790af5f 100644 --- a/tests/Unit/Server/BuilderTest.php +++ b/tests/Unit/Server/BuilderTest.php @@ -15,6 +15,7 @@ use Mcp\Capability\Registry; use Mcp\Capability\Registry\ElementReference; use Mcp\Capability\Registry\Loader\LoaderInterface; use Mcp\Capability\Registry\ReferenceHandlerInterface; +use Mcp\Capability\RegistryInterface; use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\LogicException; use Mcp\Schema\Content\TextContent; @@ -32,6 +33,8 @@ use Mcp\Server\Handler\Request\InitializeHandler; use Mcp\Server\Protocol; use Mcp\Server\Session\SessionInterface; use Mcp\Server\Stateless\StatelessProtocol; +use Mcp\Server\Subscription\InMemoryNotificationBus; +use Mcp\Server\Subscription\PublishingEventDispatcher; use Mcp\Tests\Unit\Server\Extension\ThingExtension; use Mcp\Tests\Unit\Server\Extension\ThingListHandler; use Mcp\Tests\Unit\Server\Extension\ThingListRequest; @@ -404,6 +407,42 @@ final class BuilderTest extends TestCase $this->fail('CallToolHandler not found in request handlers'); } + + public function testBuildingWithASuppliedRegistryDoesNotAnnounceTheLoadAsAChange(): void + { + // Otherwise every build publishes one list_changed per element, for no change. + $bus = new InMemoryNotificationBus(); + $registry = new Registry(new PublishingEventDispatcher($bus)); + + Server::builder() + ->setRegistry($registry) + ->setNotificationBus($bus) + ->addTool(static fn (): string => 'ok', 'alpha') + ->addTool(static fn (): string => 'ok', 'beta') + ->build(); + + $this->assertSame(0, $bus->cursor()); + $this->assertTrue($registry->hasTool('alpha')); + $this->assertTrue($registry->hasTool('beta')); + + // A real change after the build is still published. + $registry->unregisterTool('alpha'); + + $this->assertSame(1, $bus->cursor()); + } + + public function testAThirdPartyRegistryIsStillLoadedThroughThePlainLoader(): void + { + $registry = $this->createMock(RegistryInterface::class); + $registry->expects($this->once()) + ->method('registerTool') + ->with($this->callback(static fn (Tool $tool): bool => 'alpha' === $tool->name)); + + Server::builder() + ->setRegistry($registry) + ->addTool(static fn (): string => 'ok', 'alpha') + ->build(); + } } /** diff --git a/tests/Unit/Server/ProtocolTest.php b/tests/Unit/Server/ProtocolTest.php index 997c39d..175db74 100644 --- a/tests/Unit/Server/ProtocolTest.php +++ b/tests/Unit/Server/ProtocolTest.php @@ -25,7 +25,9 @@ use Mcp\Schema\Request\PingRequest; use Mcp\Server\Handler\Notification\NotificationHandlerInterface; use Mcp\Server\Handler\Request\RequestHandlerInterface; use Mcp\Server\Protocol; +use Mcp\Server\Session\InMemorySessionStore; use Mcp\Server\Session\SessionInterface; +use Mcp\Server\Session\SessionManager; use Mcp\Server\Session\SessionManagerInterface; use Mcp\Server\Transport\TransportInterface; use Mcp\Tests\Unit\Fixtures\ThrowingRequest; @@ -793,6 +795,63 @@ final class ProtocolTest extends TestCase $this->assertStringNotContainsString('Unexpected error', $message['error']['message']); } + #[TestDox('Failure while dispatching an outbound request is answered under the inbound request id')] + public function testOutboundRequestFailureIsAnsweredUnderInboundRequestId(): void + { + $handler = $this->createMock(RequestHandlerInterface::class); + $handler->method('supports')->willReturn(true); + $handler->method('handle')->willReturnCallback(static function (): Response { + // Suspend with an outbound, id-less request, as sampling/elicitation handlers do. + \Fiber::suspend(['type' => 'request', 'request' => new PingRequest(), 'timeout' => 5]); + + return new Response(1, []); + }); + + $exception = new \RuntimeException('Transport unavailable'); + $this->transport->method('attachFiberToSession')->willThrowException($exception); + + $sessionManager = new SessionManager(new InMemorySessionStore()); + + $events = []; + $dispatcher = $this->createMock(EventDispatcherInterface::class); + $dispatcher->method('dispatch')->willReturnCallback(static function (object $event) use (&$events): object { + $events[] = $event; + + return $event; + }); + + $protocol = new Protocol( + requestHandlers: [$handler], + notificationHandlers: [], + messageFactory: MessageFactory::make(), + sessionManager: $sessionManager, + eventDispatcher: $dispatcher, + ); + + $session = $sessionManager->create(); + $session->save(); + $sessionId = $session->getId(); + $protocol->processInput( + $this->transport, + '{"jsonrpc": "2.0", "id": 1, "method": "ping"}', + $sessionId + ); + + $errorEvents = array_values(array_filter($events, static fn (object $event): bool => $event instanceof ErrorEvent)); + $this->assertCount(1, $errorEvents); + $this->assertSame($exception, $errorEvents[0]->getThrowable()); + $this->assertSame(1, $errorEvents[0]->getError()->getId()); + + $outgoing = $protocol->consumeOutgoingMessages($sessionId); + $errors = array_values(array_filter( + array_map(static fn (array $outgoingMessage): array => json_decode($outgoingMessage['message'], true), $outgoing), + static fn (array $message): bool => isset($message['error']), + )); + $this->assertCount(1, $errors); + $this->assertSame(1, $errors[0]['id']); + $this->assertSame(Error::INTERNAL_ERROR, $errors[0]['error']['code']); + } + #[TestDox('Notification handler exceptions are caught and logged')] public function testNotificationHandlerExceptionsAreCaught(): void { diff --git a/tests/Unit/Server/Session/FileSessionStoreTest.php b/tests/Unit/Server/Session/FileSessionStoreTest.php index a2b8729..ed22e86 100644 --- a/tests/Unit/Server/Session/FileSessionStoreTest.php +++ b/tests/Unit/Server/Session/FileSessionStoreTest.php @@ -16,6 +16,8 @@ use Mcp\Exception\RuntimeException; use Mcp\Server\Session\FileSessionStore; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; +use Psr\Log\LogLevel; use Symfony\Component\Uid\UuidV4; class FileSessionStoreTest extends TestCase @@ -109,4 +111,121 @@ class FileSessionStoreTest extends TestCase throw $e; } } + + #[TestDox('logs a warning when a session file cannot be read')] + public function testUnreadableSessionFileLogsWarning(): void + { + $logger = new WarningCollectingLogger(); + $store = new FileSessionStore($this->directory, logger: $logger); + $id = new UuidV4(); + + $store->write($id, 'payload'); + + $path = $this->directory.\DIRECTORY_SEPARATOR.$id->toRfc4122(); + chmod($path, 0000); + clearstatcache(true, $path); + + if (is_readable($path)) { + $this->markTestSkipped('Permission bits do not restrict reads here (running as root, or a filesystem that ignores them).'); + } + + $this->assertFalse($store->read($id)); + $this->assertCount(1, $logger->warnings); + $this->assertSame('Failed to read session file.', $logger->warnings[0]['message']); + $this->assertSame($path, $logger->warnings[0]['context']['path']); + } + + #[TestDox('logs a warning when a session file cannot be written')] + public function testUnwritableSessionFileLogsWarning(): void + { + $logger = new WarningCollectingLogger(); + $store = new FileSessionStore($this->directory, logger: $logger); + + chmod($this->directory, 0555); + clearstatcache(true, $this->directory); + + if (is_writable($this->directory)) { + $this->markTestSkipped('Permission bits do not restrict writes here (running as root, or a filesystem that ignores them).'); + } + + $this->assertFalse($store->write(new UuidV4(), 'payload')); + $this->assertCount(1, $logger->warnings); + $this->assertSame('Failed to write session file.', $logger->warnings[0]['message']); + } + + #[TestDox('logs a warning when the session directory cannot be opened for garbage collection')] + public function testGcLogsWarningWhenDirectoryVanished(): void + { + $logger = new WarningCollectingLogger(); + $store = new FileSessionStore($this->directory, logger: $logger); + + rmdir($this->directory); + + $this->assertSame([], $store->gc()); + $this->assertCount(1, $logger->warnings); + $this->assertSame('Failed to open session directory for garbage collection.', $logger->warnings[0]['message']); + $this->assertSame($this->directory, $logger->warnings[0]['context']['directory']); + } + + #[TestDox('gc() warns and does not report a session whose file survived deletion')] + public function testGcLogsWarningWhenExpiredFileCannotBeDeleted(): void + { + $logger = new WarningCollectingLogger(); + $store = new FileSessionStore($this->directory, ttl: 60, logger: $logger); + $id = new UuidV4(); + $store->write($id, 'payload'); + + $path = $this->directory.\DIRECTORY_SEPARATOR.$id->toRfc4122(); + touch($path, time() - 120); + + chmod($this->directory, 0555); + clearstatcache(true, $this->directory); + + if (is_writable($this->directory)) { + $this->markTestSkipped('Permission bits do not restrict writes here (running as root, or a filesystem that ignores them).'); + } + + // The file is still there, so its id must not be reported as deleted. + $this->assertSame([], $store->gc()); + $this->assertFileExists($path); + $this->assertCount(1, $logger->warnings); + $this->assertSame('Failed to delete expired session file.', $logger->warnings[0]['message']); + $this->assertSame($path, $logger->warnings[0]['context']['path']); + } + + #[TestDox('stays silent on the happy path')] + public function testHappyPathLogsNothing(): void + { + $logger = new WarningCollectingLogger(); + $store = new FileSessionStore($this->directory, logger: $logger); + $id = new UuidV4(); + + $store->write($id, 'payload'); + $store->read($id); + $store->destroy($id); + $store->gc(); + + $this->assertSame([], $logger->warnings); + } +} + +/** + * Keeps every warning with its context, so a silent failure can be told apart + * from one the operator was told about. + */ +final class WarningCollectingLogger extends AbstractLogger +{ + /** @var list}> */ + public array $warnings = []; + + /** + * @param string|\Stringable $message + * @param array $context + */ + public function log($level, $message, array $context = []): void + { + if (LogLevel::WARNING === $level) { + $this->warnings[] = ['message' => (string) $message, 'context' => $context]; + } + } } diff --git a/tests/Unit/Server/Session/SessionTest.php b/tests/Unit/Server/Session/SessionTest.php index 40483d3..a6b1697 100644 --- a/tests/Unit/Server/Session/SessionTest.php +++ b/tests/Unit/Server/Session/SessionTest.php @@ -245,6 +245,21 @@ class SessionTest extends TestCase $this->assertFalse($this->session->has('nonexistent')); } + public function testForgetDoesNotCreateIntermediateSegments(): void + { + $this->session->forget('a.b'); + + $this->assertSame([], $this->session->all()); + } + + public function testForgetDoesNotOverwriteNonArrayIntermediateValue(): void + { + $this->session->set('key', 'string_value'); + $this->session->forget('key.nested'); + + $this->assertSame('string_value', $this->session->get('key')); + } + public function testSessionCanStoreVariousDataTypes(): void { $this->session->set('string', 'value'); diff --git a/tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php b/tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php index 29c86d6..fd86d02 100644 --- a/tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php +++ b/tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php @@ -293,6 +293,88 @@ class OidcDiscoveryTest extends TestCase $this->assertSame('https://auth.example.com/.well-known/openid-configuration', $requestedUrls[1]); } + #[TestDox('issuer with path and trailing slash is discovered and matched verbatim')] + public function testIssuerWithPathAndTrailingSlash(): void + { + $this->skipIfPsrHttpClientIsMissing(); + + $factory = new Psr17Factory(); + $requestedUrls = []; + $issuer = 'https://auth.example.com/application/o/mcp/'; + $validMetadata = [ + 'issuer' => $issuer, + 'authorization_endpoint' => 'https://auth.example.com/application/o/authorize/', + 'token_endpoint' => 'https://auth.example.com/application/o/token/', + 'jwks_uri' => 'https://auth.example.com/application/o/mcp/jwks/', + 'code_challenge_methods_supported' => ['S256'], + ]; + + $httpClient = $this->createMock(ClientInterface::class); + $httpClient->expects($this->exactly(3)) + ->method('sendRequest') + ->willReturnCallback(static function (RequestInterface $request) use ($factory, &$requestedUrls, $validMetadata): ResponseInterface { + $requestedUrls[] = (string) $request->getUri(); + + if (3 !== \count($requestedUrls)) { + return $factory->createResponse(404); + } + + return $factory->createResponse(200)->withBody( + $factory->createStream(json_encode($validMetadata, \JSON_THROW_ON_ERROR)), + ); + }); + + $discovery = new OidcDiscovery( + httpClient: $httpClient, + requestFactory: $factory, + ); + + $metadata = $discovery->discover($issuer); + + $this->assertSame($issuer, $metadata['issuer']); + $this->assertSame('https://auth.example.com/.well-known/oauth-authorization-server/application/o/mcp', $requestedUrls[0]); + $this->assertSame('https://auth.example.com/.well-known/openid-configuration/application/o/mcp', $requestedUrls[1]); + $this->assertSame('https://auth.example.com/application/o/mcp/.well-known/openid-configuration', $requestedUrls[2]); + } + + #[TestDox('issuer without path but with trailing slash is discovered and matched verbatim')] + public function testIssuerWithoutPathWithTrailingSlash(): void + { + $this->skipIfPsrHttpClientIsMissing(); + + $factory = new Psr17Factory(); + $requestedUrls = []; + $issuer = 'https://auth.example.com/'; + $validMetadata = [ + 'issuer' => $issuer, + 'authorization_endpoint' => 'https://auth.example.com/authorize', + 'token_endpoint' => 'https://auth.example.com/oauth/token', + 'jwks_uri' => 'https://auth.example.com/.well-known/jwks.json', + 'code_challenge_methods_supported' => ['S256'], + ]; + + $httpClient = $this->createMock(ClientInterface::class); + $httpClient->expects($this->once()) + ->method('sendRequest') + ->willReturnCallback(static function (RequestInterface $request) use ($factory, &$requestedUrls, $validMetadata): ResponseInterface { + $requestedUrls[] = (string) $request->getUri(); + + return $factory->createResponse(200)->withBody( + $factory->createStream(json_encode($validMetadata, \JSON_THROW_ON_ERROR)), + ); + }); + + $discovery = new OidcDiscovery( + httpClient: $httpClient, + requestFactory: $factory, + ); + + $metadata = $discovery->discover($issuer); + + $this->assertSame($issuer, $metadata['issuer']); + $this->assertSame(['https://auth.example.com/.well-known/oauth-authorization-server'], $requestedUrls); + } + private function skipIfPsrHttpClientIsMissing(): void { if (!interface_exists(ClientInterface::class)) { diff --git a/tests/Unit/Server/Transport/StreamableHttpTransportTest.php b/tests/Unit/Server/Transport/StreamableHttpTransportTest.php index 932f5c7..186432f 100644 --- a/tests/Unit/Server/Transport/StreamableHttpTransportTest.php +++ b/tests/Unit/Server/Transport/StreamableHttpTransportTest.php @@ -12,19 +12,23 @@ namespace Mcp\Tests\Unit\Server\Transport; use Mcp\Exception\InvalidArgumentException; +use Mcp\Schema\JsonRpc\Error; use Mcp\Server\Transport\Http\Middleware\CorsMiddleware; use Mcp\Server\Transport\Http\Middleware\DnsRebindingProtectionMiddleware; use Mcp\Server\Transport\Http\Middleware\ProtocolVersionMiddleware; use Mcp\Server\Transport\StreamableHttpTransport; +use Mcp\Server\Transport\TransportInterface; use Nyholm\Psr7\Factory\Psr17Factory; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; +use Psr\Clock\ClockInterface; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; +use Symfony\Component\Uid\Uuid; final class StreamableHttpTransportTest extends TestCase { @@ -347,6 +351,92 @@ final class StreamableHttpTransportTest extends TestCase new StreamableHttpTransport($request, $this->factory, $this->factory, null, [], maxBodyBytes: 0); } + #[TestDox('an immediate response is consumed once and not replayed on a later POST')] + public function testImmediateResponseIsNotReplayedOnSecondPost(): void + { + $request = $this->factory + ->createServerRequest('POST', 'http://localhost/') + ->withHeader('Host', 'localhost') + ->withBody($this->factory->createStream('{"jsonrpc":"2.0","id":1,"method":"ping"}')); + + $transport = new StreamableHttpTransport($request, $this->factory, $this->factory); + + $calls = 0; + $transport->onMessage(static function (TransportInterface $transport, string $payload) use (&$calls): void { + if (1 === ++$calls) { + $transport->send('{"jsonrpc":"2.0","id":1,"result":{}}', ['status_code' => 200]); + } + }); + + $first = $transport->listen(); + + $this->assertSame(200, $first->getStatusCode()); + $this->assertSame('{"jsonrpc":"2.0","id":1,"result":{}}', (string) $first->getBody()); + + $second = $transport->listen(); + + $this->assertSame(2, $calls); + $this->assertSame(202, $second->getStatusCode()); + $this->assertSame('', (string) $second->getBody()); + } + + #[TestDox('the polling loop times out a pending request via the injected clock')] + public function testPollingLoopTimesOutPendingRequestViaInjectedClock(): void + { + $request = $this->factory + ->createServerRequest('POST', 'http://localhost/') + ->withHeader('Host', 'localhost') + ->withBody($this->factory->createStream('{"jsonrpc":"2.0","id":1,"method":"ping"}')); + + $requestedAt = 1_000_000; + + // Frozen 121s after the pending request was issued — past its 120s timeout. + $clock = new class($requestedAt + 121) implements ClockInterface { + public function __construct(private readonly int $timestamp) + { + } + + public function now(): \DateTimeImmutable + { + return (new \DateTimeImmutable())->setTimestamp($this->timestamp); + } + }; + + $transport = new StreamableHttpTransport( + $request, + $this->factory, + $this->factory, + clock: $clock, + ); + + $received = null; + $fiber = new \Fiber(static function () use (&$received) { + $received = \Fiber::suspend(); + + return null; + }); + $fiber->start(); + + $transport->onMessage(static function (TransportInterface $transport) use ($fiber): void { + $transport->attachFiberToSession($fiber, Uuid::v4()); + }); + $transport->setOutgoingMessagesProvider(static fn (): array => []); + $transport->setResponseFinder(static fn () => null); + $transport->setPendingRequestsProvider(static fn (): array => [ + ['request_id' => 1, 'timestamp' => $requestedAt, 'timeout' => 120], + ]); + + $response = $transport->listen(); + + $this->assertSame('text/event-stream', $response->getHeaderLine('Content-Type')); + + $this->expectOutputString(''); + $response->getBody()->getContents(); + + $this->assertTrue($fiber->isTerminated()); + $this->assertInstanceOf(Error::class, $received); + } + private function stubAuth401(): MiddlewareInterface { return new class($this->factory) implements MiddlewareInterface {