From 6ea71947a5400a3bd76cc9c6d70d830e86540792 Mon Sep 17 00:00:00 2001 From: Faneraiy14 Date: Wed, 19 Aug 2026 17:11:11 +0300 Subject: [PATCH 1/3] Cast variadic tool parameters element-by-element SchemaGenerator::buildVariadicParameterSchema() already advertises a variadic parameter (e.g. `int ...$scores`) as a JSON "array" schema, so a spec-conformant client sends an array for it. But ReferenceHandler::prepareArguments() never special-cased ReflectionParameter::isVariadic(): it fell through to the regular single-value branch and passed the whole array to castArgumentType(), which tried to cast it as one scalar and failed - e.g. "Cannot cast value to integer" for `int ...$scores` given [1, 2, 3]. Any tool handler declaring a variadic parameter was therefore unreachable through the advertised schema, confirmed by direct reproduction against ReferenceHandler::handle() before this change. Casts each array element individually via the existing castArgumentType() (unchanged - it already inspects the variadic parameter's element type, not "array", per PHP reflection semantics) and appends to the result in order; variadic is always the last parameter, so this preserves correct positional ordering. A non-array value for a variadic argument now fails clearly with an invalid-params error instead of the confusing cast message. --- src/Capability/Registry/ReferenceHandler.php | 25 +++++++++- .../Registry/ReferenceHandlerTest.php | 46 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/Capability/Registry/ReferenceHandler.php b/src/Capability/Registry/ReferenceHandler.php index 99e58442..0991920c 100644 --- a/src/Capability/Registry/ReferenceHandler.php +++ b/src/Capability/Registry/ReferenceHandler.php @@ -99,10 +99,33 @@ 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()) { + // SchemaGenerator advertises variadic parameters as a JSON "array" + // schema (see buildVariadicParameterSchema()), so the incoming value + // here is an array whose elements each need casting to the variadic's + // element type. Falling through to castArgumentType() below would try + // to cast the whole array as a single scalar and fail (e.g. "Cannot + // cast value to integer" for `int ...$extra`). Variadic is always the + // last parameter, so appending here preserves correct final ordering. + $values = $arguments[$paramName] ?? []; + if (!\is_array($values)) { + throw RegistryException::invalidParams(\sprintf('Parameter `%s` must be an array of values.', $paramName)); + } + foreach (array_values($values) as $value) { + try { + $finalArgs[] = $this->castArgumentType($value, $parameter); + } catch (InvalidArgumentException $e) { + throw RegistryException::invalidParams($e->getMessage(), $e); + } catch (\Throwable $e) { + throw RegistryException::internalError("Error processing parameter `{$paramName}`: {$e->getMessage()}", $e); + } + } + continue; + } + // Check if parameter is a special injectable type $type = $parameter->getType(); if ($type instanceof \ReflectionNamedType && !$type->isBuiltin()) { diff --git a/tests/Unit/Capability/Registry/ReferenceHandlerTest.php b/tests/Unit/Capability/Registry/ReferenceHandlerTest.php index dadca9f5..3ac5e036 100644 --- a/tests/Unit/Capability/Registry/ReferenceHandlerTest.php +++ b/tests/Unit/Capability/Registry/ReferenceHandlerTest.php @@ -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; @@ -141,4 +142,49 @@ 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 testHandleThrowsRegistryExceptionWhenVariadicArgumentIsNotAnArray(): 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 an array of values.'); + + (new ReferenceHandler())->handle($reference, [ + '_session' => $this->createMock(SessionInterface::class), + 'name' => 'bad', + 'scores' => 'not-an-array', + ]); + } } From cf300c4701f5f0216b7122921c365984745e6cd4 Mon Sep 17 00:00:00 2001 From: Faneraiy14 Date: Sat, 29 Aug 2026 12:20:08 +0300 Subject: [PATCH 2/3] Address chr-hertel's review: wrap scalars, reject objects, index cast errors - Non-array variadic argument is now wrapped as a single-element list instead of being rejected: MCP prompt arguments are always Record per protocol, so a variadic prompt parameter legitimately receives a plain string, not an array. Rejecting it regressed behaviour that worked on main. - A JSON object (non-list array) is now explicitly rejected instead of being silently flattened into positional args by array_values(). - Cast-failure errors now include the parameter name and element index, so a failure inside a large array is actually actionable. - Trimmed the inline rationale comment; the PR description already covers it. Tests updated: replaced the "throws on non-array" test (no longer correct) with tests for the wrap-as-single-element, reject-object, and indexed-error-message behaviours. --- src/Capability/Registry/ReferenceHandler.php | 21 +++++----- .../Registry/ReferenceHandlerTest.php | 39 +++++++++++++++++-- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/Capability/Registry/ReferenceHandler.php b/src/Capability/Registry/ReferenceHandler.php index 0991920c..23390c13 100644 --- a/src/Capability/Registry/ReferenceHandler.php +++ b/src/Capability/Registry/ReferenceHandler.php @@ -103,24 +103,23 @@ private function prepareArguments(\ReflectionFunctionAbstract $reflection, array $paramPosition = $parameter->getPosition(); if ($parameter->isVariadic()) { - // SchemaGenerator advertises variadic parameters as a JSON "array" - // schema (see buildVariadicParameterSchema()), so the incoming value - // here is an array whose elements each need casting to the variadic's - // element type. Falling through to castArgumentType() below would try - // to cast the whole array as a single scalar and fail (e.g. "Cannot - // cast value to integer" for `int ...$extra`). Variadic is always the - // last parameter, so appending here preserves correct final ordering. + // Each element is cast individually below; a non-array value is + // wrapped as a single element, since prompt arguments arrive as + // plain strings (Record per the protocol) even + // though tool arguments follow the advertised "array" schema. $values = $arguments[$paramName] ?? []; if (!\is_array($values)) { - throw RegistryException::invalidParams(\sprintf('Parameter `%s` must be an array of values.', $paramName)); + $values = [$values]; + } elseif (!array_is_list($values)) { + throw RegistryException::invalidParams(\sprintf('Parameter `%s` must be a list of values, not an object.', $paramName)); } - foreach (array_values($values) as $value) { + foreach ($values as $index => $value) { try { $finalArgs[] = $this->castArgumentType($value, $parameter); } catch (InvalidArgumentException $e) { - throw RegistryException::invalidParams($e->getMessage(), $e); + throw RegistryException::invalidParams(\sprintf('Parameter `%s[%d]`: %s', $paramName, $index, $e->getMessage()), $e); } catch (\Throwable $e) { - throw RegistryException::internalError("Error processing parameter `{$paramName}`: {$e->getMessage()}", $e); + throw RegistryException::internalError(\sprintf('Error processing parameter `%s[%d]`: %s', $paramName, $index, $e->getMessage()), $e); } } continue; diff --git a/tests/Unit/Capability/Registry/ReferenceHandlerTest.php b/tests/Unit/Capability/Registry/ReferenceHandlerTest.php index 3ac5e036..73b8f134 100644 --- a/tests/Unit/Capability/Registry/ReferenceHandlerTest.php +++ b/tests/Unit/Capability/Registry/ReferenceHandlerTest.php @@ -173,18 +173,51 @@ public function testHandleTreatsOmittedVariadicArgumentAsZeroElements(): void $this->assertSame(0, $result); } - public function testHandleThrowsRegistryExceptionWhenVariadicArgumentIsNotAnArray(): void + public function testHandleWrapsANonArrayVariadicArgumentAsASingleElement(): void + { + // Tool arguments follow the advertised "array" schema, but MCP prompt + // arguments are always Record 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 testHandleIncludesParameterNameAndIndexWhenAVariadicElementFailsToCast(): 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 an array of values.'); + $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' => 'not-an-array', + 'scores' => ['1', 'not-a-number', '3'], ]); } } From 18a7cf6591092735d1779c47c285d6148b80a2fb Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sat, 29 Aug 2026 15:43:40 +0200 Subject: [PATCH 3/3] Pin argument order for an injectable before a variadic --- .../Registry/ReferenceHandlerTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/Unit/Capability/Registry/ReferenceHandlerTest.php b/tests/Unit/Capability/Registry/ReferenceHandlerTest.php index 73b8f134..6ddeec36 100644 --- a/tests/Unit/Capability/Registry/ReferenceHandlerTest.php +++ b/tests/Unit/Capability/Registry/ReferenceHandlerTest.php @@ -206,6 +206,23 @@ public function testHandleThrowsRegistryExceptionWhenVariadicArgumentIsAnObjectN ]); } + 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);