From 3162b8867d4d29caaaaad054b10ebadc685a2407 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sat, 10 Oct 2026 03:55:36 +0200 Subject: [PATCH 1/2] [Client] Fix connecting to a server with an empty version Accept an empty `serverInfo.version`, and close the transport when initialization fails on a malformed result, wrapped in a `ConnectionException`. --- CHANGELOG.md | 1 + src/Client.php | 9 +++++++- src/Schema/Implementation.php | 3 ++- tests/Unit/ClientTest.php | 28 ++++++++++++++++++++++-- tests/Unit/Schema/ImplementationTest.php | 11 ++++++---- 5 files changed, 44 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e34852a92..dcd5a3670 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * [BC Break] Reject a `Tool` input schema whose `properties` is not an object or whose `required` is neither a list nor `null`, instead of silently replacing the member. Reject a `completion/complete` whose `argument` is missing `name` or `value`, instead of completing against an empty prefix. * 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). +* Fix `Client::connect()` rejecting an empty `serverInfo.version`, as the Python SDK sends, and leaving the transport open when initialization fails on a malformed result: the transport is now closed and the failure wrapped in a `ConnectionException`. * Fix stateless SSE streams holding back frames until close when PHP output buffering is enabled. * Reject a recognized `Mcp-Param-*` header whose mirrored argument is absent from the body with `-32020`, instead of accepting the request (SEP-2243). * Fix `RequestEvent`, `ResponseEvent` and `ErrorEvent` not being dispatched for `2026-07-28` requests. diff --git a/src/Client.php b/src/Client.php index 02ec67e4a..c714a01ce 100644 --- a/src/Client.php +++ b/src/Client.php @@ -17,6 +17,7 @@ use Mcp\Client\Protocol; use Mcp\Client\Transport\TransportInterface; use Mcp\Exception\ConnectionException; +use Mcp\Exception\ExceptionInterface; use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\RequestCancelledException; use Mcp\Exception\RequestException; @@ -103,13 +104,19 @@ public function connect(TransportInterface $transport): void $this->logger->info('Client connected and initialized', ['attempt' => $attempt]); return; - } catch (ConnectionException $e) { + } catch (ExceptionInterface $e) { // initialize() flags the session before sending the initialized // notification, so a failure in between leaves the flag set. $this->protocol->getState()->setInitialized(false); $transport->close(); + // Anything else, like a malformed initialize result, is not + // transient and would fail every retry the same way. + if (!$e instanceof ConnectionException) { + throw new ConnectionException('Initialization failed: '.$e->getMessage(), 0, $e); + } + if ($attempt === $maxAttempts) { throw $e; } diff --git a/src/Schema/Implementation.php b/src/Schema/Implementation.php index 2cbeb4d3b..eed94f2e2 100644 --- a/src/Schema/Implementation.php +++ b/src/Schema/Implementation.php @@ -51,7 +51,8 @@ public static function fromArray(array $data): self if (!isset($data['name']) || !\is_string($data['name']) || '' === $data['name']) { throw new InvalidArgumentException('Invalid or missing "name" in Implementation data.'); } - if (!isset($data['version']) || !\is_string($data['version']) || '' === $data['version']) { + // Only typed as a string, and the Python SDK sends "" when none is configured. + if (!isset($data['version']) || !\is_string($data['version'])) { throw new InvalidArgumentException('Invalid or missing "version" in Implementation data.'); } diff --git a/tests/Unit/ClientTest.php b/tests/Unit/ClientTest.php index 424614908..58bce5fba 100644 --- a/tests/Unit/ClientTest.php +++ b/tests/Unit/ClientTest.php @@ -157,6 +157,25 @@ public function testFailureAfterHandshakeResultDoesNotLeaveClientConnected(): vo $this->assertFalse($client->isConnected()); } + #[TestDox('a malformed handshake result fails with a ConnectionException and closes the transport')] + public function testMalformedHandshakeResultClosesTheTransport(): void + { + $transport = new FakeTransport([FakeTransport::MALFORMED]); + + $client = Client::builder()->setMaxRetries(2)->build(); + + try { + $client->connect($transport); + $this->fail(\sprintf('Expected a "%s" to be thrown.', ConnectionException::class)); + } catch (ConnectionException $e) { + $this->assertInstanceOf(InvalidArgumentException::class, $e->getPrevious()); + } + + $this->assertSame(1, $transport->connectCalls, 'a malformed result is not retried'); + $this->assertSame(1, $transport->closeCalls); + $this->assertFalse($client->isConnected()); + } + #[TestDox('a timed out attempt does not leave state behind that fails the retry')] public function testTimedOutAttemptDoesNotPoisonTheRetry(): void { @@ -197,6 +216,9 @@ final class FakeTransport extends BaseTransport /** The initialize request is answered, but the connection breaks right after. */ public const BREAK_AFTER_ACCEPT = 'break_after_accept'; + /** The initialize request is answered with a result lacking `serverInfo.version`. */ + public const MALFORMED = 'malformed'; + public int $connectCalls = 0; public int $closeCalls = 0; @@ -206,7 +228,7 @@ final class FakeTransport extends BaseTransport private array $outbox = []; /** - * @param list $attempts How each successive connect() call behaves + * @param list $attempts How each successive connect() call behaves */ public function __construct(private array $attempts = [self::ACCEPT]) { @@ -256,7 +278,9 @@ public function send(string $data): void : ['result' => [ 'protocolVersion' => ProtocolVersion::V2025_11_25->value, 'capabilities' => [], - 'serverInfo' => ['name' => 'Test Server', 'version' => '1.0.0'], + 'serverInfo' => self::MALFORMED === $this->outcome + ? ['name' => 'Test Server'] + : ['name' => 'Test Server', 'version' => '1.0.0'], ]]; $this->outbox[] = json_encode(['jsonrpc' => '2.0', 'id' => $message['id']] + $answer, \JSON_THROW_ON_ERROR); diff --git a/tests/Unit/Schema/ImplementationTest.php b/tests/Unit/Schema/ImplementationTest.php index 0da76c4a6..3667cd57e 100644 --- a/tests/Unit/Schema/ImplementationTest.php +++ b/tests/Unit/Schema/ImplementationTest.php @@ -120,12 +120,15 @@ public function testFromArrayThrowsOnMissingVersion(): void Implementation::fromArray(['name' => 'my-client']); } - public function testFromArrayThrowsOnEmptyVersion(): void + /** + * The spec types the version as a plain string, and the Python SDK sends an + * empty one when the server has none configured. + */ + public function testFromArrayAcceptsEmptyVersion(): void { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Invalid or missing "version" in Implementation data.'); + $implementation = Implementation::fromArray(['name' => 'my-server', 'version' => '']); - Implementation::fromArray(['name' => 'my-client', 'version' => '']); + $this->assertSame('', $implementation->version); } public function testFromArrayThrowsOnNonStringVersion(): void From 2569a22f394b541697231979cdf487eb4e677874 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 13 Oct 2026 00:26:50 +0200 Subject: [PATCH 2/2] Apply suggestion from @chr-hertel --- src/Schema/Implementation.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Schema/Implementation.php b/src/Schema/Implementation.php index eed94f2e2..c8b755234 100644 --- a/src/Schema/Implementation.php +++ b/src/Schema/Implementation.php @@ -51,7 +51,6 @@ public static function fromArray(array $data): self if (!isset($data['name']) || !\is_string($data['name']) || '' === $data['name']) { throw new InvalidArgumentException('Invalid or missing "name" in Implementation data.'); } - // Only typed as a string, and the Python SDK sends "" when none is configured. if (!isset($data['version']) || !\is_string($data['version'])) { throw new InvalidArgumentException('Invalid or missing "version" in Implementation data.'); }