[Server] Answer 404 for a session that holds no attributes

Author: chr-hertel

CHANGELOG.md

@@ -48,6 +48,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
 * [BC Break] Reject `tools/call` when `arguments` is not a JSON object with `-32600`, including `"arguments": []`.
 * [BC Break] Reject `tools/call` with `-32600` when its message has a member name starting with a NUL byte, as its arguments cannot keep their JSON types then.
 * [BC Break] `array<string, T>` parameters generate `{"type": "object"}` instead of `{"type": "array"}`, and `array<int, T>` parameters accept a JSON array or a JSON object.
+* [BC Break] Add `SessionInterface::isUnreadable()`, which a custom session implements. A session the store holds without attributes, because a handler cleared it or its data is corrupt, is answered with `404` so the client opens a new one; only a session the store fails to return is answered with `503`.
 
 ### Client
 

src/Server/Protocol.php

@@ -1204,15 +1204,15 @@ private function resolveSession(TransportInterface $transport, ?Uuid $sessionId,
      *
      * A session that does not record who opened it serves no one.
      *
-     * @throws SessionStoreException when the session exists but its data cannot be read
+     * @throws SessionStoreException when the store holds the session but fails to return it
      */
     private function servesPrincipal(SessionInterface $session, ?string $principal): bool
     {
         if ($session->get(self::SESSION_PRINCIPAL, false) === self::fingerprint($principal)) {
             return true;
         }
 
-        if ([] === $session->all()) {
+        if ($session->isUnreadable()) {
             throw new SessionStoreException(\sprintf('Session "%s" could not be read.', $session->getId()->toRfc4122()));
         }
 

src/Server/Session/Session.php

@@ -184,6 +184,13 @@ public function all(): array
         return $this->readData();
     }
 
+    public function isUnreadable(): bool
+    {
+        $this->readData();
+
+        return !$this->stored && $this->store->exists($this->id);
+    }
+
     public function hydrate(array $attributes): void
     {
         $this->data = $attributes;

src/Server/Session/SessionInterface.php

@@ -11,6 +11,7 @@
 
 namespace Mcp\Server\Session;
 
+use Mcp\Exception\SessionStoreException;
 use Symfony\Component\Uid\Uuid;
 
 /**
@@ -70,6 +71,13 @@ public function pull(string $key, mixed $default = null): mixed;
      */
     public function all(): array;
 
+    /**
+     * Whether the store holds this session but failed to return it, unlike a session without attributes.
+     *
+     * @throws SessionStoreException if the store cannot be reached
+     */
+    public function isUnreadable(): bool;
+
     /**
      * Set all attributes of the session, typically for hydration.
      * This will overwrite existing attributes.

tests/Unit/Server/Session/SessionTest.php

@@ -353,6 +353,34 @@ public function testSaveBeforeReadInitializesData(): void
         $this->assertTrue($session->save());
     }
 
+    public function testNewSessionIsNotUnreadable(): void
+    {
+        $this->assertFalse((new Session(new InMemorySessionStore()))->isUnreadable());
+    }
+
+    public function testSessionTheStoreHoldsAndReturnsIsNotUnreadable(): void
+    {
+        $store = new InMemorySessionStore();
+        $id = Uuid::v4();
+        $store->write($id, '[]');
+
+        $this->assertFalse((new Session($store, $id))->isUnreadable());
+    }
+
+    public function testSessionTheStoreHoldsButFailsToReturnIsUnreadable(): void
+    {
+        $store = new class extends InMemorySessionStore {
+            public function read(Uuid $id): string|false
+            {
+                return false;
+            }
+        };
+        $id = Uuid::v4();
+        $store->write($id, '{"key":"value"}');
+
+        $this->assertTrue((new Session($store, $id))->isUnreadable());
+    }
+
     public function testAllReturnsEmptyArrayForNullPayload(): void
     {
         $store = $this->getMockBuilder(InMemorySessionStore::class)

tests/Unit/Server/Transport/SessionBindingTest.php

@@ -59,6 +59,7 @@ protected function setUp(): void
             ->setServerInfo('test', '1.0.0')
             ->setSession($this->store)
             ->addTool(self::whoami(...), name: 'whoami')
+            ->addTool(self::forget(...), name: 'forget')
             ->build();
     }
 
@@ -159,6 +160,36 @@ public function testDeleteDuringAStoreOutageKeepsTheSession(): void
         $this->assertSame('alice', $this->text($this->call($session, 'alice.ide.1')));
     }
 
+    public function testSessionClearedByAHandlerIsNotFound(): void
+    {
+        $session = $this->initialize('alice.ide.1');
+        $forget = json_encode(['jsonrpc' => '2.0', 'id' => 2, 'method' => 'tools/call', 'params' => ['name' => 'forget', 'arguments' => new \stdClass()]], \JSON_THROW_ON_ERROR);
+
+        $this->assertSame(200, $this->serve($this->request('POST', $forget, 'alice.ide.1', $session))->getStatusCode());
+
+        $this->assertSame(404, $this->call($session, 'alice.ide.1')->getStatusCode());
+        $this->assertSame(404, $this->serve($this->request('DELETE', '', 'alice.ide.1', $session))->getStatusCode());
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function provideDataWithoutAttributes(): iterable
+    {
+        yield 'empty' => ['[]'];
+        yield 'not JSON' => ['{'];
+        yield 'not a list of attributes' => ['null'];
+    }
+
+    #[DataProvider('provideDataWithoutAttributes')]
+    public function testSessionWhoseDataHoldsNoAttributesIsNotFound(string $data): void
+    {
+        $session = Uuid::v4();
+        $this->store->write($session, $data);
+
+        $this->assertSame(404, $this->call($session->toRfc4122(), 'alice.ide.1')->getStatusCode());
+    }
+
     public function testSessionFromBeforeTheUpgradeIsNotFound(): void
     {
         $session = Uuid::v4();
@@ -301,6 +332,13 @@ public function testSessionOpenedWithoutPrincipalOnlyServesRequestsWithoutOne():
         $this->assertSame(404, $this->call($session, 'alice.ide.1')->getStatusCode());
     }
 
+    public static function forget(RequestContext $context): string
+    {
+        $context->getSession()->clear();
+
+        return 'forgotten';
+    }
+
     public static function whoami(RequestContext $context): string
     {
         return $context->getAccessToken()?->getSubject() ?? 'anonymous';