test(mcp): surface stdio connect diagnostics in prompt smoke test - #1950
Merged
Conversation
Collaborator
Author
|
This is just a test observability fix, no actual behavioral or test changes yet. |
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.
Fixes the opaque Windows failure of
SmokeMcpPromptSkillTests.ManagerDiscoversAndLoadsPromptOverStdio(serverKind: \"python\").Problem
On Windows CI (run 31794198529), the python theory case fails after 1m10s with
Assert.IsType() Failure: Value is nullat line 37. The manager swallows the MCP SDKTimeoutException(60sInitializationTimeoutdefault) into anUnreachablestatus, and this test never callsAssertConnectednor passes anITestOutputHelper— so CI shows only an opaque null and the real startup error goes toNullLogger.Prior context
AssertConnected+ test logger but applied it only toSmokeMcpServerHttpHeaderTests.McpSdkCatalogNotificationIntegrationTestswith an explicit note: "This PR does not prove or fix an underlying production MCP startup defect... the next run will retain the exception needed for a causal follow-up." That retention was never applied to this test.Fix
Diagnostics-first, per the #1904 pattern — no timeout jiggling:
ITestOutputHelperinto the test.harness.AssertConnected(\"smoke\")before the skill lookup, so the next Windows failure names the manager's real exception instead of an opaque null.This is the prerequisite for a causal fix (expected: python stdio child exceeds the 60s init budget on Windows). If the retained error confirms a timeout, the follow-up is to raise the stdio
InitializationTimeoutinMcpClientManager.BuildClientOptions.