QueuedWriteAdvancesOnHandoffTest: the successor's caller can miss its terminal when the first write's ack is late (1/330, Plugins shard 3) #3363

Activity

1. rbuergi

   added

   bugSomething isn't workingSomething isn't working

   on Sep 5, 2026
2. rbuergi commented on Sep 5, 2026

   

   rbuergi

   on Sep 5, 2026

   ContributorAuthor

   More actions

   Closed — fixed in MeshWeaver.Plugins#1372, and it was the test, not the handoff

   Verified on Plugins main: the two settle signals are present in
   QueuedWriteAdvancesOnHandoffTest.

   Not a second ordering hole on the successor's side. The write was fine in the failing run — the
   store poll that precedes the failing assertion had already converged on name="second" with
   the description, and secondError was null. Only the caller's terminal had not arrived yet, and the
   test read it synchronously the instant the store converged.

   Those are two decoupled paths, and MeshNodeStreamCache documents them as such:

   • the per-path Update queue advances on a bounded signal (QueueAdvanceBound), while
   • "each write's subscription to the owner's PatchDataResponse must keep running to deliver the
     real terminal (value / RLS denial) to its caller", and
   • UpdateRemote bounds its own owner-response wait at UpdateResponseWaitBound (~2 s) before
     falling back to an optimistic emit.

   So the owner can have written storage, and the 100 ms poll can observe it, while the terminal is
   still in flight. The failing run took [2 s] — UpdateResponseWaitBound exactly. That is the
   1-in-330 window.

   The fix, and why it is not a widened bound

   Each write now completes its own AsyncSubject<Unit> from the subscription's onNext and
   onError, awaited before any terminal is read — the producer→test signal shape AGENTS.md
   prescribes.

   🚨 The assertion is not weakened. If a terminal never arrives, the
   Within(TestTimeouts.Convergence) wait still fails — and now fails saying the caller was never
   settled, rather than NotBeNull at whatever instant the store happened to converge. Late and
   never were indistinguishable before; they are not now. That distinction is the whole difference
   between this and raising a timeout.

   No entry was added to a known-flakes file, per this issue's own instruction that "an allowance is
   not a fix".

   On the filing

   The attribution in the issue was right and saved time: neither #3362 (a rename nothing referenced)
   nor #1369 (two Blazor render paths) can reach the queued-write handoff, and I confirmed that
   independently before looking anywhere else.
3. rbuergi

   closed this as completedon Sep 5, 2026
4. rbuergi commented on Sep 6, 2026

   

   rbuergi

   on Sep 6, 2026

   ContributorAuthor

   More actions

   Investigated. Static reading of UpdateRemote and MeshNodeStreamCache's per-path queue finds no path that delivers a null terminal to a caller whose error is null: every OnNext carries the lambda's output (EmitTerminal), the base node (the two no-op branches), or a re-attempt's value (ChainTerminal); the base sources filter nulls (BaseStateSource Where(change.Value is not null)); a null through the typed-content Select would throw into OnError; and the assertion helper's Emit() fails on completion without a value. 40 runs of the test locally against core main (34c3eecec+): 40/40.

   So the next occurrence has to name its sequence. MeshWeaver.Plugins#1391 makes both callers record every notification they receive (OnNext with name/description/version, OnError with type and message, OnCompleted) and carries the two sequences in the failure messages. Not a fix; the fix needs that sequence.