Skip to content

[Capability] Keep a nullable or union key of an array shape inside the shape - #613

Open
Revinand wants to merge 1 commit into
modelcontextprotocol:mainfrom
Revinand:fix/schema-array-shape-nullable-members
Open

Revinand wants to merge 1 commit into
modelcontextprotocol:mainfrom
Revinand:fix/schema-array-shape-nullable-members

Conversation

@Revinand

Copy link
Copy Markdown

Fixes #612

Why it fails

SchemaGenerator decides whether a parameter is nullable by searching its type string for null (str_contains() in parseParametersInfo(), stripos() on the DocBlock type in getParameterTypeString()). For array{name: string, nickname: string|null} the null of the nickname key matches, so the parameter becomes array|null and the shape is dropped. Shape keys that are unions, such as ?string or int|string, went to mapSimpleTypeToJsonSchema() whole and fell back to object.

Change

  • Both checks now use hasNullBranch(), which looks for null among the top-level branches from splitUnion(). A null inside a shape or generic no longer makes the parameter nullable; array{id: int}|null still does.
  • parsePropertyDefinition() maps each branch of a key's union, so string|null becomes ["string", "null"], ?string becomes ["null", "string"] and int|string becomes ["integer", "string"].
  • A CHANGELOG.md entry under a new 0.9.1 heading.

The third substring check, in getTypeStringFromReflection(), works on the reflection type, which cannot hold a shape, so I left it as is.

Verification

PHP 8.5.11 (composer:2 image), composer install.

  • New test testKeepsNullableAndUnionKeysOfArrayShapesInsideTheShape: on main it fails because nullUnionMember gets ['array', 'null'] with empty items. With the change it passes.
  • Unit: 2669 tests, integration: 108 tests, all green with the change.
  • phpstan and php-cs-fixer fix --dry-run: clean.
  • End to end with the stdio server from the issue: on main tools/call returns -32602. With the change tools/list shows the shape and both {"name": "Ada", "nickname": null} and {"name": "Ada", "nickname": "Countess"} are accepted.

Not run

The interop and conformance suites.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Capability] An array shape with a nullable or union key rejects valid tool arguments

1 participant