[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-12T13:50:45.429194Z	2efd7c0	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".

[chatgpt-codex-connector]

Migrate remaining success-path tests to declared capabilities

In the non-local test configuration, tests/test_stateless_server_requests.rs:24 still defines StatelessServer with an empty ServerHandler implementation, but the assertions at lines 100–103 and 245–248 require its valid tools/list requests to succeed. This branch now returns -32601 instead, so both tests fail. The default-info handlers in test_discover_http_client_startup.rs have the same problem, and the resource gate likewise breaks Calculator-backed session-store/stale-session tests that still call list_all_resources() despite Calculator advertising only tools. Update the remaining success-path fixtures to declare the capabilities they exercise, or change their requests to supported methods.

Useful? React with 👍 / 👎.

[xixi-box]

Addressed in 2efd7c0. Thanks for catching the remaining fixtures.

• The stateless/discover HTTP success-path handlers now advertise tools; the shared TestServer fixture does too.
• Calculator-backed session-store and stale-session tests now use the supported tools/list method, including the raw inline-protocol request.
• The affected HTTP/stateless/session test binaries passed locally (16 tests), and test_server_initialization passed (22 tests).
• I also corrected the commit-message lint issue by replacing the last commit with a short conventional subject.

The full non-local CI run is being checked on this commit.

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