Skip to content

Commit ce46033

Browse files
authored
[Server] Stop a retried resources/read from leaving out its caching hints (#667)
1 parent 05b6e64 commit ce46033

6 files changed

Lines changed: 28 additions & 12 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
2222
* 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.
2323
* 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.
2424
* Tool, prompt and resource results and `TextContent` encode arrays and objects as compact JSON instead of pretty-printed JSON.
25+
* 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`.
2526
* 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.
2627
* 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`.
2728
* 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.

‎docs/run/caching.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,6 @@ conservative default stands until you change it.
2424

2525
A `ReadResourceResult` may set its own `ttlMs`/`cacheScope`, which win over the policy.
2626

27-
Results produced by an [MRTR retry](../handlers/input-required.md) are never given hints: their inputs
28-
are not part of any cache key.
27+
Results produced by an [MRTR retry](../handlers/input-required.md) always carry `ttlMs: 0` and
28+
`cacheScope: "private"`, whatever the policy or the result says: their inputs are not part of any cache
29+
key, so they must not be cached.

‎src/Server/Wire/Rev2026Codec.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function encodeResult(string $method, array $result, bool $cacheable = tr
5858
$stamped = $this->stampResultType($method, $result);
5959

6060
return $this->stampServerInfo(
61-
$cacheable ? $this->fillCacheHints($method, $stamped) : $stamped,
61+
$this->fillCacheHints($method, $stamped, $cacheable),
6262
);
6363
}
6464

@@ -94,7 +94,7 @@ private function stampResultType(string $method, array $result): array
9494
*
9595
* @return array<string, mixed>
9696
*/
97-
private function fillCacheHints(string $method, array $result): array
97+
private function fillCacheHints(string $method, array $result, bool $cacheable): array
9898
{
9999
if (ResultType::Complete->value !== ($result['resultType'] ?? null)) {
100100
return $result;
@@ -104,6 +104,10 @@ private function fillCacheHints(string $method, array $result): array
104104
return $result;
105105
}
106106

107+
if (!$cacheable) {
108+
return [...$result, 'ttlMs' => 0, 'cacheScope' => CacheScope::Private->value];
109+
}
110+
107111
$ttl = $result['ttlMs'] ?? null;
108112
$scope = $result['cacheScope'] ?? null;
109113

‎src/Server/Wire/WireCodecInterface.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@ interface WireCodecInterface
3131
*
3232
* @param string $method the request method the result answers
3333
* @param array<string, mixed> $result the neutral result body
34-
* @param bool $cacheable whether this answer may carry caching hints at all — false for one
35-
* produced by a multi round-trip retry, whose inputs are not part of
36-
* any cache key
34+
* @param bool $cacheable whether this answer may be cached — false for one produced by a
35+
* multi round-trip retry, whose inputs are not part of any cache key
3736
*
3837
* @return array<string, mixed>
3938
*/

‎tests/Unit/Server/Stateless/StatelessProtocolTest.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,7 +1166,7 @@ public function testFirstRoundReadIsCacheable(): void
11661166
$this->assertArrayHasKey('cacheScope', $answer['body']['result']);
11671167
}
11681168

1169-
#[TestDox('a result produced by a multi round-trip retry carries no caching hints')]
1169+
#[TestDox('a result produced by a multi round-trip retry carries hints that forbid caching it')]
11701170
public function testMrtrRetryResultIsNotCacheable(): void
11711171
{
11721172
$answer = self::call(
@@ -1181,10 +1181,8 @@ public function testMrtrRetryResultIsNotCacheable(): void
11811181
);
11821182

11831183
$this->assertSame('complete', $answer['body']['result']['resultType']);
1184-
// The inputs are not part of any cache key, so the answer must not be
1185-
// presented as reusable.
1186-
$this->assertArrayNotHasKey('ttlMs', $answer['body']['result']);
1187-
$this->assertArrayNotHasKey('cacheScope', $answer['body']['result']);
1184+
$this->assertSame(0, $answer['body']['result']['ttlMs']);
1185+
$this->assertSame('private', $answer['body']['result']['cacheScope']);
11881186
}
11891187

11901188
#[TestDox('a listen stream acknowledges, then carries what the client asked for')]

‎tests/Unit/Server/Wire/CachePolicyTest.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,17 @@ public function testNonCacheableMethodIsUntouched(): void
110110
$this->assertArrayNotHasKey('ttlMs', $encoded);
111111
$this->assertArrayNotHasKey('cacheScope', $encoded);
112112
}
113+
114+
#[TestDox('the result of a retry forbids caching, whatever the policy or the result say')]
115+
public function testRetryResultForbidsCachingWhateverThePolicyOrItsHandlerSay(): void
116+
{
117+
$codec = new Rev2026Codec(null, CachePolicy::default(60_000, CacheScope::Public));
118+
119+
foreach ([['contents' => []], ['contents' => [], 'ttlMs' => 500, 'cacheScope' => 'public']] as $result) {
120+
$encoded = $codec->encodeResult('resources/read', $result, cacheable: false);
121+
122+
$this->assertSame(0, $encoded['ttlMs']);
123+
$this->assertSame('private', $encoded['cacheScope']);
124+
}
125+
}
113126
}

0 commit comments

Comments
 (0)