From 29da3163d17188c07340bc78d7624c10ea57c050 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 5 Nov 2026 01:31:40 +0100 Subject: [PATCH] [Server] Stop a retried `resources/read` from leaving out its caching hints --- CHANGELOG.md | 1 + docs/run/caching.md | 5 +++-- src/Server/Wire/Rev2026Codec.php | 8 ++++++-- src/Server/Wire/WireCodecInterface.php | 5 ++--- .../Unit/Server/Stateless/StatelessProtocolTest.php | 8 +++----- tests/Unit/Server/Wire/CachePolicyTest.php | 13 +++++++++++++ 6 files changed, 28 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 232dacb19..e70a32234 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix tools, prompts and resources sending `content`, `messages` and `contents` as a JSON object instead of an array when the handler returned a non-list array, e.g. the result of `array_filter()`. Named keys in nested results of mixed arrays no longer overwrite each other. * Fix `SecureUrl` rejecting plain http on hosts under the reserved `.localhost` TLD (RFC 6761) and on IPv4 loopback addresses other than `127.0.0.1`, so `ProtectedResourceMetadata`, `OidcDiscovery` and `JwtTokenValidator::fromIssuer()` failed for local development setups like `http://shop.localhost:8080`. Plain http is now accepted for `localhost`, any `*.localhost`, `127.0.0.0/8` and `::1`; every other host still requires https. * Tool, prompt and resource results and `TextContent` encode arrays and objects as compact JSON instead of pretty-printed JSON. +* Fix the result of a retried `resources/read` leaving out the required `ttlMs` and `cacheScope`, which the TypeScript SDK's client rejects, or keeping the hints its handler set: it now always carries `ttlMs: 0` and `cacheScope: private`. * Fix tool name validation not following the spec's rule (1 to 128 characters of `A-Z`, `a-z`, `0-9`, `_`, `-` and `.`): names of 65 to 128 characters no longer log a warning, while names with a `/` or a trailing newline now do. * 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`. * Fix a nullable tool parameter with a default (e.g. `?string $category = 'books'`) receiving the default when the client explicitly sends `null`: `ReferenceHandler` now passes the `null` through. diff --git a/docs/run/caching.md b/docs/run/caching.md index 777b66c77..28a6c1646 100644 --- a/docs/run/caching.md +++ b/docs/run/caching.md @@ -24,5 +24,6 @@ conservative default stands until you change it. A `ReadResourceResult` may set its own `ttlMs`/`cacheScope`, which win over the policy. -Results produced by an [MRTR retry](../handlers/input-required.md) are never given hints: their inputs -are not part of any cache key. +Results produced by an [MRTR retry](../handlers/input-required.md) always carry `ttlMs: 0` and +`cacheScope: "private"`, whatever the policy or the result says: their inputs are not part of any cache +key, so they must not be cached. diff --git a/src/Server/Wire/Rev2026Codec.php b/src/Server/Wire/Rev2026Codec.php index d3b21efff..32db243d3 100644 --- a/src/Server/Wire/Rev2026Codec.php +++ b/src/Server/Wire/Rev2026Codec.php @@ -58,7 +58,7 @@ public function encodeResult(string $method, array $result, bool $cacheable = tr $stamped = $this->stampResultType($method, $result); return $this->stampServerInfo( - $cacheable ? $this->fillCacheHints($method, $stamped) : $stamped, + $this->fillCacheHints($method, $stamped, $cacheable), ); } @@ -94,7 +94,7 @@ private function stampResultType(string $method, array $result): array * * @return array */ - private function fillCacheHints(string $method, array $result): array + private function fillCacheHints(string $method, array $result, bool $cacheable): array { if (ResultType::Complete->value !== ($result['resultType'] ?? null)) { return $result; @@ -104,6 +104,10 @@ private function fillCacheHints(string $method, array $result): array return $result; } + if (!$cacheable) { + return [...$result, 'ttlMs' => 0, 'cacheScope' => CacheScope::Private->value]; + } + $ttl = $result['ttlMs'] ?? null; $scope = $result['cacheScope'] ?? null; diff --git a/src/Server/Wire/WireCodecInterface.php b/src/Server/Wire/WireCodecInterface.php index e183c4b5d..08f7cc98e 100644 --- a/src/Server/Wire/WireCodecInterface.php +++ b/src/Server/Wire/WireCodecInterface.php @@ -31,9 +31,8 @@ interface WireCodecInterface * * @param string $method the request method the result answers * @param array $result the neutral result body - * @param bool $cacheable whether this answer may carry caching hints at all — false for one - * produced by a multi round-trip retry, whose inputs are not part of - * any cache key + * @param bool $cacheable whether this answer may be cached — false for one produced by a + * multi round-trip retry, whose inputs are not part of any cache key * * @return array */ diff --git a/tests/Unit/Server/Stateless/StatelessProtocolTest.php b/tests/Unit/Server/Stateless/StatelessProtocolTest.php index 81ff7eddf..aae8ec828 100644 --- a/tests/Unit/Server/Stateless/StatelessProtocolTest.php +++ b/tests/Unit/Server/Stateless/StatelessProtocolTest.php @@ -1166,7 +1166,7 @@ public function testFirstRoundReadIsCacheable(): void $this->assertArrayHasKey('cacheScope', $answer['body']['result']); } - #[TestDox('a result produced by a multi round-trip retry carries no caching hints')] + #[TestDox('a result produced by a multi round-trip retry carries hints that forbid caching it')] public function testMrtrRetryResultIsNotCacheable(): void { $answer = self::call( @@ -1181,10 +1181,8 @@ public function testMrtrRetryResultIsNotCacheable(): void ); $this->assertSame('complete', $answer['body']['result']['resultType']); - // The inputs are not part of any cache key, so the answer must not be - // presented as reusable. - $this->assertArrayNotHasKey('ttlMs', $answer['body']['result']); - $this->assertArrayNotHasKey('cacheScope', $answer['body']['result']); + $this->assertSame(0, $answer['body']['result']['ttlMs']); + $this->assertSame('private', $answer['body']['result']['cacheScope']); } #[TestDox('a listen stream acknowledges, then carries what the client asked for')] diff --git a/tests/Unit/Server/Wire/CachePolicyTest.php b/tests/Unit/Server/Wire/CachePolicyTest.php index 4a65aa502..de81ff19c 100644 --- a/tests/Unit/Server/Wire/CachePolicyTest.php +++ b/tests/Unit/Server/Wire/CachePolicyTest.php @@ -110,4 +110,17 @@ public function testNonCacheableMethodIsUntouched(): void $this->assertArrayNotHasKey('ttlMs', $encoded); $this->assertArrayNotHasKey('cacheScope', $encoded); } + + #[TestDox('the result of a retry forbids caching, whatever the policy or the result say')] + public function testRetryResultForbidsCachingWhateverThePolicyOrItsHandlerSay(): void + { + $codec = new Rev2026Codec(null, CachePolicy::default(60_000, CacheScope::Public)); + + foreach ([['contents' => []], ['contents' => [], 'ttlMs' => 500, 'cacheScope' => 'public']] as $result) { + $encoded = $codec->encodeResult('resources/read', $result, cacheable: false); + + $this->assertSame(0, $encoded['ttlMs']); + $this->assertSame('private', $encoded['cacheScope']); + } + } }