Update to MCP 2026-07-28 without backwards compat. - #256
Conversation
Require self-describing 2026-07-28 requests and expose server discovery so callers no longer depend on initialization state. Return cache and result metadata required by the new wire contract, and reject legacy or malformed requests with actionable errors. Co-Authored-By: HAL 9000
|
👋 Thanks for assigning @benthecarman as a reviewer! |
|
Yeah based on the goose team's comments this seems like the better way to go |
Dispatch unsupported methods before validating request metadata so legacy and unknown methods receive method-not-found errors. Compare protocol versions before validating other metadata so unsupported clients receive actionable negotiation details. Reuse the production protocol version in integration tests and keep request handling helpers below the entry point. Co-Authored-By: HAL 9000
Send modern request metadata from the live test harness and replace the legacy initialization check with server discovery coverage. Verify cache and server metadata on discovery, listing, and live tool responses so the stateless wire contract is exercised against a node. Co-Authored-By: HAL 9000
Describe the required per-request metadata and discovery flow so MCP clients use the server without the removed initialization handshake. Call out the latest-only compatibility boundary and supported stdio method surface. Co-Authored-By: HAL 9000
09cdb76 to
c5b1b2b
Compare
benthecarman
left a comment
There was a problem hiding this comment.
overall looks good to me, just some final review comments on correctness from claude
| Ok(ToolCallResult::success(text)) | ||
| }, | ||
| Err(e) => ToolCallResult::error(format!("{}: {}", e.category(), e.message)), | ||
| Err(e) if e.is_tool_execution() => { |
There was a problem hiding this comment.
Argument-validation errors now violate the spec (tools/mod.rs:66). I earlier relayed that converting invalid-argument errors to JSON-RPC -32602 was spec-prescribed — that was wrong. The spec’s tools Error Handling section explicitly lists input validation errors (“value out of range”, wrong format) under Tool Execution Errors reported with isError: true, reserving protocol errors for unknown tools and schema-shape failures. The PR moves all 37 tools’ argument parsing to hard -32602 envelope errors, which breaks the LLM self-correction loop the spec designed isError for. This is the most consequential finding after the notification bug.
| return error_response(id, INVALID_PARAMS, "Missing required parameter: name"); | ||
| }; | ||
| let tool_args = match params.get("arguments") { | ||
| Some(arguments) if !arguments.is_object() => { |
There was a problem hiding this comment.
"arguments": null is rejected while an omitted key succeeds (main.rs:175). Confirmed by running the binary — many client serializers emit null for “no arguments,” and the old code tolerated it.
| }; | ||
|
|
||
| let id = message.get("id").cloned(); | ||
| let response_id = id |
There was a problem hiding this comment.
ldk-server-mcp/src/protocol.rs — JsonRpcRequest::from_value: the !valid early return runs before the let Some(id) = id else { return Ok(None) } notification bail-out, so a malformed notification (e.g. {"jsonrpc":"2.0","method":"notifications/cancelled","params":null}) gets an id: null error response on stdout. JSON-RPC 2.0 and MCP both say notifications MUST NOT receive a response, and this crate’s CLAUDE.md says they’re ignored. Also, an explicit "id": null now maps to Some(Value::Null) and gets rejected, where serde previously folded it to None (silent).
Fix: move the notification bail-out above the validity check and treat a null id as absent, e.g. let Some(id) = id.filter(|id| !id.is_null()) else { return Ok(None); }; — then simplify response_id and the id clause of valid, which can assume a present, non-null id. Please also extend test_notification_no_response with a params: null (or array-params) notification asserting no output line.
Alternative to #255 (which I only saw after opening this).
Here we don't keep the old version around, just go for the new thing.
(cc @benthecarman let me know what you prefer).