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

[figitaki]

Community review. connor4312 has the final call. The method-based dispatch reads well, and the RequestMethod trait fixes the drift concern from the first round. I wrote tests for a few edge cases, and three things came out of it. All three are reproduced; details are inline.

1. The pre-init InvalidParams arm catches every method, not just initialize. So malformed first requests skip the 2026-07-28 version check and the method-served check. (blocking, in my view)
2. After a rebase onto main, skills/get and resources/directory/read answer malformed params with -32602 instead of -32601 when the server doesn't declare the extension or directoryRead. Their capability checks (#1392, #1409) run inside the match arms, after validate_method_served. (blocking once rebased) skills/list is fine: malformed cursors still get -32601.
3. inline_lifecycle_tells_removed_methods_from_invalid_params never finishes. It waits on server_task after discover, while serve() is still waiting for the next request. (blocking)

Results, all with the CI feature set: on main, on this PR, on this PR plus the guard from comment 1, and on a rebased branch with both fixes, the full suite has zero failures when the hanging test is skipped. Run alone, the hanging test times out after 300s on this PR, with or without the guard. It doesn't exist on main.

Non-blocking: servers that used to see malformed known-method requests in on_custom_request won't anymore. That's the correct behavior, but it's observable, so a changelog line would help (VERSIONING.md).

On ServerRequest, +1 to a follow-up. I'm happy to take that PR once this lands, unless you want it.

If it's useful, I can send the regression tests and both fixes as separate patches (tests separate from fixes). I'm not opening a competing PR.

AI disclosure: I used Claude Code to review the diff and draft these comments, and a coding agent to write the tests and run the suites. I reviewed the code, the test output and the response bodies myself.

[figitaki]

This arm answers -32602 for any method before the handshake, not just initialize. On main, a first request that isn't initialize goes through the _meta check and then the inline path, where handle_request checks the protocol version and calls validate_method_served. With this arm, a malformed first request skips both:

First request	main	this PR	PR + guard
malformed params, unsupported protocolVersion in _meta	-32022	-32602	-32022
malformed logging/setLevel ({"level": 7}), 2026-07-28	—	-32602	-32601
well-formed logging/setLevel (control), 2026-07-28	-32601	-32601	-32601

The guard only intercepts initialize, and lets any other InvalidParams fall through to request => request:

ClientRequest::InvalidParams(invalid) if invalid.method() == "initialize" => { /* existing -32602 + continue */ }

malformed_initialize_before_handshake_is_invalid_params still passes with the guard.

[figitaki]

Heads-up for the rebase: main now gates skills/get / skills/list and resources/directory/read inside their match arms with validate_skills_capability. Those arms run after this pre-check. On a rebased branch, with a server that doesn't declare the skills extension (or directoryRead):

• skills/get with malformed params → -32602 (expected -32601)
• resources/directory/read with malformed params → -32602 (expected -32601)

Moving the skills and directoryRead checks in here, next to the tasks check, gives -32601 for both, and regression tests for both pass with it. skills/list isn't affected: a bad cursor object still gets -32601.

Every capability-gated method added later will need the same treatment. A table-driven test would catch the next one: for each gated method, turn its capability off, send malformed params, and assert -32601.

[figitaki]

This test doesn't finish. It sends discover and then awaits server_task, but after answering a discover request serve() goes back to waiting for the next message (the bootstrap-probe branch in serve_server_with_ct_inner continues the loop), so the await never returns. Run alone, it times out at 300s on this branch, with or without the guard. Awaiting server_task only after a request that selects the lifecycle, or dropping the discover step, should let it finish. (blocking: it hangs a CI run that includes it)