Repository navigation
Apply the test discovery timeouts to Sampling_Stdio - #1950
Merged
tarekgh merged 1 commit intoOct 22, 2026
Merged
Conversation
5 tasks done
modelcontextprotocol#1943 and modelcontextprotocol#1946 give stdio test_server clients an unbounded server/discover probe when a test passes no options, and leave options a test passes unchanged. Sampling_Stdio passes options for its sampling handler without pinning a protocol version, so its test_server case still used the 5-second probe and could fall back to initialize while the discover answer was on its way; the server then rejects initialize. Start its test_server options from TestMcpClient.CreateOptions(), as CONTRIBUTING.md asks; the everything server keeps the default probe.
girishkvs
force-pushed
the
test_server_discover_timeout
branch
from
October 22, 2026 21:55
a35062c to
69eb00b
Compare
girishkvs
changed the title
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The localized test change follows the established helper pattern while preserving sampling behavior and legacy-server defaults.
No open issues 🎉
What changed in this PR
This test-only follow-up to #1943 and #1946 applies the shared discovery timeout configuration to Sampling_Stdio, reducing slow-startup failures without changing SDK behavior.
Changes:
- Initialize
test_serverclient options withTestMcpClient.CreateOptions()before assigning the sampling handler. - Preserve the
everythingserver’s default probe timeout and existing sampling behavior.
| File | Description |
|---|---|
| tests/ModelContextProtocol.Tests/ClientIntegrationTests.cs | Apply test discovery timeouts to the controlled-server sampling test. |
Review effort: Balanced
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.
Summary
This is a test-only follow-up to #1943 and #1946. Both give stdio
test_serverclients the test discovery timeouts only when a test passes no client options:ClientIntegrationTestFixturenow usesTestMcpClient.CreateOptions()for them, and options a test passes stay unchanged.Sampling_Stdiopasses options for its sampling handler without pinning a protocol version, so onmainitstest_servercase still uses the default 5-secondserver/discoverprobe. When the TestServer starts slowly, the client falls back toinitializewhile the discover answer is still on its way, and the server, which has already selected 2026-07-28, rejects it:Sampling_Stdionow starts itstest_serveroptions fromTestMcpClient.CreateOptions()and then sets its sampling handler, as CONTRIBUTING.md asks since Prevent discovery probe timeouts in controlled test connections #1946. Theeverythingserver keeps the default probe, as in the fixture.ClientIntegrationTestscall sites that pass options pin2025-11-25, so they never probe. SDK behavior is unchanged; this does not fix the underlying fallback race.CI evidence
ClientIntegrationTests.Sampling_Stdio(clientId: "test_server")on .NET 8. Prevent discovery probe timeouts in controlled test connections #1946 does not change this test's options.Validation
mainwith Prevent discovery probe timeouts in controlled test connections #1946 by holding the TestServer'sserver/discoveranswer for 7 seconds, a local-only change that is not in this PR.ClientIntegrationTestson .NET 8 had one failure,Sampling_Stdio(clientId: "test_server"), with the CI message (58 passed, 2 skipped). With this change, all passed (59 passed, 2 skipped).ClientIntegrationTestsandJuly2026ProtocolFallbackTests(91 tests): 20 runs on .NET 8 and one run each on .NET 10 and .NET Framework 4.7.2, with zero failures. Each run had 89 passed and the 2 OpenAI-key skips.