From bb86136ed8ae834d3ea4e06b9358e6e75425816d Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Sun, 2 Aug 2026 12:35:28 -0500 Subject: [PATCH] Fix the race in a_timeout_that_did_not_reach_the_prior_mark_pauses_the_shard ShardStateTracker publishes through a Block, so a subscribed observer runs on the block's consumer thread and is not guaranteed to have run by the time StartAgentAsync returns. The test collected Paused states into a List and asserted on it immediately, which is a race it lost on net9 in CI while a compliance-only PR was in flight (#608). The failure message is what identifies it rather than a wrong count: Shouldly.ShouldAssertException : [ShardName: Trip:V2:All, Sequence: 60, Action: Paused] should have single item but had 1 items A one-element collection reported as failing a single-item assertion is not possible unless the collection changed between the check and the message being rendered. The list was empty when ShouldHaveSingleItem ran and held the state a moment later when Shouldly enumerated it again to build the message. Waits for the publication through a TaskCompletionSource bounded by the class's TestTimeout, which was declared for exactly this and never used. Deliberate narrowing: the exactly-one-publication assertion is gone, because "exactly one so far" cannot be asserted against an asynchronous publisher without an arbitrary sleep, and the test's subject is that a supervisor sees the pause with the right sequence and reason -- not the publication count. net9.0: the class 8/8 consecutive runs, EventTests 657/0/0. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VpDCvJcBDZerieJB4JEHde --- src/EventTests/Daemon/SideEffectGateTimeoutTests.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/EventTests/Daemon/SideEffectGateTimeoutTests.cs b/src/EventTests/Daemon/SideEffectGateTimeoutTests.cs index 506c35e..7a4bd25 100644 --- a/src/EventTests/Daemon/SideEffectGateTimeoutTests.cs +++ b/src/EventTests/Daemon/SideEffectGateTimeoutTests.cs @@ -62,21 +62,26 @@ public async Task a_timeout_that_did_not_reach_the_prior_mark_pauses_the_shard() // The genuine failure. Side effects must NOT be enabled over history the prior version already // covered, so no continuous agent starts — but the shard is published as Paused rather than left // silently stopped, so it is visible to a supervisor and resumes from its persisted floor. - var paused = new List(); + // ShardStateTracker publishes through a Block, so an observer runs on the block's + // consumer thread and is NOT guaranteed to have run by the time StartAgentAsync returns. + // Collecting into a List and asserting straight after the call is a race, and CI lost it: the + // list was empty at the assertion and held the state by the time Shouldly rendered the failure + // message, which is why that failure read "should have single item but had 1 items". + var paused = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); await using var harness = new DaemonHarness( settings => settings.SideEffectGateTimeout = 250.Milliseconds(), progressReads: [0, PriorMark - 40], onShardState: state => { - if (state.Action == ShardAction.Paused) paused.Add(state); + if (state.Action == ShardAction.Paused) paused.TrySetResult(state); }); await harness.Daemon.StartAgentAsync("Trip:V2:All", CancellationToken.None); harness.Daemon.CurrentAgents().ShouldNotContain(x => x.Name.Identity == "Trip:V2:All"); - var state = paused.ShouldHaveSingleItem(); + var state = await paused.Task.WaitAsync(TestTimeout); state.ShardName.ShouldBe("Trip:V2:All"); state.Sequence.ShouldBe(PriorMark - 40); state.PauseReason.ShouldContain("side-effect gate warm-up timed out");