From cc97489bc167bd5fd5030bc7b230867c8e0c1823 Mon Sep 17 00:00:00 2001 From: Valery Gutu Date: Fri, 5 Jun 2026 17:30:50 +0300 Subject: [PATCH 1/3] Allow pre-built instance handlers in element registration `HandlerResolver::resolve()` rejected `[$instance, 'methodName']` array handlers even though the `Handler` type alias and the runtime invoker already support them, so `Server::builder()->addTool(handler: [$obj, 'bar'])` threw "Invalid array handler format" at build time. Relax the array-shape guard to accept an object in slot 0 and normalize it to its class name for reflection (the same idiom getHandlerDescription() already uses). This unblocks dependency-injected handler objects that the container-less `new $className()` fallback cannot build. Fixes #369 --- CHANGELOG.md | 5 +++++ src/Capability/Discovery/HandlerResolver.php | 15 ++++++++++----- .../Capability/Discovery/HandlerResolverTest.php | 13 +++++++++++-- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8db034bd..a71e75c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to `mcp/sdk` will be documented in this file. +0.7.0 +----- + +* Allow registering an element handler as a pre-built object instance (`[$instance, 'methodName']`) via `Builder::addTool()`, `addResource()`, `addResourceTemplate()`, and `addPrompt()`. `HandlerResolver` previously rejected instances with "Invalid array handler format" even though the `Handler` type already permitted them — this unblocks handler objects with constructor dependencies that the container-less fallback cannot build. + 0.6.0 ----- diff --git a/src/Capability/Discovery/HandlerResolver.php b/src/Capability/Discovery/HandlerResolver.php index cd78aaf4..9f5d2786 100644 --- a/src/Capability/Discovery/HandlerResolver.php +++ b/src/Capability/Discovery/HandlerResolver.php @@ -11,11 +11,14 @@ namespace Mcp\Capability\Discovery; +use Mcp\Capability\Registry\ElementReference; use Mcp\Exception\InvalidArgumentException; /** * Utility class to validate and resolve MCP element handlers. * + * @phpstan-import-type Handler from ElementReference + * * @author Kyrian Obikwelu */ class HandlerResolver @@ -25,11 +28,12 @@ class HandlerResolver * * A handler can be: * - A Closure: function() { ... } - * - An array: [ClassName::class, 'methodName'] (instance method) + * - An array: [ClassName::class, 'methodName'] (resolved on a new or container-provided instance) + * - An array: [$instance, 'methodName'] (method on a pre-built object instance) * - An array: [ClassName::class, 'staticMethod'] (static method, if callable) * - A string: InvokableClassName::class (which will resolve to its '__invoke' method) * - * @param \Closure|array{0: string, 1: string}|string $handler the handler to resolve + * @param Handler $handler the handler to resolve * * @throws InvalidArgumentException If the handler format is invalid, the class/method doesn't exist, * or the method is unsuitable (e.g., private, abstract). @@ -41,10 +45,11 @@ public static function resolve(\Closure|array|string $handler): \ReflectionMetho } if (\is_array($handler)) { - if (2 !== \count($handler) || !isset($handler[0]) || !isset($handler[1]) || !\is_string($handler[0]) || !\is_string($handler[1])) { - throw new InvalidArgumentException('Invalid array handler format. Expected [ClassName::class, \'methodName\'].'); + if (2 !== \count($handler) || !isset($handler[0]) || !isset($handler[1]) || !(\is_string($handler[0]) || \is_object($handler[0])) || !\is_string($handler[1])) { + throw new InvalidArgumentException('Invalid array handler format. Expected [ClassName::class, \'methodName\'] or [$instance, \'methodName\'].'); } - [$className, $methodName] = $handler; + [$classOrObject, $methodName] = $handler; + $className = \is_object($classOrObject) ? $classOrObject::class : $classOrObject; if (!class_exists($className)) { throw new InvalidArgumentException(\sprintf('Handler class "%s" not found for array handler.', $className)); } diff --git a/tests/Unit/Capability/Discovery/HandlerResolverTest.php b/tests/Unit/Capability/Discovery/HandlerResolverTest.php index 6a87a0b3..24e91d44 100644 --- a/tests/Unit/Capability/Discovery/HandlerResolverTest.php +++ b/tests/Unit/Capability/Discovery/HandlerResolverTest.php @@ -38,6 +38,15 @@ public function testResolvesValidArrayHandler(): void $this->assertEquals(ValidHandlerClass::class, $resolved->getDeclaringClass()->getName()); } + public function testResolvesValidInstanceArrayHandler(): void + { + $handler = [new ValidHandlerClass(), 'publicMethod']; + $resolved = HandlerResolver::resolve($handler); + $this->assertInstanceOf(\ReflectionMethod::class, $resolved); + $this->assertEquals('publicMethod', $resolved->getName()); + $this->assertEquals(ValidHandlerClass::class, $resolved->getDeclaringClass()->getName()); + } + public function testResolvesValidInvokableClassStringHandler(): void { $handler = ValidInvokableClass::class; @@ -59,14 +68,14 @@ public function testResolvesStaticMethodsForManualRegistration(): void public function testThrowsForInvalidArrayHandlerFormatCount(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage("Invalid array handler format. Expected [ClassName::class, 'methodName']."); + $this->expectExceptionMessage('Invalid array handler format. Expected [ClassName::class, \'methodName\'] or [$instance, \'methodName\'].'); HandlerResolver::resolve([ValidHandlerClass::class]); /* @phpstan-ignore argument.type */ } public function testThrowsForInvalidArrayHandlerFormatTypes(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage("Invalid array handler format. Expected [ClassName::class, 'methodName']."); + $this->expectExceptionMessage('Invalid array handler format. Expected [ClassName::class, \'methodName\'] or [$instance, \'methodName\'].'); HandlerResolver::resolve([ValidHandlerClass::class, 123]); /* @phpstan-ignore argument.type */ } From b5b7544c6e2ccbadcd364cc19e6854351156b025 Mon Sep 17 00:00:00 2001 From: Valery Gutu Date: Fri, 5 Jun 2026 19:43:03 +0300 Subject: [PATCH 2/3] Use 0.6.1 changelog heading and test instance handler end-to-end This is a bugfix with no new public API or BC break, so the changelog entry belongs under 0.6.1 rather than a new 0.7.0 section. Add an end-to-end test that registers a pre-built instance handler with a constructor dependency the container-less new $className() fallback cannot satisfy, then drives a real tools/call through CallToolHandler and asserts the injected instance is the one invoked. --- CHANGELOG.md | 2 +- tests/Unit/Server/BuilderTest.php | 36 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a71e75c9..32dee04c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ All notable changes to `mcp/sdk` will be documented in this file. -0.7.0 +0.6.1 ----- * Allow registering an element handler as a pre-built object instance (`[$instance, 'methodName']`) via `Builder::addTool()`, `addResource()`, `addResourceTemplate()`, and `addPrompt()`. `HandlerResolver` previously rejected instances with "Invalid array handler format" even though the `Handler` type already permitted them — this unblocks handler objects with constructor dependencies that the container-less fallback cannot build. diff --git a/tests/Unit/Server/BuilderTest.php b/tests/Unit/Server/BuilderTest.php index 4435639c..19308793 100644 --- a/tests/Unit/Server/BuilderTest.php +++ b/tests/Unit/Server/BuilderTest.php @@ -83,6 +83,25 @@ public function testCustomReferenceHandlerIsUsedForToolCalls(): void $this->assertSame('intercepted', $result); } + #[TestDox('A pre-built instance handler with constructor dependencies is registered and invoked on that instance')] + public function testPreBuiltInstanceHandlerIsInvokedOnTheGivenInstance(): void + { + // The handler's constructor requires an argument the container-less + // `new $className()` fallback can never satisfy. If the tool call + // succeeds, it can only be because the pre-built instance — carrying + // its injected dependency — was the one invoked. + $handler = new GreetingService('World'); + + $server = Server::builder() + ->setServerInfo('test', '1.0.0') + ->addTool(handler: [$handler, 'greet'], name: 'greet', description: 'Greets using the injected name') + ->build(); + + $result = $this->callTool($server, 'greet'); + + $this->assertSame('Hello, World', $result); + } + #[TestDox('enableExtension() registers an extension and announces its capability payload')] public function testEnableExtensionRegistersExtension(): void { @@ -166,3 +185,20 @@ private function callTool(Server $server, string $toolName): mixed $this->fail('CallToolHandler not found in request handlers'); } } + +/** + * A handler whose constructor dependency cannot be satisfied by the + * container-less `new $className()` fallback, so it must be registered as a + * pre-built instance. + */ +final class GreetingService +{ + public function __construct(private string $name) + { + } + + public function greet(): string + { + return 'Hello, '.$this->name; + } +} From 70eb7fe814ca38f1d6eb31dea08027d1135d3961 Mon Sep 17 00:00:00 2001 From: Valery Gutu Date: Mon, 8 Jun 2026 13:09:21 +0300 Subject: [PATCH 3/3] Address review: reject Closure array handlers, fix abstract-method message - HandlerResolver: extract the array-handler guard into a readable $hasValidTarget check that accepts strings and objects but excludes \Closure, so [$closure, 'method'] falls through to the format error instead of the misleading "method not found in class Closure". - Fix the abstract-method rejection message ("must be abstract" -> "must not be abstract"), a pre-existing typo in the same branch. - Tighten the CHANGELOG entry wording. - Cover Closure rejection with a regression test. --- CHANGELOG.md | 2 +- src/Capability/Discovery/HandlerResolver.php | 10 ++++++++-- .../Unit/Capability/Discovery/HandlerResolverTest.php | 9 ++++++++- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32dee04c..b1844e66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to `mcp/sdk` will be documented in this file. 0.6.1 ----- -* Allow registering an element handler as a pre-built object instance (`[$instance, 'methodName']`) via `Builder::addTool()`, `addResource()`, `addResourceTemplate()`, and `addPrompt()`. `HandlerResolver` previously rejected instances with "Invalid array handler format" even though the `Handler` type already permitted them — this unblocks handler objects with constructor dependencies that the container-less fallback cannot build. +* Allow `[$instance, 'methodName']` as an element handler in `Builder::addTool()`, `addResource()`, `addResourceTemplate()`, and `addPrompt()`. Unblocks handlers with constructor dependencies that the container-less `new $className()` fallback cannot build. 0.6.0 ----- diff --git a/src/Capability/Discovery/HandlerResolver.php b/src/Capability/Discovery/HandlerResolver.php index 9f5d2786..8cadcd3c 100644 --- a/src/Capability/Discovery/HandlerResolver.php +++ b/src/Capability/Discovery/HandlerResolver.php @@ -45,7 +45,13 @@ public static function resolve(\Closure|array|string $handler): \ReflectionMetho } if (\is_array($handler)) { - if (2 !== \count($handler) || !isset($handler[0]) || !isset($handler[1]) || !(\is_string($handler[0]) || \is_object($handler[0])) || !\is_string($handler[1])) { + // A Closure in slot 0 must fall through to the format error rather than + // be treated as an instance, where "$closure::class" would yield the + // misleading class name "Closure". + $target = $handler[0] ?? null; + $hasValidTarget = (\is_string($target) || \is_object($target)) && !$target instanceof \Closure; + + if (2 !== \count($handler) || !$hasValidTarget || !isset($handler[1]) || !\is_string($handler[1])) { throw new InvalidArgumentException('Invalid array handler format. Expected [ClassName::class, \'methodName\'] or [$instance, \'methodName\'].'); } [$classOrObject, $methodName] = $handler; @@ -75,7 +81,7 @@ public static function resolve(\Closure|array|string $handler): \ReflectionMetho throw new InvalidArgumentException(\sprintf('Handler method "%s::%s" must be public.', $className, $methodName)); } if ($reflectionMethod->isAbstract()) { - throw new InvalidArgumentException(\sprintf('Handler method "%s::%s" must be abstract.', $className, $methodName)); + throw new InvalidArgumentException(\sprintf('Handler method "%s::%s" must not be abstract.', $className, $methodName)); } if ($reflectionMethod->isConstructor() || $reflectionMethod->isDestructor()) { throw new InvalidArgumentException(\sprintf('Handler method "%s::%s" cannot be a constructor or destructor.', $className, $methodName)); diff --git a/tests/Unit/Capability/Discovery/HandlerResolverTest.php b/tests/Unit/Capability/Discovery/HandlerResolverTest.php index 24e91d44..9076a7e5 100644 --- a/tests/Unit/Capability/Discovery/HandlerResolverTest.php +++ b/tests/Unit/Capability/Discovery/HandlerResolverTest.php @@ -79,6 +79,13 @@ public function testThrowsForInvalidArrayHandlerFormatTypes(): void HandlerResolver::resolve([ValidHandlerClass::class, 123]); /* @phpstan-ignore argument.type */ } + public function testThrowsForClosureInArrayHandler(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid array handler format. Expected [ClassName::class, \'methodName\'] or [$instance, \'methodName\'].'); + HandlerResolver::resolve([static fn () => null, 'method']); + } + public function testThrowsForNonExistentClassInArrayHandler(): void { $this->expectException(InvalidArgumentException::class); @@ -138,7 +145,7 @@ public function testThrowsForDestructorAsHandler(): void public function testThrowsForAbstractMethodHandler(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Handler method "Mcp\Tests\Unit\Capability\Discovery\AbstractHandlerClass::abstractMethod" must be abstract.'); + $this->expectExceptionMessage('Handler method "Mcp\Tests\Unit\Capability\Discovery\AbstractHandlerClass::abstractMethod" must not be abstract.'); HandlerResolver::resolve([AbstractHandlerClass::class, 'abstractMethod']); }