Skip to content
Open
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

All notable changes to `mcp/sdk` will be documented in this file.

0.9.1
-----

* Fix tools with an array shape parameter whose keys are nullable or unions (e.g. `array{name: string, nickname: string|null}`) rejecting valid arguments with `-32602`: a `null` inside the shape no longer turns the parameter into `array|null`, and such keys get their own types instead of `object`.

0.9.0
-----

Expand Down
26 changes: 22 additions & 4 deletions src/Capability/Discovery/SchemaGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ private function parseParametersInfo(\ReflectionMethod|\ReflectionFunction $refl
$allowsNull = true;
} elseif ($hasDefault && null === $defaultValue) {
$allowsNull = true;
} elseif (str_contains($typeString, 'null') || 'mixed' === strtolower($typeString)) {
} elseif (self::hasNullBranch($typeString) || 'mixed' === strtolower($typeString)) {
$allowsNull = true;
}

Expand Down Expand Up @@ -609,7 +609,7 @@ private function getParameterTypeString(\ReflectionParameter $rp, ?Param $paramT
// Otherwise, use the DocBlock type if it was valid and non-generic
if (null !== $docBlockType && !$isDocBlockTypeGeneric) {
// Consider if DocBlock adds nullability missing from reflection
if (false !== stripos($docBlockType, 'null') && $reflectionTypeString && false === stripos($reflectionTypeString, 'null') && !str_ends_with($reflectionTypeString, '|null')) {
if (self::hasNullBranch($docBlockType) && $reflectionTypeString && false === stripos($reflectionTypeString, 'null') && !str_ends_with($reflectionTypeString, '|null')) {
// If reflection didn't capture null, but docblock did, append |null (if not already mixed)
if ('mixed' !== $reflectionTypeString) {
return $reflectionTypeString.'|null';
Expand Down Expand Up @@ -733,6 +733,17 @@ public static function splitUnion(string $type): array
return array_values(array_filter($branches, static fn (string $branch): bool => '' !== $branch));
}

/**
* Whether `null` is one of the top-level alternatives of a type, as in
* `string|null`. A `null` inside a generic or a shape, as in
* `array{name: string|null}`, belongs to that inner type and does not
* make the whole type nullable.
*/
private static function hasNullBranch(string $type): bool
{
return \in_array('null', array_map(strtolower(...), self::splitUnion($type)), true);
}

/**
* Maps a PHP type string (potentially a union) to an array of JSON Schema type names.
*
Expand Down Expand Up @@ -936,9 +947,16 @@ private function parsePropertyDefinition(string $propDefinition, array &$propert
],
];
}
// Simple type
// Simple type, or a union of them such as `string|null` or `?string`
else {
$properties[$propName] = ['type' => $this->mapSimpleTypeToJsonSchema($propType)];
if (str_starts_with($propType, '?')) {
$propType = 'null|'.substr($propType, 1);
}
$types = array_values(array_unique(array_map(
fn (string $branch): string => 'null' === strtolower($branch) ? 'null' : $this->mapSimpleTypeToJsonSchema($branch),
self::splitUnion($propType),
)));
$properties[$propName] = ['type' => 1 === \count($types) ? $types[0] : $types];
}
}
}
Expand Down
16 changes: 16 additions & 0 deletions tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,22 @@ public function nullableTypedArrays(
): void {
}

/**
* A `null` or `|` inside an array shape belongs to one of its keys.
*
* @param array{name: string, nickname: string|null} $nullUnionMember Shape with a nullable key
* @param array{name: string, nickname: ?string} $nullableMember Shape with a nullable key
* @param array{id: int|string} $unionMember Shape with a union key
* @param array{id: int}|null $nullableShape Nullable shape
*/
public function arrayShapesWithUnionMembers(
array $nullUnionMember,
array $nullableMember,
array $unionMember,
$nullableShape,
): void {
}

// ===== NULLABLE AND OPTIONAL SCENARIOS =====

/**
Expand Down
33 changes: 33 additions & 0 deletions tests/Unit/Capability/Discovery/SchemaGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,39 @@ public function testRecoversItemsTypeForNullableTypedArrays(): void
$this->assertEqualsCanonicalizing(['nullableStrings'], $schema['required']);
}

public function testKeepsNullableAndUnionKeysOfArrayShapesInsideTheShape(): void
{
$method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'arrayShapesWithUnionMembers');
$schema = $this->schemaGenerator->generate($method);
// The `null` of a key must neither make the parameter nullable nor turn it back into a plain array.
$this->assertEquals([
'type' => 'object',
'description' => 'Shape with a nullable key',
'properties' => ['name' => ['type' => 'string'], 'nickname' => ['type' => ['string', 'null']]],
'required' => ['name', 'nickname'],
], $schema['properties']['nullUnionMember']);
$this->assertEquals([
'type' => 'object',
'description' => 'Shape with a nullable key',
'properties' => ['name' => ['type' => 'string'], 'nickname' => ['type' => ['null', 'string']]],
'required' => ['name', 'nickname'],
], $schema['properties']['nullableMember']);
$this->assertEquals([
'type' => 'object',
'description' => 'Shape with a union key',
'properties' => ['id' => ['type' => ['integer', 'string']]],
'required' => ['id'],
], $schema['properties']['unionMember']);
// A top-level `|null` still makes the shape nullable.
$this->assertEquals([
'type' => ['object', 'null'],
'description' => 'Nullable shape',
'properties' => ['id' => ['type' => 'integer']],
'required' => ['id'],
], $schema['properties']['nullableShape']);
$this->assertEqualsCanonicalizing(['nullUnionMember', 'nullableMember', 'unionMember', 'nullableShape'], $schema['required']);
}

public function testHandlesNullableTypeHintsAndOptionalParameters(): void
{
$method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'nullableAndOptional');
Expand Down
Loading