fix(services): use refresh channel in Register instead of inline check

Simpler approach: Register() sends a non-blocking signal on a new
chRefresh channel; the run() goroutine picks it up and calls update()
immediately. This avoids duplicating per-service check logic and removes
the lock-ordering complexity from the previous implementation.

Author: Fletch153

pkg/services/health.go

@@ -47,8 +47,9 @@ func CopyHealth(dest, src map[string]error) {
 // HealthChecker is a services.Service which monitors other services and can be probed for system health.
 type HealthChecker struct {
 	StateMachine
-	chStop chan struct{}
-	chDone chan struct{}
+	chStop    chan struct{}
+	chDone    chan struct{}
+	chRefresh chan struct{}
 
 	cfg HealthCheckerConfig
 
@@ -130,12 +131,13 @@ func (cfg HealthCheckerConfig) New() *HealthChecker {
 	cfg.initVerSha()
 	cfg.setNoopHooks()
 	return &HealthChecker{
-		cfg:      cfg,
-		services: make(map[string]HealthReporter, 10),
-		healthy:  make(map[string]error, 10),
-		ready:    make(map[string]error, 10),
-		chStop:   make(chan struct{}),
-		chDone:   make(chan struct{}),
+		cfg:       cfg,
+		services:  make(map[string]HealthReporter, 10),
+		healthy:   make(map[string]error, 10),
+		ready:     make(map[string]error, 10),
+		chStop:    make(chan struct{}),
+		chDone:    make(chan struct{}),
+		chRefresh: make(chan struct{}, 1),
 	}
 }
 
@@ -171,6 +173,8 @@ func (c *HealthChecker) run() {
 		select {
 		case <-ticker.C:
 			c.update(ctx)
+		case <-c.chRefresh:
+			c.update(ctx)
 		case <-c.chStop:
 			return
 		}
@@ -218,38 +222,21 @@ func (c *HealthChecker) Register(service HealthReporter) error {
 		return fmt.Errorf("misconfigured check %#v for %T", name, service)
 	}
 
-	func() {
-		c.servicesMu.Lock()
-		defer c.servicesMu.Unlock()
-		if testing.Testing() {
-			if orig, ok := c.services[name]; ok {
-				panic(fmt.Errorf("duplicate name %q: service names must be unique: types %T & %T", name, service, orig))
-			}
+	c.servicesMu.Lock()
+	defer c.servicesMu.Unlock()
+	if testing.Testing() {
+		if orig, ok := c.services[name]; ok {
+			panic(fmt.Errorf("duplicate name %q: service names must be unique: types %T & %T", name, service, orig))
 		}
-		c.services[name] = service
-	}()
-
-	// If already started, immediately populate the health state for this
-	// service so callers don't wait up to <interval> for the next tick.
-	// servicesMu must be released before calling IfStarted to avoid a
-	// deadlock with Start(), which holds the StateMachine lock while
-	// calling update() (which acquires servicesMu.RLock).
-	c.IfStarted(func() {
-		ctx := context.Background()
-		ready := service.Ready()
+	}
+	c.services[name] = service
 
-		c.stateMu.Lock()
-		defer c.stateMu.Unlock()
-		c.ready[name] = ready
-		for n, err := range service.HealthReport() {
-			c.healthy[n] = err
-			value := 0
-			if err == nil {
-				value = 1
-			}
-			c.cfg.SetStatus(ctx, name, value)
-		}
-	})
+	// Signal the polling goroutine to refresh immediately so the newly
+	// registered service is visible without waiting for the next tick.
+	select {
+	case c.chRefresh <- struct{}{}:
+	default: // refresh already pending
+	}
 
 	return nil
 }

pkg/services/health_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -70,9 +71,9 @@ func (r *testHealthReporter) HealthReport() map[string]error {
 	return map[string]error{r.name: r.healthy}
 }
 
-// TestHealthChecker_Register_ImmediateUpdate verifies that Register() immediately
-// populates the health state when the HealthChecker is already running, so
-// callers don't wait up to 15 s for the next polling tick.
+// TestHealthChecker_Register_ImmediateUpdate verifies that Register() triggers an
+// immediate poll when the HealthChecker is already running, so the newly
+// registered service is visible well within a polling interval (15 s).
 func TestHealthChecker_Register_ImmediateUpdate(t *testing.T) {
 	hc := HealthCheckerConfig{Ver: "test", Sha: "abc1234"}.New()
 	if err := hc.Start(); err != nil {
@@ -85,18 +86,23 @@ func TestHealthChecker_Register_ImmediateUpdate(t *testing.T) {
 		t.Fatalf("Register: %v", err)
 	}
 
-	// No sleep needed — Register() must have synchronously populated the state.
-	ready, readyErrs := hc.IsReady()
-	assert.True(t, ready, "expected ready after Register, got errors: %v", readyErrs)
-	assert.Nil(t, readyErrs["svc"], "expected svc ready, got: %v", readyErrs["svc"])
+	// The refresh is async (goroutine scheduling); a short poll is sufficient
+	// and far faster than the 15-second polling interval.
+	assert.Eventually(t, func() bool {
+		_, errs := hc.IsReady()
+		_, ok := errs["svc"]
+		return ok && errs["svc"] == nil
+	}, time.Second, time.Millisecond, "service should become ready quickly after Register")
 
-	healthy, healthyErrs := hc.IsHealthy()
-	assert.True(t, healthy, "expected healthy after Register, got errors: %v", healthyErrs)
-	assert.Nil(t, healthyErrs["svc"], "expected svc healthy, got: %v", healthyErrs["svc"])
+	assert.Eventually(t, func() bool {
+		_, errs := hc.IsHealthy()
+		_, ok := errs["svc"]
+		return ok && errs["svc"] == nil
+	}, time.Second, time.Millisecond, "service should become healthy quickly after Register")
 }
 
 // TestHealthChecker_Register_ImmediateUpdate_Unhealthy verifies that an unhealthy
-// service registered on a running HealthChecker is immediately visible.
+// service registered on a running HealthChecker is reflected quickly.
 func TestHealthChecker_Register_ImmediateUpdate_Unhealthy(t *testing.T) {
 	hc := HealthCheckerConfig{Ver: "test", Sha: "abc1234"}.New()
 	if err := hc.Start(); err != nil {
@@ -110,11 +116,15 @@ func TestHealthChecker_Register_ImmediateUpdate_Unhealthy(t *testing.T) {
 		t.Fatalf("Register: %v", err)
 	}
 
-	ready, readyErrs := hc.IsReady()
-	assert.False(t, ready, "expected not ready")
+	assert.Eventually(t, func() bool {
+		_, errs := hc.IsReady()
+		_, ok := errs["svc2"]
+		return ok
+	}, time.Second, time.Millisecond, "service should appear in ready map quickly after Register")
+
+	_, readyErrs := hc.IsReady()
 	assert.Equal(t, notReady, readyErrs["svc2"])
 
-	healthy, healthyErrs := hc.IsHealthy()
-	assert.False(t, healthy, "expected not healthy")
+	_, healthyErrs := hc.IsHealthy()
 	assert.Equal(t, notReady, healthyErrs["svc2"])
 }