CI: CrossHubPatchAtomicityTest drops concurrent cross-hub adds on loaded shards — the ack path is bounded, the merge is not the suspect #945

Split out of #826, which tracks a different shape ("8 distinct tests each failed exactly once"). This one is a named test failing repeatedly on unrelated PRs, always on the same shard.

Evidence (2026-08-08)

MeshWeaver.AI.Test.CrossHubPatchAtomicityTest.ConcurrentCrossHubPatches_DoNotDropAQueuedMessage, shard 4 both times:

Neither diff goes near cross-hub patching. Both pass locally on the same head, first attempt.

Why this is not "just a flaky test"

The test asserts a product invariant: 6 mirrors × 48 concurrent cross-hub stream.Update adds against one node, four rounds, and every key must survive. A failure means the owner genuinely dropped acked writes — the exact loss class #590/#648/#667 were about. The test doing its job under load is more likely than the test being wrong.

What I ruled OUT

The merge is atomic. DataExtensions.ApplyMeshNodePatchInTurn performs the read-merge-write inside primary.Update(...) on the owner's action-block turn — a pure, bounded transform with no IO and no nested subscribe, explicitly written that way so back-to-back patches each merge onto the freshest node. Sibling keys cannot be lost there.

The remaining suspect: the bounded ACK path

In the same function, success is acked OFF-turn by postSub:

• it waits for a post-commit emission containing the stamped write, .Timeout(20s);
• then IPostCommitFlush.Flush(...), .Timeout(10s).

On a saturated shard those bounds are reachable. Two things to check, in order:

1. Does an expired ack turn into a lost write, or only a lost acknowledgement? If the merge committed and only the ack timed out, the key should still be present and the test would pass — so a MISSING key means either the merge never ran for that patch, or the client never sent it. Instrument which.
2. Does the client retry on a NACK/timeout, and if so against what base? A retry that re-diffs against a stale mirror base could emit a patch that replaces PendingUserMessages wholesale instead of adding one key — that WOULD drop siblings. Worth confirming the diff is always a minimal per-key delta under contention.

Repro status — honest

I could not reproduce locally: 18 cores, test passes in ~23 s even with other builds running. It needs a genuinely saturated 2-core-class runner. Suggested next step is instrumenting the ack/merge path rather than more local re-runs.

Cost

Every occurrence blocks a green PR and burns a CI cycle; it hit two unrelated PRs in one morning.

🤖 Generated with Claude Code