Skip to content

[Schema] Fix _meta guard key in Request and Notification jsonSerialize() - #485

Open
chr-hertel wants to merge 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-30-jsonserialize-guard
Open

[Schema] Fix _meta guard key in Request and Notification jsonSerialize()#485
chr-hertel wants to merge 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-30-jsonserialize-guard

Conversation

@chr-hertel

Copy link
Copy Markdown
Member

The guard in Request::jsonSerialize() checked $params['meta'] while the write targets $params['_meta'], so a _meta provided by the message class was silently overwritten by withMeta().

Fixes the key in both Request and Notification, with a regression test each.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fix matches the intended behavior and is covered by regression tests.

Pull request overview

Corrects JSON-RPC metadata precedence so class-provided _meta values are preserved.

Changes:

  • Fixes _meta guards in request and notification serialization.
  • Adds regression coverage for both message types.
File summaries
File Description
src/Schema/JsonRpc/Request.php Checks the correct metadata key.
src/Schema/JsonRpc/Notification.php Checks the correct metadata key.
tests/Unit/Schema/JsonRpc/RequestTest.php Tests request metadata precedence.
tests/Unit/Schema/JsonRpc/NotificationTest.php Tests notification metadata precedence.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

2 participants