feat(server): move to rmcp 3.1 and pin what the transport may serve - #54
Merged
Conversation
Stage 1 of #34: the SDK bump, with every behaviour it would otherwise change by default made explicit. No MCP revision is adopted here — audit schema v1 is untouched and stays v1. MSRV moves to 1.88, which rmcp 3.1.0 requires. The port is mechanical where the SDK renamed things: `call_tool` returns `CallToolResponse`, the enum wrapping `CallToolResult`, and the outcome classification reads `is_error` off its completed variant; `model::Meta` became `RequestMetaObject`; `ListToolsResult` gained three fields and is written out by name so a later one cannot be inherited silently. The rest is what the SDK would have decided for us. Revisions served are declared in `SUPPORTED_PROTOCOL_VERSIONS` and returned from `supported_protocol_versions`, rather than inheriting every revision rmcp knows. The hand-written `initialize` tests that same list instead of `KNOWN_VERSIONS`: the SDK negotiates again after the handler returns, using the handler's answer as its fallback, so a handler that echoed an unsupported revision made the SDK echo it too. The record now names the revision negotiated rather than the one requested, and `get_info` pins its default instead of inheriting `ProtocolVersion:: default()`, which moves with the SDK. That is not sufficient on its own, and the adversarial review found why. rmcp routes a request to its handshake-free lifecycle on the mere presence of `_meta.io.modelcontextprotocol/protocolVersion` — whatever revision that key names, never consulting the supported list — and synthesises the peer with the SDK's own build identity. A client naming 2025-11-25, which this build does serve, could therefore reach a tool with no `initialize` behind it and land in the audit stream as `rmcp`/<version>: a client the server never spoke to, which is the attribution an audit trail can least afford. `call_tool` and `list_tools` now refuse such a request, and the refused call is recorded with `client` absent rather than with the placeholder. Two transport defaults are likewise set by name in a shared `http_server_config`, which the integration tests now serve so the configuration is testable at all. `allowed_hosts` defaults to loopback only, which would refuse every deployment not addressed as `localhost`, containers included. `max_request_body_bytes` is a 4 MiB POST cap with no rmcp 2.2 equivalent: kept as a memory bound but pinned, because it also ceilings `add_attachment` independently of the operator's `global.max_attachment_bytes` — #52 reconciles the two. Each decision is pinned by a test that fails without it, including an HTTP-level reproduction of the handshake-free call and a non-loopback `Host`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage 1 of #34: the SDK bump, with every behaviour it would otherwise change by default made explicit. No MCP revision is adopted here — audit schema v1 is untouched. MSRV moves to 1.85 → 1.88, which rmcp 3.1.0 requires (
rust-versionin its manifest); therust-msrvCI job moves with it.The mechanical half
call_toolreturnsCallToolResponse— the enum wrappingCallToolResultfor revisions with intermediate results — and the outcome classification readsis_erroroff its completed variant.model::MetabecameRequestMetaObject.ListToolsResultgained three fields and is written out by name, so a later SDK field cannot be inherited silently; the 2026-07-28 cache hints stay absent while that revision is unserved.The half that needed deciding
Revisions served are declared in
SUPPORTED_PROTOCOL_VERSIONSand returned fromsupported_protocol_versions, instead of inheriting every revision rmcp knows. The hand-writteninitializetests that same list rather thanKNOWN_VERSIONS: the SDK negotiates again after the handler returns, using the handler's answer as its fallback, so a handler that echoed an unsupported revision made the SDK echo it too. Theinitializerecord now names the revision negotiated rather than the one requested, andget_infopins its default instead of inheritingProtocolVersion::default(), which moves with the SDK.That is not sufficient on its own — the adversarial review found why, and it is the reason this PR is bigger than a bump. rmcp routes a request to its handshake-free lifecycle on the mere presence of
_meta.io.modelcontextprotocol/protocolVersion, whatever revision that key names, never consulting the supported list; that path synthesises the peer with the SDK's own build identity. A client declaring 2025-11-25 — a revision this build does serve — could therefore reach a tool with noinitializebehind it and land in the audit stream asrmcp/3.1.0: a client the server never spoke to, which is the attribution an audit trail can least afford. It was reproduced end to end over HTTP (including from inside a live session, carrying that session's real id) and over stdio.call_toolandlist_toolsnow refuse such a request, and the refused call is recorded withclientabsent rather than with the placeholder.Two transport defaults are likewise set by name, in a shared
http_server_configthat the integration tests now serve — the harness previously built its own config, so these were untestable.allowed_hostsdefaults to loopback only, which would refuse every deployment not addressed aslocalhost, containers included.max_request_body_bytesis a 4 MiB POST cap with no rmcp 2.2 equivalent: kept as a memory bound but pinned, because it also ceilingsadd_attachmentindependently of the operator'sglobal.max_attachment_bytes— #52 reconciles the two.Verification
cargo fmt --check, both clippy legs,cargo test --workspace --all-targets --locked(347),cargo deny checkandtyposare clean.Adversarial review across four lenses plus a mutation verifier. It returned three blockers — the handshake-free path (found independently by three reviewers, and again by the stage 2 design work), the MSRV conflict, and DESIGN.md documenting a protection the code did not have — plus majors on the body cap and on
InitializeEvent::protocol_version's rustdoc, which this change made false. All are fixed here. Every decision is pinned by a test that fails without it: an HTTP-level reproduction of the handshake-free call (verified red without the guard, serving bug content), a non-loopbackHost(403 without the config), and both halves of the version narrowing (red when either is widened toKNOWN_VERSIONS).Two things the review surfaced that are deliberately not here: #52 (body cap vs
max_attachment_bytes) and #53 (the server identifies itself to clients asrmcp— pre-existing, identical in rmcp 2.2, so not a regression from this bump).