address Copilot review findings on PR #146

- Add statusMonitor.setAndBroadcast that always broadcasts to SSE
  subscribers; PokeBox now uses it so a config edit that flips only a
  non-level field (ConsoleEnabled, Enabled, Name, …) still pushes to
  open /bx tabs. The chatter-suppression in set() is desirable for
  the steady-state poll loop but defeats the purpose of an
  out-of-band config-change refresh.
- PokeBox now distinguishes transient DB errors from "not found":
  on err != nil we log and keep the last-known-good snapshot; only
  box == nil drops the cached entry. Avoids poisoning the UI when
  SQLite hiccups.
- Gear.Start stores the unsubscribe func returned by Subscribe and
  Gear.Stop now calls it before shutting the monitor down. Closes
  the events-adapter forwarder goroutine cleanly and stops new
  PokeBox calls from reaching a winding-down monitor.
- Subscribe call now references events.EventTypeBoxConfigChanged
  (cast to string for the gear.EventPublisher API) instead of a raw
  "box.config_changed" literal — keeps publisher and subscriber on
  the same constant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: sarg3nt

gearbox/internal/gears/bx/gear.go

@@ -16,6 +16,7 @@ import (
 	"github.com/a-h/templ"
 	"github.com/go-chi/chi/v5"
 
+	"github.com/sarg3nt/gearbox/internal/framework/events"
 	"github.com/sarg3nt/gearbox/internal/framework/gear"
 )
 
@@ -26,8 +27,9 @@ func init() {
 // Gear implements the Bx fleet-view gear.
 type Gear struct {
 	gear.BaseGear
-	handlers *Handlers
-	monitor  *statusMonitor
+	handlers           *Handlers
+	monitor            *statusMonitor
+	configChangeUnsub  func() // returned by EventHub.Subscribe; called in Stop
 }
 
 // Info returns gear metadata.
@@ -66,7 +68,7 @@ func (g *Gear) Start(ctx context.Context) error {
 	}
 	if g.monitor != nil {
 		if hub := g.GetEventHub(); hub != nil {
-			hub.Subscribe("box.config_changed", func(e gear.Event) {
+			g.configChangeUnsub = hub.Subscribe(string(events.EventTypeBoxConfigChanged), func(e gear.Event) {
 				if e.ServerID == "" {
 					return
 				}
@@ -77,8 +79,15 @@ func (g *Gear) Start(ctx context.Context) error {
 	return nil
 }
 
-// Stop signals the background poller to wind down.
+// Stop signals the background poller to wind down and releases the
+// event subscription so the forwarder goroutine inside the events
+// adapter exits cleanly and no further PokeBox calls reach a
+// shutting-down monitor.
 func (g *Gear) Stop(ctx context.Context) error {
+	if g.configChangeUnsub != nil {
+		g.configChangeUnsub()
+		g.configChangeUnsub = nil
+	}
 	if g.monitor != nil {
 		g.monitor.Stop()
 	}

gearbox/internal/gears/bx/status.go

@@ -121,16 +121,24 @@ func (m *statusMonitor) run(ctx context.Context) {
 // box-config-changed event subscriber so settings edits (enable toggle,
 // console-enabled toggle, agent URL change, …) reflect on /bx without
 // waiting on the next 30s tick. Safe to call concurrently with the
-// regular poll loop — m.set is mutex-guarded.
+// regular poll loop — set/setAndBroadcast are mutex-guarded.
 func (m *statusMonitor) PokeBox(ctx context.Context, boxID string) {
 	if m.db == nil {
 		return
 	}
 	box, err := m.db.GetBoxByBoxID(boxID)
-	if err != nil || box == nil {
-		// Box was deleted, or the DB call failed — drop any stale
-		// snapshot for that ID so /bx renders default-state rather
-		// than the last-known-good values for a now-missing box.
+	if err != nil {
+		// Transient DB error — don't poison the snapshot by dropping
+		// last-known-good values. Log and bail; the next 30s poll
+		// will reconcile.
+		if m.deps.Logger != nil {
+			m.deps.Logger.Warn("bx PokeBox: db lookup failed; preserving last-known status", "box_id", boxID, "error", err)
+		}
+		return
+	}
+	if box == nil {
+		// Box was deleted — drop the stale snapshot so /bx doesn't
+		// keep rendering last-known-good values for a now-missing box.
 		m.mu.Lock()
 		delete(m.statuses, boxID)
 		m.mu.Unlock()
@@ -143,7 +151,11 @@ func (m *statusMonitor) PokeBox(ctx context.Context, boxID string) {
 		}
 	}
 	status := m.probe(ctx, box, apiKey)
-	m.set(status)
+	// Force-broadcast: a config edit may have flipped only a non-level
+	// field (e.g. ConsoleEnabled). The default set() suppresses chatter
+	// on equal-level updates, which is exactly the case we need to NOT
+	// suppress here.
+	m.setAndBroadcast(status)
 }
 
 // pollAll fans out a reachability check per configured box. The check is
@@ -271,6 +283,19 @@ func (m *statusMonitor) set(s BoxStatus) {
 	}
 }
 
+// setAndBroadcast stores a status and ALWAYS broadcasts to subscribers,
+// even when level/reachable haven't changed. Used by PokeBox so a flip
+// of a non-level field (ConsoleEnabled, Enabled, Name, …) reaches open
+// /bx tabs immediately — the chatter-suppression in set() is desirable
+// for the steady-state poll loop but defeats the purpose of an
+// out-of-band config-change refresh.
+func (m *statusMonitor) setAndBroadcast(s BoxStatus) {
+	m.mu.Lock()
+	m.statuses[s.BoxID] = s
+	m.mu.Unlock()
+	m.broadcast(s)
+}
+
 // Subscribe returns a channel of status events and an unsubscribe func.
 // Events are best-effort: a slow consumer that doesn't drain its channel
 // will simply miss events (we never block the publisher).