You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Server] Keep JSON types of tool call arguments - #678
json_decode(assoc: true) makes {} and [] the same value, so tool arguments got validated against the wrong JSON type - and the schema can't restore what's already lost. tools/call arguments now keep their JSON types, the rest of the message stays arrays since its typed parsers lose nothing that way.
The parameter type decides what a handler gets: array still gets arrays at every level, \stdClass, object, mixed and untyped ones get \stdClass. So not \stdClass everywhere like Preserve native JSON types when passing arguments to tool handlers #587 asks for, but you can opt in per parameter.
arguments must be a JSON object per spec, so null and [] are refused with -32600 - PHP clients encoding an empty array will notice, our own Client doesn't send that.
array<string, T> generates object instead of array, which only worked around the lost {}. An empty default is advertised as {}.
OriginatingRequestBinder sorts \stdClass as well, otherwise a retry with reordered nested members fails verification.
A PHP object can't hold a member name starting with NUL. Such a message falls back to arrays and parses like before, except tools/call, which can't keep its argument types then and is refused with -32600 - that's why this drops the NUL datasets and the raw handler from #665.
Preserves JSON object and array distinctions in tool arguments, addressing validation failures from #586 and supporting type-preserving handler inputs from #587.
Changes:
Preserve JSON types through parsing and validation, with conversion based on handler parameter types.
Update map schemas, empty defaults, and request-state binding.
Add regression coverage and document breaking changes.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The iterable conversion rejects valid Traversable arguments such as ArrayIterator before handler invocation, so existing Traversable values must be preserved.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Preserve Traversable arguments for iterable parameters
Routing iterable through castToArray() also rejects valid Traversable arguments supplied programmatically: an ArrayIterator accepted by the PHP handler now throws Cannot cast value to array before invocation. Keep existing Traversable values unchanged for iterable parameters, while recursively converting JSON objects and arrays.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Rejecting NUL-prefixed member names and changing request-state canonicalization introduce intentional breaking changes to protocol interoperability and multi-instance compatibility that warrant human review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
breaking changeBreaking the Backwards Compatibility PromiseServerIssues & PRs related to the Server component
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #586
Closes #587
json_decode(assoc: true)makes{}and[]the same value, so tool arguments got validated against the wrong JSON type - and the schema can't restore what's already lost.tools/callarguments now keep their JSON types, the rest of the message stays arrays since its typed parsers lose nothing that way.arraystill gets arrays at every level,\stdClass,object,mixedand untyped ones get\stdClass. So not\stdClasseverywhere like Preserve native JSON types when passing arguments to tool handlers #587 asks for, but you can opt in per parameter.argumentsmust be a JSON object per spec, sonulland[]are refused with-32600- PHP clients encoding an empty array will notice, our ownClientdoesn't send that.array<string, T>generatesobjectinstead ofarray, which only worked around the lost{}. An empty default is advertised as{}.OriginatingRequestBindersorts\stdClassas well, otherwise a retry with reordered nested members fails verification.A PHP object can't hold a member name starting with NUL. Such a message falls back to arrays and parses like before, except
tools/call, which can't keep its argument types then and is refused with-32600- that's why this drops the NUL datasets and therawhandler from #665.BC breaks are in the CHANGELOG.