Skip to content

Commit b973893

Browse files
authored
[Server] Serve an extension's methods under the modern lifecycle (#454)
* [Server] Serve an extension's methods under the modern lifecycle The modern dispatcher takes the method-to-extension map the builder collects, so a method belonging to an extension this server does not serve is answered -32601 naming the extension instead of a bare "no handler found". It is still an unknown method - the server genuinely does not implement it - but the caller can now act on the answer. * [Server] Reject two extensions claiming the same RPC method The message factory resolves a contested method to whichever class registered first, while extensionMethods kept the last one — so error messages could name the wrong extension. Also fixes a test that claimed to prove a method gets named by its extension while asserting the opposite; the case it meant to cover (an enabled extension with no handler for one of its methods) had no coverage at all.
1 parent 7e38c94 commit b973893

5 files changed

Lines changed: 165 additions & 4 deletions

File tree

src/Server/Builder.php

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,9 @@ final class Builder
236236
/** @var list<class-string<\Mcp\Schema\JsonRpc\Request>|class-string<\Mcp\Schema\JsonRpc\Notification>> */
237237
private array $extensionMessages = [];
238238

239+
/** @var array<string, string> RPC method to the extension identifier defining it */
240+
private array $extensionMethods = [];
241+
239242
/**
240243
* @var LoaderInterface[]
241244
*/
@@ -389,7 +392,8 @@ public function setCapabilities(ServerCapabilities $serverCapabilities): self
389392
* for extensions that only announce a capability.
390393
*
391394
* @throws InvalidArgumentException if the identifier is not a valid `_meta` prefix
392-
* @throws LogicException if the same extension is enabled more than once
395+
* @throws LogicException if the same extension is enabled more than once, or
396+
* two enabled extensions define the same RPC method
393397
*/
394398
public function enableExtension(ExtensionInterface ...$extensions): self
395399
{
@@ -405,7 +409,19 @@ public function enableExtension(ExtensionInterface ...$extensions): self
405409
// Without this the method cannot be decoded at all, so nothing
406410
// downstream ever sees it.
407411
foreach ($extension->getMessages() as $message) {
412+
$method = $message::getMethod();
413+
414+
// The message factory resolves a method to whichever class was
415+
// registered first, so a second owner here would silently lose
416+
// the dispatch race while still being named in error messages.
417+
if (isset($this->extensionMethods[$method]) && $this->extensionMethods[$method] !== $id) {
418+
throw new LogicException(\sprintf('Method "%s" is already claimed by extension "%s", so extension "%s" cannot also define it.', $method, $this->extensionMethods[$method], $id));
419+
}
420+
408421
$this->extensionMessages[] = $message;
422+
// Recorded even though the handler answers it, so a server with
423+
// the extension *off* can say so instead of "no such method".
424+
$this->extensionMethods[$method] = $id;
409425
}
410426

411427
foreach ($extension->getRequestHandlers() as $handler) {
@@ -836,6 +852,7 @@ public function buildStateless(array $supportedVersions = [ProtocolVersion::V202
836852
: null,
837853
cachePolicy: $this->cachePolicy,
838854
notificationBus: $this->notificationBus,
855+
extensionMethods: $this->extensionMethods,
839856
);
840857
}
841858

src/Server/Stateless/StatelessProtocol.php

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ final class StatelessProtocol
8989
/**
9090
* @param iterable<RequestHandlerInterface<ResultInterface>> $requestHandlers
9191
* @param list<ProtocolVersion> $supportedVersions
92+
* @param array<string, string> $extensionMethods RPC method to the extension identifier defining it
9293
*/
9394
public function __construct(
9495
private readonly iterable $requestHandlers,
@@ -102,6 +103,7 @@ public function __construct(
102103
private readonly ?RequestStateCodec $requestStateCodec = null,
103104
?CachePolicy $cachePolicy = null,
104105
private readonly ?NotificationBusInterface $notificationBus = null,
106+
private readonly array $extensionMethods = [],
105107
) {
106108
$this->codec = $codec ?? new Rev2026Codec($configuration->serverInfo, $cachePolicy);
107109

@@ -396,7 +398,7 @@ private function dispatch(string $method, array $decoded, RequestMeta $meta, str
396398
} catch (\Throwable $e) {
397399
$this->logger->warning('Rejected an unparseable modern-era request.', ['method' => $method, 'exception' => $e]);
398400

399-
return StatelessResult::error(Error::forMethodNotFound(\sprintf('Method "%s" is not supported.', $method), $id), 404);
401+
return StatelessResult::error($this->unknownMethod($method, $id), 404);
400402
}
401403

402404
$request = $messages[0] ?? null;
@@ -410,7 +412,7 @@ private function dispatch(string $method, array $decoded, RequestMeta $meta, str
410412

411413
return StatelessResult::error(
412414
$unknownMethod
413-
? Error::forMethodNotFound($request->getMessage(), $id)
415+
? $this->unknownMethod($method, $id)
414416
: Error::forInvalidRequest($request->getMessage(), $id),
415417
$unknownMethod ? 404 : 400,
416418
);
@@ -510,7 +512,28 @@ private function dispatch(string $method, array $decoded, RequestMeta $meta, str
510512
return $this->encode($method, $id, $result->result, null === $input);
511513
}
512514

513-
return StatelessResult::error(Error::forMethodNotFound(\sprintf('No handler found for method "%s".', $method), $id), 404);
515+
return StatelessResult::error($this->unknownMethod($method, $id), 404);
516+
}
517+
518+
/**
519+
* A method with no handler, said as precisely as the server can.
520+
*
521+
* An extension's method is still `-32601` when the extension is off — the
522+
* server genuinely does not implement it — but naming the extension turns
523+
* an opaque refusal into something the caller can act on.
524+
*/
525+
private function unknownMethod(string $method, string|int $id): Error
526+
{
527+
$extension = $this->extensionMethods[$method] ?? null;
528+
529+
if (null !== $extension) {
530+
return Error::forMethodNotFound(
531+
\sprintf('Method "%s" belongs to the "%s" extension, which this server does not serve.', $method, $extension),
532+
$id,
533+
);
534+
}
535+
536+
return Error::forMethodNotFound(\sprintf('No handler found for method "%s".', $method), $id);
514537
}
515538

516539
/**

tests/Unit/Server/BuilderTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,15 @@ public function testEnableExtensionRegistersItsMessages(): void
203203
$this->assertInstanceOf(ThingListRequest::class, $decoded[0]);
204204
}
205205

206+
#[TestDox('enableExtension() throws when two enabled extensions define the same RPC method')]
207+
public function testEnableExtensionRejectsClaimedMethod(): void
208+
{
209+
$this->expectException(LogicException::class);
210+
$this->expectExceptionMessage('com.example/things.list');
211+
212+
Server::builder()->enableExtension(new ThingExtension('com.example/things-a'), new ThingExtension('com.example/things-b'));
213+
}
214+
206215
#[TestDox('A method-providing extension contributes the handlers serving its methods')]
207216
public function testEnableExtensionRegistersItsHandlers(): void
208217
{
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the official PHP MCP SDK.
5+
*
6+
* A collaboration between Symfony and the PHP Foundation.
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Mcp\Tests\Unit\Server\Extension;
13+
14+
use Mcp\Schema\Extension\ExtensionIdentifier;
15+
use Mcp\Schema\Extension\ExtensionInterface;
16+
17+
/**
18+
* An extension that declares a method it does not serve: {@see ThingListRequest}
19+
* is registered so the method decodes, but no handler answers it.
20+
*/
21+
final class UnservedThingExtension implements ExtensionInterface
22+
{
23+
public function getId(): ExtensionIdentifier
24+
{
25+
return new ExtensionIdentifier('com.example/unserved-things');
26+
}
27+
28+
public function getCapabilities(): array
29+
{
30+
return [];
31+
}
32+
33+
public function getMessages(): array
34+
{
35+
return [ThingListRequest::class];
36+
}
37+
38+
public function getRequestHandlers(): iterable
39+
{
40+
return [];
41+
}
42+
}

tests/Unit/Server/Stateless/StatelessProtocolTest.php

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
use Mcp\Server\Stateless\StatelessResult;
3535
use Mcp\Server\Subscription\InMemoryNotificationBus;
3636
use Mcp\Server\Wire\CachePolicy;
37+
use Mcp\Tests\Unit\Server\Extension\ThingExtension;
38+
use Mcp\Tests\Unit\Server\Extension\UnservedThingExtension;
3739
use PHPUnit\Framework\Attributes\DataProvider;
3840
use PHPUnit\Framework\Attributes\TestDox;
3941
use PHPUnit\Framework\TestCase;
@@ -791,6 +793,74 @@ public function testAcknowledgmentReflectsWhatTheServerCanDo(): void
791793
$this->assertSame(['toolsListChanged' => true], (array) $first['params']['notifications']);
792794
}
793795

796+
#[TestDox('an extension method is served by the extension that claims it')]
797+
public function testExtensionMethodIsServed(): void
798+
{
799+
$protocol = Server::builder()
800+
->setServerInfo('test-server', '1.0.0')
801+
->enableExtension(new ThingExtension())
802+
->buildStateless([ProtocolVersion::V2026_07_28]);
803+
804+
$answer = self::callWithHeaders($protocol, 'com.example/things.list', [], [
805+
'MCP-Protocol-Version' => ProtocolVersion::V2026_07_28->value,
806+
'Mcp-Method' => 'com.example/things.list',
807+
]);
808+
809+
$this->assertSame(200, $answer['status']);
810+
$this->assertSame(['a', 'b'], $answer['body']['result']['things']);
811+
}
812+
813+
#[TestDox('the extension is advertised under capabilities.extensions')]
814+
public function testExtensionIsAdvertised(): void
815+
{
816+
$protocol = Server::builder()
817+
->setServerInfo('test-server', '1.0.0')
818+
->enableExtension(new ThingExtension())
819+
->buildStateless([ProtocolVersion::V2026_07_28]);
820+
821+
$answer = self::call($protocol, 'server/discover');
822+
823+
$this->assertSame(['flavour' => 'vanilla'], (array) $answer['body']['result']['capabilities']['extensions']['com.example/things']);
824+
}
825+
826+
#[TestDox('a method of an extension this server has never heard of stays generic')]
827+
public function testUnknownExtensionMethodStaysGeneric(): void
828+
{
829+
$protocol = Server::builder()
830+
->setServerInfo('test-server', '1.0.0')
831+
->buildStateless([ProtocolVersion::V2026_07_28]);
832+
833+
$answer = self::callWithHeaders($protocol, 'com.example/things.list', [], [
834+
'MCP-Protocol-Version' => ProtocolVersion::V2026_07_28->value,
835+
'Mcp-Method' => 'com.example/things.list',
836+
]);
837+
838+
$this->assertSame(404, $answer['status']);
839+
$this->assertSame(Error::METHOD_NOT_FOUND, $answer['body']['error']['code']);
840+
// The extension was never enabled, so it never entered the method map
841+
// — there is nothing to name it by.
842+
$this->assertStringContainsString('com.example/things.list', $answer['body']['error']['message']);
843+
$this->assertStringNotContainsString('extension', $answer['body']['error']['message']);
844+
}
845+
846+
#[TestDox('a method of an extension this server does not serve says so by name')]
847+
public function testUnservedExtensionMethodNamesItsExtension(): void
848+
{
849+
$protocol = Server::builder()
850+
->setServerInfo('test-server', '1.0.0')
851+
->enableExtension(new UnservedThingExtension())
852+
->buildStateless([ProtocolVersion::V2026_07_28]);
853+
854+
$answer = self::callWithHeaders($protocol, 'com.example/things.list', [], [
855+
'MCP-Protocol-Version' => ProtocolVersion::V2026_07_28->value,
856+
'Mcp-Method' => 'com.example/things.list',
857+
]);
858+
859+
$this->assertSame(404, $answer['status']);
860+
$this->assertSame(Error::METHOD_NOT_FOUND, $answer['body']['error']['code']);
861+
$this->assertStringContainsString('com.example/unserved-things', $answer['body']['error']['message']);
862+
}
863+
794864
#[TestDox('a notification is acknowledged with no body, never answered')]
795865
public function testNotificationIsAcknowledged(): void
796866
{

0 commit comments

Comments
 (0)