Skip to content

Fix flaky stderr test: call WaitForExit() after WaitForExitAsync() to flush ErrorDataReceived events#1278

Merged
stephentoub merged 2 commits intomainfrom
copilot/fix-flaky-stderr-test
Feb 15, 2026
Merged

Fix flaky stderr test: call WaitForExit() after WaitForExitAsync() to flush ErrorDataReceived events#1278
stephentoub merged 2 commits intomainfrom
copilot/fix-flaky-stderr-test

Conversation

Copy link
Contributor

Copilot AI commented Feb 15, 2026 •
edited
Loading

CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked is flaky on macOS net8.0 because Process.WaitForExitAsync() does not guarantee that all ErrorDataReceived events have been dispatched — only the synchronous WaitForExit() (no-arg overload) does.

In GetUnexpectedExitExceptionAsync, the code intended to flush stderr before building the error message, but was using only WaitForExitAsync. The fix adds a WaitForExit() call after it, which is effectively free since the process has already exited:

#if NET
await _process.WaitForExitAsync(cancellationToken).ConfigureAwait(false);
#endif
_process.WaitForExit(); // ensures all ErrorDataReceived callbacks have completed

The .NET Framework path already used WaitForExit() and was not affected.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@stephentoub
Fix flaky stderr test: call WaitForExit() after WaitForExitAsync() to…
ae007a5 
… ensure ErrorDataReceived events are flushed

WaitForExitAsync(CancellationToken) does not guarantee that all
ErrorDataReceived events have been dispatched, unlike the synchronous
WaitForExit() (no arguments). This caused a race condition where the
stderr callback in StdioClientTransportTests might not be invoked
before the process was disposed, leading to flaky test failures on
macOS net8.0.

Co-authored-by: stephentoub <[email protected]>
@stephentoub
Copy link
Contributor

stephentoub commented Feb 15, 2026

@copilot it looks like this same fix was made and reverted in https://github.com/modelcontextprotocol/csharp-sdk/pull/980/commits... any ideas why? Still believe this is the right fix?

Copilot AI changed the title [WIP] Fix flaky stderr test causing CI failures Fix flaky stderr test: call WaitForExit() after WaitForExitAsync() to flush ErrorDataReceived events Feb 15, 2026
Copilot AI requested a review from stephentoub February 15, 2026 14:27
Copy link
Contributor Author

Copilot AI commented Feb 15, 2026

@copilot it looks like this same fix was made and reverted in https://github.com/modelcontextprotocol/csharp-sdk/pull/980/commits... any ideas why? Still believe this is the right fix?

Looking at PR #980 history: the same WaitForExit() fix was initially applied (and you even suggested the exact pattern), but was then incorrectly reverted. The CI failure that prompted the revert was on Mono (stack trace shows Mono format: [0x001c8] in <69cc817959bb4cc3885cc3ba22a5c8d7>:0), where ErrorDataReceived never fires at all. The Copilot agent confused that Mono-specific failure with the WaitForExit fix being wrong, and reverted it instead of recognizing it needed the skip attribute for Mono (which it then added separately).

So what actually happened was two distinct issues:

  1. Mono: ErrorDataReceived never fires → fixed by the skip attribute (SkipUnless = IsStdErrCallbackSupported) which is already in place
  2. .NET (macOS net8.0, etc.): ErrorDataReceived fires but WaitForExitAsync doesn't guarantee it's been dispatched → fixed by adding WaitForExit() after WaitForExitAsync()

The revert only addressed issue 1 and left issue 2 unfixed, which is why the test is still flaky on .NET. This fix addresses issue 2. The WaitForExit() (no-arg) overload guarantees all async event handling has completed, and since the process has already exited at this point, it returns essentially immediately.

@stephentoub stephentoub marked this pull request as ready for review February 15, 2026 14:31
@stephentoub stephentoub merged commit 3caac28 into main Feb 15, 2026
11 checks passed
@stephentoub stephentoub deleted the copilot/fix-flaky-stderr-test branch February 15, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephentoub stephentoub Awaiting requested review from stephentoub

Assignees

@stephentoub stephentoub

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stephentoub