Preserve unrecognized keys on open schema types - #9
Conversation
Types the MCP schema declares open with `[key: string]: unknown` now keep every key they do not model and re-emit it on serialization. The parser reads index signatures from both interface declarations and inline object literals, neither of which is reachable through `getProperties()` or the inline member pattern. A type carrying one gains an `additionalProperties` property flagged as an open bag, which the DTO generator renders as: - a `KNOWN_KEYS` constant listing the wire keys the class models, including const properties such as `type` - a trailing constructor parameter, sorted last so inherited properties keep their positions - `fromArray()` collecting the remainder through `array_diff_key()` - `toArray()` returning `$result + $this->additionalProperties`, a union rather than a merge so a modelled field always wins over a preserved key of the same name The property is declared once per inheritance chain while every class computes its own `KNOWN_KEYS`, so a subclass subtracts its own fields before collecting and the merge runs exactly once, in the class that declares the property. `ToolInputSchema` and `ToolOutputSchema` are opted in by name. The wire schema for 2025-11-25 sets no `additionalProperties: false` on either, so extra JSON Schema keywords are valid on them, and the TypeScript transcription carries no index signature to detect. The list drops out once the generator targets a revision that declares them open. Interfaces consisting solely of an index signature remain plain maps and gain no bag. The bag is excluded from generated array shapes, as it is not a wire key.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @felipemarcos. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
This PR updates the TypeScript generator (and regenerated PHP DTOs) so MCP types declared as “open” preserve unrecognized keys during fromArray() hydration and re-emit them during toArray() serialization, addressing schema round-trip loss (notably JSON Schema keywords like $defs for tool schemas).
Changes:
- Add an “open bag” (
additionalProperties) mechanism plusKNOWN_KEYSlists so DTOs retain and re-emit unmodelled wire keys. - Teach the parser/synthetic extractor to detect index signatures and opt specific inline types (ToolInputSchema/ToolOutputSchema) into open-bag behavior.
- Regenerate affected PHP DTOs and update skill reference/schema artifacts to reflect the new property.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Server/Tools/DTO/ToolOutputSchema.php | Preserve unknown JSON Schema keywords via additionalProperties. |
| src/Server/Tools/DTO/ToolInputSchema.php | Preserve unknown JSON Schema keywords via additionalProperties. |
| src/Server/Tools/DTO/ListToolsResult.php | Collect/forward unknown keys for open result types. |
| src/Server/Tools/DTO/CallToolResult.php | Collect/forward unknown keys for open result types. |
| src/Server/Resources/DTO/ReadResourceResult.php | Collect/forward unknown keys for open result types. |
| src/Server/Resources/DTO/ListResourceTemplatesResult.php | Collect/forward unknown keys for open paginated result types. |
| src/Server/Resources/DTO/ListResourcesResult.php | Collect/forward unknown keys for open paginated result types. |
| src/Server/Prompts/DTO/ListPromptsResult.php | Collect/forward unknown keys for open paginated result types. |
| src/Server/Prompts/DTO/GetPromptResult.php | Collect/forward unknown keys for open result types. |
| src/Server/Core/DTO/CompleteResult.php | Collect/forward unknown keys for open result types. |
| src/Common/Traits/ValidatesRequiredFields.php | Add helper to compute “additional/unmodelled” fields. |
| src/Common/Tasks/DTO/ListTasksResult.php | Collect/forward unknown keys for open paginated result types. |
| src/Common/Tasks/DTO/GetTaskResult.php | Intersection wrapper updated to pass additionalProperties (but currently with issues). |
| src/Common/Tasks/DTO/CancelTaskResult.php | Intersection wrapper updated to pass additionalProperties (but currently with issues). |
| src/Common/Protocol/DTO/Result.php | Introduce open-bag storage/merge at the base result level. |
| src/Common/Protocol/DTO/PaginatedResult.php | Add KNOWN_KEYS + open-bag collection for paginated base result type. |
| src/Common/Protocol/DTO/InitializeResult.php | Collect/forward unknown keys for open result types. |
| src/Common/Protocol/DTO/GetTaskPayloadResult.php | Collect/forward unknown keys for open result types. |
| src/Common/JsonRpc/DTO/RequestParamsMeta.php | Preserve unknown meta keys via open bag. |
| src/Client/Tasks/DTO/CreateTaskResult.php | Collect/forward unknown keys for open result types. |
| src/Client/Sampling/DTO/CreateMessageResult.php | Collect/forward unknown keys for open result types. |
| src/Client/Roots/DTO/ListRootsResult.php | Collect/forward unknown keys for open result types. |
| src/Client/Elicitation/DTO/ElicitResult.php | Collect/forward unknown keys for open result types. |
| skill/reference/server.md | Update reference counts for tool schema properties. |
| skill/reference/common.md | Reference tables updated (currently includes non-wire additionalProperties as “Key Properties”). |
| skill/data/schema-server.json | Skill schema data updated to include new property. |
| skill/data/schema-common.json | Skill schema data updated to include new property. |
| generator/src/writers/index.ts | Emit additionalFields() helper into generated PHP trait. |
| generator/src/types/index.ts | Add open-bag property marker + helper builder. |
| generator/src/parser/index.ts | Detect interface index signatures and synthesize open-bag property. |
| generator/src/generators/dto.ts | Generate KNOWN_KEYS, open-bag ctor param ordering, and merge-on-serialize logic. |
| generator/src/extractors/synthetic-dto.ts | Detect inline index signatures and opt-in tool schemas by name. |
Comments suppressed due to low confidence (3)
src/Common/Tasks/DTO/GetTaskResult.php:156
- fromArray() is currently reading $additionalProperties from a literal 'additionalProperties' wire key, which means unrecognized keys are still dropped and nothing is preserved unless callers add that synthetic key themselves. For open schema types, this should be computed as “everything except the modelled keys” via additionalFields().
self::asInt($data['ttl']),
self::asArrayOrNull($data['_meta'] ?? null),
self::asArrayOrNull($data['additionalProperties'] ?? null),
self::asStringOrNull($data['statusMessage'] ?? null),
self::asIntOrNull($data['pollInterval'] ?? null)
src/Common/Tasks/DTO/CancelTaskResult.php:156
- fromArray() is currently reading $additionalProperties from a literal 'additionalProperties' wire key, which means unrecognized keys are still dropped and nothing is preserved unless callers add that synthetic key themselves. For open schema types, this should be computed as “everything except the modelled keys” via additionalFields().
self::asInt($data['ttl']),
self::asArrayOrNull($data['_meta'] ?? null),
self::asArrayOrNull($data['additionalProperties'] ?? null),
self::asStringOrNull($data['statusMessage'] ?? null),
self::asIntOrNull($data['pollInterval'] ?? null)
skill/reference/common.md:109
- This table lists “Key Properties” (wire keys), but it now includes the PHP-side catch-all bag
additionalProperties, which is not a wire key per the PR design (it merges into serialization output). Including it here is misleading to consumers reading the reference docs.
| Result | Result from operation | _meta?: { [key: stri..., additionalProperties?: { [key: stri... |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Intersection type wrappers such as `GetTaskResult` and `CancelTaskResult` collect the keys they do not model and re-emit them on serialization, matching the shape the DTO generator produces. The wrapper sources the bag from its base type, so the property is declared and merged in the one class that owns it. `KNOWN_KEYS` is recomputed on the wrapper rather than inherited, because the wrapper also models the keys of every merged type and must subtract all of them before collecting the remainder through `additionalFields()`. The bag is appended after the optional parameters in the constructor signature, its docblock, and the `parent::__construct()` arguments, so every other parameter keeps its position. It is excluded from the wrapper's own property declarations, assignments, getters and `toArray()` writes, as it is not a wire key.
The skill schema data and reference tables list the fields a type carries on the wire, so the open bag is left out of both. `extractProperties()` skips it for interfaces, which covers the JSON data files and the markdown tables together, as both read the same schema map. The bag is also excluded from the common-field set a union draws its discriminator from: it is shared by every open member of a union yet distinguishes none of them, so it must not reach the fallback that takes the first common field.
|
Both findings are fixed, pushed as a4a2e69 and 67cb8fd. The parameter-order note and the suppressed Verified against the regenerated output:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
generator/src/types/index.ts:24
- There is an orphaned JSDoc block before OPEN_BAG_PROPERTY (a docblock with no symbol to attach to), caused by inserting the constant between the comment and TsProperty. Removing the stray block avoids confusing generated docs and keeps TypeScript comments aligned with their declarations.
/**
* Represents a property extracted from a TypeScript interface.
*/
/**
Closes #8.
Types the MCP schema declares open with
[key: string]: unknownnow keep every key they do not model and re-emit it on serialization.How it works
The parser reads index signatures from both interface declarations and inline object literals, neither of which is reachable through
getProperties()or the inline member pattern. A type carrying one gains anadditionalPropertiesproperty flagged as an open bag, which the DTO generator renders as:KNOWN_KEYSconstant listing the wire keys the class models, including const properties such astypefromArray()collecting the remainder througharray_diff_key()toArray()returning$result + $this->additionalProperties, a union rather than a merge so a modelled field always wins over a preserved key of the same nameThe property is declared once per inheritance chain while every class computes its own
KNOWN_KEYS, so a subclass subtracts its own fields before collecting and the merge runs exactly once, in the class that declares the property.Generated shape, on a subclass:
Types affected
Resultand its subclasses,GetTaskPayloadResult, andRequestParamsMetaare detected from their index signatures.ToolInputSchemaandToolOutputSchemaare opted in by name. The wire schema for 2025-11-25 sets noadditionalProperties: falseon either, so extra JSON Schema keywords are valid on them, and the TypeScript transcription carries no index signature to detect. The list drops out once the generator targets a revision that declares them open.Interfaces consisting solely of an index signature remain plain maps and gain no bag. The bag is excluded from generated array shapes, as it is not a wire key.
Compatibility
The bag is a trailing optional constructor parameter and no existing parameter moves, so positional construction at the previous arity continues to work.
fromArray()andtoArray()signatures are unchanged.Passing an array whose keys the class models produces byte-identical serialized output.
Note for reviewers
src/is generator output. The generator change and the regenerated PHP are committed together so a cleannpm run generatereproduces the tree.