Fix flaky pending-messages-modified E2E test across SDKs

[stephentoub]

The Should_Emit_Pending_Messages_Modified_Event_When_Message_Queue_Changes event-fidelity test was failing intermittently in the C# SDK CI leg (runs/26224349644). It was the only test in its fixture not using the standard SendAndWaitAsync + event-collector pattern, instead going through a custom helper that did two independently-timed awaits and used an async void local function to backfill existing messages -- a combination prone to timing races and lost exceptions.

Approach

Refactor the test to the standard pattern (subscribe, SendAndWait, then filter the collected events) in all four affected SDKs so the same fix doesn't have to be rediscovered language-by-language. While here, eliminate the underlying anti-patterns the bug rode in on:

• C# (TestHelper.GetFinalAssistantMessageAsync): replace async void CheckExistingMessages with async Task CheckExistingMessagesAsync, drained deterministically in a finally so the backfill cannot outlive the enclosing using disposals. The dotnet/ tree now has zero async void methods.
• Node (sdkTestHelper.ts): replace new Promise<T>(async (resolve, reject) => ...) -- the JS analog of async void, where synchronous throws inside the executor are silently lost -- with idiomatic async/await in getFinalAssistantMessage and getExistingFinalResponse.
• Python / Go: pure test refactor; helpers were already clean.

Validation

All four EventFidelity E2E suites pass after the change. Full E2E suites also clean:

• C#: refactored test 10/10, EventFidelity 8/8, Session + Tools 54/55 (1 unrelated skip)
• Node: 280/289 (2 file failures in an unrelated pending_work_resume.e2e.test.ts are a known local-env auth race; not touched by this PR)
• Python: 300 passed, 7 skipped
• Go: all e2e tests pass

Lints/typechecks clean in all four languages; unused imports cleaned up.

---

PR description generated by Copilot.

[Copilot]

Pull request overview

This PR fixes intermittent flakiness in the cross-SDK “pending messages modified” event-fidelity E2E test by refactoring the test flow to a consistent “subscribe/collect → sendAndWait → assert against collected events” pattern, and by removing helper implementations that could lose exceptions or race on timing.

Changes:

• Refactors the pending-messages-modified E2E test in Python/Node/Go/.NET to use send_and_wait / sendAndWait / SendAndWait and then inspect the collected events.
• Hardens helper logic in .NET and Node by eliminating async void / new Promise(async ...)-style patterns that can drop exceptions or create timing races.
• Cleans up now-unused imports and simplifies event acquisition logic in the affected tests.

Show a summary per file

File	Description
python/e2e/test_event_fidelity_e2e.py	Switches the flaky test to send_and_wait + event list collection, consistent with other tests in the file.
nodejs/test/e2e/harness/sdkTestHelper.ts	Reworks getFinalAssistantMessage to avoid new Promise(async ...) and makes the “existing vs future” logic fully async/await.
nodejs/test/e2e/event_fidelity.e2e.test.ts	Refactors the flaky test to sendAndWait + collected events, removing the split await pattern.
go/internal/e2e/event_fidelity_e2e_test.go	Refactors the flaky test to use SendAndWait and a mutex-protected event snapshot instead of a timeout channel wait.
dotnet/test/Harness/TestHelper.cs	Replaces an async void backfill with an observable Task and drains it deterministically to avoid outliving disposals.
dotnet/test/E2E/EventFidelityE2ETests.cs	Refactors the flaky test to SendAndWaitAsync + a single event collector, matching the fixture’s dominant pattern.

Copilot's findings

Comments suppressed due to low confidence (1)

dotnet/test/Harness/TestHelper.cs:89

• GetFinalAssistantMessageAsync uses a CancellationTokenSource for timeouts, but the backfill task (CheckExistingMessagesAsync → GetExistingMessagesAsync → session.GetEventsAsync()) doesn't receive the cancellation token. Because the method now awaits backfill in finally, a slow/hung GetEventsAsync call can delay (or prevent) the timeout exception from surfacing and potentially hang tests. Consider threading cts.Token into GetExistingMessagesAsync and passing it to session.GetEventsAsync (and/or otherwise ensuring backfill can't outlive the timeout).

        // Backfill from already-delivered messages so we don't lose events that arrived
        // between SendAsync returning and the subscription being installed. Run it
        // concurrently with the live subscription, but keep the Task observable so any
        // exception is propagated through tcs (not the unobserved-task handler) and so
        // we can drain it deterministically below.
        var backfill = CheckExistingMessagesAsync();

        using var registration = cts.Token.Register(
            static state => ((TaskCompletionSource<AssistantMessageEvent>)state!).TrySetException(
                new TimeoutException("Timeout waiting for assistant message")),
            tcs);

        try
        {
            return await tcs.Task;
        }
        finally
        {
            // Drain the backfill before our `using` scopes (cts, subscription) dispose.
            // Any exception was already routed through tcs above, so swallow here.
            try { await backfill.ConfigureAwait(false); } catch { }
        }

        async Task CheckExistingMessagesAsync()
        {
            try
            {
                var (existingFinal, existingIdle) = await GetExistingMessagesAsync(session, alreadyIdle);
                lock (stateLock)

• Files reviewed: 6/6 changed files
• Comments generated: 1

[github-actions]

Cross-SDK Consistency Review ✅

This PR applies the same fix across all four SDK implementations (Node.js, Python, Go, and .NET), which is exactly the right approach for test consistency.

Pattern applied uniformly:

• Subscribe to events with a simple collector before sending
• Use sendAndWait / send_and_wait / SendAndWait / SendAndWaitAsync in one round trip
• Filter the collected events after the call returns

Additional cleanups are also consistent:

• .NET: async void → async Task (with deterministic drain in finally)
• Node.js: new Promise(async ...) anti-pattern → idiomatic async/await
• Python/Go: pure test refactor, no anti-patterns to clean up

No cross-SDK inconsistencies found. All changes are in test/harness code, not public API surface.

Generated by SDK Consistency Review Agent for issue #1362 · ● 108K · ◷