Skip to content

fix(tests): deterministically gate two Windows-flaky actor cold-start tests - #1572

Merged
Aaronontheweb merged 2 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/flaky-cold-start-actor-tests
Jul 3, 2026
Merged

fix(tests): deterministically gate two Windows-flaky actor cold-start tests#1572
Aaronontheweb merged 2 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/flaky-cold-start-actor-tests

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Problem

Two actor tests flake only on the Windows CI runner (Ubuntu + macOS stay green in the same job). They surfaced most recently on the Test-windows-latest job of #1539, but neither test's file — nor any actor source — is touched by that PR, so these are pre-existing flakes that can red-X any PR:

  • DiscordSessionBindingContractTests.Approval_response_sends_feedback
    Assert.Contains() Failure: Filter not matched in collection after AwaitAssert failed, timeout [00:00:03] is over after [2] attempts
  • SessionMemoryObserverActorTests.DistillMemories_only_persists_accepted_proposals_for_future_dedup
    Timeout 00:00:05 while waiting for a message of type SessionDistillationCompleted

Root cause (same class in both)

A fixed short timeout racing a persistent-actor cold start under ThreadPool starvation. The assertions are correct; the deadlines were sized for the work, not the worst-case scheduling latency on a saturated Windows runner. The 2 attempts in 3.66s detail is the tell — the poll loop's retry continuation itself was starved, so it wasn't "the work took 3s," it was "the harness couldn't run."

  • Binding test: DiscordSessionBindingActor is a ReceivePersistentActor. Output only renders after recovery → InitializePipeline → hydration → Active → UnstashAll; every OutputReceived before Active is stashed. The first AwaitAssertAsync (default 3s, from akka.test.default-timeout) had no readiness gate, so it polled GetPostedTexts() while that entire cold start was still in flight. CI logged "session pipeline initialized" ~3 ms after the poll gave up.
  • Observer test: SessionMemoryObserverActor is a ReceivePersistentActor whose command handlers stash until RecoveryCompleted. It's an early persistent actor, so the in-memory journal/snapshot plugin cold start + recovery consumed the 5s ExpectMsg budget before the (fast) FakeChatClient distillation ran. CI logged session_observer_recovery_complete right at the deadline.

This is not a production defect — stash-until-RecoveryCompleted is the correct Akka.Persistence synchronization the parent legitimately relies on. It's a test cold-start-budget artifact, so the fix lives in the tests.

Fix — gate on real readiness signals (no widened timeouts, no sleeps)

  • Binding test: await pipeline.Created.WaitAsync(ct) before the poll — a linear await on the signal RecordingSessionPipeline already exposes (completes inside CreateAsync, once recovery + init are done). This is byte-for-byte the pattern the Reminder_delivery_* and Stashes_messages_during_init siblings already use. The remaining tail is in-process, millisecond-scale, and the 3s poll now covers only that.
  • Observer test: Ask an empty RecordAcceptedDistillationProposals([]) first. That command is answered immediately post-recovery with no Persist and no state mutation (empty-list early return), so it's a side-effect-free readiness ack; the generous 30s ceiling absorbs cold start without polling and the behavioral 5s windows are then measured from a warm actor. Mirrors the existing ReminderManagerActorTests generous-Ask precedent and CLAUDE.md's "Ask<Ack> so callers know a state transition occurred" rule.

Neither fix introduces Thread.Sleep/Task.Delay; both block on a real signal.

Validation

  • dotnet build src/Netclaw.Actors.Tests — clean (0 warnings/errors)
  • The two named tests — pass
  • Full DiscordSessionBindingContractTests + SessionMemoryObserverActorTests classes — 83/83 pass
  • dotnet slopwatch analyze — 0 issues
  • ./scripts/Add-FileHeaders.ps1 -Verify — all headers present

Scope / follow-up

This fixes exactly the two tests that failed. Two independent specialist passes (Akka lifecycle + .NET concurrency) flagged that ~40 sibling tests across SessionBindingContractTests, DiscordSessionBindingContractTests, and SessionMemoryObserverActorTests share the same anti-pattern (a fixed poll/ExpectMsg racing first-actor cold start), and that DistillMemories_records_accepted_proposals_for_recovery_after_ack is a near-twin of the observer flake. I kept this PR tightly scoped to the observed failures rather than rewriting the family. If the maintainers want suite-wide insurance, the low-risk lever is a class-level akka.test.timefactor on those base test classes (the concurrency pass's recommendation) — happy to open a separate PR for that.

Both tests failed only on the Windows CI runner (Ubuntu + macOS green) with
timeouts, not assertion-logic failures. Root cause in both: a fixed short
timeout racing a persistent-actor cold start under ThreadPool starvation.

- DiscordSessionBindingContractTests.Approval_response_sends_feedback: the
  default 3s AwaitAssert poll raced the binding actor's full cold start
  (recovery -> init -> hydrate -> active -> unstash -> render). Under CPU
  starvation the poll loop got only ~2 attempts before the deadline. Gate on
  `await pipeline.Created.WaitAsync(ct)` first -- a linear await on the real
  readiness signal, matching the Reminder_delivery_* / Stashes_messages_during_init
  siblings -- so the 3s poll only covers the fast in-process output tail.

- SessionMemoryObserverActorTests.DistillMemories_only_persists_accepted_proposals_for_future_dedup:
  the 5s ExpectMsg budget was consumed by first-persistent-actor journal cold
  start + recovery (commands stash until RecoveryCompleted). Gate on an empty
  RecordAcceptedDistillationProposals Ask -- answered immediately post-recovery
  with no Persist and no state change, a side-effect-free readiness ack -- so the
  behavioral windows are measured from a warm actor.

No production change: stash-until-RecoveryCompleted is the correct Akka
synchronization; this is a test cold-start-budget artifact. No Thread.Sleep /
Task.Delay introduced -- both fixes block on real signals.
@Aaronontheweb Aaronontheweb added the tests All issues related to testing, quality assurance, and smoke testing. label Jul 3, 2026
@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) July 3, 2026 19:40
@Aaronontheweb Aaronontheweb added the bug Something isn't working label Jul 3, 2026
@Aaronontheweb
Aaronontheweb merged commit 9a0187e into netclaw-dev:dev Jul 3, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working tests All issues related to testing, quality assurance, and smoke testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant