fix(server): answer a known method with invalid params with -32602

[talhayme]

Fixes #1347.

A request that names a method the SDK defines but whose params do not deserialize falls through the untagged ClientRequest to CustomRequest, and the default on_custom_request answers -32601 with the method name. The client is told the server has no tools/call when the problem is its own arguments.

Change: before calling on_custom_request, the server checks whether the method is one ClientRequest defines (ClientRequest::known_method_params_error, crate-private). If it is, it reads the request again as that method's type to recover the deserialization error and answers -32602 with it, for example invalid params for tools/call: invalid type: integer 7, expected a string. A method the SDK does not define still reaches on_custom_request unchanged, and so do the methods this lifecycle does not serve, which keep -32601 as their typed arms answer them (SEP-2575): initialize, ping, logging/setLevel, resources/subscribe and resources/unsubscribe on 2026-07-28, subscriptions/listen on the legacy lifecycle.

Test: test_known_method_with_invalid_params_is_invalid_params sends the rows from the issue (tools/call with a numeric name, with string arguments, without params; prompts/get and resources/read with a numeric field) over a duplex transport and checks each gets -32602 naming the method, and that none reaches on_custom_request. Without the change it fails on the first row. cargo clippy (all three CI variants) and cargo test -p rmcp pass with the CI feature sets, and the conformance server suites pass locally (2025-11-25: 83/83, 2026-07-28: 153/153, including server-stateless).

Not in this PR: the smaller point in the issue, a non-object params answered -32600 without the request's id. Happy to open that separately.

AI disclosure: written with Claude Code; I reviewed the change and ran the tests above.

🤖 Generated with Claude Code

[github-actions]

@connor4312 is on triage rotation this week and is the reviewer for this one.

Labels come from the PR labeler. Please review the change.

_{Automated triage. Labels are a first pass and the assigned maintainer has the final say.}

[connor4312]

I think this is identifying a valid problem but this seems like not the right approach. Having to maintain the method lists and mapping in multiple places is fragile and likely to drift over time. Fixing this however requires some rework up to the deserialization step where the distinction between unknown methods and incorrect params is lost.

Instead I would suggest reworking deserialization a little bit into something like this (rough example) where the method is identified in order to deserialize (or fail on) the specific request params

impl<'de> Deserialize<'de> for IncomingRequest {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        let raw = RawRequest::deserialize(deserializer)?;

        Ok(match raw.method.as_str() {
            "tools/call" => {
                Self::CallTool(serde_json::from_value(raw.params))
            }
// ...
            _ => Self::Custom(raw),
        })
    }
}

[connor4312]

Alternative design idea from Gemini, leaning on macros:

---

1. Derive the method name from the request type itself (no manual 18-entry table):
   Every typed request is already parameterized by M: ConstString (Request<M, P>, RequestOptionalParam<M, P>, RequestNoParam<M>). Add a trait:

   pub(crate) trait RequestMethod {
       const METHOD: &'static str;
   }
   impl<M: ConstString, P> RequestMethod for Request<M, P> {
       const METHOD: &'static str = M::VALUE;
   }
   impl<M: ConstString, P> RequestMethod for RequestOptionalParam<M, P> {
       const METHOD: &'static str = M::VALUE;
   }
   impl<M: ConstString> RequestMethod for RequestNoParam<M> {
       const METHOD: &'static str = M::VALUE;
   }

2. In ts_union!, dispatch Deserialize by "method" instead of #[serde(untagged)]:
   • Peek at "method" in the buffered map/value.
   • Match "method" against $(<$V as RequestMethod>::METHOD => ...)*:
     • Deserialize only $V (which is also $O(1)$ instead of trying up to 18 untagged variants sequentially).
     • If $V::deserialize fails, capture the serde error right there (plus _meta into extensions) into a non-public variant on the #[non_exhaustive] enum (e.g., InvalidParams { method: &'static str, error: String, extensions: Extensions }) instead of falling through to CustomRequest and re-parsing later.
   • Only if "method" does not match any <$V as RequestMethod>::METHOD does it deserialize as CustomRequest.
3. Move lifecycle/capability gating in handle_request into a single pre-check on request.method() (not duplicated in both the match arms and a not_served slice):
   Before match request, run one validate_method_served(self, request.method(), legacy_request, &context)? check that handles:
   • !legacy_request: reject initialize, ping, logging/setLevel, resources/subscribe, resources/unsubscribe with -32601 (fixing the bug where SetLevelRequest and InitializeRequest with valid params currently bypass the !legacy_request check).
   • legacy_request: reject subscriptions/listen with -32601.
   • tasks/get | tasks/update | tasks/cancel: run validate_tasks_capability.
     Then in match request, the individual arms don't need if !legacy_request / validate_tasks_capability at all, and ClientRequest::InvalidParams { method, error, .. } simply returns Err(McpError::invalid_params(format!("invalid params for {method}: {error}"), None)).

[talhayme]

Thanks, agreed — the second list was the weak part. I'll rework it along those lines in this PR:

1. a RequestMethod trait so each request type carries its method name;
2. a Deserialize for ClientRequest that dispatches on method, deserializes only that variant, and keeps its error in a #[doc(hidden)] InvalidParams variant (the enum is already #[non_exhaustive]), with CustomRequest only for unknown methods;
3. one validate_method_served pre-check, which also closes the bypass you spotted for initialize and logging/setLevel with valid params on 2026-07-28.

Should ServerRequest (sampling and elicitation on the client side) get the same treatment here, or in a follow-up?

AI disclosure: drafted with Claude Code; I reviewed it.

[SIDDARTHAREDDY8]

Looked through the diff and the dispatch code in crates/rmcp/src/handler/server.rs at this head. I verified the check! list in known_method_params_error covers all 18 typed ClientRequest variants, so no known method is missed there. Two things I wanted to flag:

1. The not_served list for the 2026-07-28 lifecycle includes initialize and logging/setLevel, but their typed arms further up in the same match do not have a lifecycle gate: InitializeRequest always calls self.initialize(...), and the SetLevelRequest arm calls self.set_level(...) unconditionally, unlike PingRequest/SubscribeRequest/UnsubscribeRequest, which return method_not_found when !legacy_request. So on a 2026-07-28 connection a well-formed initialize or setLevel (for a handler that overrides set_level; the default returns method-not-found) is served, while a malformed one now gets -32601 instead of -32602. Is that asymmetry intended? If these methods are meant to be unserved in the new lifecycle, the typed arms would need the same gate; if they are served, they would arguably not belong in not_served so bad params report -32602 like tools/call does.

2. The new test in crates/rmcp/tests/test_custom_request.rs only exercises the default lifecycle. The not_served branch has no coverage: for example a malformed ping or resources/subscribe after 2026-07-28 negotiation staying at -32601, and a malformed subscriptions/listen on a legacy connection staying at -32601. That branching is the part most likely to drift as the SEP-2575 lifecycle rules evolve, so a case or two there would help.

One smaller question: in known_method_params_error, when the re-parse as the method's own type succeeds, the fallback message is "params do not match the method" and the real serde error is discarded. Is that path reachable in practice? I could not construct a case where the untagged dispatch fails but the single-method parse succeeds, so if it is defensive only, a short comment saying so would save the next reader the same detour.