Avoid blocking on completion signals when the waiter has left

[kevindharmawan]

The controller and the view changer both used unbuffered channels to report completion, where the receiver can legitimately stop waiting and leave the sender blocked forever.

Controller: Decide stops waiting on deliverChan when the view aborts, but decide was still blocked sending on it, so the run loop stalled until shutdown. Now uses a per-decision delivered channel closed after delivery. In the aborted case decide no longer returns early, so the decision-count increment, rotation check, pruning and leader-token acquisition now run, since the decision was in fact delivered.

View changer: once commitInFlightProposal returned via timeout or stop, nothing read inFlightDecideChan or inFlightSyncChan. A late Decide or Sync then blocked the in-flight view's run goroutine, which blocked Abort() since it waits on that goroutine, which blocked the view changer in its own cleanup. No later attempt could start to drain the channel. Decide could at least escape on stopChan, while Sync had no escape at all. Now each attempt owns an inFlightAttempt holding its own buffered decideCh and syncCh, cleared when the attempt ends, with non-blocking sends. The in-flight view's Decider and Sync are callbacks bound to that attempt at creation, so a late Decide or Sync can only signal its own attempt and cannot disturb a later one.

Adds regression tests for all three completion paths.

[HagarMeir]

Behavior change worth confirming: previously decide blocked on the unbuffered deliverChan send when the waiter had left (view aborted), so the post-delivery bookkeeping below (incrementCurrentDecisionsInView, checkIfRotate/changeView, MaybePruneRevokedRequests, acquireLeaderToken) never ran in the aborted case. It now always runs once delivery happened. This unblocks the run loop (the real fix) and is defensible because the decision was in fact delivered, but it means a rotation changeView and leader-token acquisition can now fire for a view that is being aborted, racing the in-progress view change. changeView guards against going backwards (latestView > newViewNumber), which mitigates it, but please confirm the extra increment/rotate cannot momentarily double-count or start a spurious rotation view before the pending abort/view-change is processed.

[HagarMeir]

Dead state: id, view, and sequence on inFlightAttempt (and ViewChanger.inFlightAttemptSeq that feeds id) are written but never read anywhere — attempt identity is established purely by pointer equality (v.inFlightAttempt == attempt in the cleanup defer). Only decideCh, syncCh, and viewRef are actually used. Consider dropping the unused fields and the inFlightAttemptSeq counter to reduce clutter.

[HagarMeir]

These public Decide/Sync methods (and currentInFlightAttempt) are now dead in production. In-flight views are wired with the per-attempt callbacks (Decider: callbacks, Sync: callbacks), and nothing else wires ViewChanger as a Decider/Synchronizer (the heartbeat handler is c.controller, and consensus.go never passes the ViewChanger as a Decider/Sync). Only the new tests call them. Beyond being dead code, they are a latent foot-gun: when invoked with a nil attempt they still call Application.Deliver/Synchronizer.Sync without stopping any view or signaling. Consider removing them rather than keeping them as a compatibility shim.

[HagarMeir]

Dead branch: because this select has a default, it never blocks, so the case <-v.stopChan: branch can never be the distinguishing choice — and all three branches just return anyway. The stopChan case is unreachable/pointless here (same in syncInFlight, which correctly omits it). Simplify to a plain non-blocking send:

	select {
	case attempt.decideCh <- struct{}{}:
	default:
	}

[HagarMeir]

🤖 This PR was reviewed by Claude. See the inline comments above for detailed findings.