Merge prism-php#1009 — refactor(tool): prevent circular references and improve handler resolution for invokable subclasses

Author: wishborn

docs/core-concepts/tools-function-calling.md

@@ -253,8 +253,7 @@ class SearchTool extends Tool
         $this
             ->as('search')
             ->for('useful when you need to search for current events')
-            ->withStringParameter('query', 'Detailed search query. Best to search one topic at a time.')
-            ->using($this);
+            ->withStringParameter('query', 'Detailed search query. Best to search one topic at a time.');
     }
 
     public function __invoke(string $query): string

src/Concerns/CallsTools.php

@@ -126,12 +126,12 @@ protected function executeToolsWithConcurrency(array $tools, array $groupedToolC
      * @param  Tool[]  $tools
      * @return array{toolResult: ToolResult, events: array<int, ToolResultEvent|ArtifactEvent>}
      */
-    protected function executeToolCall(array $tools, ToolCall $toolCall, string $messageId): array
+    protected static function executeToolCall(array $tools, ToolCall $toolCall, string $messageId): array
     {
         $events = [];
 
         try {
-            $tool = $this->resolveTool($toolCall->name, $tools);
+            $tool = self::resolveTool($toolCall->name, $tools);
             $output = call_user_func_array(
                 $tool->handle(...),
                 $toolCall->arguments()
@@ -224,8 +224,10 @@ protected function executeToolCall(array $tools, ToolCall $toolCall, string $mes
 
     /**
      * @param  Tool[]  $tools
+     *
+     * @throws PrismException
      */
-    protected function resolveTool(string $name, array $tools): Tool
+    protected static function resolveTool(string $name, array $tools): Tool
     {
         try {
             return collect($tools)

src/Tool.php

@@ -39,7 +39,7 @@ class Tool
     /** @var array <int, string> */
     protected array $requiredParameters = [];
 
-    /** @var Closure():mixed|callable():mixed */
+    /** @var Closure():mixed|callable():mixed|null */
     protected $fn;
 
     /** @var null|false|Closure(Throwable,array<int|string,mixed>):string */
@@ -68,6 +68,10 @@ public function for(string $description): self
 
     public function using(Closure|callable $fn): self
     {
+        if ($fn === $this) {
+            return $this;
+        }
+
         $this->fn = $fn;
 
         return $this;
@@ -262,7 +266,9 @@ public function failedHandler(): null|false|Closure
     public function handle(...$args): string|ToolOutput|ToolError
     {
         try {
-            $value = call_user_func($this->fn, ...$args);
+            $callable = $this->resolveHandler();
+
+            $value = call_user_func($callable, ...$args);
 
             if (is_string($value)) {
                 return $value;
@@ -281,6 +287,35 @@ public function handle(...$args): string|ToolOutput|ToolError
         }
     }
 
+    /**
+     * Resolve the callable handler for this tool.
+     *
+     * Priority: explicit $fn > invokable subclass (__invoke) > error.
+     * Also unwraps SerializableClosure wrappers that break named arguments.
+     */
+    protected function resolveHandler(): callable
+    {
+        $fn = $this->fn;
+
+        if ($fn === null && method_exists($this, '__invoke')) {
+            $fn = $this;
+        }
+
+        if ($fn === null) {
+            throw new PrismException("Tool handler not defined for tool: {$this->name}");
+        }
+
+        // After ProcessDriver deserialization, $fn may become a
+        // SerializableClosure\Serializers\Native whose __invoke doesn't
+        // forward PHP 8 named arguments. Unwrap via getClosure() to
+        // recover the real Closure so named-arg spreading works.
+        if (is_object($fn) && method_exists($fn, 'getClosure')) {
+            return $fn->getClosure();
+        }
+
+        return $fn;
+    }
+
     protected function shouldHandleErrors(): bool
     {
         return $this->failedHandler !== false;

src/Tools/LaravelMcpTool.php

@@ -17,8 +17,7 @@ class LaravelMcpTool extends Tool
     public function __construct(private readonly \Laravel\Mcp\Server\Tool $tool)
     {
         $this->as($tool->name())
-            ->for($tool->description())
-            ->using($this);
+            ->for($tool->description());
 
         $data = $tool->toArray();
         $properties = $data['inputSchema']['properties'] ?? [];

tests/ToolTest.php

@@ -85,6 +85,47 @@ public function __invoke(string $query): string
         ->toBe('The event is at 3pm eastern');
 });
 
+it('handles invokable subclass with using($this) without circular reference', function (): void {
+    $tool = new class extends Tool
+    {
+        public function __construct()
+        {
+            parent::__construct();
+            $this->as('test_tool')
+                ->for('A test tool')
+                ->withParameter(new StringSchema('query', 'the query'))
+                ->using($this);
+        }
+
+        public function __invoke(string $query): string
+        {
+            return "Result: $query";
+        }
+    };
+
+    expect($tool->handle(query: 'hello'))->toBe('Result: hello');
+});
+
+it('invokable subclass works without calling using() at all', function (): void {
+    $tool = new class extends Tool
+    {
+        public function __construct()
+        {
+            parent::__construct();
+            $this->as('auto_tool')
+                ->for('Auto-detected invokable')
+                ->withParameter(new StringSchema('input', 'the input'));
+        }
+
+        public function __invoke(string $input): string
+        {
+            return "Auto: $input";
+        }
+    };
+
+    expect($tool->handle(input: 'test'))->toBe('Auto: test');
+});
+
 it('can have fluent parameters', function (): void {
     $tool = (new Tool)
         ->as('test tool')