fix(raft): address coderabbitai feedback — ShutdownTimeout clamp, transfer error propagation, deterministic test

ShutdownTimeout zero-value panic (critical):
NewNode now clamps ShutdownTimeout to 5*SendTimeout when the caller passes
zero, preventing a panic in time.NewTicker inside waitForMsgsLanded. The
normal path through initRaftNode already sets it explicitly; this guard
protects direct callers (e.g. tests) that omit the field.

Leadership transfer error propagation (major):
When store-lag abdication calls leadershipTransfer() and it fails, the
error is now returned instead of being logged and silently continuing.
Continuing after a failed transfer left the node as leader-without-worker,
stalling the cluster.

Deterministic abdication test (major):
Replace time.Sleep(10ms) + t.Fatal-in-goroutine with channel-based
synchronization: leader runFn signals leaderStarted; the test goroutine
waits up to 50ms for that signal and calls t.Error (safe from goroutines)
if it arrives, then cancels the context either way.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: auricom

pkg/raft/election.go

@@ -146,6 +146,7 @@ func (d *DynamicLeaderElection) Run(ctx context.Context) error {
 							Msg("became leader but store is significantly behind raft state; abdicating to prevent stalled block production")
 						if tErr := d.node.leadershipTransfer(); tErr != nil {
 							d.logger.Error().Err(tErr).Msg("leadership transfer failed after store-lag abdication")
+							return fmt.Errorf("leadership transfer failed after store-lag abdication: %w", tErr)
 						}
 						continue
 					}

pkg/raft/election_test.go

@@ -241,10 +241,15 @@ func TestDynamicLeaderElectionRun(t *testing.T) {
 					startedCh:  fStarted,
 					isSyncedFn: func(*RaftBlockState) (int, error) { return -5, nil },
 				}
-				// Leader must never start
+				// Signal if leader ever starts — it must not.
+				leaderStarted := make(chan struct{}, 1)
 				leader := &testRunnable{runFn: func(ctx context.Context) error {
-					t.Fatal("leader should not start when store is significantly behind raft")
-					return nil
+					select {
+					case leaderStarted <- struct{}{}:
+					default:
+					}
+					<-ctx.Done()
+					return ctx.Err()
 				}}
 
 				logger := zerolog.Nop()
@@ -257,7 +262,14 @@ func TestDynamicLeaderElectionRun(t *testing.T) {
 					leaderCh <- false
 					<-fStarted
 					leaderCh <- true
-					time.Sleep(10 * time.Millisecond)
+					// Wait for abdication to complete (transfer + continue) then verify
+					// the leader was never started before cancelling.
+					select {
+					case <-leaderStarted:
+						t.Error("leader should not start when store is significantly behind raft")
+					case <-time.After(50 * time.Millisecond):
+						// leadership transferred without starting leader — expected
+					}
 					cancel()
 				}()
 				return d, ctx, cancel

pkg/raft/node.go

@@ -82,6 +82,15 @@ func buildRaftConfig(cfg *Config, logger zerolog.Logger) *raft.Config {
 }
 
 func NewNode(cfg *Config, logger zerolog.Logger) (*Node, error) {
+	// Clamp ShutdownTimeout so waitForMsgsLanded never receives a zero or
+	// negative interval (which would panic in time.NewTicker). Callers such as
+	// initRaftNode already set this, but direct callers in tests may not.
+	if cfg.ShutdownTimeout <= 0 {
+		cfgCopy := *cfg
+		cfgCopy.ShutdownTimeout = 5 * cfg.SendTimeout
+		cfg = &cfgCopy
+	}
+
 	if err := os.MkdirAll(cfg.RaftDir, 0755); err != nil {
 		return nil, fmt.Errorf("create raft dir: %w", err)
 	}