From 2c2456f4ba62488372a60f8b31c35a5b6f2e0e6f Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Mon, 11 May 2026 21:49:05 +0200 Subject: [PATCH] [Server] Untangle origin tracking from Registry Drop the isManual flag from ElementReference and the discovery-state methods from RegistryInterface. The Registry becomes a flat last-write-wins map; DiscoveryLoader owns its own owned-set bookkeeping so re-running discovery only removes elements it itself contributed. Adds ChainLoader for explicit composition. Manual-over-discovered precedence is preserved via loader ordering, not a flag on the reference. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 8 + src/Capability/Discovery/Discoverer.php | 8 +- src/Capability/Discovery/DiscoveryState.php | 17 ++ src/Capability/Registry.php | 215 +++++-------- src/Capability/Registry/ElementReference.php | 1 - .../Registry/Loader/ArrayLoader.php | 8 +- .../Registry/Loader/ChainLoader.php | 38 +++ .../Registry/Loader/DiscoveryLoader.php | 151 +++++++++- src/Capability/Registry/PromptReference.php | 3 +- src/Capability/Registry/ResourceReference.php | 3 +- .../Registry/ResourceTemplateReference.php | 3 +- src/Capability/Registry/ToolReference.php | 3 +- src/Capability/RegistryInterface.php | 47 ++- src/Server/Builder.php | 20 +- ...mbinedRegistrationTest-resources_list.json | 4 +- ...onTest-resources_read-config_priority.json | 2 +- .../Discovery/DiscoveryStateTest.php | 164 ++++++++++ .../Capability/Discovery/DiscoveryTest.php | 10 - .../Registry/Loader/ChainLoaderTest.php | 57 ++++ .../Registry/Loader/DiscoveryLoaderTest.php | 284 ++++++++++++++++++ .../Loader/Stub/MutableDiscoverer.php | 27 ++ .../Registry/Loader/Stub/RecordingLoader.php | 30 ++ .../Registry/Loader/Stub/ToolWriterLoader.php | 37 +++ tests/Unit/Capability/RegistryTest.php | 201 ++++--------- 24 files changed, 1003 insertions(+), 338 deletions(-) create mode 100644 src/Capability/Registry/Loader/ChainLoader.php create mode 100644 tests/Unit/Capability/Discovery/DiscoveryStateTest.php create mode 100644 tests/Unit/Capability/Registry/Loader/ChainLoaderTest.php create mode 100644 tests/Unit/Capability/Registry/Loader/DiscoveryLoaderTest.php create mode 100644 tests/Unit/Capability/Registry/Loader/Stub/MutableDiscoverer.php create mode 100644 tests/Unit/Capability/Registry/Loader/Stub/RecordingLoader.php create mode 100644 tests/Unit/Capability/Registry/Loader/Stub/ToolWriterLoader.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f025385..1451fd87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ All notable changes to `mcp/sdk` will be documented in this file. ----- * Allow overriding the default name pattern for Discovery +* Add `ChainLoader` to compose multiple `LoaderInterface` implementations via explicit ordering. +* Add `RegistryInterface::unregisterTool()`, `unregisterResource()`, `unregisterResourceTemplate()`, `unregisterPrompt()` — idempotent removals. +* Add `RegistryInterface::hasTool()`, `hasResource()`, `hasResourceTemplate()`, `hasPrompt()` — by-name existence checks. +* `DiscoveryLoader` now refreshes only its own previously written entries; manual registrations (via `Builder::addTool()` etc. or runtime `$registry->registerTool()` calls) survive rediscovery, and a same-name manual registration takes precedence over discovery on collision. +* [BC Break] Removed `ElementReference::$isManual` public property and the `bool $isManual` parameter from all `*Reference` constructors. Origin tracking is no longer carried on the element; manual-over-discovered precedence is encoded by loader execution order. +* [BC Break] `RegistryInterface::registerTool()`, `registerResource()`, `registerResourceTemplate()`, `registerPrompt()` lost their trailing `bool $isManual = false` parameter. Callers using positional arguments must drop the flag. +* [BC Break] Removed `RegistryInterface::clear()`, `getDiscoveryState()`, `setDiscoveryState()`. Rediscovery now goes through `DiscoveryLoader::load()` directly. +* `Registry::register*()` semantics changed to plain last-write-wins (overwrites silently) and the methods now return the stored `*Reference`. The previous "discovered registration is ignored when a manual one already exists" precedence rule still applies, but is now enforced by `DiscoveryLoader` via reference-identity tracking — and still emits a debug log when a discovery is skipped due to a conflicting registration. 0.5.0 ----- diff --git a/src/Capability/Discovery/Discoverer.php b/src/Capability/Discovery/Discoverer.php index b3216087..0b71c368 100644 --- a/src/Capability/Discovery/Discoverer.php +++ b/src/Capability/Discovery/Discoverer.php @@ -243,7 +243,7 @@ private function processMethod(\ReflectionMethod $method, array &$discoveredCoun meta: $instance->meta, outputSchema: $outputSchema, ); - $tools[$name] = new ToolReference($tool, [$className, $methodName], false); + $tools[$name] = new ToolReference($tool, [$className, $methodName]); ++$discoveredCount['tools']; break; @@ -261,7 +261,7 @@ private function processMethod(\ReflectionMethod $method, array &$discoveredCoun $instance->icons, $instance->meta, ); - $resources[$instance->uri] = new ResourceReference($resource, [$className, $methodName], false); + $resources[$instance->uri] = new ResourceReference($resource, [$className, $methodName]); ++$discoveredCount['resources']; break; @@ -282,7 +282,7 @@ private function processMethod(\ReflectionMethod $method, array &$discoveredCoun } $prompt = new Prompt($name, $instance->title, $description, $arguments, $instance->icons, $instance->meta); $completionProviders = $this->getCompletionProviders($method); - $prompts[$name] = new PromptReference($prompt, [$className, $methodName], false, $completionProviders); + $prompts[$name] = new PromptReference($prompt, [$className, $methodName], $completionProviders); ++$discoveredCount['prompts']; break; @@ -295,7 +295,7 @@ private function processMethod(\ReflectionMethod $method, array &$discoveredCoun $meta = $instance->meta ?? null; $resourceTemplate = new ResourceTemplate($instance->uriTemplate, $name, $description, $mimeType, $annotations, $meta); $completionProviders = $this->getCompletionProviders($method); - $resourceTemplates[$instance->uriTemplate] = new ResourceTemplateReference($resourceTemplate, [$className, $methodName], false, $completionProviders); + $resourceTemplates[$instance->uriTemplate] = new ResourceTemplateReference($resourceTemplate, [$className, $methodName], $completionProviders); ++$discoveredCount['resourceTemplates']; break; } diff --git a/src/Capability/Discovery/DiscoveryState.php b/src/Capability/Discovery/DiscoveryState.php index e5c089e5..59a73ec8 100644 --- a/src/Capability/Discovery/DiscoveryState.php +++ b/src/Capability/Discovery/DiscoveryState.php @@ -72,6 +72,23 @@ public function getResourceTemplates(): array return $this->resourceTemplates; } + /** + * Returns the subset of this state whose keys are absent from $next. + * + * Asymmetric by design: entries whose keys exist in both states are excluded + * regardless of value. Used to identify owned entries that a fresh discovery + * no longer produces. + */ + public function obsoletedBy(self $next): self + { + return new self( + array_diff_key($this->tools, $next->tools), + array_diff_key($this->resources, $next->resources), + array_diff_key($this->prompts, $next->prompts), + array_diff_key($this->resourceTemplates, $next->resourceTemplates), + ); + } + /** * Check if this state contains any discovered elements. */ diff --git a/src/Capability/Registry.php b/src/Capability/Registry.php index 2a327ae4..c8fdca3e 100644 --- a/src/Capability/Registry.php +++ b/src/Capability/Registry.php @@ -11,7 +11,6 @@ namespace Mcp\Capability; -use Mcp\Capability\Discovery\DiscoveryState; use Mcp\Capability\Registry\PromptReference; use Mcp\Capability\Registry\ResourceReference; use Mcp\Capability\Registry\ResourceTemplateReference; @@ -68,129 +67,120 @@ public function __construct( ) { } - public function registerTool(Tool $tool, callable|array|string $handler, bool $isManual = false): void + public function registerTool(Tool $tool, callable|array|string $handler): ToolReference { - $toolName = $tool->name; - $existing = $this->tools[$toolName] ?? null; - - if ($existing && !$isManual && $existing->isManual) { - $this->logger->debug( - \sprintf('Ignoring discovered tool "%s" as it conflicts with a manually registered one.', $toolName), - ); - - return; - } - - if (!$this->nameValidator->isValid($toolName)) { + if (!$this->nameValidator->isValid($tool->name)) { $this->logger->warning( - \sprintf('Tool name "%s" is invalid. Tool names should only contain letters (a-z, A-Z), numbers, dots, hyphens, underscores, and forward slashes.', $toolName), + \sprintf('Tool name "%s" is invalid. Tool names should only contain letters (a-z, A-Z), numbers, dots, hyphens, underscores, and forward slashes.', $tool->name), ); } - $this->tools[$toolName] = new ToolReference($tool, $handler, $isManual); + $reference = new ToolReference($tool, $handler); + $this->tools[$tool->name] = $reference; $this->eventDispatcher?->dispatch(new ToolListChangedEvent()); + + return $reference; } - public function registerResource(Resource $resource, callable|array|string $handler, bool $isManual = false): void + public function registerResource(Resource $resource, callable|array|string $handler): ResourceReference { - $uri = $resource->uri; - $existing = $this->resources[$uri] ?? null; - - if ($existing && !$isManual && $existing->isManual) { - $this->logger->debug( - \sprintf('Ignoring discovered resource "%s" as it conflicts with a manually registered one.', $uri), - ); - - return; - } - - $this->resources[$uri] = new ResourceReference($resource, $handler, $isManual); + $reference = new ResourceReference($resource, $handler); + $this->resources[$resource->uri] = $reference; $this->eventDispatcher?->dispatch(new ResourceListChangedEvent()); + + return $reference; } public function registerResourceTemplate( ResourceTemplate $template, callable|array|string $handler, array $completionProviders = [], - bool $isManual = false, - ): void { - $uriTemplate = $template->uriTemplate; - $existing = $this->resourceTemplates[$uriTemplate] ?? null; - - if ($existing && !$isManual && $existing->isManual) { - $this->logger->debug( - \sprintf('Ignoring discovered template "%s" as it conflicts with a manually registered one.', $uriTemplate), - ); - - return; - } - - $this->resourceTemplates[$uriTemplate] = new ResourceTemplateReference( - $template, - $handler, - $isManual, - $completionProviders, - ); + ): ResourceTemplateReference { + $reference = new ResourceTemplateReference($template, $handler, $completionProviders); + $this->resourceTemplates[$template->uriTemplate] = $reference; $this->eventDispatcher?->dispatch(new ResourceTemplateListChangedEvent()); + + return $reference; } public function registerPrompt( Prompt $prompt, callable|array|string $handler, array $completionProviders = [], - bool $isManual = false, - ): void { - $promptName = $prompt->name; - $existing = $this->prompts[$promptName] ?? null; - - if ($existing && !$isManual && $existing->isManual) { - $this->logger->debug( - \sprintf('Ignoring discovered prompt "%s" as it conflicts with a manually registered one.', $promptName), - ); + ): PromptReference { + $reference = new PromptReference($prompt, $handler, $completionProviders); + $this->prompts[$prompt->name] = $reference; + $this->eventDispatcher?->dispatch(new PromptListChangedEvent()); + + return $reference; + } + + public function unregisterTool(string $name): void + { + if (!isset($this->tools[$name])) { return; } - $this->prompts[$promptName] = new PromptReference($prompt, $handler, $isManual, $completionProviders); + unset($this->tools[$name]); - $this->eventDispatcher?->dispatch(new PromptListChangedEvent()); + $this->eventDispatcher?->dispatch(new ToolListChangedEvent()); } - public function clear(): void + public function unregisterResource(string $uri): void { - $clearCount = 0; - - foreach ($this->tools as $name => $tool) { - if (!$tool->isManual) { - unset($this->tools[$name]); - ++$clearCount; - } - } - foreach ($this->resources as $uri => $resource) { - if (!$resource->isManual) { - unset($this->resources[$uri]); - ++$clearCount; - } - } - foreach ($this->prompts as $name => $prompt) { - if (!$prompt->isManual) { - unset($this->prompts[$name]); - ++$clearCount; - } + if (!isset($this->resources[$uri])) { + return; } - foreach ($this->resourceTemplates as $uriTemplate => $template) { - if (!$template->isManual) { - unset($this->resourceTemplates[$uriTemplate]); - ++$clearCount; - } + + unset($this->resources[$uri]); + + $this->eventDispatcher?->dispatch(new ResourceListChangedEvent()); + } + + public function unregisterResourceTemplate(string $uriTemplate): void + { + if (!isset($this->resourceTemplates[$uriTemplate])) { + return; } - if ($clearCount > 0) { - $this->logger->debug(\sprintf('Removed %d discovered elements from internal registry.', $clearCount)); + unset($this->resourceTemplates[$uriTemplate]); + + $this->eventDispatcher?->dispatch(new ResourceTemplateListChangedEvent()); + } + + public function unregisterPrompt(string $name): void + { + if (!isset($this->prompts[$name])) { + return; } + + unset($this->prompts[$name]); + + $this->eventDispatcher?->dispatch(new PromptListChangedEvent()); + } + + public function hasTool(string $name): bool + { + return isset($this->tools[$name]); + } + + public function hasResource(string $uri): bool + { + return isset($this->resources[$uri]); + } + + public function hasResourceTemplate(string $uriTemplate): bool + { + return isset($this->resourceTemplates[$uriTemplate]); + } + + public function hasPrompt(string $name): bool + { + return isset($this->prompts[$name]); } public function hasTools(): bool @@ -338,59 +328,6 @@ public function getPrompt(string $name): PromptReference return $this->prompts[$name] ?? throw new PromptNotFoundException($name); } - /** - * Get the current discovery state (only discovered elements, not manual ones). - */ - public function getDiscoveryState(): DiscoveryState - { - return new DiscoveryState( - tools: array_filter($this->tools, static fn ($tool) => !$tool->isManual), - resources: array_filter($this->resources, static fn ($resource) => !$resource->isManual), - prompts: array_filter($this->prompts, static fn ($prompt) => !$prompt->isManual), - resourceTemplates: array_filter($this->resourceTemplates, static fn ($template) => !$template->isManual), - ); - } - - /** - * Set the discovery state, replacing all discovered elements. - * Manual elements are preserved. - */ - public function setDiscoveryState(DiscoveryState $state): void - { - // Clear existing discovered elements - $this->clear(); - - // Import new discovered elements - foreach ($state->getTools() as $name => $tool) { - $this->tools[$name] = $tool; - } - - foreach ($state->getResources() as $uri => $resource) { - $this->resources[$uri] = $resource; - } - - foreach ($state->getPrompts() as $name => $prompt) { - $this->prompts[$name] = $prompt; - } - - foreach ($state->getResourceTemplates() as $uriTemplate => $template) { - $this->resourceTemplates[$uriTemplate] = $template; - } - - // Dispatch events for the imported elements - if ($this->eventDispatcher instanceof EventDispatcherInterface) { - if (!empty($state->getTools())) { - $this->eventDispatcher->dispatch(new ToolListChangedEvent()); - } - if (!empty($state->getResources()) || !empty($state->getResourceTemplates())) { - $this->eventDispatcher->dispatch(new ResourceListChangedEvent()); - } - if (!empty($state->getPrompts())) { - $this->eventDispatcher->dispatch(new PromptListChangedEvent()); - } - } - } - /** * Calculate next cursor for pagination. * diff --git a/src/Capability/Registry/ElementReference.php b/src/Capability/Registry/ElementReference.php index 6425ba13..a49b2eb3 100644 --- a/src/Capability/Registry/ElementReference.php +++ b/src/Capability/Registry/ElementReference.php @@ -23,7 +23,6 @@ class ElementReference */ public function __construct( public readonly \Closure|array|string $handler, - public readonly bool $isManual = false, ) { } } diff --git a/src/Capability/Registry/Loader/ArrayLoader.php b/src/Capability/Registry/Loader/ArrayLoader.php index 34055a2d..bce3adce 100644 --- a/src/Capability/Registry/Loader/ArrayLoader.php +++ b/src/Capability/Registry/Loader/ArrayLoader.php @@ -124,7 +124,7 @@ public function load(RegistryInterface $registry): void meta: $data['meta'] ?? null, outputSchema: $data['outputSchema'] ?? null, ); - $registry->registerTool($tool, $data['handler'], true); + $registry->registerTool($tool, $data['handler']); $handlerDesc = $this->getHandlerDescription($data['handler']); $this->logger->debug("Registered manual tool {$name} from handler {$handlerDesc}"); @@ -164,7 +164,7 @@ public function load(RegistryInterface $registry): void icons: $data['icons'] ?? null, meta: $data['meta'] ?? null, ); - $registry->registerResource($resource, $data['handler'], true); + $registry->registerResource($resource, $data['handler']); $handlerDesc = $this->getHandlerDescription($data['handler']); $this->logger->debug("Registered manual resource {$name} from handler {$handlerDesc}"); @@ -203,7 +203,7 @@ public function load(RegistryInterface $registry): void meta: $data['meta'] ?? null, ); $completionProviders = $this->getCompletionProviders($reflection); - $registry->registerResourceTemplate($template, $data['handler'], $completionProviders, true); + $registry->registerResourceTemplate($template, $data['handler'], $completionProviders); $handlerDesc = $this->getHandlerDescription($data['handler']); $this->logger->debug("Registered manual template {$name} from handler {$handlerDesc}"); @@ -261,7 +261,7 @@ public function load(RegistryInterface $registry): void meta: $data['meta'] ?? null ); $completionProviders = $this->getCompletionProviders($reflection); - $registry->registerPrompt($prompt, $data['handler'], $completionProviders, true); + $registry->registerPrompt($prompt, $data['handler'], $completionProviders); $handlerDesc = $this->getHandlerDescription($data['handler']); $this->logger->debug("Registered manual prompt {$name} from handler {$handlerDesc}"); diff --git a/src/Capability/Registry/Loader/ChainLoader.php b/src/Capability/Registry/Loader/ChainLoader.php new file mode 100644 index 00000000..6c30a160 --- /dev/null +++ b/src/Capability/Registry/Loader/ChainLoader.php @@ -0,0 +1,38 @@ + + */ +final class ChainLoader implements LoaderInterface +{ + /** + * @param LoaderInterface[] $loaders + */ + public function __construct( + private readonly array $loaders, + ) { + } + + public function load(RegistryInterface $registry): void + { + foreach ($this->loaders as $loader) { + $loader->load($registry); + } + } +} diff --git a/src/Capability/Registry/Loader/DiscoveryLoader.php b/src/Capability/Registry/Loader/DiscoveryLoader.php index a2ec40f9..d2b95384 100644 --- a/src/Capability/Registry/Loader/DiscoveryLoader.php +++ b/src/Capability/Registry/Loader/DiscoveryLoader.php @@ -12,7 +12,10 @@ namespace Mcp\Capability\Registry\Loader; use Mcp\Capability\Discovery\DiscovererInterface; +use Mcp\Capability\Discovery\DiscoveryState; use Mcp\Capability\RegistryInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; /** * @internal @@ -21,10 +24,12 @@ */ final class DiscoveryLoader implements LoaderInterface { + private DiscoveryState $owned; + /** - * @param string[] $scanDirs - * @param array|string[] $excludeDirs - * @param string[] $namePatterns + * @param string[] $scanDirs + * @param string[] $excludeDirs + * @param string[] $namePatterns */ public function __construct( private string $basePath, @@ -32,13 +37,149 @@ public function __construct( private array $excludeDirs, private DiscovererInterface $discoverer, private array $namePatterns = DiscovererInterface::DEFAULT_NAME_PATERNS, + private LoggerInterface $logger = new NullLogger(), ) { + $this->owned = new DiscoveryState(); } public function load(RegistryInterface $registry): void { - $discoveryState = $this->discoverer->discover($this->basePath, $this->scanDirs, $this->excludeDirs, $this->namePatterns); + $discovered = $this->discoverer->discover($this->basePath, $this->scanDirs, $this->excludeDirs, $this->namePatterns); + + $this->unregisterOwned($registry, $this->owned->obsoletedBy($discovered)); + $this->owned = $this->writeDiscovered($registry, $discovered); + } + + /** + * Unregisters entries we previously wrote that the registry still attributes to us. + * Entries overwritten by someone else are left untouched (identity check fails). + */ + private function unregisterOwned(RegistryInterface $registry, DiscoveryState $obsolete): void + { + foreach ($obsolete->getTools() as $name => $owned) { + if ($registry->hasTool($name) && $registry->getTool($name) === $owned) { + $registry->unregisterTool($name); + } + } + foreach ($obsolete->getResources() as $uri => $owned) { + if ($registry->hasResource($uri) && $registry->getResource($uri, false) === $owned) { + $registry->unregisterResource($uri); + } + } + foreach ($obsolete->getResourceTemplates() as $uriTemplate => $owned) { + if ($registry->hasResourceTemplate($uriTemplate) && $registry->getResourceTemplate($uriTemplate) === $owned) { + $registry->unregisterResourceTemplate($uriTemplate); + } + } + foreach ($obsolete->getPrompts() as $name => $owned) { + if ($registry->hasPrompt($name) && $registry->getPrompt($name) === $owned) { + $registry->unregisterPrompt($name); + } + } + } + + /** + * Writes the discovered state into the registry, skipping entries that a conflicting + * registration already holds. Returns the new owned state (only the writes we actually performed). + */ + private function writeDiscovered(RegistryInterface $registry, DiscoveryState $discovered): DiscoveryState + { + $tools = []; + foreach ($discovered->getTools() as $name => $reference) { + if (!$this->mayWriteTool($registry, $name)) { + continue; + } + $tools[$name] = $registry->registerTool($reference->tool, $reference->handler); + } + + $resources = []; + foreach ($discovered->getResources() as $uri => $reference) { + if (!$this->mayWriteResource($registry, $uri)) { + continue; + } + $resources[$uri] = $registry->registerResource($reference->resource, $reference->handler); + } + + $resourceTemplates = []; + foreach ($discovered->getResourceTemplates() as $uriTemplate => $reference) { + if (!$this->mayWriteResourceTemplate($registry, $uriTemplate)) { + continue; + } + $resourceTemplates[$uriTemplate] = $registry->registerResourceTemplate( + $reference->resourceTemplate, + $reference->handler, + $reference->completionProviders, + ); + } + + $prompts = []; + foreach ($discovered->getPrompts() as $name => $reference) { + if (!$this->mayWritePrompt($registry, $name)) { + continue; + } + $prompts[$name] = $registry->registerPrompt( + $reference->prompt, + $reference->handler, + $reference->completionProviders, + ); + } + + return new DiscoveryState($tools, $resources, $prompts, $resourceTemplates); + } + + private function mayWriteTool(RegistryInterface $registry, string $name): bool + { + if (!$registry->hasTool($name) || $registry->getTool($name) === ($this->owned->getTools()[$name] ?? null)) { + return true; + } + + $this->logger->debug(\sprintf( + 'Ignoring discovered tool "%s": a conflicting manual or runtime registration already exists.', + $name, + )); + + return false; + } + + private function mayWriteResource(RegistryInterface $registry, string $uri): bool + { + if (!$registry->hasResource($uri) || $registry->getResource($uri, false) === ($this->owned->getResources()[$uri] ?? null)) { + return true; + } + + $this->logger->debug(\sprintf( + 'Ignoring discovered resource "%s": a conflicting manual or runtime registration already exists.', + $uri, + )); + + return false; + } + + private function mayWriteResourceTemplate(RegistryInterface $registry, string $uriTemplate): bool + { + if (!$registry->hasResourceTemplate($uriTemplate) || $registry->getResourceTemplate($uriTemplate) === ($this->owned->getResourceTemplates()[$uriTemplate] ?? null)) { + return true; + } + + $this->logger->debug(\sprintf( + 'Ignoring discovered resource template "%s": a conflicting manual or runtime registration already exists.', + $uriTemplate, + )); + + return false; + } + + private function mayWritePrompt(RegistryInterface $registry, string $name): bool + { + if (!$registry->hasPrompt($name) || $registry->getPrompt($name) === ($this->owned->getPrompts()[$name] ?? null)) { + return true; + } + + $this->logger->debug(\sprintf( + 'Ignoring discovered prompt "%s": a conflicting manual or runtime registration already exists.', + $name, + )); - $registry->setDiscoveryState($discoveryState); + return false; } } diff --git a/src/Capability/Registry/PromptReference.php b/src/Capability/Registry/PromptReference.php index 5de3a195..ec7d0219 100644 --- a/src/Capability/Registry/PromptReference.php +++ b/src/Capability/Registry/PromptReference.php @@ -29,10 +29,9 @@ class PromptReference extends ElementReference public function __construct( public readonly Prompt $prompt, \Closure|array|string $handler, - bool $isManual = false, public readonly array $completionProviders = [], ) { - parent::__construct($handler, $isManual); + parent::__construct($handler); } /** diff --git a/src/Capability/Registry/ResourceReference.php b/src/Capability/Registry/ResourceReference.php index d65f461e..bec8b9b5 100644 --- a/src/Capability/Registry/ResourceReference.php +++ b/src/Capability/Registry/ResourceReference.php @@ -28,9 +28,8 @@ class ResourceReference extends ElementReference public function __construct( public readonly Resource $resource, callable|array|string $handler, - bool $isManual = false, ) { - parent::__construct($handler, $isManual); + parent::__construct($handler); } /** diff --git a/src/Capability/Registry/ResourceTemplateReference.php b/src/Capability/Registry/ResourceTemplateReference.php index ef2d915a..49d03b39 100644 --- a/src/Capability/Registry/ResourceTemplateReference.php +++ b/src/Capability/Registry/ResourceTemplateReference.php @@ -36,10 +36,9 @@ class ResourceTemplateReference extends ElementReference public function __construct( public readonly ResourceTemplate $resourceTemplate, callable|array|string $handler, - bool $isManual = false, public readonly array $completionProviders = [], ) { - parent::__construct($handler, $isManual); + parent::__construct($handler); $this->compileTemplate(); } diff --git a/src/Capability/Registry/ToolReference.php b/src/Capability/Registry/ToolReference.php index 9aa5a3c9..36c0835f 100644 --- a/src/Capability/Registry/ToolReference.php +++ b/src/Capability/Registry/ToolReference.php @@ -28,9 +28,8 @@ class ToolReference extends ElementReference public function __construct( public readonly Tool $tool, callable|array|string $handler, - bool $isManual = false, ) { - parent::__construct($handler, $isManual); + parent::__construct($handler); } /** diff --git a/src/Capability/RegistryInterface.php b/src/Capability/RegistryInterface.php index 67295681..124704b8 100644 --- a/src/Capability/RegistryInterface.php +++ b/src/Capability/RegistryInterface.php @@ -11,7 +11,6 @@ namespace Mcp\Capability; -use Mcp\Capability\Discovery\DiscoveryState; use Mcp\Capability\Registry\ElementReference; use Mcp\Capability\Registry\PromptReference; use Mcp\Capability\Registry\ResourceReference; @@ -35,21 +34,25 @@ interface RegistryInterface { /** - * Registers a tool with its handler. + * Registers a tool with its handler. Overwrites any prior registration of the same name. + * Returns the stored reference, whose identity callers may track to detect later overwrites. * * @param Handler $handler */ - public function registerTool(Tool $tool, callable|array|string $handler, bool $isManual = false): void; + public function registerTool(Tool $tool, callable|array|string $handler): ToolReference; /** - * Registers a resource with its handler. + * Registers a resource with its handler. Overwrites any prior registration of the same URI. + * Returns the stored reference, whose identity callers may track to detect later overwrites. * * @param Handler $handler */ - public function registerResource(Resource $resource, callable|array|string $handler, bool $isManual = false): void; + public function registerResource(Resource $resource, callable|array|string $handler): ResourceReference; /** * Registers a resource template with its handler and completion providers. + * Overwrites any prior registration of the same URI template. + * Returns the stored reference, whose identity callers may track to detect later overwrites. * * @param Handler $handler * @param array $completionProviders @@ -58,11 +61,12 @@ public function registerResourceTemplate( ResourceTemplate $template, callable|array|string $handler, array $completionProviders = [], - bool $isManual = false, - ): void; + ): ResourceTemplateReference; /** * Registers a prompt with its handler and completion providers. + * Overwrites any prior registration of the same name. + * Returns the stored reference, whose identity callers may track to detect later overwrites. * * @param Handler $handler * @param array $completionProviders @@ -71,24 +75,35 @@ public function registerPrompt( Prompt $prompt, callable|array|string $handler, array $completionProviders = [], - bool $isManual = false, - ): void; + ): PromptReference; /** - * Clear discovered elements from registry. + * Removes a tool by name. No-op if absent. */ - public function clear(): void; + public function unregisterTool(string $name): void; /** - * Get the current discovery state (only discovered elements, not manual ones). + * Removes a resource by URI. No-op if absent. */ - public function getDiscoveryState(): DiscoveryState; + public function unregisterResource(string $uri): void; /** - * Set discovery state, replacing all discovered elements. - * Manual elements are preserved. + * Removes a resource template by URI template. No-op if absent. */ - public function setDiscoveryState(DiscoveryState $state): void; + public function unregisterResourceTemplate(string $uriTemplate): void; + + /** + * Removes a prompt by name. No-op if absent. + */ + public function unregisterPrompt(string $name): void; + + public function hasTool(string $name): bool; + + public function hasResource(string $uri): bool; + + public function hasResourceTemplate(string $uriTemplate): bool; + + public function hasPrompt(string $name): bool; /** * @return bool true if any tools are registered diff --git a/src/Server/Builder.php b/src/Server/Builder.php index 280272e6..caff5785 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -19,6 +19,7 @@ use Mcp\Capability\Registry\Container; use Mcp\Capability\Registry\ElementReference; use Mcp\Capability\Registry\Loader\ArrayLoader; +use Mcp\Capability\Registry\Loader\ChainLoader; use Mcp\Capability\Registry\Loader\DiscoveryLoader; use Mcp\Capability\Registry\Loader\LoaderInterface; use Mcp\Capability\Registry\ReferenceHandler; @@ -526,28 +527,29 @@ public function build(): Server $container = $this->container ?? new Container(); $registry = $this->registry ?? new Registry($this->eventDispatcher, $logger); $subscriptionManager = $this->subscriptionManager ?? new SessionSubscriptionManager($logger); - $loaders = [ - ...$this->loaders, - new ArrayLoader($this->tools, $this->resources, $this->resourceTemplates, $this->prompts, $logger, $this->schemaGenerator), - ]; - $sessionManager = $this->sessionManager ?? new SessionManager( $this->sessionStore ?? new InMemorySessionStore(), $logger, ); + // ArrayLoader runs before DiscoveryLoader so manual entries are seen first; DiscoveryLoader's + // identity check then preserves them against same-name discovered entries. + $loaders = [ + ...$this->loaders, + new ArrayLoader($this->tools, $this->resources, $this->resourceTemplates, $this->prompts, $logger, $this->schemaGenerator), + ]; + if (null !== $this->discoveryBasePath) { if (null !== $this->discoverer || class_exists(Finder::class)) { $discoverer = $this->discoverer ?? $this->createDiscoverer($logger); - $loaders[] = new DiscoveryLoader($this->discoveryBasePath, $this->discoveryScanDirs, $this->discoveryExcludeDirs, $discoverer, $this->discoveryNamePatterns); + $loaders[] = new DiscoveryLoader($this->discoveryBasePath, $this->discoveryScanDirs, $this->discoveryExcludeDirs, $discoverer, $this->discoveryNamePatterns, $logger); } else { $logger->warning('File-based discovery requires symfony/finder. Skipping automatic discovery. Run: composer require symfony/finder'); } } - foreach ($loaders as $loader) { - $loader->load($registry); - } + $loader = 1 === \count($loaders) ? $loaders[0] : new ChainLoader($loaders); + $loader->load($registry); $messageFactory = MessageFactory::make(); diff --git a/tests/Inspector/Http/snapshots/HttpCombinedRegistrationTest-resources_list.json b/tests/Inspector/Http/snapshots/HttpCombinedRegistrationTest-resources_list.json index 2c7912e4..f3646e57 100644 --- a/tests/Inspector/Http/snapshots/HttpCombinedRegistrationTest-resources_list.json +++ b/tests/Inspector/Http/snapshots/HttpCombinedRegistrationTest-resources_list.json @@ -1,9 +1,9 @@ { "resources": [ { - "name": "priority_config_discovered", + "name": "priority_config_manual", "uri": "config://priority", - "description": "A resource discovered via attributes.\n\nThis will be overridden by a manual registration with the same URI." + "description": "Manually registered resource that overrides a discovered one." } ] } diff --git a/tests/Inspector/Http/snapshots/HttpCombinedRegistrationTest-resources_read-config_priority.json b/tests/Inspector/Http/snapshots/HttpCombinedRegistrationTest-resources_read-config_priority.json index 053dcceb..85eef65a 100644 --- a/tests/Inspector/Http/snapshots/HttpCombinedRegistrationTest-resources_read-config_priority.json +++ b/tests/Inspector/Http/snapshots/HttpCombinedRegistrationTest-resources_read-config_priority.json @@ -3,7 +3,7 @@ { "uri": "config://priority", "mimeType": "text/plain", - "text": "Discovered Priority Config: Low" + "text": "Manual Priority Config: HIGH (overrides discovered)" } ] } diff --git a/tests/Unit/Capability/Discovery/DiscoveryStateTest.php b/tests/Unit/Capability/Discovery/DiscoveryStateTest.php new file mode 100644 index 00000000..b5b60b12 --- /dev/null +++ b/tests/Unit/Capability/Discovery/DiscoveryStateTest.php @@ -0,0 +1,164 @@ +tool('t1'); + $t2 = $this->tool('t2'); + + $owned = new DiscoveryState( + tools: ['t1' => $t1, 't2' => $t2], + ); + $next = new DiscoveryState( + tools: ['t2' => $this->tool('t2'), 't3' => $this->tool('t3')], + ); + + $obsolete = $owned->obsoletedBy($next); + + $this->assertSame(['t1' => $t1], $obsolete->getTools()); + } + + public function testObsoletedByIsAsymmetricAndIgnoresValuesOnSharedKeys(): void + { + // Same key, different reference instance — must NOT be reported as obsolete. + $owned = new DiscoveryState( + tools: ['t' => $this->tool('t')], + ); + $next = new DiscoveryState( + tools: ['t' => $this->tool('t')], + ); + + $this->assertTrue($owned->obsoletedBy($next)->isEmpty()); + } + + public function testObsoletedByOnEmptyNextReturnsAllOwned(): void + { + $owned = new DiscoveryState( + tools: ['t' => $this->tool('t')], + resources: ['r://x' => $this->resource('r://x')], + prompts: ['p' => $this->prompt('p')], + resourceTemplates: ['x://{id}' => $this->template('x://{id}')], + ); + + $obsolete = $owned->obsoletedBy(new DiscoveryState()); + + $this->assertSame(['t'], array_keys($obsolete->getTools())); + $this->assertSame(['r://x'], array_keys($obsolete->getResources())); + $this->assertSame(['p'], array_keys($obsolete->getPrompts())); + $this->assertSame(['x://{id}'], array_keys($obsolete->getResourceTemplates())); + } + + public function testObsoletedByOnEmptyOwnedReturnsEmpty(): void + { + $next = new DiscoveryState( + tools: ['t' => $this->tool('t')], + ); + + $this->assertTrue((new DiscoveryState())->obsoletedBy($next)->isEmpty()); + } + + public function testObsoletedByKeepsKindsIndependent(): void + { + // A key shared across different kinds must not cancel out. + $owned = new DiscoveryState( + tools: ['shared' => $this->tool('shared')], + prompts: ['shared' => $this->prompt('shared')], + ); + $next = new DiscoveryState( + tools: ['shared' => $this->tool('shared')], + // 'shared' prompt is gone. + ); + + $obsolete = $owned->obsoletedBy($next); + + $this->assertSame([], $obsolete->getTools()); + $this->assertSame(['shared'], array_keys($obsolete->getPrompts())); + } + + public function testObsoletedByAcrossAllKindsAtOnce(): void + { + $owned = new DiscoveryState( + tools: ['keep_t' => $this->tool('keep_t'), 'drop_t' => $this->tool('drop_t')], + resources: ['r://keep' => $this->resource('r://keep'), 'r://drop' => $this->resource('r://drop')], + prompts: ['keep_p' => $this->prompt('keep_p'), 'drop_p' => $this->prompt('drop_p')], + resourceTemplates: ['keep://{id}' => $this->template('keep://{id}'), 'drop://{id}' => $this->template('drop://{id}')], + ); + $next = new DiscoveryState( + tools: ['keep_t' => $this->tool('keep_t')], + resources: ['r://keep' => $this->resource('r://keep')], + prompts: ['keep_p' => $this->prompt('keep_p')], + resourceTemplates: ['keep://{id}' => $this->template('keep://{id}')], + ); + + $obsolete = $owned->obsoletedBy($next); + + $this->assertSame(['drop_t'], array_keys($obsolete->getTools())); + $this->assertSame(['r://drop'], array_keys($obsolete->getResources())); + $this->assertSame(['drop_p'], array_keys($obsolete->getPrompts())); + $this->assertSame(['drop://{id}'], array_keys($obsolete->getResourceTemplates())); + } + + private function tool(string $name): ToolReference + { + return new ToolReference( + new Tool( + name: $name, + title: null, + inputSchema: ['type' => 'object', 'properties' => [], 'required' => null], + description: null, + annotations: null, + icons: null, + meta: null, + outputSchema: null, + ), + static fn () => null, + ); + } + + private function resource(string $uri): ResourceReference + { + return new ResourceReference( + new Resource(uri: $uri, name: 'r', description: null, mimeType: 'text/plain'), + static fn () => null, + ); + } + + private function prompt(string $name): PromptReference + { + return new PromptReference( + new Prompt(name: $name, description: null, arguments: []), + static fn () => [], + ); + } + + private function template(string $uriTemplate): ResourceTemplateReference + { + return new ResourceTemplateReference( + new ResourceTemplate(uriTemplate: $uriTemplate, name: 'tpl', description: null, mimeType: 'text/plain'), + static fn () => null, + ); + } +} diff --git a/tests/Unit/Capability/Discovery/DiscoveryTest.php b/tests/Unit/Capability/Discovery/DiscoveryTest.php index 1451361d..b4197262 100644 --- a/tests/Unit/Capability/Discovery/DiscoveryTest.php +++ b/tests/Unit/Capability/Discovery/DiscoveryTest.php @@ -41,7 +41,6 @@ public function testDiscoversAllElementTypesCorrectlyFromFixtureFiles(): void $this->assertCount(5, $tools); $this->assertArrayHasKey('greet_user', $tools); - $this->assertFalse($tools['greet_user']->isManual); $this->assertEquals('greet_user', $tools['greet_user']->tool->name); $this->assertEquals('Greets a user by name.', $tools['greet_user']->tool->description); $this->assertEquals([DiscoverableToolHandler::class, 'greet'], $tools['greet_user']->handler); @@ -54,7 +53,6 @@ public function testDiscoversAllElementTypesCorrectlyFromFixtureFiles(): void $this->assertArrayHasKey('InvokableCalculator', $tools); $this->assertInstanceOf(ToolReference::class, $tools['InvokableCalculator']); - $this->assertFalse($tools['InvokableCalculator']->isManual); $this->assertEquals([InvocableToolFixture::class, '__invoke'], $tools['InvokableCalculator']->handler); $this->assertArrayHasKey('inc_file_name_tool', $tools); @@ -68,44 +66,36 @@ public function testDiscoversAllElementTypesCorrectlyFromFixtureFiles(): void $this->assertCount(3, $resources); $this->assertArrayHasKey('app://info/version', $resources); - $this->assertFalse($resources['app://info/version']->isManual); $this->assertEquals('app_version', $resources['app://info/version']->resource->name); $this->assertEquals('text/plain', $resources['app://info/version']->resource->mimeType); $this->assertArrayHasKey('invokable://config/status', $resources); - $this->assertFalse($resources['invokable://config/status']->isManual); $this->assertEquals([InvocableResourceFixture::class, '__invoke'], $resources['invokable://config/status']->handler); $prompts = $discovery->getPrompts(); $this->assertCount(4, $prompts); $this->assertArrayHasKey('creative_story_prompt', $prompts); - $this->assertFalse($prompts['creative_story_prompt']->isManual); $this->assertCount(2, $prompts['creative_story_prompt']->prompt->arguments); $this->assertEquals(CompletionProviderFixture::class, $prompts['creative_story_prompt']->completionProviders['genre']); $this->assertArrayHasKey('simpleQuestionPrompt', $prompts); - $this->assertFalse($prompts['simpleQuestionPrompt']->isManual); $this->assertArrayHasKey('InvokableGreeterPrompt', $prompts); - $this->assertFalse($prompts['InvokableGreeterPrompt']->isManual); $this->assertEquals([InvocablePromptFixture::class, '__invoke'], $prompts['InvokableGreeterPrompt']->handler); $this->assertArrayHasKey('content_creator', $prompts); - $this->assertFalse($prompts['content_creator']->isManual); $this->assertCount(3, $prompts['content_creator']->completionProviders); $templates = $discovery->getResourceTemplates(); $this->assertCount(4, $templates); $this->assertArrayHasKey('product://{region}/details/{productId}', $templates); - $this->assertFalse($templates['product://{region}/details/{productId}']->isManual); $this->assertEquals('product_details_template', $templates['product://{region}/details/{productId}']->resourceTemplate->name); $this->assertEquals(CompletionProviderFixture::class, $templates['product://{region}/details/{productId}']->completionProviders['region']); $this->assertEqualsCanonicalizing(['region', 'productId'], $templates['product://{region}/details/{productId}']->getVariableNames()); $this->assertArrayHasKey('invokable://user-profile/{userId}', $templates); - $this->assertFalse($templates['invokable://user-profile/{userId}']->isManual); $this->assertEquals([InvocableResourceTemplateFixture::class, '__invoke'], $templates['invokable://user-profile/{userId}']->handler); } diff --git a/tests/Unit/Capability/Registry/Loader/ChainLoaderTest.php b/tests/Unit/Capability/Registry/Loader/ChainLoaderTest.php new file mode 100644 index 00000000..5488818e --- /dev/null +++ b/tests/Unit/Capability/Registry/Loader/ChainLoaderTest.php @@ -0,0 +1,57 @@ +load(new Registry()); + + $this->assertSame(['A', 'B', 'C'], $calls->getArrayCopy()); + } + + public function testLastWriterWinsForConflictingKeys(): void + { + $registry = new Registry(); + + $first = new ToolWriterLoader('shared', static fn () => 'first'); + $second = new ToolWriterLoader('shared', static fn () => 'second'); + + (new ChainLoader([$first, $second]))->load($registry); + + $this->assertSame('second', ($registry->getTool('shared')->handler)()); + } + + public function testEmptyChainIsNoop(): void + { + $registry = new Registry(); + + (new ChainLoader([]))->load($registry); + + $this->assertFalse($registry->hasTools()); + $this->assertFalse($registry->hasResources()); + $this->assertFalse($registry->hasResourceTemplates()); + $this->assertFalse($registry->hasPrompts()); + } +} diff --git a/tests/Unit/Capability/Registry/Loader/DiscoveryLoaderTest.php b/tests/Unit/Capability/Registry/Loader/DiscoveryLoaderTest.php new file mode 100644 index 00000000..eaab2dfa --- /dev/null +++ b/tests/Unit/Capability/Registry/Loader/DiscoveryLoaderTest.php @@ -0,0 +1,284 @@ +registry = new Registry(); + } + + public function testLoadRegistersAllDiscoveredElements(): void + { + $loader = new DiscoveryLoader('/base', [], [], new MutableDiscoverer(new DiscoveryState( + tools: ['t1' => new ToolReference($this->makeTool('t1'), static fn () => 't1')], + resources: ['r://1' => new ResourceReference($this->makeResource('r://1'), static fn () => 'r1')], + prompts: ['p1' => new PromptReference($this->makePrompt('p1'), static fn () => [])], + resourceTemplates: ['t://{id}' => new ResourceTemplateReference($this->makeTemplate('t://{id}'), static fn () => 'tpl')], + ))); + + $loader->load($this->registry); + + $this->assertInstanceOf(ToolReference::class, $this->registry->getTool('t1')); + $this->assertInstanceOf(ResourceReference::class, $this->registry->getResource('r://1', false)); + $this->assertInstanceOf(PromptReference::class, $this->registry->getPrompt('p1')); + $this->assertInstanceOf(ResourceTemplateReference::class, $this->registry->getResourceTemplate('t://{id}')); + } + + public function testLoadTwiceUnregistersStaleAndKeepsNew(): void + { + $discoverer = new MutableDiscoverer(new DiscoveryState( + tools: ['t1' => new ToolReference($this->makeTool('t1'), static fn () => 't1')], + resources: ['r://1' => new ResourceReference($this->makeResource('r://1'), static fn () => 'r1')], + )); + $loader = new DiscoveryLoader('/base', [], [], $discoverer); + + $loader->load($this->registry); + + // Second discovery: t1 is gone, t2 appears; resource r://1 still present, new r://2 added. + $discoverer->state = new DiscoveryState( + tools: ['t2' => new ToolReference($this->makeTool('t2'), static fn () => 't2')], + resources: [ + 'r://1' => new ResourceReference($this->makeResource('r://1'), static fn () => 'r1-updated'), + 'r://2' => new ResourceReference($this->makeResource('r://2'), static fn () => 'r2'), + ], + ); + $loader->load($this->registry); + + $this->assertInstanceOf(ToolReference::class, $this->registry->getTool('t2')); + $this->assertInstanceOf(ResourceReference::class, $this->registry->getResource('r://2', false)); + + $updatedResource = $this->registry->getResource('r://1', false); + $this->assertInstanceOf(ResourceReference::class, $updatedResource); + $this->assertSame('r1-updated', ($updatedResource->handler)()); + + $this->expectException(ToolNotFoundException::class); + $this->registry->getTool('t1'); + } + + public function testLoadTwiceUnregistersStalePromptsAndTemplates(): void + { + $discoverer = new MutableDiscoverer(new DiscoveryState( + prompts: [ + 'p1' => new PromptReference($this->makePrompt('p1'), static fn () => []), + 'p2' => new PromptReference($this->makePrompt('p2'), static fn () => []), + ], + resourceTemplates: [ + 't1://{id}' => new ResourceTemplateReference($this->makeTemplate('t1://{id}'), static fn () => 'tpl1'), + 't2://{id}' => new ResourceTemplateReference($this->makeTemplate('t2://{id}'), static fn () => 'tpl2'), + ], + )); + $loader = new DiscoveryLoader('/base', [], [], $discoverer); + $loader->load($this->registry); + + // Second discovery: drop p1 and t1, keep p2 and t2, add p3 and t3. + $discoverer->state = new DiscoveryState( + prompts: [ + 'p2' => new PromptReference($this->makePrompt('p2'), static fn () => []), + 'p3' => new PromptReference($this->makePrompt('p3'), static fn () => []), + ], + resourceTemplates: [ + 't2://{id}' => new ResourceTemplateReference($this->makeTemplate('t2://{id}'), static fn () => 'tpl2'), + 't3://{id}' => new ResourceTemplateReference($this->makeTemplate('t3://{id}'), static fn () => 'tpl3'), + ], + ); + $loader->load($this->registry); + + $this->assertInstanceOf(PromptReference::class, $this->registry->getPrompt('p2')); + $this->assertInstanceOf(PromptReference::class, $this->registry->getPrompt('p3')); + $this->assertInstanceOf(ResourceTemplateReference::class, $this->registry->getResourceTemplate('t2://{id}')); + $this->assertInstanceOf(ResourceTemplateReference::class, $this->registry->getResourceTemplate('t3://{id}')); + + $missing = 0; + try { + $this->registry->getPrompt('p1'); + } catch (PromptNotFoundException) { + ++$missing; + } + try { + $this->registry->getResourceTemplate('t1://{id}'); + } catch (ResourceNotFoundException) { + ++$missing; + } + $this->assertSame(2, $missing); + } + + public function testLoadOverwritesPreviousRegistrationOnSameKey(): void + { + $discoverer = new MutableDiscoverer(new DiscoveryState( + tools: ['t' => new ToolReference($this->makeTool('t'), static fn () => 'v1')], + prompts: ['p' => new PromptReference($this->makePrompt('p'), static fn () => [], ['arg' => EnumCompletionProvider::class])], + )); + $loader = new DiscoveryLoader('/base', [], [], $discoverer); + $loader->load($this->registry); + + // Same names, different handlers / completion providers. + $discoverer->state = new DiscoveryState( + tools: ['t' => new ToolReference($this->makeTool('t'), static fn () => 'v2')], + prompts: ['p' => new PromptReference($this->makePrompt('p'), static fn () => [], ['arg' => ListCompletionProvider::class])], + ); + $loader->load($this->registry); + + $this->assertSame('v2', ($this->registry->getTool('t')->handler)()); + $this->assertSame(['arg' => ListCompletionProvider::class], $this->registry->getPrompt('p')->completionProviders); + } + + public function testLoadPreservesConflictingRuntimeRegistration(): void + { + // Application registers a tool directly. + $this->registry->registerTool($this->makeTool('shared'), static fn () => 'runtime'); + + // Discovery later finds the same name. The manual registration wins — + // discovery does not clobber entries it doesn't own. + $discoverer = new MutableDiscoverer(new DiscoveryState( + tools: ['shared' => new ToolReference($this->makeTool('shared'), static fn () => 'discovered')], + )); + (new DiscoveryLoader('/base', [], [], $discoverer))->load($this->registry); + + $this->assertSame('runtime', ($this->registry->getTool('shared')->handler)()); + } + + public function testLoadPreservesRuntimeOverrideOfPreviouslyOwnedEntry(): void + { + // First discovery owns 'shared'. + $discoverer = new MutableDiscoverer(new DiscoveryState( + tools: ['shared' => new ToolReference($this->makeTool('shared'), static fn () => 'discovered-v1')], + )); + $loader = new DiscoveryLoader('/base', [], [], $discoverer); + $loader->load($this->registry); + + // Developer overrides at runtime. + $this->registry->registerTool($this->makeTool('shared'), static fn () => 'runtime'); + + // Rediscovery still finds 'shared'; loader sees the registry no longer holds its instance and steps aside. + $discoverer->state = new DiscoveryState( + tools: ['shared' => new ToolReference($this->makeTool('shared'), static fn () => 'discovered-v2')], + ); + $loader->load($this->registry); + + $this->assertSame('runtime', ($this->registry->getTool('shared')->handler)()); + } + + public function testLoadDoesNotUnregisterRuntimeAdditions(): void + { + $discoverer = new MutableDiscoverer(new DiscoveryState( + tools: ['discovered_tool' => new ToolReference($this->makeTool('discovered_tool'), static fn () => 'discovered')], + )); + $loader = new DiscoveryLoader('/base', [], [], $discoverer); + + $loader->load($this->registry); + + // Application registers a tool directly between two discovery runs. + $this->registry->registerTool($this->makeTool('runtime_tool'), static fn () => 'runtime'); + + // Second discovery run with a different state. The runtime tool must survive. + $discoverer->state = new DiscoveryState( + tools: ['discovered_tool_v2' => new ToolReference($this->makeTool('discovered_tool_v2'), static fn () => 'v2')], + ); + $loader->load($this->registry); + + $this->assertInstanceOf(ToolReference::class, $this->registry->getTool('runtime_tool')); + $this->assertInstanceOf(ToolReference::class, $this->registry->getTool('discovered_tool_v2')); + + $this->expectException(ToolNotFoundException::class); + $this->registry->getTool('discovered_tool'); + } + + public function testEmptySecondLoadUnregistersAllPreviouslyDiscovered(): void + { + $discoverer = new MutableDiscoverer(new DiscoveryState( + tools: ['t' => new ToolReference($this->makeTool('t'), static fn () => 't')], + resources: ['r://x' => new ResourceReference($this->makeResource('r://x'), static fn () => 'rx')], + prompts: ['p' => new PromptReference($this->makePrompt('p'), static fn () => [])], + resourceTemplates: ['x://{id}' => new ResourceTemplateReference($this->makeTemplate('x://{id}'), static fn () => 'tpl')], + )); + $loader = new DiscoveryLoader('/base', [], [], $discoverer); + $loader->load($this->registry); + + $discoverer->state = new DiscoveryState(); + $loader->load($this->registry); + + $exceptions = 0; + try { + $this->registry->getTool('t'); + } catch (ToolNotFoundException) { + ++$exceptions; + } + try { + $this->registry->getResource('r://x', false); + } catch (ResourceNotFoundException) { + ++$exceptions; + } + try { + $this->registry->getPrompt('p'); + } catch (PromptNotFoundException) { + ++$exceptions; + } + try { + $this->registry->getResourceTemplate('x://{id}'); + } catch (ResourceNotFoundException) { + ++$exceptions; + } + $this->assertSame(4, $exceptions); + } + + private function makeTool(string $name): Tool + { + return new Tool( + name: $name, + title: null, + inputSchema: ['type' => 'object', 'properties' => [], 'required' => null], + description: null, + annotations: null, + icons: null, + meta: null, + outputSchema: null, + ); + } + + private function makeResource(string $uri): Resource + { + return new Resource(uri: $uri, name: 'r', description: null, mimeType: 'text/plain'); + } + + private function makePrompt(string $name): Prompt + { + return new Prompt(name: $name, description: null, arguments: []); + } + + private function makeTemplate(string $uriTemplate): ResourceTemplate + { + return new ResourceTemplate(uriTemplate: $uriTemplate, name: 'tpl', description: null, mimeType: 'text/plain'); + } +} diff --git a/tests/Unit/Capability/Registry/Loader/Stub/MutableDiscoverer.php b/tests/Unit/Capability/Registry/Loader/Stub/MutableDiscoverer.php new file mode 100644 index 00000000..0b5b298b --- /dev/null +++ b/tests/Unit/Capability/Registry/Loader/Stub/MutableDiscoverer.php @@ -0,0 +1,27 @@ +state; + } +} diff --git a/tests/Unit/Capability/Registry/Loader/Stub/RecordingLoader.php b/tests/Unit/Capability/Registry/Loader/Stub/RecordingLoader.php new file mode 100644 index 00000000..a1eccae1 --- /dev/null +++ b/tests/Unit/Capability/Registry/Loader/Stub/RecordingLoader.php @@ -0,0 +1,30 @@ + $calls + */ + public function __construct(private string $name, private \ArrayObject $calls) + { + } + + public function load(RegistryInterface $registry): void + { + $this->calls->append($this->name); + } +} diff --git a/tests/Unit/Capability/Registry/Loader/Stub/ToolWriterLoader.php b/tests/Unit/Capability/Registry/Loader/Stub/ToolWriterLoader.php new file mode 100644 index 00000000..d8ae3f13 --- /dev/null +++ b/tests/Unit/Capability/Registry/Loader/Stub/ToolWriterLoader.php @@ -0,0 +1,37 @@ +registerTool(new Tool( + name: $this->toolName, + title: null, + inputSchema: ['type' => 'object', 'properties' => [], 'required' => null], + description: null, + annotations: null, + icons: null, + meta: null, + outputSchema: null, + ), $this->handler); + } +} diff --git a/tests/Unit/Capability/RegistryTest.php b/tests/Unit/Capability/RegistryTest.php index 90ac8879..5173aaff 100644 --- a/tests/Unit/Capability/RegistryTest.php +++ b/tests/Unit/Capability/RegistryTest.php @@ -82,48 +82,18 @@ public function testGetToolReturnsRegisteredTool(): void $this->assertInstanceOf(ToolReference::class, $toolRef); $this->assertEquals($tool->name, $toolRef->tool->name); $this->assertEquals($handler, $toolRef->handler); - $this->assertFalse($toolRef->isManual); } - public function testRegisterToolWithManualFlag(): void + public function testRegisterToolOverwritesPriorRegistration(): void { - $tool = $this->createValidTool('test_tool'); - $handler = static fn () => 'result'; + $first = $this->createValidTool('test_tool'); + $second = $this->createValidTool('test_tool'); - $this->registry->registerTool($tool, $handler, true); + $this->registry->registerTool($first, static fn () => 'first'); + $this->registry->registerTool($second, static fn () => 'second'); $toolRef = $this->registry->getTool('test_tool'); - $this->assertTrue($toolRef->isManual); - } - - public function testRegisterToolIgnoresDiscoveredWhenManualExists(): void - { - $manualTool = $this->createValidTool('test_tool'); - $discoveredTool = $this->createValidTool('test_tool'); - - $this->registry->registerTool($manualTool, static fn () => 'manual', true); - - $this->logger - ->expects($this->once()) - ->method('debug') - ->with('Ignoring discovered tool "test_tool" as it conflicts with a manually registered one.'); - - $this->registry->registerTool($discoveredTool, static fn () => 'discovered'); - - $toolRef = $this->registry->getTool('test_tool'); - $this->assertTrue($toolRef->isManual); - } - - public function testRegisterToolOverridesDiscoveredWithManual(): void - { - $discoveredTool = $this->createValidTool('test_tool'); - $manualTool = $this->createValidTool('test_tool'); - - $this->registry->registerTool($discoveredTool, static fn () => 'discovered'); - $this->registry->registerTool($manualTool, static fn () => 'manual', true); - - $toolRef = $this->registry->getTool('test_tool'); - $this->assertTrue($toolRef->isManual); + $this->assertEquals('second', ($toolRef->handler)()); } public function testGetToolThrowsExceptionForUnregisteredTool(): void @@ -169,36 +139,18 @@ public function testGetResourceReturnsRegisteredResource(): void $this->assertInstanceOf(ResourceReference::class, $resourceRef); $this->assertEquals($resource->uri, $resourceRef->resource->uri); $this->assertEquals($handler, $resourceRef->handler); - $this->assertFalse($resourceRef->isManual); - } - - public function testRegisterResourceWithManualFlag(): void - { - $resource = $this->createValidResource('test://resource'); - $handler = static fn () => 'content'; - - $this->registry->registerResource($resource, $handler, true); - - $resourceRef = $this->registry->getResource('test://resource'); - $this->assertTrue($resourceRef->isManual); } - public function testRegisterResourceIgnoresDiscoveredWhenManualExists(): void + public function testRegisterResourceOverwritesPriorRegistration(): void { - $manualResource = $this->createValidResource('test://resource'); - $discoveredResource = $this->createValidResource('test://resource'); - - $this->registry->registerResource($manualResource, static fn () => 'manual', true); + $first = $this->createValidResource('test://resource'); + $second = $this->createValidResource('test://resource'); - $this->logger - ->expects($this->once()) - ->method('debug') - ->with('Ignoring discovered resource "test://resource" as it conflicts with a manually registered one.'); - - $this->registry->registerResource($discoveredResource, static fn () => 'discovered'); + $this->registry->registerResource($first, static fn () => 'first'); + $this->registry->registerResource($second, static fn () => 'second'); $resourceRef = $this->registry->getResource('test://resource'); - $this->assertTrue($resourceRef->isManual); + $this->assertEquals('second', ($resourceRef->handler)()); } public function testGetResourceThrowsExceptionForUnregisteredResource(): void @@ -244,7 +196,6 @@ public function testGetResourceTemplateReturnsRegisteredTemplate(): void $this->assertInstanceOf(ResourceTemplateReference::class, $templateRef); $this->assertEquals($template->uriTemplate, $templateRef->resourceTemplate->uriTemplate); $this->assertEquals($handler, $templateRef->handler); - $this->assertFalse($templateRef->isManual); } public function testGetResourcePrefersDirectResourceOverTemplate(): void @@ -300,22 +251,16 @@ public function testRegisterResourceTemplateWithCompletionProviders(): void $this->assertEquals($completionProviders, $templateRef->completionProviders); } - public function testRegisterResourceTemplateIgnoresDiscoveredWhenManualExists(): void + public function testRegisterResourceTemplateOverwritesPriorRegistration(): void { - $manualTemplate = $this->createValidResourceTemplate('test://{id}'); - $discoveredTemplate = $this->createValidResourceTemplate('test://{id}'); - - $this->registry->registerResourceTemplate($manualTemplate, static fn () => 'manual', [], true); - - $this->logger - ->expects($this->once()) - ->method('debug') - ->with('Ignoring discovered template "test://{id}" as it conflicts with a manually registered one.'); + $first = $this->createValidResourceTemplate('test://{id}'); + $second = $this->createValidResourceTemplate('test://{id}'); - $this->registry->registerResourceTemplate($discoveredTemplate, static fn () => 'discovered'); + $this->registry->registerResourceTemplate($first, static fn () => 'first'); + $this->registry->registerResourceTemplate($second, static fn () => 'second'); $templateRef = $this->registry->getResourceTemplate('test://{id}'); - $this->assertTrue($templateRef->isManual); + $this->assertEquals('second', ($templateRef->handler)()); } public function testResourceTemplateMatchingPrefersMoreSpecificMatches(): void @@ -375,7 +320,6 @@ public function testGetPromptReturnsRegisteredPrompt(): void $this->assertInstanceOf(PromptReference::class, $promptRef); $this->assertEquals($prompt->name, $promptRef->prompt->name); $this->assertEquals($handler, $promptRef->handler); - $this->assertFalse($promptRef->isManual); } public function testRegisterPromptWithCompletionProviders(): void @@ -389,22 +333,16 @@ public function testRegisterPromptWithCompletionProviders(): void $this->assertEquals($completionProviders, $promptRef->completionProviders); } - public function testRegisterPromptIgnoresDiscoveredWhenManualExists(): void + public function testRegisterPromptOverwritesPriorRegistration(): void { - $manualPrompt = $this->createValidPrompt('test_prompt'); - $discoveredPrompt = $this->createValidPrompt('test_prompt'); + $first = $this->createValidPrompt('test_prompt'); + $second = $this->createValidPrompt('test_prompt'); - $this->registry->registerPrompt($manualPrompt, static fn () => 'manual', [], true); - - $this->logger - ->expects($this->once()) - ->method('debug') - ->with('Ignoring discovered prompt "test_prompt" as it conflicts with a manually registered one.'); - - $this->registry->registerPrompt($discoveredPrompt, static fn () => 'discovered'); + $this->registry->registerPrompt($first, static fn () => 'first'); + $this->registry->registerPrompt($second, static fn () => 'second'); $promptRef = $this->registry->getPrompt('test_prompt'); - $this->assertTrue($promptRef->isManual); + $this->assertEquals('second', ($promptRef->handler)()); } public function testGetPromptThrowsExceptionForUnregisteredPrompt(): void @@ -415,70 +353,55 @@ public function testGetPromptThrowsExceptionForUnregisteredPrompt(): void $this->registry->getPrompt('non_existent_prompt'); } - public function testClearRemovesOnlyDiscoveredElements(): void - { - $manualTool = $this->createValidTool('manual_tool'); - $discoveredTool = $this->createValidTool('discovered_tool'); - $manualResource = $this->createValidResource('test://manual'); - $discoveredResource = $this->createValidResource('test://discovered'); - $manualPrompt = $this->createValidPrompt('manual_prompt'); - $discoveredPrompt = $this->createValidPrompt('discovered_prompt'); - $manualTemplate = $this->createValidResourceTemplate('manual://{id}'); - $discoveredTemplate = $this->createValidResourceTemplate('discovered://{id}'); - - $this->registry->registerTool($manualTool, static fn () => 'manual', true); - $this->registry->registerTool($discoveredTool, static fn () => 'discovered'); - $this->registry->registerResource($manualResource, static fn () => 'manual', true); - $this->registry->registerResource($discoveredResource, static fn () => 'discovered'); - $this->registry->registerPrompt($manualPrompt, static fn () => [], [], true); - $this->registry->registerPrompt($discoveredPrompt, static fn () => []); - $this->registry->registerResourceTemplate($manualTemplate, static fn () => 'manual', [], true); - $this->registry->registerResourceTemplate($discoveredTemplate, static fn () => 'discovered'); - - // Test that all elements exist - $this->registry->getTool('manual_tool'); - $this->registry->getResource('test://manual'); - $this->registry->getPrompt('manual_prompt'); - $this->registry->getResourceTemplate('manual://{id}'); - $this->registry->getTool('discovered_tool'); - $this->registry->getResource('test://discovered'); - $this->registry->getPrompt('discovered_prompt'); - $this->registry->getResourceTemplate('discovered://{id}'); - - $this->registry->clear(); - - // Manual elements should still exist - $this->registry->getTool('manual_tool'); - $this->registry->getResource('test://manual'); - $this->registry->getPrompt('manual_prompt'); - $this->registry->getResourceTemplate('manual://{id}'); - - // Test that all discovered elements throw exceptions + public function testUnregisterToolRemovesRegisteredTool(): void + { + $tool = $this->createValidTool('test_tool'); + $this->registry->registerTool($tool, static fn () => 'result'); + + $this->registry->unregisterTool('test_tool'); + $this->expectException(ToolNotFoundException::class); - $this->registry->getTool('discovered_tool'); + $this->registry->getTool('test_tool'); + } - $this->expectException(ResourceNotFoundException::class); - $this->registry->getResource('test://discovered'); + public function testUnregisterToolIsIdempotentForAbsentName(): void + { + $this->registry->unregisterTool('never_registered'); - $this->expectException(PromptNotFoundException::class); - $this->registry->getPrompt('discovered_prompt'); + $this->assertFalse($this->registry->hasTools()); + } + + public function testUnregisterResourceRemovesRegisteredResource(): void + { + $resource = $this->createValidResource('test://resource'); + $this->registry->registerResource($resource, static fn () => 'content'); + + $this->registry->unregisterResource('test://resource'); $this->expectException(ResourceNotFoundException::class); - $this->registry->getResourceTemplate('discovered://{id}'); + $this->registry->getResource('test://resource', false); } - public function testClearLogsNothingWhenNoDiscoveredElements(): void + public function testUnregisterResourceTemplateRemovesRegisteredTemplate(): void { - $manualTool = $this->createValidTool('manual_tool'); - $this->registry->registerTool($manualTool, static fn () => 'manual', true); + $template = $this->createValidResourceTemplate('test://{id}'); + $this->registry->registerResourceTemplate($template, static fn () => 'content'); + + $this->registry->unregisterResourceTemplate('test://{id}'); - $this->logger - ->expects($this->never()) - ->method('debug'); + $this->expectException(ResourceNotFoundException::class); + $this->registry->getResourceTemplate('test://{id}'); + } - $this->registry->clear(); + public function testUnregisterPromptRemovesRegisteredPrompt(): void + { + $prompt = $this->createValidPrompt('test_prompt'); + $this->registry->registerPrompt($prompt, static fn () => []); + + $this->registry->unregisterPrompt('test_prompt'); - $this->registry->getTool('manual_tool'); + $this->expectException(PromptNotFoundException::class); + $this->registry->getPrompt('test_prompt'); } public function testRegisterToolHandlesStringHandler(): void