Skip to content

fix: resolve .NET E2E race conditions in cancellation and multi-client tests#1034

Merged
SteveSandersonMS merged 1 commit intomainfrom
fix/dotnet-e2e-race-conditions
Apr 7, 2026
Merged

fix: resolve .NET E2E race conditions in cancellation and multi-client tests#1034
SteveSandersonMS merged 1 commit intomainfrom
fix/dotnet-e2e-race-conditions

Conversation

@SteveSandersonMS
Copy link
Copy Markdown
Contributor

@SteveSandersonMS SteveSandersonMS commented Apr 7, 2026

Hopefully fixes two flaky .NET E2E test failures:

1. SendAndWait_Throws_OperationCanceledException_When_Token_CancelledShould_Create_A_Session_With_Customized_SystemMessage_Config

CI failure (Windows)

The cancellation test cancelled the token but never waited for the agent to go idle. The agent's in-flight sleep 10 request leaked into the proxy during the next test, hitting the wrong snapshot and causing a 500 error.

Fix: Wait for SessionIdleEvent after the cancellation assertion, and verify Aborted == true.

2. One_Client_Approves_Permission_And_Both_See_The_Result

CI failure (macOS)

SendAndWaitAsync returns when session1 goes idle, but client2's PermissionCompletedEvent may not have propagated yet. The immediate Assert.Contains was a race.

Fix: Set up GetNextEventOfTypeAsync<PermissionCompletedEvent>(session2) before sending, then await it before asserting.

@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner April 7, 2026 13:40
Copilot AI review requested due to automatic review settings April 7, 2026 13:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Stabilizes .NET E2E tests by adding explicit event synchronization to avoid cross-test leakage after cancellation and to remove a race in multi-client permission propagation.

Changes:

  • In SessionTests, adds a SessionIdleEvent waiter after canceling SendAndWaitAsync to ensure the agent becomes idle before the test ends.
  • In MultiClientTests, sets up and awaits PermissionCompletedEvent on the second client before asserting both clients observed the completed permission.
Show a summary per file
File Description
dotnet/test/SessionTests.cs Adds waiting for session.idle after cancellation to prevent in-flight work leaking into the next test.
dotnet/test/MultiClientTests.cs Waits for client2’s PermissionCompletedEvent before asserting, reducing a race between clients.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment on lines 447 to 450
// Set up wait for tool execution to start BEFORE sending
var toolStartTask = TestHelper.GetNextEventOfTypeAsync<ToolExecutionStartEvent>(session);
var sessionIdleTask = TestHelper.GetNextEventOfTypeAsync<SessionIdleEvent>(session);

Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sessionIdleTask is created immediately after CreateSessionAsync(). Because session events are dispatched asynchronously, this waiter can race and capture an already-queued initial session.idle from session creation/resume rather than the post-cancellation idle you intend to wait for. To make this deterministic, set up the SessionIdleEvent waiter after you’ve observed ToolExecutionStartEvent (and before calling cts.Cancel()), so it can only match the idle corresponding to this in-flight operation.

Copilot uses AI. Check for mistakes.
@github-actions

This comment has been minimized.

…t tests

SessionTests: SendAndWait_Throws_OperationCanceledException_When_Token_Cancelled
did not wait for the agent to go idle after cancellation, leaking proxy
requests into subsequent tests (causing Customized_SystemMessage_Config to fail).

MultiClientTests: One_Client_Approves_Permission_And_Both_See_The_Result
asserted PermissionCompletedEvent on client2 immediately after session1 went
idle, but event propagation to client2 is async and may not have arrived yet.
@SteveSandersonMS SteveSandersonMS force-pushed the fix/dotnet-e2e-race-conditions branch from a6e33ac to 90e5214 Compare April 7, 2026 13:45
@SteveSandersonMS SteveSandersonMS merged commit b4fa5d9 into main Apr 7, 2026
16 checks passed
@SteveSandersonMS SteveSandersonMS deleted the fix/dotnet-e2e-race-conditions branch April 7, 2026 13:48
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Cross-SDK Consistency Review ✅

This PR only modifies .NET test files to fix E2E race conditions — no changes to the public SDK API surface.

Changes reviewed:

  • dotnet/test/SessionTests.cs — adds AbortAsync() + SessionIdleEvent waits in cancellation/timeout tests to prevent agent state from leaking between tests
  • dotnet/test/MultiClientTests.cs — sets up PermissionCompletedEvent listener before sending to avoid a race in client2 assertions

These are .NET-specific test stability improvements. No features were added or modified, so there's nothing requiring equivalent changes in the Node.js, Python, or Go SDKs. No cross-SDK consistency issues identified.

Generated by SDK Consistency Review Agent for issue #1034 ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants