From d95348a9487e6c06d2552896e8607332d6db0b9c Mon Sep 17 00:00:00 2001
From: Dale Seo <5466341+DaleSeo@users.noreply.github.com>
Date: Sun, 22 Mar 2026 18:20:16 -0400
Subject: [PATCH] fix: prevent CallToolResult and GetTaskPayloadResult from
 shadowing CustomResult 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()`)
---
 crates/rmcp/src/model.rs      | 44 ++++++++++++++++++++++++++++++++++-
 crates/rmcp/src/model/task.rs | 21 ++++++++++++++++-
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs
index fa47ea788..482384354 100644
--- a/crates/rmcp/src/model.rs
+++ b/crates/rmcp/src/model.rs
@@ -2691,7 +2691,7 @@ pub type ElicitationCompletionNotification =
 ///
 /// Contains the content returned by the tool execution and an optional
 /// flag indicating whether the operation resulted in an error.
-#[derive(Default, Debug, Serialize, Deserialize, Clone, PartialEq)]
+#[derive(Default, Debug, Serialize, Clone, PartialEq)]
 #[serde(rename_all = "camelCase")]
 #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
 #[non_exhaustive]
@@ -2710,6 +2710,48 @@ pub struct CallToolResult {
     pub meta: Option<Meta>,
 }
 
+// Custom Deserialize implementation that:
+// 1. Defaults `content` to `[]` when the field is missing (lenient per Postel's law)
+// 2. Requires at least one known field to be present, so that `CallToolResult` doesn't
+//    greedily match arbitrary JSON objects when used inside `#[serde(untagged)]` enums
+//    (e.g. `ServerResult`), which would shadow `CustomResult`.
+impl<'de> Deserialize<'de> for CallToolResult {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        #[derive(Deserialize)]
+        #[serde(rename_all = "camelCase")]
+        struct Helper {
+            content: Option<Vec<Content>>,
+            structured_content: Option<Value>,
+            is_error: Option<bool>,
+            #[serde(rename = "_meta")]
+            meta: Option<Meta>,
+        }
+
+        let helper = Helper::deserialize(deserializer)?;
+
+        if helper.content.is_none()
+            && helper.structured_content.is_none()
+            && helper.is_error.is_none()
+            && helper.meta.is_none()
+        {
+            return Err(serde::de::Error::custom(
+                "expected at least one known CallToolResult field \
+                 (content, structuredContent, isError, or _meta)",
+            ));
+        }
+
+        Ok(CallToolResult {
+            content: helper.content.unwrap_or_default(),
+            structured_content: helper.structured_content,
+            is_error: helper.is_error,
+            meta: helper.meta,
+        })
+    }
+}
+
 impl CallToolResult {
     /// Create a successful tool result with unstructured content
     pub fn success(content: Vec<Content>) -> Self {
diff --git a/crates/rmcp/src/model/task.rs b/crates/rmcp/src/model/task.rs
index 8373aa243..343c925ef 100644
--- a/crates/rmcp/src/model/task.rs
+++ b/crates/rmcp/src/model/task.rs
@@ -123,7 +123,7 @@ pub struct GetTaskResult {
 /// (e.g., `CallToolResult` for `tools/call`). This is represented as
 /// an open object. The payload is the original request's result
 /// serialized as a JSON value.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Serialize)]
 #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
 #[non_exhaustive]
 pub struct GetTaskPayloadResult(pub Value);
@@ -135,6 +135,25 @@ impl GetTaskPayloadResult {
     }
 }
 
+// Custom Deserialize that always fails, so that `GetTaskPayloadResult` is skipped
+// during `#[serde(untagged)]` enum deserialization (e.g. `ServerResult`).
+// The payload has the same JSON shape as `CustomResult(Value)`, so they are
+// indistinguishable.  `CustomResult` acts as the catch-all instead.
+// `GetTaskPayloadResult` should be constructed programmatically via `::new()`.
+impl<'de> serde::Deserialize<'de> for GetTaskPayloadResult {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        // Consume the value so the deserializer state stays consistent.
+        serde::de::IgnoredAny::deserialize(deserializer)?;
+        Err(serde::de::Error::custom(
+            "GetTaskPayloadResult cannot be deserialized directly; \
+             use CustomResult as the catch-all",
+        ))
+    }
+}
+
 /// Response to a `tasks/cancel` request.
 ///
 /// Per spec, `CancelTaskResult = allOf[Result, Task]` — same shape as `GetTaskResult`.