From 538fb7e248d549d4edc2b9be6b553a30ee7e2736 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 19 Aug 2026 03:45:30 +0200 Subject: [PATCH 1/2] [Server] Make resource subscribe/unsubscribe errors protocol-version-aware (SEP-2164) ReadResourceHandler already picked -32602 vs -32002 based on the negotiated revision; ResourceSubscribeHandler and ResourceUnsubscribeHandler still hard-coded the retired -32002 for every client. Bring them in line. --- .../Request/ResourceSubscribeHandler.php | 8 +- .../Request/ResourceUnsubscribeHandler.php | 8 +- .../Request/ResourceSubscribeHandlerTest.php | 127 ++++++++++++++++++ .../ResourceUnsubscribeHandlerTest.php | 127 ++++++++++++++++++ 4 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 tests/Unit/Server/Handler/Request/ResourceSubscribeHandlerTest.php create mode 100644 tests/Unit/Server/Handler/Request/ResourceUnsubscribeHandlerTest.php diff --git a/src/Server/Handler/Request/ResourceSubscribeHandler.php b/src/Server/Handler/Request/ResourceSubscribeHandler.php index b5421f87..9e9ec50f 100644 --- a/src/Server/Handler/Request/ResourceSubscribeHandler.php +++ b/src/Server/Handler/Request/ResourceSubscribeHandler.php @@ -18,6 +18,7 @@ use Mcp\Schema\JsonRpc\Response; use Mcp\Schema\Request\ResourceSubscribeRequest; use Mcp\Schema\Result\EmptyResult; +use Mcp\Server\RequestContext; use Mcp\Server\Resource\SubscriptionManagerInterface; use Mcp\Server\Session\SessionInterface; use Psr\Log\LoggerInterface; @@ -57,7 +58,12 @@ public function handle(Request $request, SessionInterface $session): Response|Er } catch (ResourceNotFoundException $e) { $this->logger->error('Resource not found', ['uri' => $uri, 'exception' => $e]); - return Error::forResourceNotFound($e->getMessage(), $request->getId()); + // SEP-2164 retired -32002 in favour of the JSON-RPC code that + // already meant this. Older peers still expect the old one, so the + // revision answering the request decides. + return (new RequestContext($session, $request))->getProtocolVersion()->usesInvalidParamsForResourceNotFound() + ? Error::forInvalidParams($e->getMessage(), $request->getId(), ['uri' => $uri]) + : Error::forResourceNotFound($e->getMessage(), $request->getId()); } $this->logger->debug('Subscribing to resource', ['uri' => $uri]); diff --git a/src/Server/Handler/Request/ResourceUnsubscribeHandler.php b/src/Server/Handler/Request/ResourceUnsubscribeHandler.php index 50ab8bc1..6c3f3961 100644 --- a/src/Server/Handler/Request/ResourceUnsubscribeHandler.php +++ b/src/Server/Handler/Request/ResourceUnsubscribeHandler.php @@ -18,6 +18,7 @@ use Mcp\Schema\JsonRpc\Response; use Mcp\Schema\Request\ResourceUnsubscribeRequest; use Mcp\Schema\Result\EmptyResult; +use Mcp\Server\RequestContext; use Mcp\Server\Resource\SubscriptionManagerInterface; use Mcp\Server\Session\SessionInterface; use Psr\Log\LoggerInterface; @@ -57,7 +58,12 @@ public function handle(Request $request, SessionInterface $session): Response|Er } catch (ResourceNotFoundException $e) { $this->logger->error('Resource not found', ['uri' => $uri, 'exception' => $e]); - return Error::forResourceNotFound($e->getMessage(), $request->getId()); + // SEP-2164 retired -32002 in favour of the JSON-RPC code that + // already meant this. Older peers still expect the old one, so the + // revision answering the request decides. + return (new RequestContext($session, $request))->getProtocolVersion()->usesInvalidParamsForResourceNotFound() + ? Error::forInvalidParams($e->getMessage(), $request->getId(), ['uri' => $uri]) + : Error::forResourceNotFound($e->getMessage(), $request->getId()); } $this->logger->debug('Unsubscribing from resource', ['uri' => $uri]); diff --git a/tests/Unit/Server/Handler/Request/ResourceSubscribeHandlerTest.php b/tests/Unit/Server/Handler/Request/ResourceSubscribeHandlerTest.php new file mode 100644 index 00000000..d16519ec --- /dev/null +++ b/tests/Unit/Server/Handler/Request/ResourceSubscribeHandlerTest.php @@ -0,0 +1,127 @@ +registry = $this->createMock(RegistryInterface::class); + $this->subscriptionManager = $this->createMock(SubscriptionManagerInterface::class); + $this->session = $this->createMock(SessionInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->handler = new ResourceSubscribeHandler($this->registry, $this->subscriptionManager, $this->logger); + } + + public function testSupportsResourceSubscribeRequest(): void + { + $request = $this->createRequest('file://test.txt'); + + $this->assertTrue($this->handler->supports($request)); + } + + public function testHandleSuccessfulSubscribe(): void + { + $uri = 'file://test.txt'; + $request = $this->createRequest($uri); + + $this->registry + ->expects($this->once()) + ->method('getResource') + ->with($uri) + ->willReturn($this->createMock(\Mcp\Capability\Registry\ResourceReference::class)); + + $this->subscriptionManager + ->expects($this->once()) + ->method('subscribe') + ->with($this->session, $uri); + + $response = $this->handler->handle($request, $this->session); + + $this->assertInstanceOf(Response::class, $response); + $this->assertEquals($request->getId(), $response->id); + $this->assertInstanceOf(EmptyResult::class, $response->result); + } + + /** + * @dataProvider provideResourceNotFoundRevisions + */ + public function testHandleResourceNotFoundIsProtocolVersionAware(?string $negotiated, int $expectedCode): void + { + $uri = 'file://nonexistent/file.txt'; + $request = $this->createRequest($uri); + $exception = new ResourceNotFoundException($uri); + + $this->registry + ->expects($this->once()) + ->method('getResource') + ->with($uri) + ->willThrowException($exception); + + $this->session + ->method('get') + ->with('protocol_version') + ->willReturn($negotiated); + + $this->subscriptionManager + ->expects($this->never()) + ->method('subscribe'); + + $response = $this->handler->handle($request, $this->session); + + $this->assertInstanceOf(Error::class, $response); + $this->assertEquals($request->getId(), $response->id); + $this->assertEquals($expectedCode, $response->code); + } + + /** + * @return iterable + */ + public static function provideResourceNotFoundRevisions(): iterable + { + yield 'no negotiated revision' => [null, Error::RESOURCE_NOT_FOUND]; + yield '2025-11-25' => ['2025-11-25', Error::RESOURCE_NOT_FOUND]; + yield '2026-07-28' => ['2026-07-28', Error::INVALID_PARAMS]; + } + + private function createRequest(string $uri): ResourceSubscribeRequest + { + return ResourceSubscribeRequest::fromArray([ + 'jsonrpc' => '2.0', + 'method' => ResourceSubscribeRequest::getMethod(), + 'id' => 'test-request-'.uniqid(), + 'params' => [ + 'uri' => $uri, + ], + ]); + } +} diff --git a/tests/Unit/Server/Handler/Request/ResourceUnsubscribeHandlerTest.php b/tests/Unit/Server/Handler/Request/ResourceUnsubscribeHandlerTest.php new file mode 100644 index 00000000..daa82f52 --- /dev/null +++ b/tests/Unit/Server/Handler/Request/ResourceUnsubscribeHandlerTest.php @@ -0,0 +1,127 @@ +registry = $this->createMock(RegistryInterface::class); + $this->subscriptionManager = $this->createMock(SubscriptionManagerInterface::class); + $this->session = $this->createMock(SessionInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->handler = new ResourceUnsubscribeHandler($this->registry, $this->subscriptionManager, $this->logger); + } + + public function testSupportsResourceUnsubscribeRequest(): void + { + $request = $this->createRequest('file://test.txt'); + + $this->assertTrue($this->handler->supports($request)); + } + + public function testHandleSuccessfulUnsubscribe(): void + { + $uri = 'file://test.txt'; + $request = $this->createRequest($uri); + + $this->registry + ->expects($this->once()) + ->method('getResource') + ->with($uri) + ->willReturn($this->createMock(\Mcp\Capability\Registry\ResourceReference::class)); + + $this->subscriptionManager + ->expects($this->once()) + ->method('unsubscribe') + ->with($this->session, $uri); + + $response = $this->handler->handle($request, $this->session); + + $this->assertInstanceOf(Response::class, $response); + $this->assertEquals($request->getId(), $response->id); + $this->assertInstanceOf(EmptyResult::class, $response->result); + } + + /** + * @dataProvider provideResourceNotFoundRevisions + */ + public function testHandleResourceNotFoundIsProtocolVersionAware(?string $negotiated, int $expectedCode): void + { + $uri = 'file://nonexistent/file.txt'; + $request = $this->createRequest($uri); + $exception = new ResourceNotFoundException($uri); + + $this->registry + ->expects($this->once()) + ->method('getResource') + ->with($uri) + ->willThrowException($exception); + + $this->session + ->method('get') + ->with('protocol_version') + ->willReturn($negotiated); + + $this->subscriptionManager + ->expects($this->never()) + ->method('unsubscribe'); + + $response = $this->handler->handle($request, $this->session); + + $this->assertInstanceOf(Error::class, $response); + $this->assertEquals($request->getId(), $response->id); + $this->assertEquals($expectedCode, $response->code); + } + + /** + * @return iterable + */ + public static function provideResourceNotFoundRevisions(): iterable + { + yield 'no negotiated revision' => [null, Error::RESOURCE_NOT_FOUND]; + yield '2025-11-25' => ['2025-11-25', Error::RESOURCE_NOT_FOUND]; + yield '2026-07-28' => ['2026-07-28', Error::INVALID_PARAMS]; + } + + private function createRequest(string $uri): ResourceUnsubscribeRequest + { + return ResourceUnsubscribeRequest::fromArray([ + 'jsonrpc' => '2.0', + 'method' => ResourceUnsubscribeRequest::getMethod(), + 'id' => 'test-request-'.uniqid(), + 'params' => [ + 'uri' => $uri, + ], + ]); + } +} From eec45c32fffd1dcecabd86a64fdd44014f1a5b59 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sat, 22 Aug 2026 02:44:09 +0200 Subject: [PATCH 2/2] Remove unnecessary comments Co-authored-by: Christopher Hertel --- src/Server/Handler/Request/ResourceSubscribeHandler.php | 3 --- src/Server/Handler/Request/ResourceUnsubscribeHandler.php | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/Server/Handler/Request/ResourceSubscribeHandler.php b/src/Server/Handler/Request/ResourceSubscribeHandler.php index 9e9ec50f..1e8147b0 100644 --- a/src/Server/Handler/Request/ResourceSubscribeHandler.php +++ b/src/Server/Handler/Request/ResourceSubscribeHandler.php @@ -58,9 +58,6 @@ public function handle(Request $request, SessionInterface $session): Response|Er } catch (ResourceNotFoundException $e) { $this->logger->error('Resource not found', ['uri' => $uri, 'exception' => $e]); - // SEP-2164 retired -32002 in favour of the JSON-RPC code that - // already meant this. Older peers still expect the old one, so the - // revision answering the request decides. return (new RequestContext($session, $request))->getProtocolVersion()->usesInvalidParamsForResourceNotFound() ? Error::forInvalidParams($e->getMessage(), $request->getId(), ['uri' => $uri]) : Error::forResourceNotFound($e->getMessage(), $request->getId()); diff --git a/src/Server/Handler/Request/ResourceUnsubscribeHandler.php b/src/Server/Handler/Request/ResourceUnsubscribeHandler.php index 6c3f3961..522c5128 100644 --- a/src/Server/Handler/Request/ResourceUnsubscribeHandler.php +++ b/src/Server/Handler/Request/ResourceUnsubscribeHandler.php @@ -58,9 +58,6 @@ public function handle(Request $request, SessionInterface $session): Response|Er } catch (ResourceNotFoundException $e) { $this->logger->error('Resource not found', ['uri' => $uri, 'exception' => $e]); - // SEP-2164 retired -32002 in favour of the JSON-RPC code that - // already meant this. Older peers still expect the old one, so the - // revision answering the request decides. return (new RequestContext($session, $request))->getProtocolVersion()->usesInvalidParamsForResourceNotFound() ? Error::forInvalidParams($e->getMessage(), $request->getId(), ['uri' => $uri]) : Error::forResourceNotFound($e->getMessage(), $request->getId());