[Server] Answer a DELETE of an unknown or ended session with 404 (#638)

* [Server] Answer a DELETE of an unknown or ended session with 404

The spec requires a 404 for any request that carries the id of a terminated session.

* Make InMemorySessionStore::exists() honor the TTL

Author: nicolas-grekas

CHANGELOG.md

@@ -8,8 +8,10 @@ All notable changes to `mcp/sdk` will be documented in this file.
 ### Server
 
 * [BC Break] `DocBlockParser::parseDocBlock()` no longer accepts `null`; pass `ReflectionMethod::getDocComment()` as is
+* [BC Break] Fix a `DELETE` of an unknown or ended session being answered `200` instead of the `404` the spec requires: `Protocol::destroySession()` and `BaseTransport::handleSessionEnd()` return `false` for such a session, as does the listener of `TransportInterface::onSessionEnd()`. A listener that returns nothing keeps the `200`.
 * Fix tools with an array shape parameter whose keys are nullable or unions (e.g. `array{name: string, nickname: string|null}`) rejecting valid arguments with `-32602`: a `null` inside the shape no longer turns the parameter into `array|null`, and such keys get their own types instead of `object`, including lists and nested shapes (`string[]|null`, `array{x: int}|null`).
 * Fix `array<string|null>` and other generics with a union element getting `items` of type `object`. A `null` inside a generic no longer makes the parameter nullable, and neither does a DocBlock class whose name contains `null`.
+* Fix `InMemorySessionStore::exists()` reporting a session past its TTL as existing, while `read()` already treated it as gone.
 * Fix `Psr16NotificationBus` subscribers missing the notifications published after the bus stayed quiet for longer than its TTL: the sequence counter no longer expires, and a subscriber whose cursor is ahead of a lost counter reads what the bus holds.
 * Add `Psr6SessionStore` and `Psr6NotificationBus`, which work like `Psr16SessionStore` and `Psr16NotificationBus` over a PSR-6 cache pool. `Builder::setDiscovery()` also accepts a PSR-6 pool as `$cache`.
 

src/Server/Protocol.php

@@ -1191,14 +1191,23 @@ private static function findRequestId(array $messages): string|int|null
 
     /**
      * Destroy a specific session.
+     *
+     * @return bool false when the session does not exist
      */
-    public function destroySession(Uuid $sessionId): void
+    public function destroySession(Uuid $sessionId): bool
     {
+        try {
+            $exists = $this->sessionManager->exists($sessionId);
+        } catch (SessionStoreException) {
+            // The session may well exist, so it is not reported as unknown.
+            $exists = true;
+        }
+
         // Destroyed even when a request of the session holds the lock past the timeout, or the
         // lock is unavailable: the client ended the session, and Session::save() does not write
         // back a session that was destroyed since it was read. An unknown session id has nothing to lock.
         try {
-            $locked = null !== $this->sessionLock && $this->sessionManager->exists($sessionId) && $this->acquireSessionLock($sessionId);
+            $locked = null !== $this->sessionLock && $exists && $this->acquireSessionLock($sessionId);
         } catch (SessionLockException) {
             $locked = false;
         }
@@ -1212,6 +1221,8 @@ public function destroySession(Uuid $sessionId): void
         }
 
         $this->logger->info('Session destroyed.', ['session_id' => $sessionId->toRfc4122()]);
+
+        return $exists;
     }
 
     /**

src/Server/Session/InMemorySessionStore.php

@@ -30,7 +30,9 @@ public function __construct(
 
     public function exists(Uuid $id): bool
     {
-        return isset($this->store[$id->toRfc4122()]);
+        $session = $this->store[$id->toRfc4122()] ?? null;
+
+        return null !== $session && $this->clock->now()->getTimestamp() - $session['timestamp'] <= $this->ttl;
     }
 
     public function read(Uuid $id): string|false

src/Server/Transport/BaseTransport.php

@@ -152,10 +152,15 @@ protected function handleMessage(string $payload, ?Uuid $sessionId, ?AccessToken
         }
     }
 
-    protected function handleSessionEnd(?Uuid $sessionId): void
+    /**
+     * @return bool false when the listener reported that the session does not exist
+     */
+    protected function handleSessionEnd(?Uuid $sessionId): bool
     {
         if ($sessionId && \is_callable($this->sessionEndListener)) {
-            ($this->sessionEndListener)($sessionId);
+            return false !== ($this->sessionEndListener)($sessionId);
         }
+
+        return true;
     }
 }

src/Server/Transport/ManagesTransportCallbacks.php

@@ -31,7 +31,7 @@ trait ManagesTransportCallbacks
     /** @var callable(TransportInterface<mixed>, string, ?Uuid, ?AccessToken): void */
     protected $messageListener;
 
-    /** @var callable(Uuid): void */
+    /** @var callable(Uuid): bool */
     protected $sessionEndListener;
 
     /** @var callable(Uuid): array<int, array{message: string, context: array<string, mixed>}> */

src/Server/Transport/StreamableHttpTransport.php

@@ -242,7 +242,9 @@ protected function handleDeleteRequest(): ResponseInterface
             return $this->createErrorResponse(Error::forInvalidRequest(self::SESSION_HEADER.' header is required.'), 400);
         }
 
-        $this->handleSessionEnd($this->sessionId);
+        if (!$this->handleSessionEnd($this->sessionId)) {
+            return $this->createErrorResponse(Error::forInvalidRequest('Session not found or has expired.'), 404);
+        }
 
         return $this->responseFactory->createResponse(200);
     }

src/Server/Transport/TransportInterface.php

@@ -83,7 +83,7 @@ public function onMessage(callable $listener): void;
      *
      * The transport calls this when a client disconnects or explicitly ends their session.
      *
-     * @param callable(Uuid $sessionId): void $listener The callback function to execute when destroying a session
+     * @param callable(Uuid $sessionId): bool $listener The callback function to execute when destroying a session, returning false when the session does not exist
      */
     public function onSessionEnd(callable $listener): void;
 

tests/Unit/Server/ProtocolSessionLockTest.php

@@ -416,8 +416,7 @@ public function testDestroyingASessionLocks(): void
         $protocol = $this->protocol($lock);
         $sessionId = $this->existingSession();
 
-        $protocol->destroySession($sessionId);
-
+        $this->assertTrue($protocol->destroySession($sessionId));
         $this->assertSame(["acquire $sessionId", "destroy $sessionId", "release $sessionId"], $this->log);
         $this->assertFalse($this->store->exists($sessionId));
     }
@@ -442,8 +441,7 @@ public function testDestroyingAnUnknownSessionTakesNoLock(): void
         $protocol = $this->protocol($lock);
         $sessionId = Uuid::v4();
 
-        $protocol->destroySession($sessionId);
-
+        $this->assertFalse($protocol->destroySession($sessionId));
         $this->assertSame(["destroy $sessionId"], $this->log);
     }
 

tests/Unit/Server/Transport/StreamableHttpTransportTest.php

@@ -19,6 +19,7 @@
 use Mcp\Server;
 use Mcp\Server\RequestContext;
 use Mcp\Server\Session\FileSessionLock;
+use Mcp\Server\Session\InMemorySessionStore;
 use Mcp\Server\Session\Session;
 use Mcp\Server\Session\SessionLockInterface;
 use Mcp\Server\Suspension\RequestSuspension;
@@ -34,6 +35,7 @@
 use PHPUnit\Framework\Attributes\DataProvider;
 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;
@@ -149,6 +151,40 @@ public function testDuplicateSessionIdHeadersReturnBadRequest(): void
         $this->assertStringContainsString('must not be repeated', (string) $response->getBody());
     }
 
+    #[TestDox('a DELETE of an unknown or already ended session is answered 404')]
+    public function testDeleteOfAnEndedSessionIsNotFound(): void
+    {
+        $server = Server::builder()->setServerInfo('test', '1.0')->build();
+        $sessionId = $this->initialize($server);
+
+        $this->assertSame(200, $this->delete($server, $sessionId)->getStatusCode());
+
+        $response = $this->delete($server, $sessionId);
+
+        $this->assertSame(404, $response->getStatusCode());
+        $this->assertStringContainsString('Session not found', (string) $response->getBody());
+    }
+
+    #[TestDox('a DELETE of an expired session is answered 404')]
+    public function testDeleteOfAnExpiredSessionIsNotFound(): void
+    {
+        $clock = new class implements ClockInterface {
+            public \DateTimeImmutable $now;
+
+            public function now(): \DateTimeImmutable
+            {
+                return $this->now;
+            }
+        };
+        $clock->now = new \DateTimeImmutable('2026-10-28 12:00:00');
+        $server = Server::builder()->setServerInfo('test', '1.0')->setSession(new InMemorySessionStore(60, $clock))->build();
+        $sessionId = $this->initialize($server);
+
+        $clock->now = $clock->now->modify('+61 seconds');
+
+        $this->assertSame(404, $this->delete($server, $sessionId)->getStatusCode());
+    }
+
     #[TestDox('pass-through middleware disables defaults without a warning log')]
     public function testPassthroughMiddlewareDisablesDefaultsWithoutWarning(): void
     {
@@ -841,6 +877,31 @@ private function post(InterleavingSessionStore $store, string $body, string $ses
         return $builder->build()->run(new StreamableHttpTransport($request, $this->factory, $this->factory));
     }
 
+    /**
+     * Opens a session on $server and returns its id.
+     */
+    private function initialize(Server $server): string
+    {
+        $request = $this->factory
+            ->createServerRequest('POST', 'http://localhost/')
+            ->withHeader('Host', 'localhost')
+            ->withHeader('Content-Type', 'application/json')
+            ->withHeader('Accept', 'application/json, text/event-stream')
+            ->withBody($this->factory->createStream('{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}'));
+
+        return $server->run(new StreamableHttpTransport($request, $this->factory, $this->factory))->getHeaderLine(StreamableHttpTransport::SESSION_HEADER);
+    }
+
+    private function delete(Server $server, string $sessionId): ResponseInterface
+    {
+        $request = $this->factory
+            ->createServerRequest('DELETE', 'http://localhost/')
+            ->withHeader('Host', 'localhost')
+            ->withHeader(StreamableHttpTransport::SESSION_HEADER, $sessionId);
+
+        return $server->run(new StreamableHttpTransport($request, $this->factory, $this->factory));
+    }
+
     /**
      * Reads an SSE response; the stream calls ob_flush() itself, so the output is captured by a handler, not a plain buffer.
      */