Skip to content

Commit 15a1bad

Browse files
committed
Use TaskCompletionSource for thread-safe callback wait in C# cancel test
Addresses review feedback: the previous poll read observedRequest (written by the callback thread) from the test continuation without synchronization. Switch to the TaskCompletionSource pattern already used by the direct-RPC test in this file, so the callback result is handed off safely and awaited with a timeout. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f7849882-a2a2-4587-a602-da7718889a8c
1 parent 4ae594b commit 15a1bad

1 file changed

Lines changed: 7 additions & 13 deletions

File tree

dotnet/test/E2E/McpOAuthE2ETests.cs

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -204,13 +204,13 @@ public async Task Should_Cancel_Pending_MCP_OAuth_Request()
204204
{
205205
await using var oauthServer = await OAuthMcpServer.StartAsync(ExpectedToken);
206206
var serverName = "oauth-cancelled-mcp";
207-
McpAuthContext? observedRequest = null;
207+
var authRequest = new TaskCompletionSource<McpAuthContext>(TaskCreationOptions.RunContinuationsAsynchronously);
208208

209209
await using var session = await CreateSessionAsync(new SessionConfig
210210
{
211211
OnMcpAuthRequest = request =>
212212
{
213-
observedRequest = request;
213+
authRequest.TrySetResult(request);
214214
return Task.FromResult<McpAuthResult?>(McpAuthResult.Cancel());
215215
},
216216
McpServers = new Dictionary<string, McpServerConfig>
@@ -228,19 +228,13 @@ public async Task Should_Cancel_Pending_MCP_OAuth_Request()
228228
// The MCP connection is kicked off by session.create, but the SDK only registers its
229229
// `mcp.oauth_required` event interest once create returns. If the server's initial 401
230230
// wins that race, the runtime records `needs-auth` WITHOUT invoking the host callback,
231-
// so `observedRequest` is briefly null even after `needs-auth` is observed. A later
232-
// auth retry (now that interest is registered) invokes the callback with the same
233-
// `Initial` reason. Wait for the callback rather than sampling it the instant
231+
// so the callback fires only on a later auth retry (now that interest is registered),
232+
// with the same `Initial` reason. Await the callback rather than sampling it the instant
234233
// `needs-auth` first appears, which is what made this test flaky.
235-
await TestHelper.WaitForConditionAsync(
236-
() => Task.FromResult(observedRequest is not null),
237-
timeout: TimeSpan.FromSeconds(60),
238-
pollInterval: TimeSpan.FromMilliseconds(200),
239-
timeoutMessage: $"{serverName} OAuth request reaching the host callback");
234+
var observedRequest = await authRequest.Task.WaitAsync(TimeSpan.FromSeconds(60));
240235

241-
Assert.NotNull(observedRequest);
242-
Assert.NotEmpty(observedRequest!.RequestId);
243-
Assert.Equal(serverName, observedRequest!.ServerName);
236+
Assert.NotEmpty(observedRequest.RequestId);
237+
Assert.Equal(serverName, observedRequest.ServerName);
244238
Assert.Equal(McpOauthRequestReason.Initial, observedRequest.Reason);
245239
}
246240

0 commit comments

Comments
 (0)