Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/Capability/Registry.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
10 changes: 8 additions & 2 deletions src/Server/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
Expand Down
68 changes: 68 additions & 0 deletions tests/Unit/Capability/RegistryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
39 changes: 39 additions & 0 deletions tests/Unit/Server/BuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}
}

/**
Expand Down