Skip to content

Comments

Make LoggingMessageNotificationParams.Data required per MCP spec#1353

Merged
stephentoub merged 2 commits intomainfrom
copilot/fix-logging-message-data-requirement
Feb 23, 2026
Merged

Make LoggingMessageNotificationParams.Data required per MCP spec#1353
stephentoub merged 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
@stephentoub stephentoub merged commit 571e4dc into main Feb 23, 2026
17 of 18 checks passed
@stephentoub stephentoub deleted the copilot/fix-logging-message-data-requirement branch February 23, 2026 16:37
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.

3 participants