Allow pre-built instance handlers in element registration#375
Open
valeriudev wants to merge 2 commits into
Open
Allow pre-built instance handlers in element registration#375valeriudev wants to merge 2 commits into
valeriudev wants to merge 2 commits into
Conversation
`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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Registering an element with a pre-built object instance as the handler throws at build time:
This is the only place that rejects an instance: the
Handlertype alias already declaresarray{0: object|string, 1: string},getHandlerDescription()already formats object handlers, and the runtime invoker (ReferenceHandler) already dispatches them via itsis_callablebranch.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]idiomgetHandlerDescription()already uses. The guard can't be replaced byis_callable(), sinceis_callable([Class::class, 'instanceMethod'])isfalseand 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