fix: preserve legacy fallback after rejected discovery - #1269
Open
JulienCote wants to merge 2 commits into
Open
JulienCote wants to merge 2 commits into
JulienCote wants to merge 2 commits into
Conversation
github-actions
Bot
added
T-test
A dual-lifecycle stdio client probes a server with server/discover before deciding whether to use the modern per-request metadata lifecycle or the legacy initialize handshake. rmcp previously marked the connection as requiring per-request metadata before the discovery handler completed. When discovery returned an error identifying a legacy server, the auto client correctly fell back to initialize on the same connection, but the server retained the modern metadata requirement and rejected later legacy requests. Derive discovery support from supported_protocol_versions. Servers that advertise only initialization-based versions return MethodNotFound without dispatching discovery. Keep rejected discovery requests in the pre-initialization loop, and require per-request metadata only after an inline request establishes the modern lifecycle. Add separate regression coverage for legacy-only version configuration and for rejected discovery followed by initialize and a metadata-light tools/list request. Update the HTTP discovery test server to advertise the discovery lifecycle. Validation: - cargo +nightly fmt --all - cargo clippy --all-targets --all-features -- -D warnings - affected client/server lifecycle tests pass - cargo build --all-features - cargo test --all-features is blocked by the unrelated Windows-only transport::auth::tests::default_http_client_preserves_connection_failure_cause assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2437c70d-f526-4b60-8ee4-effca05aadd4
JulienCote
force-pushed
the
fix/legacy-discovery-fallback
branch
from
September 14, 2026 21:41
60b94ea to
61e1d60
Compare
The reverse cancellation test uses InitializeResult only when the client feature is enabled without local support. Import it in the matching configuration so the all-features-except-local CI jobs compile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
Fix legacy fallback after a rejected
server/discoverrequest.Motivation and Context
A client supporting both lifecycle models should probe with
server/discover, then fall back toinitializewhen the server reports that it is legacy.rmcp previously marked the connection as requiring modern per-request metadata before discovery completed. If discovery returned an error, the client correctly fell back to
initialize, but the server retained the modern metadata requirement and rejected subsequent legacy requests.This change:
MethodNotFoundwithout dispatching discovery.How Has This Been Tested?
Added separate regression tests covering:
initializeand a metadata-lighttools/list.cargo test --all-featuresotherwise passed 568 tests, but the unrelated Windows-specifictransport::auth::tests::default_http_client_preserves_connection_failure_causeassertion failed due to the platform's connection-refused error shape.Breaking Changes
None. This restores the expected backward-compatible lifecycle behavior.
Types of changes
Checklist
Additional context
The stdio backward-compatibility rules require clients to fall back on discovery errors other than recognized modern errors. The server therefore must not commit the connection to the modern lifecycle until discovery succeeds