fix(client): fall back from discover for any non-modern error - #1133
Open
ip2a wants to merge 1 commit into
Open
Conversation
`ClientLifecycleMode::Auto` only fell back to the legacy `initialize` handshake when `server/discover` failed with JSON-RPC `-32601` (`METHOD_NOT_FOUND`). Deployed legacy servers commonly reject an unknown pre-`initialize` request with other implementation-defined errors (`-32600`, `-32602`) or with session-middleware errors before method dispatch, so Auto broke against servers that previously worked. Per the 2026-07-28 backward-compatibility guidance, the fallback MUST NOT be keyed to one specific error code: the client falls back to `initialize` for every discover failure that does not positively identify a modern server. The recognized modern failures — a negotiated version mismatch, a missing-required-capability error, or a header validation failure — are surfaced instead of triggering a fallback. Fixes modelcontextprotocol#1040.
DaleSeo
reviewed
Aug 5, 2026
| // Fall back to the legacy `initialize` handshake. The 2026-07-28 | ||
| // backward-compatibility guidance is explicit that this MUST NOT | ||
| // be keyed to one specific error code. | ||
| Err(_) => { |
Member
There was a problem hiding this comment.
Err(_) also catches 401 and 403, as well as client-side failures like NoPreferredProtocolVersion. Neither indicates that the peer is legacy, but Auto now attempts initialize and may hide the original actionable error.
| // must not trigger a legacy fallback. Surface the error. | ||
| Err(error) if is_modern_server_error(&error) => return Err(error), | ||
| // Any other outcome — `-32601`, `-32602`, `-32600`, another | ||
| // implementation-defined error, or no response at all — means |
Member
There was a problem hiding this comment.
discover_startup waits in expect_response until a message comes in or the transport closes. So, if a legacy server is open but silently ignores server/discover, it never reaches this branch.
9 tasks
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.
ClientLifecycleMode::Autoonly fell back to the legacyinitializehandshake whenserver/discoverfailed with JSON-RPC-32601(METHOD_NOT_FOUND). Deployed legacy servers commonly reject an unknown pre-initializerequest with other implementation-defined errors (-32600,-32602) or with session-middleware errors before method dispatch, so Auto broke against servers that previously worked.Per the 2026-07-28 backward-compatibility guidance, the fallback MUST NOT be keyed to one specific error code: the client falls back to
initializefor every discover failure that does not positively identify a modern server. The recognized modern failures — a negotiated version mismatch, a missing-required-capability error, or a header validation failure — are surfaced instead of triggering a fallback.Fixes #1040.
Motivation and Context
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context