From c66039e78cd5e2ba257bc9af5b1e3e0177d80859 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Thu, 4 Jun 2026 14:41:49 -0700 Subject: [PATCH] Fix flaky AddIncomingMessageFilter_Multiple_Filters_Execute_In_Order test The test asserts each incoming message filter runs exactly three times, once per message: initialize, notifications/initialized, and tools/list. The initialize and tools/list exchanges are request/response, so awaiting the client calls guarantees their filter passes have completed. The notifications/initialized notification is sent fire-and-forget by the client, so it has no synchronization point and may still be in flight when ListToolsAsync returns, leaving the strict counts at two and failing the test. Add a TaskCompletionSource that the outermost filter completes once it has finished processing the initialized notification, and await it before snapshotting the log. This follows the existing pattern used elsewhere in this file and makes the test deterministic without weakening the strict count assertions. --- ...cpServerBuilderExtensionsMessageFilterTests.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsMessageFilterTests.cs b/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsMessageFilterTests.cs index a39d4896f..c59c6a09e 100644 --- a/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsMessageFilterTests.cs +++ b/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsMessageFilterTests.cs @@ -100,6 +100,12 @@ public async Task AddIncomingMessageFilter_Intercepts_Request_Messages() [Fact] public async Task AddIncomingMessageFilter_Multiple_Filters_Execute_In_Order() { + // The client sends notifications/initialized fire-and-forget, so unlike the initialize and + // tools/list request/response exchanges it has no synchronization point the test can await. + // Signal once the outermost filter finishes processing it so the strict counts below observe a + // complete, stable log instead of racing the still-in-flight notification. + var initializedNotificationProcessed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + McpServerBuilder .WithMessageFilters(filters => { @@ -109,6 +115,11 @@ public async Task AddIncomingMessageFilter_Multiple_Filters_Execute_In_Order() logger.LogInformation("MessageFilter1 before"); await next(context, cancellationToken); logger.LogInformation("MessageFilter1 after"); + + if (context.JsonRpcMessage is JsonRpcNotification { Method: NotificationMethods.InitializedNotification }) + { + initializedNotificationProcessed.TrySetResult(true); + } }); filters.AddIncomingFilter((next) => async (context, cancellationToken) => @@ -127,6 +138,10 @@ public async Task AddIncomingMessageFilter_Multiple_Filters_Execute_In_Order() await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + // Wait for the fire-and-forget initialized notification to flow through the filter pipeline + // before snapshotting the log; otherwise the strict counts below can race the notification. + await initializedNotificationProcessed.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken); + var logMessages = MockLoggerProvider.LogMessages .Where(m => m.Category.StartsWith("MessageFilter")) .Select(m => m.Message)