[xixi-box]

Fixes #1445

Why

The default ServerHandler implementations for prompts/list, completion/complete, resources/list, resources/templates/list, and tools/list returned successful empty results even when get_info() did not advertise the corresponding capability. This can mislead capability discovery and conformance clients.

Change

• Return JSON-RPC -32601 (method not found) when the corresponding capability is absent from get_info().
• Keep the empty success result when that capability is explicitly advertised.
• Extend the JS streamable-HTTP regression to cover all four unsupported methods on a tools-only server.
• Add a lightweight Rust duplex regression test for no capabilities, tools-only, and fully advertised capabilities. Unlike the npm-backed JS test, it also runs under --all-features.
• Align existing test fixtures with their declared capabilities and document the compatibility impact in crates/rmcp/CHANGELOG.md.

Compatibility note

This intentionally changes observable wire behavior: servers that do not advertise prompts, completions, resources, or tools but previously relied on the corresponding default empty success response will now return -32601 for those methods. To retain an empty default result, advertise the relevant capability. No Rust trait signatures or public types change; this is a protocol-conformance bug fix.

Verification

• New Rust duplex regression passed with client,server and with --all-features.
• Existing lifecycle/cache-hint tests passed (24 tests); server initialization tests passed (22 tests).
• HTTP discover, stateless request, session-store, and stale-session test binaries passed (16 tests total) after updating capability-aware fixtures.
• JS Streamable HTTP regression passed, including the new resource-method assertions.
• cargo fmt --all -- --check, node --check, and git diff --check passed.
• GitHub CI (including the full non-local matrix and Commitlint) passed on 2efd7c0.

The original regression was reproduced on f852ec0e9281 (fail before / pass after the initial fix).

AI assistance: OpenCode and oss-scout generated and verified the initial patch; ChatGPT helped review, extend the fix, add regression coverage, and verify the follow-up.

[chatgpt-codex-connector]

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-14T08:06:02.216636Z	e4684ee	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

@jokemanfire 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.}

[jokemanfire]

Verified at a4b6647 (base f852ec0) by running a probe pair, not by reading the diff. Three asks, all small; none is about correctness of the two hunks you wrote.

Verification

Probe: serve_directly on a tokio::io::duplex(4096) pair at ProtocolVersion::V_2026_07_28, server = ServerCapabilities::builder().enable_tools().build() and no method overrides; same probe binary run on upstream/main and on your head.

call	f852ec0 (main)	a4b6647 (head)
prompts/list	Ok	Err -32601: prompts/list
completion/complete	Ok	Err -32601: completion/complete
resources/list	Ok	Ok
resources/templates/list	Ok	Ok
tools/list	Ok	Ok

With enable_prompts().enable_completions() declared, both calls still return Ok (completion values=[]), so the "keep the empty success when advertised" half works too. CI at a4b6647 is 24/24 green.

Your regression test is real: crates/rmcp/tests/test_with_js.rs is executed by the Run Tests (no local feature) job (cargo test -p rmcp --features <all except local>), not by Run Tests. Locally, cargo test -p rmcp --all-features --test test_with_js compiles the file to 0 tests because of #![cfg(not(feature = "local"))] — worth knowing before anyone concludes the JS check passed from a --all-features run.

No macro regression: #[prompt_handler] injects its own list_prompts and a get_info with prompts enabled (crates/rmcp-macros/src/prompt_handler.rs:57, :110), so the new trait default only affects handlers that rely on it.

Asks

1. resources/list and resources/templates/list keep the same hole. Rows 3-4 above: a server that declares no resources capability still answers both with an empty success, while read_resource, get_prompt, set_level and subscribe already return -32601 from their defaults. Issue #1445 calls this out explicitly ("list_resources, list_resource_templates and list_tools have the same Ok(default) defaults"). Gating the two resource defaults is two lines and the same shape as what you wrote; if you would rather keep this PR narrow, please open the follow-up and say so in the body so it does not get lost.

2. Pin the behaviour with a Rust test. Right now the only regression coverage is the npm-backed JS file — it is a genuine check (it does run in CI), but it costs a node install and disappears entirely under --all-features. A duplex test like crates/rmcp/tests/test_handler_cache_hints.rs:99-130 can assert both directions cheaply: tools-only server → -32601 for both methods, declared server → Ok.

3. Say the behaviour change out loud. Servers that never declared prompts/completions but relied on the default empty success will start receiving errors from the next release. A line in the PR body and in the release notes (see VERSIONING.md) is enough; it is a fix, not an API break, but clients of such servers should not be surprised.

I am not approving until these are answered — ask 1 is the one I would like to see resolved in this PR.

[xixi-box]

@jokemanfire Thanks for the careful before/after probe and for clarifying which CI configuration actually runs the JS regression test. I agree with all three asks.

1. Resource capability checks: You're right that resources/list and resources/templates/list have the same mismatch. I'll address both in this PR, guarding their default implementations with the advertised resources capability and updating the JS expectations accordingly. I'll also review the analogous tools/list default noted in ServerHandler defaults answer prompts/list and completion/complete when neither capability is declared #1445 so its scope is explicit.
2. Rust regression coverage: I'll add a lightweight duplex-based Rust test for the unsupported (-32601) and explicitly advertised (successful empty result) cases. The JS test can then remain additional cross-SDK coverage rather than the sole regression check.
3. Behavior-change note: I'll make the compatibility impact explicit in the PR description and call it out for the release notes: servers that did not advertise these capabilities but previously relied on the empty default results will now get method not found. This is an intentional protocol-conformance fix, even though it changes observable behavior.

I haven't made those follow-up changes yet; I'll update this thread once the changes and checks are ready for another review.

[xixi-box]

@jokemanfire Follow-up pushed in 292142d. Thanks again for the detailed review.

1. Capability gaps: The default resources/list and resources/templates/list now check capabilities.resources, and I also included the analogous tools/list check from ServerHandler defaults answer prompts/list and completion/complete when neither capability is declared #1445. All return -32601 when unadvertised and preserve the empty success when advertised. The JS regression and existing test fixtures were updated accordingly.
2. Rust coverage: Added test_default_list_capabilities.rs using a duplex pair. It checks no capabilities, tools-only, and fully advertised capabilities, including --all-features. The new test passed in both ordinary and all-features builds; the existing lifecycle/cache-hint tests (24) and JS Streamable HTTP regression also passed locally.
3. Compatibility: Updated the PR description and the [Unreleased] section of crates/rmcp/CHANGELOG.md to explicitly describe the wire-behavior change and how servers can retain the empty default behavior.

The latest GitHub CI is still pending; I'd appreciate another look once it finishes.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 292142dccb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[xixi-box]

@jokemanfire Final follow-up: 2efd7c0 replaces the previous last commit. It fixes the remaining success-path test fixtures noted by Codex (including the shared server fixture and Calculator-backed session tests) and shortens the commit message to satisfy Commitlint.

GitHub CI is now green, including Run Tests (no local feature), Run Tests, and Lint Commit Messages. Conformance and Scorecard also passed; CodeQL is still running. All three requests from your review are reflected in the PR code, native Rust test coverage, and changelog/compatibility note. Thanks for taking another look.

[SIDDARTHAREDDY8]

I read the changes in crates/rmcp/src/handler/server.rs and the new test_default_list_capabilities.rs, plus the fixture updates. Two checks and one question:

1. This completes an existing pattern rather than inventing one: get_prompt, call_tool, read_resource, subscribe and set_level already returned method_not_found from their defaults, so gating the five list/complete defaults on the advertised capability is consistent with the rest of server.rs.

2. The gate lives only in the default implementations, so a server that overrides list_tools (or the other methods) is unaffected even if it does not advertise the capability. The CHANGELOG entry says "default ... handlers", which matches, but it is worth confirming that is the intended scope, since the behavior-change note could be read as broader.

3. get_info() is now consulted on every default list call. If a handler builds its ServerConfig dynamically from runtime state, the capability seen here could disagree with what was advertised during initialize. Is get_info() expected to be cheap and stable per handler? If so, a line in the trait docs saying so would pin the assumption down.

The test matrix (no capabilities, tools only, all enabled) covers the combinations well, and the fixture updates in the other test files show the blast radius was walked through.

[xixi-box]

@SIDDARTHAREDDY8 Thanks for the thoughtful review. Yes, the scope is intentional: these checks apply only to the ServerHandler default methods. An explicit override retains its existing behavior; this PR does not introduce a new dispatcher-wide capability gate.

get_info() is already used for capability advertisement during initialization/discovery and by other request-time checks. The default handlers now consult it as well, so implementations should keep it inexpensive and keep the advertised capabilities consistent for the lifetime of a session. The SDK does not currently cache a per-session snapshot of get_info(); a handler that changes its capability declarations dynamically could therefore become inconsistent with initialization. I added this expectation to the get_info() trait documentation in b1305af.

That commit also merges the latest main (v3.10.0 release) and preserves the compatibility note under [Unreleased]. The new CI run is pending.

[jokemanfire]

All three asks from my review at a4b6647 are resolved at b1305af, and I re-ran the gates rather than reading the diff.

Verification at b1305af (base 53e64a9, i.e. after merging main)

• test_default_list_capabilities (new): 1 passed. It covers capabilities {}, {tools} and {tools,resources,prompts,completions} against all five methods, so the two resource defaults you gated in 2efd7c0 are pinned, not just the original two.
• test_handler_cache_hints 8 passed, test_client_lifecycle_modes 16 passed, test_stateless_server_requests 4 passed, test_streamable_http_session_store 4 passed, test_streamable_http_stale_session 4 passed, test_discover_http_client_startup 4 passed, test_tool_macros 16 passed, test_prompt_macros 8 passed (features client server reqwest transport-streamable-http-server transport-streamable-http-client-reqwest, plus macros for the macro binaries). Total 65 passed, 0 failed.
• cargo fmt --check clean.

Ask 3 is satisfied by the [Unreleased] / Fixed entry, which names the five methods and the behavior change explicitly, and the get_info doc note covers the "defaults call get_info() per request" contract. The JS regression is not run locally (node install); CI's Run Tests (no local feature) job covers it, as established in the previous round.

One non-blocking observation, no action needed: the default handlers now call get_info() on every list_*/complete request, so an implementation whose get_info is expensive or allocates a fresh ServerConfig pays that per call. The new trait docs say it should be cheap and consistent, which is the right trade-off for keeping the trait surface unchanged.

LGTM from me on the code and the evidence.

[DaleSeo]

We shouldn't write the changelog manually. It's generated by the release automation.

[xixi-box]

Agreed, thanks for the clarification. I removed the manually added Unreleased changelog entry in e4684ee. The release automation can generate it instead; the implementation and regression tests remain unchanged.