MNTR: make barrier failures actually fail the barrier

[Aaronontheweb]

The bug

ClientFSM replied to a failed barrier with new Failure(...):

else if (!barrierResult.Success)
{
    response = new Failure(new Exception("barrier failed:" + ...));
}
...
@event.StateData.RunningOp.Value.Item2.Tell(response);

ClientFSM derives from FSM<,>, so the unqualified name Failure binds to the inherited nested FSMBase.Failure — an FSM termination reason — not Akka.Actor.Failure.

Two independent proofs of the binding:

• Akka.Actor.Failure declares no constructor taking an argument (only Exception and Timestamp properties), so new Failure(new Exception(...)) cannot bind to it.
• Akka.Actor.Failure is [Obsolete]. Binding to it would emit CS0618, which breaks this repo's -warnaserror build. The file compiles clean.

FutureActorRef's ask-completion switch faults on exactly three things — ISystemMessage, Status.Failure, and the obsolete Akka.Actor.Failure:

case T t:
    handled = _result.TrySetResult(t);   // ← FSMBase.Failure lands HERE
    break;

So the ask completed successfully. EnterBarrierAsync then discarded the result and logged "passed barrier {0}". Every non-success outcome — timeout, wrong barrier, client lost, duplicate node, and the post-failure short-circuit — was silently swallowed, and the node continued through a rendezvous that never happened.

Why it matters

It does not fabricate passing tests. It destroys attributability.

When one node fails an assertion and never reaches the next barrier, the remaining nodes should fail cleanly on a broken rendezvous, naming the barrier. Instead they walked into the following phase unsynchronized and generated their own unrelated failures. One node's localized, diagnosable error became a multi-node pile of mutually-confusing symptoms — which is a large part of why de-flaking this suite has been so difficult.

Barrier failures are not hypothetical. They appear in real CI runs:

Build	Barrier failures in the MNTR log
129447	ClientLost(node-9), BarrierEmpty(node-1), BarrierTimeout 'verified-first', BarrierEmpty(controller)
129814	ClientLost(node-9), BarrierEmpty(node-1)
129819	BarrierTimeout '5-up', BarrierEmpty(first), ClientLost(node-9), BarrierEmpty(node-1)

Every one was swallowed.

The change

Use Status.Failure at all three sites — matching the two places in this same file that already do it correctly (Player.cs:392 and :570 use Status.Failure for exactly this purpose, including one for the same "not connected yet" message the fixed line 424 got wrong).

Also drops the now-unused ask result, since a failure propagates as an exception rather than needing inspection.

Healthy barriers are unaffected — a successful BarrierResult still replies with the barrier name, and the ask still completes normally.

Expect the failure count to rise

This surfaces failures that were already happening and were being hidden. A CI run on this branch is expected to show more failures than before, and they should be considerably easier to attribute to the node and barrier that actually caused them. That is the intent.

Verification

• Akka.Remote.TestKit builds clean (0 warnings, 0 errors).
• Happy path unchanged: ClusterClientDiscoverySpec (barrier-heavy, 4 tests) passes 4/4 locally.

[Aaronontheweb]

LGTM - really simple fix that has probably been hiding / conflating some real failures in our MNTR test suite for years.