Skip to content

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

Merged
chr-hertel merged 2 commits into
modelcontextprotocol:mainfrom
Revinand:fix/schema-array-shape-nullable-members
Oct 27, 2026
Merged

chr-hertel merged 2 commits into
modelcontextprotocol:mainfrom
Revinand:fix/schema-array-shape-nullable-members

Conversation

@Revinand

Copy link
Copy Markdown
Contributor

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.

@chr-hertel chr-hertel added Server Issues & PRs related to the Server component bug Something isn't working labels Oct 27, 2026
chr-hertel
chr-hertel previously approved these changes Oct 27, 2026

@chr-hertel chr-hertel left a comment •

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.

Thank you @Revinand & welcome to the repo! :)

(added some more testcases - was not the only scenario)

@chr-hertel
chr-hertel force-pushed the fix/schema-array-shape-nullable-members branch from d06504d to f9e56db Compare October 27, 2026 20:56
@chr-hertel
chr-hertel merged commit ed8098c into modelcontextprotocol:main Oct 27, 2026
28 checks passed
chr-hertel added a commit to Adebayo120/php-sdk that referenced this pull request Oct 27, 2026
…nsport-leak-on

* origin/main:
  [Capability] Keep a nullable or union key of an array shape inside the shape (modelcontextprotocol#613)
  chore(deps): bump the npm_and_yarn group across 1 directory with 3 updates (modelcontextprotocol#632)
  [Server] Drop `null` from `DocBlockParser::parseDocBlock()` (modelcontextprotocol#633)
  chore(deps): bump astral-sh/setup-uv from 10.3.0 to 10.3.2 (modelcontextprotocol#628)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Server Issues & PRs related to the Server component

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

2 participants