From 09c310341b9421189e0ba0b1c58f805a2ad7c43f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 4 Sep 2026 00:16:39 +0900 Subject: [PATCH] Fix the flaky thread count assertion in the listen keepalive test ## Motivation and Context "listen keepalive is not started when the interval is nil" fails intermittently on CI. The most recent run failed on Ruby 3.2 alone while 3.1 and head passed, and a re-run of the same commit went green. The assertion samples a process-wide count: ```ruby before = Thread.list.size open_listen_stream(id: "listen-1", notifications: { toolsListChanged: true }) assert_equal before, Thread.list.size, "a nil interval must not spawn a keepalive thread" ``` `Thread.list` covers the whole process, so the count moves for reasons this test has no interest in. The observed failure reported 54 against 51: three threads had *gone away* between the samples, not appeared. This file starts threads in sixteen places, and the SSE stream and reaper threads they leave behind finish whenever they finish, so whether the count holds still depends on the test order and on the scheduler rather than on the behavior under test. The test itself is asking the right question. `setup` builds its transport with `listen_keepalive_interval: nil` precisely so opening a listen stream spawns no timer, and the point is to hold that guarantee. Only the measurement is wrong, so this compares the set of threads instead: a thread that disappears no longer registers, and a thread that appears still does. Minitest runs these tests serially and `open_listen_stream` drives the SSE body inline rather than on a thread, so a keepalive timer is the only thread that can appear across the two samples. No library code changes. ## How Has This Been Tested? The rewritten assertion was checked for sensitivity before being kept: a temporary probe pointed `open_listen_stream` at a transport built with `listen_keepalive_interval: 15` and asserted the set difference was *not* empty. It passed, confirming the assertion still catches a spawned keepalive thread rather than merely never failing. The probe was removed afterwards. The failure did not reproduce locally beforehand, across six seeds of the file on its own and six full-suite runs, which matches a timing-sensitive flake that needs a slower machine. ## Breaking Changes None. --- .../server/transports/streamable_http_transport_test.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/mcp/server/transports/streamable_http_transport_test.rb b/test/mcp/server/transports/streamable_http_transport_test.rb index 36fe9ac2..bf032b2f 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -6307,10 +6307,13 @@ def string end test "listen keepalive is not started when the interval is nil" do - before = Thread.list.size + # The set of threads, not their count: `Thread.list` is process-wide, so a thread another test left running + # that finishes between the two samples moves the count in the direction this assertion does not care about. + # Only a thread that appears is evidence of a keepalive timer. + before = Thread.list open_listen_stream(id: "listen-1", notifications: { toolsListChanged: true }) - assert_equal before, Thread.list.size, "a nil interval must not spawn a keepalive thread" + assert_empty(Thread.list - before, "a nil interval must not spawn a keepalive thread") end test "listen_keepalive_interval rejects a non-positive value" do