Skip to content

Allow pre-built instance handlers in element registration#375

Open
valeriudev wants to merge 2 commits into
modelcontextprotocol:mainfrom
valeriudev:fix/instance-method-handler
Open

Allow pre-built instance handlers in element registration#375
valeriudev wants to merge 2 commits into
modelcontextprotocol:mainfrom
valeriudev:fix/instance-method-handler

Conversation

@valeriudev
Copy link
Copy Markdown

Registering an element with a pre-built object instance as the handler throws at build time:

$foo = new Foo();
Server::builder()->addTool(handler: [$foo, 'bar'], name: 'foo')->build();
// Mcp\Exception\InvalidArgumentException: Invalid array handler format.
// Expected [ClassName::class, 'methodName'].  (HandlerResolver.php)

This is the only place that rejects an instance: the Handler type alias already declares array{0: object|string, 1: string}, getHandlerDescription() already formats object handlers, and the runtime invoker (ReferenceHandler) already dispatches them via its is_callable branch. HandlerResolver::resolve()'s guard simply required a string in slot 0, out of sync with that contract.

Fix: accept an object in slot 0 and normalize it to its class name for reflection — the same is_object($h[0]) ? $h[0]::class : $h[0] idiom getHandlerDescription() already uses. The guard can't be replaced by is_callable(), since is_callable([Class::class, 'instanceMethod']) is false and that lazy-instantiate form must keep working.

This unblocks handler objects with constructor dependencies that the container-less new $className() fallback cannot build.

Tests: added testResolvesValidInstanceArrayHandler; full unit suite, CS, and PHPStan are green.

Fixes #369

`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 modelcontextprotocol#369
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Server] Passing tool handler using instance method throws "Invalid array handler"

1 participant