Skip to content

fix: prevent CallToolResult from shadowing CustomResult in untagged enum deserialization#771

Open
DaleSeo wants to merge 1 commit intomainfrom
fix/untagged-enum-deserialization
Open

fix: prevent CallToolResult from shadowing CustomResult in untagged enum deserialization#771
DaleSeo wants to merge 1 commit intomainfrom
fix/untagged-enum-deserialization

Conversation

@DaleSeo
Copy link
Member

@DaleSeo DaleSeo commented Mar 22, 2026

Motivation and Context

After #752 added #[serde(default)] to CallToolResult.content, all fields became optional. This made CallToolResult greedily match any JSON object in the #[serde(untagged)] ServerResult enum, preventing CustomResult from ever being reached. GetTaskPayloadResult(Value) had the same problem — both it and CustomResult wrap Value, so whichever comes first wins.

Fix: replace derived Deserialize with custom impls.

  • CallToolResult now requires at least one known field, still defaults content to [] when missing.
  • GetTaskPayloadResult always fails deserialization (indistinguishable from CustomResult in JSON; construct via ::new()).

Discovered via #761 (test_custom_client_request_reaches_server failing without the local feature).

How Has This Been Tested?

All the existing tests have passed.

cargo test --features server,client --test test_custom_request -- --nocapture
cargo test --features server,client --test test_structured_output -- --nocapture
cargo test --all-features

Before this fix, the first command panicked:

---- test_custom_client_request_reaches_server stdout ----

thread 'test_custom_client_request_reaches_server' panicked at crates/rmcp/tests/test_custom_request.rs:82:18:
Expected custom result, got: CallToolResult(CallToolResult { content: [], structured_content: None, is_error: None, meta: None })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test_custom_client_request_reaches_server

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Breaking Changes

No. CallToolResult still accepts all valid payloads (including missing content). GetTaskPayloadResult can no longer be deserialized from JSON directly — construct via ::new() instead.

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

…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()`)
@github-actions github-actions bot added T-core Core library changes T-model Model/data structure changes labels Mar 22, 2026
@DaleSeo DaleSeo self-assigned this Mar 22, 2026
@DaleSeo DaleSeo marked this pull request as ready for review March 22, 2026 23:42
@DaleSeo DaleSeo requested a review from a team as a code owner March 22, 2026 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-model Model/data structure changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant