diff --git a/CHANGELOG.md b/CHANGELOG.md index eb0b7062..452ae291 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to `mcp/sdk` will be documented in this file. +0.11.0 +------ + +### Server + +* Handle `notifications/cancelled` on handshake-era servers by dropping the suspended handler fiber (for example one waiting on elicitation or sampling) and not answering the cancelled request. Unknown, already-finished and un-cancellable ids are ignored. 2026-07-28 stdio cancellation of `subscriptions/listen` is unchanged. +* [BC Break] Add `TransportInterface::setCancellationChecker()`. Transports that extend `BaseTransport` get it automatically; a custom implementor must add the setter and drop a cancelled fiber from the polling loop without sending a response. + 0.10.0 ------ diff --git a/src/Server/Protocol.php b/src/Server/Protocol.php index 90c40e81..26b7bd68 100644 --- a/src/Server/Protocol.php +++ b/src/Server/Protocol.php @@ -19,6 +19,7 @@ use Mcp\Schema\JsonRpc\Response; use Mcp\Schema\JsonRpc\ResultInterface; use Mcp\Schema\MessageFactory; +use Mcp\Schema\Notification\CancelledNotification; use Mcp\Schema\Request\InitializeRequest; use Mcp\Schema\Request\ResourceSubscribeRequest; use Mcp\Schema\Request\ResourceUnsubscribeRequest; @@ -88,6 +89,12 @@ class Protocol /** Session key for the fingerprint of the principal that opened the session, null when it was opened without one */ private const SESSION_PRINCIPAL = '_mcp.principal'; + /** Session key for handshake-era requests whose handler is suspended in a fiber of some worker */ + private const SESSION_IN_FLIGHT_REQUESTS = '_mcp.in_flight_requests'; + + /** Session key for in-flight requests a notifications/cancelled named, so the worker holding the fiber can drop it */ + private const SESSION_CANCELLED_REQUESTS = '_mcp.cancelled_requests'; + /** Requests whose effect has to outlast them, which a server without sessions cannot keep. */ private const SESSION_SCOPED_REQUESTS = [ ResourceSubscribeRequest::class, @@ -134,6 +141,20 @@ class Protocol */ private array $droppedAwaitedRequestIds = []; + /** + * The incoming request id of each suspended handler fiber, so a notifications/cancelled can find it. + * + * @var \WeakMap + */ + private \WeakMap $incomingRequestIds; + + /** + * Incoming request keys cancelled in this process, so a polling loop does not need to reload the session. + * + * @var array + */ + private array $cancelledRequestKeys = []; + private readonly ?SessionLockInterface $sessionLock; /** @@ -158,6 +179,7 @@ public function __construct( ) { $this->sessionLock = null !== $sessionLock && null !== $fiberScheduler ? new EventLoopSessionLock($sessionLock, $fiberScheduler) : $sessionLock; $this->awaitedRequestIds = new \WeakMap(); + $this->incomingRequestIds = new \WeakMap(); $this->fiberOutgoingMessages = new \WeakMap(); } @@ -188,6 +210,8 @@ public function connect(TransportInterface $transport): void $transport->setPendingRequestsProvider(fn (Uuid $sessionId, \Fiber $fiber): array => $this->getAwaitedPendingRequests($fiber, $sessionId)); + $transport->setCancellationChecker(fn (Uuid $sessionId, \Fiber $fiber): bool => $this->isHandlerCancelled($fiber, $sessionId)); + $transport->setResponseFinder($this->checkResponse(...)); $transport->setFiberYieldHandler(function (mixed $yieldedValue, ?Uuid $sessionId, \Fiber $fiber) use ($transportRef): void { @@ -462,6 +486,7 @@ private function handleRequest(TransportInterface $transport, Request $request, $dispatchResult = $this->dispatchResult(...); $errorForThrowable = $this->errorForThrowable(...); $saveResumedSession = $this->saveResumedSession(...); + $clearIncomingRequest = $this->clearIncomingRequest(...); // Weak: the fiber's state keeping this protocol alive segfaults PHP 8.1 when both are freed. $protocol = \WeakReference::create($this); /** @var bool $suspended set by reference once the fiber first suspended */ @@ -471,7 +496,7 @@ private function handleRequest(TransportInterface $transport, Request $request, // re-enters inside it each round rather than needing a new one. // Handled in the fiber, as the transport resumes it outside the catch below. /** @var McpFiber $fiber */ - $fiber = HandlerFibers::create(static function () use ($handler, $request, $session, $shim, $codec, $dispatchResult, $errorForThrowable, $saveResumedSession, &$suspended): Response|Error { + $fiber = HandlerFibers::create(static function () use ($handler, $request, $session, $shim, $codec, $dispatchResult, $errorForThrowable, $saveResumedSession, $clearIncomingRequest, &$suspended): Response|Error { try { $result = $handler->handle($request, $session); $result = $shim?->fulfill($result, $handler, $request, $session, $codec) ?? $result; @@ -492,6 +517,7 @@ private function handleRequest(TransportInterface $transport, Request $request, // A fiber that never suspended is saved by doProcessInput(); after a resume nothing else saves it. if ($suspended) { $saveResumedSession($session); + $clearIncomingRequest($request->getId(), $session); } return $result; @@ -518,7 +544,11 @@ private function handleRequest(TransportInterface $transport, Request $request, // The transport resumes the fiber from what the session holds: it must // not get the fiber if the request the fiber awaits was not stored. A // later save must not store that request either, nobody would resume it. + // The in-flight mark must be in that save, so another worker can honour a + // notifications/cancelled for this request. if (!$session instanceof EphemeralSessionInterface) { + $this->markIncomingRequestInFlight($request->getId(), $session); + try { $saved = $session->save(); } catch (\Throwable $e) { @@ -536,6 +566,7 @@ private function handleRequest(TransportInterface $transport, Request $request, $suspended = true; + $this->incomingRequestIds[$fiber] = $request->getId(); $this->trackAwaitedRequest($fiber, $awaitedRequestId); $transport->attachFiberToSession($fiber, $session->getId()); @@ -603,6 +634,10 @@ private function handleNotification(Notification $notification, SessionInterface $event = $this->dispatchEvent(new NotificationEvent($notification, $session)); $notification = $event->getNotification(); + if ($notification instanceof CancelledNotification) { + $this->cancelIncomingRequest($notification, $session); + } + foreach ($this->notificationHandlers as $handler) { if (!$handler->supports($notification)) { continue; @@ -616,6 +651,163 @@ private function handleNotification(Notification $notification, SessionInterface } } + /** + * Drops a suspended handler for $notification's request, if one is still in flight. + * + * Unknown, already finished or un-cancellable requests are ignored, as is a + * malformed notification that never becomes a {@see CancelledNotification}. + */ + private function cancelIncomingRequest(CancelledNotification $notification, SessionInterface $session): void + { + $requestId = $notification->requestId; + $key = self::requestKey($requestId); + + $this->logger->debug('Received a cancellation notification.', [ + 'request_id' => $requestId, + 'reason' => $notification->reason, + ]); + + $fiber = $this->fiberForIncomingRequest($requestId); + if (null === $fiber && !$this->hasInFlightIncomingRequest($session, $key)) { + $this->logger->debug('Ignored a cancellation for an unknown or finished request.', [ + 'request_id' => $requestId, + ]); + + return; + } + + $this->logger->info('Cancelled an in-flight request.', ['request_id' => $requestId]); + + $this->cancelledRequestKeys[$key] = true; + + if (!$session instanceof EphemeralSessionInterface) { + $session->set(self::SESSION_CANCELLED_REQUESTS.'.'.$key, true); + } + + if (null !== $fiber) { + $this->dropSuspendedHandler($fiber, $session); + } + } + + /** + * Whether a transport must drop $fiber without sending a response for its request. + * + * @param McpFiber $fiber + */ + private function isHandlerCancelled(\Fiber $fiber, Uuid $sessionId): bool + { + if (!isset($this->incomingRequestIds[$fiber])) { + return false; + } + + $requestId = $this->incomingRequestIds[$fiber]; + $key = self::requestKey($requestId); + + if (!isset($this->cancelledRequestKeys[$key])) { + if (!$this->sessionManager->exists($sessionId)) { + return false; + } + + $session = $this->sessionManager->createWithId($sessionId); + if (true !== $session->get(self::SESSION_CANCELLED_REQUESTS.'.'.$key)) { + return false; + } + + $this->cancelledRequestKeys[$key] = true; + } else { + $session = $this->sessionManager->exists($sessionId) ? $this->sessionManager->createWithId($sessionId) : null; + } + + if (null !== $session) { + $this->dropSuspendedHandler($fiber, $session); + $this->clearIncomingRequest($requestId, $session); + if (!$session instanceof EphemeralSessionInterface) { + try { + $session->save(); + } catch (\Throwable $e) { + $this->logger->error(\sprintf('Failed to save the session of a cancelled request: %s', $e->getMessage()), ['exception' => $e]); + } + } + } else { + $this->dropSuspendedHandler($fiber, null); + unset($this->incomingRequestIds[$fiber], $this->cancelledRequestKeys[$key]); + } + + return true; + } + + /** + * @param McpFiber $fiber + */ + private function dropSuspendedHandler(\Fiber $fiber, ?SessionInterface $session): void + { + $awaited = $this->awaitedRequestIds[$fiber] ?? null; + unset($this->awaitedRequestIds[$fiber]); + + if (null === $awaited || null === $session || $session instanceof EphemeralSessionInterface) { + return; + } + + $id = $awaited->toRfc4122(); + $session->forget(self::SESSION_PENDING_REQUESTS.'.'.$id); + $session->set(self::SESSION_RESPONSES.'.'.$id, null); + $session->forget(self::SESSION_RESPONSES.'.'.$id); + } + + /** + * @return McpFiber|null + */ + private function fiberForIncomingRequest(string|int $requestId): ?\Fiber + { + foreach ($this->incomingRequestIds as $fiber => $id) { + if ($id === $requestId) { + return $fiber; + } + } + + return null; + } + + private function hasInFlightIncomingRequest(SessionInterface $session, string $key): bool + { + return !$session instanceof EphemeralSessionInterface && true === $session->get(self::SESSION_IN_FLIGHT_REQUESTS.'.'.$key); + } + + private function markIncomingRequestInFlight(string|int $requestId, SessionInterface $session): void + { + $key = self::requestKey($requestId); + // A later request may reuse this id; it must not inherit a cancellation meant for the previous one. + $session->forget(self::SESSION_CANCELLED_REQUESTS.'.'.$key); + $session->set(self::SESSION_IN_FLIGHT_REQUESTS.'.'.$key, true); + } + + private function clearIncomingRequest(string|int $requestId, SessionInterface $session): void + { + $key = self::requestKey($requestId); + unset($this->cancelledRequestKeys[$key]); + + foreach ($this->incomingRequestIds as $fiber => $id) { + if ($id === $requestId) { + unset($this->incomingRequestIds[$fiber]); + } + } + + if ($session instanceof EphemeralSessionInterface) { + return; + } + + $session->forget(self::SESSION_IN_FLIGHT_REQUESTS.'.'.$key); + $session->forget(self::SESSION_CANCELLED_REQUESTS.'.'.$key); + } + + /** + * A session-key segment for a JSON-RPC request id, unique for int vs string ids. + */ + private static function requestKey(string|int $requestId): string + { + return (\is_int($requestId) ? 'i:' : 's:').strtr(base64_encode((string) $requestId), '+/', '-_'); + } + /** * Sends a request to the client and returns the request ID. */ diff --git a/src/Server/Transport/BaseTransport.php b/src/Server/Transport/BaseTransport.php index 868654aa..71de8754 100644 --- a/src/Server/Transport/BaseTransport.php +++ b/src/Server/Transport/BaseTransport.php @@ -103,6 +103,18 @@ protected function getPendingRequests(?Uuid $sessionId, \Fiber $fiber): array return []; } + /** + * @param McpFiber $fiber + */ + protected function isRequestCancelled(\Fiber $fiber): bool + { + if ($this->sessionId && \is_callable($this->cancellationChecker)) { + return ($this->cancellationChecker)($this->sessionId, $fiber); + } + + return false; + } + /** * @phpstan-return FiberResume */ diff --git a/src/Server/Transport/ManagesTransportCallbacks.php b/src/Server/Transport/ManagesTransportCallbacks.php index d315cf9d..8373d2b3 100644 --- a/src/Server/Transport/ManagesTransportCallbacks.php +++ b/src/Server/Transport/ManagesTransportCallbacks.php @@ -40,6 +40,9 @@ trait ManagesTransportCallbacks /** @var callable(Uuid, McpFiber): array> */ protected $pendingRequestsProvider; + /** @var callable(Uuid, McpFiber): bool */ + protected $cancellationChecker; + /** @var (callable(Uuid, Uuid): (Response>|Error|null))|null */ protected $responseFinder; @@ -66,6 +69,14 @@ public function setPendingRequestsProvider(callable $provider): void $this->pendingRequestsProvider = $provider; } + /** + * @param callable(Uuid, McpFiber): bool $checker + */ + public function setCancellationChecker(callable $checker): void + { + $this->cancellationChecker = $checker; + } + /** * @param callable(Uuid, Uuid):(Response>|Error|null) $finder */ diff --git a/src/Server/Transport/StdioTransport.php b/src/Server/Transport/StdioTransport.php index e556cc73..83e763af 100644 --- a/src/Server/Transport/StdioTransport.php +++ b/src/Server/Transport/StdioTransport.php @@ -449,6 +449,12 @@ private function writeError(Error $error): void private function processFibers(): void { foreach ($this->fibers as $key => $fiber) { + if ($this->isRequestCancelled($fiber)) { + unset($this->fibers[$key]); + + continue; + } + if ($fiber->isSuspended() && null !== $error = $this->resumeFiber($fiber)) { unset($this->fibers[$key]); $this->flushOutgoingMessages(); @@ -459,7 +465,10 @@ private function processFibers(): void if ($fiber->isTerminated()) { unset($this->fibers[$key]); - $this->handleFiberTermination($fiber); + // A cancellation may have arrived while this fiber was resumed, e.g. under an event loop. + if (!$this->isRequestCancelled($fiber)) { + $this->handleFiberTermination($fiber); + } } } } diff --git a/src/Server/Transport/StreamableHttpTransport.php b/src/Server/Transport/StreamableHttpTransport.php index 2f94f142..ced0a1d8 100644 --- a/src/Server/Transport/StreamableHttpTransport.php +++ b/src/Server/Transport/StreamableHttpTransport.php @@ -342,6 +342,10 @@ protected function createStreamedResponse(): ResponseInterface $this->logger->info('SSE: Starting request processing loop'); while ($fiber->isSuspended()) { + if ($this->isRequestCancelled($fiber)) { + return; + } + yield from $this->outgoingFrames(); $pendingRequests = $this->getPendingRequests($this->sessionId, $fiber); @@ -382,6 +386,10 @@ protected function createStreamedResponse(): ResponseInterface } } + if ($this->isRequestCancelled($fiber)) { + return; + } + yield from $this->finalFrame($fiber->getReturn()); } finally { $this->sessionFiber = null; diff --git a/src/Server/Transport/TransportInterface.php b/src/Server/Transport/TransportInterface.php index 76f0431e..20f90347 100644 --- a/src/Server/Transport/TransportInterface.php +++ b/src/Server/Transport/TransportInterface.php @@ -107,6 +107,16 @@ public function setOutgoingMessagesProvider(callable $provider): void; */ public function setPendingRequestsProvider(callable $provider): void; + /** + * Set a checker for whether a suspended handler's incoming request was cancelled. + * + * The transport calls this from its polling loop and must drop the fiber + * without sending a response when it returns true. + * + * @param callable(Uuid $sessionId, McpFiber $fiber): bool $checker + */ + public function setCancellationChecker(callable $checker): void; + /** * Set a finder function to check for a specific client response. * diff --git a/tests/Unit/Fixtures/PollingLoopTransport.php b/tests/Unit/Fixtures/PollingLoopTransport.php index 0019efe3..e15eaf83 100644 --- a/tests/Unit/Fixtures/PollingLoopTransport.php +++ b/tests/Unit/Fixtures/PollingLoopTransport.php @@ -11,16 +11,20 @@ namespace Mcp\Tests\Unit\Fixtures; +use Mcp\Schema\JsonRpc\Error; +use Mcp\Schema\JsonRpc\Response; use Mcp\Server\Suspension\NotificationSuspension; use Mcp\Server\Suspension\RequestSuspension; use Mcp\Server\Transport\InMemoryTransport; use Mcp\Server\Transport\TransportInterface; +use Symfony\Component\Uid\Uuid; /** * Exposes the protocol callbacks a transport's polling loop calls, to test which client requests * one stream waits on while others share its session. * * @phpstan-import-type FiberSuspend from TransportInterface + * @phpstan-import-type McpFiber from TransportInterface */ final class PollingLoopTransport extends InMemoryTransport { @@ -51,4 +55,72 @@ public function yieldFromFiber(NotificationSuspension|RequestSuspension $yielded $this->handleFiberYield($yielded, $this->sessionId, $this->sessionFiber); } + + /** + * @return McpFiber|null + */ + public function getSessionFiber(): ?\Fiber + { + return $this->sessionFiber; + } + + /** + * One turn of a handshake-era polling loop: drop a cancelled fiber, or resume + * it when its answer is in, and return what it would have sent the client. + * + * @return Response|Error|null + */ + public function pump(): Response|Error|null + { + $fiber = $this->sessionFiber; + if (null === $fiber) { + return null; + } + + if ($this->isRequestCancelled($fiber)) { + $this->sessionFiber = null; + + return null; + } + + if ($fiber->isSuspended()) { + $pendingRequests = $this->getPendingRequests($this->sessionId, $fiber); + + if ([] === $pendingRequests) { + if (null !== $error = $this->resumeHandlerFiber($fiber, $this->sessionId)) { + $this->sessionFiber = null; + + return $error; + } + } else { + foreach ($pendingRequests as $pending) { + $response = $this->checkForResponse(Uuid::fromString($pending['request_id']), $this->sessionId); + if (null === $response) { + continue; + } + + if (null !== $error = $this->resumeHandlerFiber($fiber, $this->sessionId, $response)) { + $this->sessionFiber = null; + + return $error; + } + + break; + } + } + } + + if ($fiber->isTerminated()) { + $this->sessionFiber = null; + if ($this->isRequestCancelled($fiber)) { + return null; + } + + $result = $fiber->getReturn(); + + return $result instanceof Response || $result instanceof Error ? $result : null; + } + + return null; + } } diff --git a/tests/Unit/Server/ProtocolTest.php b/tests/Unit/Server/ProtocolTest.php index 292d9acc..2c4f6e92 100644 --- a/tests/Unit/Server/ProtocolTest.php +++ b/tests/Unit/Server/ProtocolTest.php @@ -2746,6 +2746,224 @@ public function testClientResponseEventIsNotDispatchedForIdLessError(): void $protocol->processInput($this->transport, '{"jsonrpc": "2.0", "id": null, "error": {"code": -32700, "message": "Parse error"}}', $session->getId()); } + #[TestDox('notifications/cancelled drops a suspended handshake-era fiber and does not answer it')] + public function testCancelledNotificationDropsSuspendedFiberAndDoesNotAnswerIt(): void + { + $handler = $this->createMock(RequestHandlerInterface::class); + $handler->method('supports')->willReturn(true); + $handler->method('handle')->willReturnCallback(static function (Request $request, SessionInterface $session): Response { + \Fiber::suspend(new RequestSuspension(new PingRequest(), $session->getId()->toRfc4122(), 5)); + + return new Response($request->getId(), ['content' => [['type' => 'text', 'text' => 'should not be sent']]]); + }); + + $sessionManager = new SessionManager(new InMemorySessionStore(), gcProbability: 0); + $protocol = new Protocol( + requestHandlers: [$handler], + notificationHandlers: [], + messageFactory: MessageFactory::make(), + sessionManager: $sessionManager, + ); + + $session = $sessionManager->create(); + $session->set('_mcp.principal', null); + $session->save(); + $sessionId = $session->getId(); + + $stream = new PollingLoopTransport(); + $protocol->connect($stream); + $protocol->processInput($stream, '{"jsonrpc": "2.0", "id": 1, "method": "ping"}', $sessionId); + + $fiber = $stream->getSessionFiber(); + $this->assertInstanceOf(\Fiber::class, $fiber); + $this->assertTrue($fiber->isSuspended()); + $this->assertCount(1, $stream->getPendingRequestIds()); + $this->assertNotSame([], $stream->takeOutgoingMessages()); + + $protocol->processInput( + $stream, + '{"jsonrpc": "2.0", "method": "notifications/cancelled", "params": {"requestId": 1, "reason": "user cancelled"}}', + $sessionId + ); + + $this->assertNull($stream->pump()); + $this->assertNull($stream->getSessionFiber()); + $this->assertTrue($fiber->isSuspended(), 'The cancelled fiber must be dropped, not resumed.'); + $this->assertSame([], $protocol->getPendingRequests($sessionId)); + } + + #[TestDox('notifications/cancelled for an unknown request id is ignored')] + public function testCancelledNotificationForUnknownRequestIdIsIgnored(): void + { + $handler = $this->createMock(RequestHandlerInterface::class); + $handler->method('supports')->willReturn(true); + $handler->method('handle')->willReturnCallback(static function (Request $request, SessionInterface $session): Response { + \Fiber::suspend(new RequestSuspension(new PingRequest(), $session->getId()->toRfc4122(), 5)); + + return new Response($request->getId(), []); + }); + + $sessionManager = new SessionManager(new InMemorySessionStore(), gcProbability: 0); + $protocol = new Protocol( + requestHandlers: [$handler], + notificationHandlers: [], + messageFactory: MessageFactory::make(), + sessionManager: $sessionManager, + ); + + $session = $sessionManager->create(); + $session->set('_mcp.principal', null); + $session->save(); + $sessionId = $session->getId(); + + $stream = new PollingLoopTransport(); + $protocol->connect($stream); + $protocol->processInput($stream, '{"jsonrpc": "2.0", "id": 1, "method": "ping"}', $sessionId); + + $fiber = $stream->getSessionFiber(); + $this->assertInstanceOf(\Fiber::class, $fiber); + $pendingRequestIds = $stream->getPendingRequestIds(); + $this->assertCount(1, $pendingRequestIds); + + $this->transport = $this->createMock(TransportInterface::class); + $this->transport->expects($this->never())->method('send'); + $protocol->processInput( + $this->transport, + '{"jsonrpc": "2.0", "method": "notifications/cancelled", "params": {"requestId": 99}}', + $sessionId + ); + + $this->assertSame($fiber, $stream->getSessionFiber()); + $this->assertTrue($fiber->isSuspended()); + $this->assertSame($pendingRequestIds, $stream->getPendingRequestIds()); + + $protocol->processInput($stream, \sprintf('{"jsonrpc": "2.0", "id": %s, "result": {}}', json_encode($pendingRequestIds[0])), $sessionId); + $result = $stream->pump(); + $this->assertInstanceOf(Response::class, $result); + $this->assertSame(1, $result->getId()); + } + + #[TestDox('notifications/cancelled for a request that already finished is ignored')] + public function testCancelledNotificationForAFinishedRequestIsIgnored(): void + { + $handler = $this->createMock(RequestHandlerInterface::class); + $handler->method('supports')->willReturn(true); + $handler->method('handle')->willReturn(new Response(1, ['ok' => true])); + + $sent = []; + $this->transport->method('send')->willReturnCallback(static function (string $data) use (&$sent): void { + $sent[] = json_decode($data, true); + }); + + $sessionManager = new SessionManager(new InMemorySessionStore(), gcProbability: 0); + $protocol = new Protocol( + requestHandlers: [$handler], + notificationHandlers: [], + messageFactory: MessageFactory::make(), + sessionManager: $sessionManager, + ); + + $session = $sessionManager->create(); + $session->set('_mcp.principal', null); + $session->save(); + $sessionId = $session->getId(); + + $protocol->processInput($this->transport, '{"jsonrpc": "2.0", "id": 1, "method": "ping"}', $sessionId); + $this->assertCount(1, $sent); + $this->assertSame(1, $sent[0]['id']); + $this->assertSame(['ok' => true], $sent[0]['result']); + + $protocol->processInput( + $this->transport, + '{"jsonrpc": "2.0", "method": "notifications/cancelled", "params": {"requestId": 1}}', + $sessionId + ); + + $this->assertCount(1, $sent, 'A cancellation after the response must not produce another one.'); + } + + #[TestDox('a malformed notifications/cancelled is still rejected as an invalid request')] + public function testMalformedCancelledNotificationIsStillRejected(): void + { + $sent = []; + $this->transport->method('send')->willReturnCallback(static function (string $data) use (&$sent): void { + $sent[] = json_decode($data, true); + }); + + $sessionManager = new SessionManager(new InMemorySessionStore(), gcProbability: 0); + $protocol = new Protocol( + requestHandlers: [], + notificationHandlers: [], + messageFactory: MessageFactory::make(), + sessionManager: $sessionManager, + ); + + $session = $sessionManager->create(); + $session->set('_mcp.principal', null); + $session->save(); + $protocol->processInput( + $this->transport, + '{"jsonrpc": "2.0", "method": "notifications/cancelled", "params": {"requestId": 1, "reason": [1]}}', + $session->getId() + ); + + $this->assertCount(1, $sent); + $this->assertArrayHasKey('error', $sent[0]); + $this->assertSame(Error::INVALID_REQUEST, $sent[0]['error']['code']); + $this->assertArrayNotHasKey('id', $sent[0]); + } + + #[TestDox('cancelling one suspended request leaves another of a different id type')] + public function testCancelledNotificationMatchesRequestIdType(): void + { + $handler = $this->createMock(RequestHandlerInterface::class); + $handler->method('supports')->willReturn(true); + $handler->method('handle')->willReturnCallback(static function (Request $request, SessionInterface $session): Response { + \Fiber::suspend(new RequestSuspension(new PingRequest(), $session->getId()->toRfc4122(), 5)); + + return new Response($request->getId(), []); + }); + + $sessionManager = new SessionManager(new InMemorySessionStore(), gcProbability: 0); + $protocol = new Protocol( + requestHandlers: [$handler], + notificationHandlers: [], + messageFactory: MessageFactory::make(), + sessionManager: $sessionManager, + ); + + $session = $sessionManager->create(); + $session->set('_mcp.principal', null); + $session->save(); + $sessionId = $session->getId(); + + $intStream = new PollingLoopTransport(); + $protocol->connect($intStream); + $protocol->processInput($intStream, '{"jsonrpc": "2.0", "id": 5, "method": "ping"}', $sessionId); + + $stringStream = new PollingLoopTransport(); + $protocol->connect($stringStream); + $protocol->processInput($stringStream, '{"jsonrpc": "2.0", "id": "5", "method": "ping"}', $sessionId); + + $protocol->processInput( + $intStream, + '{"jsonrpc": "2.0", "method": "notifications/cancelled", "params": {"requestId": 5}}', + $sessionId + ); + + $this->assertNull($intStream->pump()); + $this->assertNull($intStream->getSessionFiber()); + $this->assertInstanceOf(\Fiber::class, $stringStream->getSessionFiber()); + $this->assertTrue($stringStream->getSessionFiber()->isSuspended()); + + $pendingRequestIds = $stringStream->getPendingRequestIds(); + $this->assertCount(1, $pendingRequestIds); + $protocol->processInput($stringStream, \sprintf('{"jsonrpc": "2.0", "id": %s, "result": {}}', json_encode($pendingRequestIds[0])), $sessionId); + $result = $stringStream->pump(); + $this->assertInstanceOf(Response::class, $result); + $this->assertSame('5', $result->getId()); + } + /** * @return iterable */ diff --git a/tests/Unit/Server/Transport/StdioSuspendedFibersTest.php b/tests/Unit/Server/Transport/StdioSuspendedFibersTest.php index dd9c28f8..3ff615f4 100644 --- a/tests/Unit/Server/Transport/StdioSuspendedFibersTest.php +++ b/tests/Unit/Server/Transport/StdioSuspendedFibersTest.php @@ -104,23 +104,55 @@ private static function call(int $id, string $tool, ?string $progressToken = nul return ['jsonrpc' => '2.0', 'id' => $id, 'method' => 'tools/call', 'params' => $params]; } + #[TestDox('cancelling a suspended elicitation drops it and does not answer that call')] + public function testCancelledElicitationIsNotAnswered(): void + { + $lines = $this->exchange([ + self::call(2, 'ask'), + ], static fn (int|string $id): array => [ + ['jsonrpc' => '2.0', 'method' => 'notifications/cancelled', 'params' => ['requestId' => 2, 'reason' => 'user cancelled']], + ], 1); + + $elicitations = array_values(array_filter($lines, static fn (array $line): bool => 'elicitation/create' === ($line['method'] ?? null))); + $this->assertCount(1, $elicitations); + $this->assertSame([], $this->answer($lines, 2)); + $this->assertSame([], array_values(array_filter( + $lines, + static fn (array $line): bool => 2 === ($line['id'] ?? null) && !isset($line['method']), + ))); + } + + #[TestDox('a cancellation for an unknown request leaves a suspended elicitation running')] + public function testUnknownCancellationLeavesSuspendedElicitation(): void + { + $lines = $this->exchange([ + self::call(2, 'ask'), + ], static fn (int|string $id): array => [ + ['jsonrpc' => '2.0', 'method' => 'notifications/cancelled', 'params' => ['requestId' => 99]], + ['jsonrpc' => '2.0', 'id' => $id, 'result' => ['action' => 'accept', 'content' => ['name' => 'kept']]], + ], 1); + + $this->assertSame('kept', $this->answer($lines, 2)['result']['content'][0]['text'] ?? null); + } + /** * Runs a handshake-era session with the given messages after the handshake. * - * @param list> $messages - * @param (\Closure(int|string, int|string): list>)|null $answer the client's messages once the server sent two requests, given their ids + * @param list> $messages + * @param ?\Closure $answer the client's messages once the server sent $answerAfter requests, given their ids + * @param int $answerAfter number of server-initiated requests to wait for before writing $answer * * @return list> */ - private function exchange(array $messages, ?\Closure $answer = null): array + private function exchange(array $messages, ?\Closure $answer = null, int $answerAfter = 2): array { $input = fopen('php://temp', 'r+'); $this->assertNotFalse($input); $requestIds = []; - $dispatcher = new class(static function (ServerRequestEvent $event) use ($input, $answer, &$requestIds): void { + $dispatcher = new class(static function (ServerRequestEvent $event) use ($input, $answer, $answerAfter, &$requestIds): void { $requestIds[] = $event->getRequest()->getId(); - if (null === $answer || 2 !== \count($requestIds)) { + if (null === $answer || $answerAfter !== \count($requestIds)) { return; } diff --git a/tests/Unit/Server/Transport/StreamableHttpTransportTest.php b/tests/Unit/Server/Transport/StreamableHttpTransportTest.php index 978552a0..ef4fd6d6 100644 --- a/tests/Unit/Server/Transport/StreamableHttpTransportTest.php +++ b/tests/Unit/Server/Transport/StreamableHttpTransportTest.php @@ -599,6 +599,45 @@ protected function isConnectionAborted(): bool $this->assertNull($received); } + #[TestDox('the polling loop drops a cancelled fiber and does not send its response')] + public function testPollingLoopDropsACancelledFiberWithoutSendingAResponse(): 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); + + $received = null; + $fiber = new \Fiber(static function () use (&$received) { + $received = \Fiber::suspend(); + + return new \Mcp\Schema\JsonRpc\Response(1, ['ok' => true]); + }); + $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 => [ + '00000000-0000-4000-8000-000000000001' => ['request_id' => '00000000-0000-4000-8000-000000000001', 'timestamp' => time(), 'timeout' => 120], + ]); + $transport->setCancellationChecker(static fn (): bool => true); + + $response = $transport->listen(); + + $this->assertSame('text/event-stream', $response->getHeaderLine('Content-Type')); + + $this->expectOutputString(''); + $this->assertSame('', $response->getBody()->getContents()); + + $this->assertTrue($fiber->isSuspended(), 'The cancelled call must not be resumed.'); + $this->assertNull($received); + } + /** * @return iterable */