diff --git a/src/Capability/Registry.php b/src/Capability/Registry.php index 794193f8..97840431 100644 --- a/src/Capability/Registry.php +++ b/src/Capability/Registry.php @@ -88,10 +88,27 @@ public function load(): void return; } + $this->loadFrom($this->loader); + + $this->loaded = true; + } + + /** + * Runs $loader with the change events its registrations would dispatch suppressed, since they + * describe the registry filling up rather than changing. + * + * Re-entrant-safe, and failure propagates. Does not mark the registry loaded: $loader is the + * caller's, and the one the constructor took is still owed its run. + */ + public function loadFrom(LoaderInterface $loader): void + { + if ($this->loading) { + return; + } + $this->loading = true; try { - $this->loader->load($this); - $this->loaded = true; + $loader->load($this); } finally { $this->loading = false; } diff --git a/src/Server/Builder.php b/src/Server/Builder.php index 90272ef3..1a5dd73b 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -522,7 +522,8 @@ public function setRegistry(RegistryInterface $registry): self * * Lazy (the default) defers loading to the first registry read so a persistent runtime does not * freeze the registry to a source not yet ready at build time. Disable to load eagerly at build. - * A registry supplied via setRegistry() is always loaded eagerly. + * A registry supplied via setRegistry() is always loaded eagerly; its own constructor loader, + * if it has one, still runs on the first read. */ public function setLazyLoading(bool $lazyLoading = true): self { @@ -1045,8 +1046,13 @@ private function resolve(): array if ($this->hasCustomRegistry) { // Builder can't inject the loader into an already-constructed instance, so load it eagerly. + // Via loadFrom(), which suppresses the change events the load would otherwise dispatch. $registry = $this->registry; - $chainLoader->load($registry); + if ($registry instanceof Registry) { + $registry->loadFrom($chainLoader); + } else { + $chainLoader->load($registry); + } $eagerlyLoaded = true; } else { $registry = new Registry($eventDispatcher, $logger, loader: $chainLoader); diff --git a/tests/Unit/Capability/RegistryTest.php b/tests/Unit/Capability/RegistryTest.php index cb325565..92782ab9 100644 --- a/tests/Unit/Capability/RegistryTest.php +++ b/tests/Unit/Capability/RegistryTest.php @@ -753,6 +753,74 @@ public function load(RegistryInterface $registry): void $this->assertTrue($registry->hasPrompts()); } + public function testListChangedEventsAreSuppressedWhenTheLoaderIsSuppliedFromOutside(): void + { + // As quiet as the deferred load above: these are the initial contents, not a change. + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->never())->method('dispatch'); + + $registry = new Registry($eventDispatcher, $this->logger); + $registry->loadFrom($this->toolLoader($this->createValidTool('loaded'))); + + $this->assertTrue($registry->hasTool('loaded')); + } + + public function testARuntimeRegistrationAfterAnExternalLoadIsStillDispatched(): void + { + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->once()) + ->method('dispatch') + ->with($this->isInstanceOf(ToolListChangedEvent::class)) + ->willReturnArgument(0); + + $registry = new Registry($eventDispatcher, $this->logger); + $registry->loadFrom($this->toolLoader($this->createValidTool('loaded'))); + + $registry->registerTool($this->createValidTool('runtime'), 'handler'); + } + + public function testAnExternalLoadDoesNotConsumeTheRegistrysOwnLoader(): void + { + $registry = new Registry($this->createMock(EventDispatcherInterface::class), $this->logger, loader: $this->toolLoader($this->createValidTool('own'))); + $registry->loadFrom($this->toolLoader($this->createValidTool('external'))); + + $this->assertTrue($registry->hasTool('external')); + $this->assertTrue($registry->hasTool('own')); + } + + public function testAnExternalLoadCanRunMoreThanOnce(): void + { + $registry = new Registry($this->createMock(EventDispatcherInterface::class), $this->logger); + $registry->loadFrom($this->toolLoader($this->createValidTool('first'))); + $registry->loadFrom($this->toolLoader($this->createValidTool('second'))); + + $this->assertTrue($registry->hasTool('first')); + $this->assertTrue($registry->hasTool('second')); + } + + public function testAFailedExternalLoadLeavesTheRegistryRetryable(): void + { + $registry = new Registry($this->createMock(EventDispatcherInterface::class), $this->logger); + $failing = new class implements LoaderInterface { + public function load(RegistryInterface $registry): void + { + // Reads during its own run, as discovery's identity check does. + $registry->hasTool('anything'); + + throw new \RuntimeException('data source not ready'); + } + }; + + foreach ([1, 2] as $attempt) { + try { + $registry->loadFrom($failing); + $this->fail('The loader was expected to fail.'); + } catch (\RuntimeException $e) { + $this->assertSame('data source not ready', $e->getMessage()); + } + } + } + public function testListChangedEventsAreStillDispatchedForRuntimeRegistrations(): void { $eventDispatcher = $this->createMock(EventDispatcherInterface::class); diff --git a/tests/Unit/Server/BuilderTest.php b/tests/Unit/Server/BuilderTest.php index 9cdf3292..790af5f5 100644 --- a/tests/Unit/Server/BuilderTest.php +++ b/tests/Unit/Server/BuilderTest.php @@ -15,6 +15,7 @@ use Mcp\Capability\Registry\ElementReference; use Mcp\Capability\Registry\Loader\LoaderInterface; use Mcp\Capability\Registry\ReferenceHandlerInterface; +use Mcp\Capability\RegistryInterface; use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\LogicException; use Mcp\Schema\Content\TextContent; @@ -32,6 +33,8 @@ use Mcp\Server\Protocol; use Mcp\Server\Session\SessionInterface; use Mcp\Server\Stateless\StatelessProtocol; +use Mcp\Server\Subscription\InMemoryNotificationBus; +use Mcp\Server\Subscription\PublishingEventDispatcher; use Mcp\Tests\Unit\Server\Extension\ThingExtension; use Mcp\Tests\Unit\Server\Extension\ThingListHandler; use Mcp\Tests\Unit\Server\Extension\ThingListRequest; @@ -404,6 +407,42 @@ private function callTool(Server $server, string $toolName): mixed $this->fail('CallToolHandler not found in request handlers'); } + + public function testBuildingWithASuppliedRegistryDoesNotAnnounceTheLoadAsAChange(): void + { + // Otherwise every build publishes one list_changed per element, for no change. + $bus = new InMemoryNotificationBus(); + $registry = new Registry(new PublishingEventDispatcher($bus)); + + Server::builder() + ->setRegistry($registry) + ->setNotificationBus($bus) + ->addTool(static fn (): string => 'ok', 'alpha') + ->addTool(static fn (): string => 'ok', 'beta') + ->build(); + + $this->assertSame(0, $bus->cursor()); + $this->assertTrue($registry->hasTool('alpha')); + $this->assertTrue($registry->hasTool('beta')); + + // A real change after the build is still published. + $registry->unregisterTool('alpha'); + + $this->assertSame(1, $bus->cursor()); + } + + public function testAThirdPartyRegistryIsStillLoadedThroughThePlainLoader(): void + { + $registry = $this->createMock(RegistryInterface::class); + $registry->expects($this->once()) + ->method('registerTool') + ->with($this->callback(static fn (Tool $tool): bool => 'alpha' === $tool->name)); + + Server::builder() + ->setRegistry($registry) + ->addTool(static fn (): string => 'ok', 'alpha') + ->build(); + } } /**