Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/Capability/Registry/ReferenceHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,32 @@ private function prepareArguments(\ReflectionFunctionAbstract $reflection, array
$finalArgs = [];

foreach ($reflection->getParameters() as $parameter) {
// TODO: Handle variadic parameters.
$paramName = $parameter->getName();
$paramPosition = $parameter->getPosition();

if ($parameter->isVariadic()) {
// Each element is cast individually below; a non-array value is
// wrapped as a single element, since prompt arguments arrive as
// plain strings (Record<string,string> per the protocol) even
// though tool arguments follow the advertised "array" schema.
$values = $arguments[$paramName] ?? [];
if (!\is_array($values)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regresses behaviour that works on main today: ['topics' => 'php'] for string ...$topics currently yields ['php'].

It matters for prompts. prompts/get arguments are Record<string, string> by protocol — the Inspector sends userIds=["101","102"] through as a literal string — so a variadic prompt parameter fails here where it works now.

Wrapping a non-array in a one-element list instead of throwing keeps both cases working.

$values = [$values];
} elseif (!array_is_list($values)) {
throw RegistryException::invalidParams(\sprintf('Parameter `%s` must be a list of values, not an object.', $paramName));
}
foreach ($values as $index => $value) {
try {
$finalArgs[] = $this->castArgumentType($value, $parameter);
} catch (InvalidArgumentException $e) {
throw RegistryException::invalidParams(\sprintf('Parameter `%s[%d]`: %s', $paramName, $index, $e->getMessage()), $e);
} catch (\Throwable $e) {
throw RegistryException::internalError(\sprintf('Error processing parameter `%s[%d]`: %s', $paramName, $index, $e->getMessage()), $e);
}
}
continue;
}

// Check if parameter is a special injectable type
$type = $parameter->getType();
if ($type instanceof \ReflectionNamedType && !$type->isBuiltin()) {
Expand Down
96 changes: 96 additions & 0 deletions tests/Unit/Capability/Registry/ReferenceHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Mcp\Capability\Registry\ElementReference;
use Mcp\Capability\Registry\ReferenceHandler;
use Mcp\Exception\InvalidArgumentException;
use Mcp\Exception\RegistryException;
use Mcp\Server\ClientGateway;
use Mcp\Server\Handler\ResourceHandlerInterface;
use Mcp\Server\Handler\ToolHandlerInterface;
Expand Down Expand Up @@ -141,4 +142,99 @@ public function testHandleThrowsForStringHandlerThatIsNeitherFunctionNorClass():

(new ReferenceHandler())->handle($reference, ['_session' => $session]);
}

public function testHandleCastsEachElementOfAnArrayArgumentForAVariadicParameter(): void
{
// SchemaGenerator advertises variadic parameters as a JSON "array" schema,
// so the array arrives here as a single named argument (not spread across
// multiple keys) and must be cast element-by-element to the variadic's type.
$closure = static fn (string $name, int ...$scores): string => \sprintf('%s:%d', $name, array_sum($scores));
$reference = new ElementReference($closure);

$result = (new ReferenceHandler())->handle($reference, [
'_session' => $this->createMock(SessionInterface::class),
'name' => 'total',
'scores' => ['1', '2', '3'],
]);

$this->assertSame('total:6', $result);
}

public function testHandleTreatsOmittedVariadicArgumentAsZeroElements(): void
{
$closure = static fn (string $name, int ...$scores): int => \count($scores);
$reference = new ElementReference($closure);

$result = (new ReferenceHandler())->handle($reference, [
'_session' => $this->createMock(SessionInterface::class),
'name' => 'empty',
]);

$this->assertSame(0, $result);
}

public function testHandleWrapsANonArrayVariadicArgumentAsASingleElement(): void
{
// Tool arguments follow the advertised "array" schema, but MCP prompt
// arguments are always Record<string,string> per the protocol - a
// client sends a plain string for a variadic prompt parameter, and
// that must still work rather than being rejected.
$closure = static fn (string $name, string ...$topics): array => $topics;
$reference = new ElementReference($closure);

$result = (new ReferenceHandler())->handle($reference, [
'_session' => $this->createMock(SessionInterface::class),
'name' => 'single',
'topics' => 'php',
]);

$this->assertSame(['php'], $result);
}

public function testHandleThrowsRegistryExceptionWhenVariadicArgumentIsAnObjectNotAList(): void
{
$closure = static fn (string $name, int ...$scores): int => \count($scores);
$reference = new ElementReference($closure);

$this->expectException(RegistryException::class);
$this->expectExceptionMessage('Parameter `scores` must be a list of values, not an object.');

(new ReferenceHandler())->handle($reference, [
'_session' => $this->createMock(SessionInterface::class),
'name' => 'bad',
'scores' => ['a' => 1, 'b' => 2],
]);
}

public function testHandleKeepsArgumentOrderWhenAnInjectableParameterPrecedesAVariadic(): void
{
// Injectable and regular parameters are assigned by position while variadic
// elements are appended, so the final order only holds because a variadic is
// always the last parameter.
$closure = static fn (ClientGateway $gateway, string $sep = ',', string ...$parts): string => implode($sep, $parts);
$reference = new ElementReference($closure);

$result = (new ReferenceHandler())->handle($reference, [
'_session' => $this->createMock(SessionInterface::class),
'_request' => new \stdClass(),
'parts' => ['a', 'b'],
]);

$this->assertSame('a,b', $result);
}

public function testHandleIncludesParameterNameAndIndexWhenAVariadicElementFailsToCast(): void
{
$closure = static fn (string $name, int ...$scores): int => \count($scores);
$reference = new ElementReference($closure);

$this->expectException(RegistryException::class);
$this->expectExceptionMessage('Parameter `scores[1]`: Cannot cast value to integer. Expected integer representation.');

(new ReferenceHandler())->handle($reference, [
'_session' => $this->createMock(SessionInterface::class),
'name' => 'bad',
'scores' => ['1', 'not-a-number', '3'],
]);
}
}