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 used to fall through the untagged ClientRequest to CustomRequest, and the default on_custom_request answered -32601 with the method name. The client was told the server has no tools/call when the problem was its own arguments.

Change (reworked after review, see the thread below):

• RequestMethod trait: every Request*<M, _> carries its method name (M::VALUE).
• ClientRequest deserializes by method instead of #[serde(untagged)]: the body is buffered once, only the named variant is parsed, and a serde error becomes the #[doc(hidden)] InvalidParams(InvalidParamsRequest) variant, which keeps the method, params, error text and _meta. CustomRequest is only for methods the SDK does not define. Serializing InvalidParams yields the custom request it came from.
• One validate_method_served pre-check in handle_request replaces the per-arm lifecycle gates and validate_tasks_capability calls: methods the lifecycle does not serve stay -32601 whatever their params (ping, logging/setLevel, resources/subscribe, resources/unsubscribe on 2026-07-28, subscriptions/listen on the legacy lifecycle, initialize once a session is inline). This also closes the bypass where initialize and logging/setLevel with well-formed params were served on 2026-07-28.
• The server answers ClientRequest::InvalidParams with -32602 naming the method, for example invalid params for tools/call: invalid type: integer 7, expected a string. Before the handshake a malformed initialize gets the same answer and the server keeps waiting.

Tests: test_deserialization::client_request_by_method (fitting params → typed variant, unfitting → InvalidParams keeping _meta, missing params, unknown method → CustomRequest); test_known_method_with_invalid_params_is_invalid_params (the rows from the issue, none reaches on_custom_request); inline_lifecycle_tells_removed_methods_from_invalid_params and test_legacy_lifecycle_keeps_unserved_method_not_found (removed methods stay -32601 on each lifecycle, served ones give -32602); malformed_initialize_before_handshake_is_invalid_params. fmt, the three CI clippy sets and cargo test -p rmcp pass with the CI feature set and with all features; conformance server suites pass locally (2025-11-25: 83/83, 2026-07-28: 153/153).

Not in this PR: a non-object params answered -32600 without the request's id (the smaller point in the issue), and the same deserialization shape for ServerRequest. Happy to open either 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.

[talhayme]

Reworked along those lines in 201a516:

• RequestMethod trait: every Request*<M, _> carries M::VALUE, so there is no hand-kept method table.
• ClientRequest now deserializes by method (ts_union! grew a #[request] arm). The body is buffered once, only the named variant is parsed, and a serde error lands in a #[doc(hidden)] InvalidParams(InvalidParamsRequest) variant that keeps the method, params, error text and _meta. CustomRequest is only for methods the SDK does not define. Serializing InvalidParams gives back the custom request it came from, so the client-side types are unaffected.
• One validate_method_served pre-check in handle_request replaces the per-arm if !legacy_request gates and validate_tasks_capability calls. It also closes the bypass @SIDDARTHAREDDY8 spotted: initialize and logging/setLevel with well-formed params are now -32601 on the 2026-07-28 lifecycle too. initialize is judged by whether the session is already inline (request metadata required, or a negotiated version ≥ 2026-07-28), since before the handshake the version is only known from its own params.
• Before the handshake a malformed initialize is answered -32602 and the server keeps waiting for a proper one (service/server.rs).

ServerRequest is unchanged here; happy to give it the same shape in a follow-up if you want it on the client side as well.

Checks: fmt, the three CI clippy sets, cargo test -p rmcp with the CI feature set and with all features, conformance 2025-11-25 83/83 and 2026-07-28 153/153.

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

[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.

[talhayme]

@SIDDARTHAREDDY8 Thanks for the close read. All three are addressed in the rework (201a516):

1. The asymmetry is gone: the lifecycle gate is now a single validate_method_served pre-check on the method name, so initialize and logging/setLevel with valid params get -32601 on 2026-07-28 like ping and the subscribe pair. inline_lifecycle_tells_removed_methods_from_invalid_params covers exactly those rows (fitting and unfitting params on removed methods, unfitting params on served ones).
2. Lifecycle coverage: that test runs on a server/discover session; test_legacy_lifecycle_keeps_unserved_method_not_found runs on a 2025-11-25 handshake and checks a malformed subscriptions/listen stays -32601 while a malformed tools/call on the same connection is -32602; malformed_initialize_before_handshake_is_invalid_params covers the pre-handshake case.
3. known_method_params_error and its "params do not match the method" fallback no longer exist. The serde error is captured at the only parse, in the InvalidParams variant, so there is no second attempt that could disagree with the first.

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