Skip to content

Commit 9c08434

Browse files
vinitkadam03vinitkadam03
committed
stash changes: Fix: concurrent tool execution
- Updated the `Tool` class to resolve callable handlers more effectively, allowing for better handling of invokable subclasses. - Simplified the `using` method to prevent circular references when using `$this`. - Refactored the `CallsTools` trait to enhance the resolution of tool calls, improving error handling and execution flow. - Added tests to ensure proper functionality of invokable subclasses and their integration with the tool system.
1 parent 5d6cc65 commit 9c08434

6 files changed

Lines changed: 135 additions & 39 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,7 @@ class SearchTool extends Tool
253253
$this
254254
->as('search')
255255
->for('useful when you need to search for current events')
256-
->withStringParameter('query', 'Detailed search query. Best to search one topic at a time.')
257-
->using($this);
256+
->withStringParameter('query', 'Detailed search query. Best to search one topic at a time.');
258257
}
259258

260259
public function __invoke(string $query): string

‎src/Concerns/CallsTools.php‎

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ protected function callTools(array $tools, array $toolCalls): array
4949
*/
5050
protected function callToolsAndYieldEvents(array $tools, array $toolCalls, string $messageId, array &$toolResults): Generator
5151
{
52-
$groupedToolCalls = $this->groupToolCallsByConcurrency($tools, $toolCalls);
52+
$resolvedCalls = $this->resolveToolCalls($tools, $toolCalls);
5353

54-
$executionResults = $this->executeToolsWithConcurrency($tools, $groupedToolCalls, $messageId);
54+
$executionResults = $this->executeToolCalls($resolvedCalls, $messageId);
5555

5656
foreach (array_keys($toolCalls) as $index) {
5757
$result = $executionResults[$index];
@@ -65,26 +65,36 @@ protected function callToolsAndYieldEvents(array $tools, array $toolCalls, strin
6565
}
6666

6767
/**
68+
* Resolve tool calls to Tool+ToolCall pairs, grouping by concurrency.
69+
* Each tool is resolved exactly once.
70+
*
71+
* Unresolvable tools (not found, duplicates) are stored with the caught
72+
* exception so executeResolvedToolCall can produce a proper error result.
73+
*
6874
* @param Tool[] $tools
6975
* @param ToolCall[] $toolCalls
70-
* @return array{concurrent: array<int, ToolCall>, sequential: array<int, ToolCall>}
76+
* @return array{concurrent: array<int, array{tool: Tool, toolCall: ToolCall}>, sequential: array<int, array{tool: ?Tool, toolCall: ToolCall, error?: PrismException}>}
7177
*/
72-
protected function groupToolCallsByConcurrency(array $tools, array $toolCalls): array
78+
protected function resolveToolCalls(array $tools, array $toolCalls): array
7379
{
7480
$concurrent = [];
7581
$sequential = [];
7682

7783
foreach ($toolCalls as $index => $toolCall) {
7884
try {
7985
$tool = $this->resolveTool($toolCall->name, $tools);
86+
} catch (PrismException $e) {
87+
$sequential[$index] = ['tool' => null, 'toolCall' => $toolCall, 'error' => $e];
88+
89+
continue;
90+
}
91+
92+
$pair = ['tool' => $tool, 'toolCall' => $toolCall];
8093

81-
if ($tool->isConcurrent()) {
82-
$concurrent[$index] = $toolCall;
83-
} else {
84-
$sequential[$index] = $toolCall;
85-
}
86-
} catch (PrismException) {
87-
$sequential[$index] = $toolCall;
94+
if ($tool->isConcurrent()) {
95+
$concurrent[$index] = $pair;
96+
} else {
97+
$sequential[$index] = $pair;
8898
}
8999
}
90100

@@ -95,43 +105,53 @@ protected function groupToolCallsByConcurrency(array $tools, array $toolCalls):
95105
}
96106

97107
/**
98-
* @param Tool[] $tools
99-
* @param array{concurrent: array<int, ToolCall>, sequential: array<int, ToolCall>} $groupedToolCalls
108+
* @param array{concurrent: array<int, array{tool: Tool, toolCall: ToolCall}>, sequential: array<int, array{tool: ?Tool, toolCall: ToolCall, error?: PrismException}>} $resolvedCalls
100109
* @return array<int, array{toolResult: ToolResult, events: array<int, ToolResultEvent|ArtifactEvent>}>
101110
*/
102-
protected function executeToolsWithConcurrency(array $tools, array $groupedToolCalls, string $messageId): array
111+
protected function executeToolCalls(array $resolvedCalls, string $messageId): array
103112
{
104113
$results = [];
105114

106-
$concurrentClosures = [];
107-
108-
foreach ($groupedToolCalls['concurrent'] as $index => $toolCall) {
109-
$concurrentClosures[$index] = fn () => $this->executeToolCall($tools, $toolCall, $messageId);
110-
}
115+
if ($resolvedCalls['concurrent'] !== []) {
116+
$closures = [];
117+
foreach ($resolvedCalls['concurrent'] as $index => $pair) {
118+
$closures[$index] = static fn (): array => self::executeResolvedToolCall($pair['tool'], $pair['toolCall'], $messageId);
119+
}
111120

112-
if ($concurrentClosures !== []) {
113-
foreach (Concurrency::run($concurrentClosures) as $index => $result) {
121+
foreach (Concurrency::run($closures) as $index => $result) {
114122
$results[$index] = $result;
115123
}
116124
}
117125

118-
foreach ($groupedToolCalls['sequential'] as $index => $toolCall) {
119-
$results[$index] = $this->executeToolCall($tools, $toolCall, $messageId);
126+
foreach ($resolvedCalls['sequential'] as $index => $pair) {
127+
$results[$index] = self::executeResolvedToolCall($pair['tool'], $pair['toolCall'], $messageId, $pair['error'] ?? null);
120128
}
121129

122130
return $results;
123131
}
124132

125133
/**
126-
* @param Tool[] $tools
134+
* Execute a tool call without capturing $this — safe for serialization
135+
* by Laravel's ProcessDriver which uses SerializableClosure.
136+
*
137+
* When $error is provided (tool resolution failed), skips execution and
138+
* returns a failed result directly.
139+
*
127140
* @return array{toolResult: ToolResult, events: array<int, ToolResultEvent|ArtifactEvent>}
128141
*/
129-
protected function executeToolCall(array $tools, ToolCall $toolCall, string $messageId): array
142+
protected static function executeResolvedToolCall(?Tool $tool, ToolCall $toolCall, string $messageId, ?PrismException $error = null): array
130143
{
131144
$events = [];
132145

133146
try {
134-
$tool = $this->resolveTool($toolCall->name, $tools);
147+
if ($error instanceof PrismException) {
148+
throw $error;
149+
}
150+
151+
if (! $tool instanceof Tool) {
152+
throw new PrismException("Tool [{$toolCall->name}] could not be resolved");
153+
}
154+
135155
$output = call_user_func_array(
136156
$tool->handle(...),
137157
$toolCall->arguments()
@@ -224,6 +244,8 @@ protected function executeToolCall(array $tools, ToolCall $toolCall, string $mes
224244

225245
/**
226246
* @param Tool[] $tools
247+
*
248+
* @throws PrismException
227249
*/
228250
protected function resolveTool(string $name, array $tools): Tool
229251
{

‎src/Tool.php‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class Tool
3939
/** @var array <int, string> */
4040
protected array $requiredParameters = [];
4141

42-
/** @var Closure():mixed|callable():mixed */
42+
/** @var Closure():mixed|callable():mixed|null */
4343
protected $fn;
4444

4545
/** @var null|false|Closure(Throwable,array<int|string,mixed>):string */
@@ -68,6 +68,10 @@ public function for(string $description): self
6868

6969
public function using(Closure|callable $fn): self
7070
{
71+
if ($fn === $this) {
72+
return $this;
73+
}
74+
7175
$this->fn = $fn;
7276

7377
return $this;
@@ -262,7 +266,9 @@ public function failedHandler(): null|false|Closure
262266
public function handle(...$args): string|ToolOutput|ToolError
263267
{
264268
try {
265-
$value = call_user_func($this->fn, ...$args);
269+
$callable = $this->resolveHandler();
270+
271+
$value = call_user_func($callable, ...$args);
266272

267273
if (is_string($value)) {
268274
return $value;
@@ -281,6 +287,35 @@ public function handle(...$args): string|ToolOutput|ToolError
281287
}
282288
}
283289

290+
/**
291+
* Resolve the callable handler for this tool.
292+
*
293+
* Priority: explicit $fn > invokable subclass (__invoke) > error.
294+
* Also unwraps SerializableClosure wrappers that break named arguments.
295+
*/
296+
protected function resolveHandler(): callable
297+
{
298+
$fn = $this->fn;
299+
300+
if ($fn === null && method_exists($this, '__invoke')) {
301+
$fn = $this;
302+
}
303+
304+
if ($fn === null) {
305+
throw new PrismException("Tool handler not defined for tool: {$this->name}");
306+
}
307+
308+
// After ProcessDriver deserialization, $fn may become a
309+
// SerializableClosure\Serializers\Native whose __invoke doesn't
310+
// forward PHP 8 named arguments. Unwrap via getClosure() to
311+
// recover the real Closure so named-arg spreading works.
312+
if (is_object($fn) && method_exists($fn, 'getClosure')) {
313+
return $fn->getClosure();
314+
}
315+
316+
return $fn;
317+
}
318+
284319
protected function shouldHandleErrors(): bool
285320
{
286321
return $this->failedHandler !== false;

‎src/Tools/LaravelMcpTool.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ class LaravelMcpTool extends Tool
1717
public function __construct(private readonly \Laravel\Mcp\Server\Tool $tool)
1818
{
1919
$this->as($tool->name())
20-
->for($tool->description())
21-
->using($this);
20+
->for($tool->description());
2221

2322
$data = $tool->toArray();
2423
$properties = $data['inputSchema']['properties'] ?? [];

‎tests/Concerns/CallsToolsConcurrentTest.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class TestToolCaller
1212
{
1313
use CallsTools {
1414
callToolsAndYieldEvents as public;
15-
groupToolCallsByConcurrency as public;
15+
resolveToolCalls as public;
1616
}
1717
}
1818

@@ -261,14 +261,14 @@ class TestToolCaller
261261
new ToolCall('call2', 'sequential', ['input' => 'test2']),
262262
];
263263

264-
$grouped = $this->caller->groupToolCallsByConcurrency(
264+
$resolved = $this->caller->resolveToolCalls(
265265
[$concurrentTool, $sequentialTool],
266266
$toolCalls
267267
);
268268

269-
expect($grouped)->toHaveKeys(['concurrent', 'sequential']);
270-
expect($grouped['concurrent'])->toHaveCount(1);
271-
expect($grouped['sequential'])->toHaveCount(1);
272-
expect($grouped['concurrent'][0]->name)->toBe('concurrent');
273-
expect($grouped['sequential'][1]->name)->toBe('sequential');
269+
expect($resolved)->toHaveKeys(['concurrent', 'sequential']);
270+
expect($resolved['concurrent'])->toHaveCount(1);
271+
expect($resolved['sequential'])->toHaveCount(1);
272+
expect($resolved['concurrent'][0]['toolCall']->name)->toBe('concurrent');
273+
expect($resolved['sequential'][1]['toolCall']->name)->toBe('sequential');
274274
});

‎tests/ToolTest.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,47 @@ public function __invoke(string $query): string
8585
->toBe('The event is at 3pm eastern');
8686
});
8787

88+
it('handles invokable subclass with using($this) without circular reference', function (): void {
89+
$tool = new class extends Tool
90+
{
91+
public function __construct()
92+
{
93+
parent::__construct();
94+
$this->as('test_tool')
95+
->for('A test tool')
96+
->withParameter(new StringSchema('query', 'the query'))
97+
->using($this);
98+
}
99+
100+
public function __invoke(string $query): string
101+
{
102+
return "Result: $query";
103+
}
104+
};
105+
106+
expect($tool->handle(query: 'hello'))->toBe('Result: hello');
107+
});
108+
109+
it('invokable subclass works without calling using() at all', function (): void {
110+
$tool = new class extends Tool
111+
{
112+
public function __construct()
113+
{
114+
parent::__construct();
115+
$this->as('auto_tool')
116+
->for('Auto-detected invokable')
117+
->withParameter(new StringSchema('input', 'the input'));
118+
}
119+
120+
public function __invoke(string $input): string
121+
{
122+
return "Auto: $input";
123+
}
124+
};
125+
126+
expect($tool->handle(input: 'test'))->toBe('Auto: test');
127+
});
128+
88129
it('can have fluent parameters', function (): void {
89130
$tool = (new Tool)
90131
->as('test tool')

0 commit comments

Comments
 (0)