Skip to content

Comments

Make LoggingMessageNotificationParams.Data required per MCP spec#1353

Open
Copilot wants to merge 2 commits intomainfrom
copilot/fix-logging-message-data-requirement
Open

Make LoggingMessageNotificationParams.Data required per MCP spec#1353
Copilot wants to merge 2 commits intomainfrom
copilot/fix-logging-message-data-requirement

Conversation

Copy link
Contributor

Copilot AI commented Feb 22, 2026

  • Verify MCP spec requires data field in LoggingMessageNotification (confirmed: data is in required array in schema)
  • Investigate JsonObject vs JsonElement: spec defines data as typeless (any JSON-serializable value = unknown), so JsonElement is correct; JsonObject would only allow JSON objects, not strings/numbers/arrays
  • Change JsonElement? Data to required JsonElement Data in LoggingMessageNotificationParams.cs
  • Update XML doc to match spec description
  • Update test: remove .Value accessor in PreservesAllProperties test
  • Update test: fix WithMinimalProperties test to provide required Data field
  • Update McpClientTests.cs: remove .Value accessor on m.Data
  • Build and run tests (all pass across net8.0, net9.0, net10.0)
  • Code review and security scan (no issues found)

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

The MCP schema defines `data` as required in LoggingMessageNotification params.
Change from `JsonElement?` (nullable, optional) to `required JsonElement`
(non-nullable, required) to match the spec.

JsonElement remains the correct type (not JsonObject) because the spec
defines data as typeless (any JSON-serializable value), not restricted
to JSON objects.

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub
Copy link
Contributor

@copilot fix the title

@stephentoub stephentoub marked this pull request as ready for review February 22, 2026 01:28
@stephentoub stephentoub added the breaking-change This issue or PR introduces a breaking change label Feb 22, 2026
Copy link
Contributor Author

Copilot AI commented Feb 22, 2026

@copilot fix the title

I don't have the ability to update the PR title directly with my available tools. Could you update it to: "Make LoggingMessageNotificationParams.Data required per MCP spec"?

Copilot AI changed the title [WIP] Fix LoggingMessageNotificationParams.Data to be required Make LoggingMessageNotificationParams.Data required per MCP spec Feb 22, 2026
Copilot AI requested a review from stephentoub February 22, 2026 01:30
@stephentoub stephentoub added this to the 1.0.0 (GA) milestone Feb 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This issue or PR introduces a breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants