Skip to content

fix: default CallToolResult content to empty vec on missing field - #752

Merged
DaleSeo merged 1 commit into
mainfrom
fix/call-tool-result-default-content
Mar 13, 2026
Merged

fix: default CallToolResult content to empty vec on missing field#752
DaleSeo merged 1 commit into
mainfrom
fix/call-tool-result-default-content

Conversation

@DaleSeo

@DaleSeoDaleSeo commented Mar 12, 2026

Copy link
Copy Markdown
Member

Closes#750.

Motivation and Context

This PR restores lenient deserialization by adds #[serde(default)] to the content field of the CallToolResult construct.

How Has This Been Tested?

Existing and new tests cover the change.

Breaking Changes

None. This is strictly more permissive. JSON payloads that previously deserialized successfully continue to work identically. Only payloads that previously failed (missing content) now succeed.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@DaleSeoDaleSeo self-assigned this Mar 12, 2026
@github-actionsgithub-actionsBot added T-test Testing related changes T-core Core library changes labels Mar 12, 2026
@DaleSeo
DaleSeoforce-pushed the fix/call-tool-result-default-content branch from 18b3a23 to 044bff3CompareMarch 12, 2026 23:46
@github-actionsgithub-actionsBot added the T-config Configuration file changes label Mar 12, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review March 12, 2026 23:53
@DaleSeo
DaleSeo requested a review from a team as a code ownerMarch 12, 2026 23:53

@jamadeojamadeo 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.

Makes sense to me. The only reason not to do this would be if an empty array would be semantically different from a missing one, but a) I don't think that's true, and b) it would not match the MCP schema anyway.

@DaleSeo
DaleSeo merged commit 44dfcf5 into mainMar 13, 2026
16 checks passed
@DaleSeo
DaleSeo deleted the fix/call-tool-result-default-content branch March 13, 2026 20:02
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
DaleSeo added a commit that referenced this pull request Mar 22, 2026
…ustomResult in untagged enums
The `#[serde(default)]` on `CallToolResult.content` (added in #752) made
all fields optional, causing `CallToolResult` to greedily match any JSON
object during `#[serde(untagged)]` deserialization of `ServerResult`.
Similarly, `GetTaskPayloadResult(Value)` matched everything before
`CustomResult(Value)` could be reached.
Fix by replacing derived `Deserialize` impls with custom ones:
- `CallToolResult`: require at least one known field to be present
- `GetTaskPayloadResult`: always fail (indistinguishable from
`CustomResult` in JSON; construct programmatically via `::new()`)
DaleSeo added a commit that referenced this pull request Mar 23, 2026
…ustomResult in untagged enums (#771)
The `#[serde(default)]` on `CallToolResult.content` (added in #752) made
all fields optional, causing `CallToolResult` to greedily match any JSON
object during `#[serde(untagged)]` deserialization of `ServerResult`.
Similarly, `GetTaskPayloadResult(Value)` matched everything before
`CustomResult(Value)` could be reached.
Fix by replacing derived `Deserialize` impls with custom ones:
- `CallToolResult`: require at least one known field to be present
- `GetTaskPayloadResult`: always fail (indistinguishable from
`CustomResult` in JSON; construct programmatically via `::new()`)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CallToolResult deserialization rejects responses with structuredContent but no content field

2 participants

@DaleSeo@jamadeo