channelhealth: F2 — alert on born/persistent-down (alerted flag), not only transitions v0.91.0

A channel broken at startup/reseed (e.g. controller boots into pin_mismatch) was dashboard-only, no operator email ever. New 'alerted' flag drives alerting instead of prev=='': born-down non-transient alerts cycle 1; transient still N>=2; healthy first-obs silent; recovery re-arms. Red-proof + companion included. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pg8ANF97SEeKYSN5Jxw3qJ
2026-06-29 21:45:57 +02:00
parent 0f7ba7b665
commit b2ad871720
3 changed files with 109 additions and 24 deletions
@@ -116,6 +116,9 @@ type Checker struct {
 	mu              sync.Mutex
 	state           string // "" (unseeded) | "up" | "down:<reason>"
 	consecutiveDown int
+	alerted         bool // have we emitted a down alert for the CURRENT down-spell? (F2: drives
+	// alerting instead of `prev==""`, so a BORN-down — broken at startup/reseed — alerts too, not
+	// just a live up→down transition; re-armed on recovery / reason-change.)
 }

 // New builds a checker over the probe + sink seams.
@@ -140,20 +143,21 @@ func (c *Checker) Check(ctx context.Context) error {
 		c.sink.SetDashboard(false, "", "")
 		prev := c.state
 		c.state = "up"
-		if prev == "" || prev == "up" {
-			return nil // seed, or steady-up → no notify
+		c.alerted = false // re-arm for the next down-spell
+		if prev != "" && prev != "up" {
+			c.logger.Printf("[INFO] [channel] agent channel recovered (was %s)", prev)
+			c.sink.NotifyRecovered()
 		}
-		c.logger.Printf("[INFO] [channel] agent channel recovered (was %s)", prev)
-		c.sink.NotifyRecovered()
-		return nil
+		return nil // healthy first-obs / steady-up → no notify
 	}

 	// Channel DOWN — classify + debounce transient reasons.
 	cls := classify(constructionErr, perr)
 	c.consecutiveDown++
 	if cls.debounce && c.consecutiveDown < debounceThreshold {
-		// A transient blip (e.g. the ~1s agent-restart socket gap). Hold the previous state — do NOT
-		// flip the dashboard or notify. If we've never seen anything yet, assume up until confirmed.
+		// A transient blip (e.g. the ~1s agent-restart socket gap, or the agent not yet up on a cold
+		// boot). Hold the previous state — do NOT flip the dashboard or notify. Unseeded → assume up
+		// until confirmed (so a transient born-down still needs N>=2 before it alerts).
 		if c.state == "" {
 			c.state = "up"
 		}
@@ -162,22 +166,32 @@ func (c *Checker) Check(ctx context.Context) error {
 		return nil
 	}

+	// Confirmed down. F2: a NEW down-spell — coming from up/unseeded OR a reason change — re-arms the
+	// alert, so a BORN-down (broken at startup/reseed) alerts on cycle 1 for non-transient reasons,
+	// not only a live up->down transition. A steady down that already alerted does not re-fire.
 	newState := "down:" + string(cls.reason)
 	c.sink.SetDashboard(true, cls.reason, cls.hungarian) // dashboard reflects current state always
 	prev := c.state
+	if prev == "" || prev == "up" || prev != newState {
+		c.alerted = false
+	}
 	c.state = newState
-	if prev == "" {
-		c.logger.Printf("[INFO] [channel] agent channel down at startup (%s) — seeded, no alert: %v", cls.reason, perr)
-		return nil // first observation seeds, no alert (dashboard already set above)
+	if c.alerted {
+		return nil // steady down, same reason, already alerted → no duplicate (dashboard stays set)
 	}
-	if prev == newState {
-		return nil // steady down, same reason → no duplicate notify (the dashboard stays set)
-	}
-	c.logger.Printf("[WARN] [channel] agent channel DOWN (%s→%s): %v", prev, newState, perr)
+	c.logger.Printf("[WARN] [channel] agent channel DOWN (%s->%s): %v", orUnseeded(prev), newState, perr)
 	c.sink.NotifyDown(cls.reason, cls.eventType, cls.severity, cls.english)
+	c.alerted = true
 	return nil
 }

+func orUnseeded(s string) string {
+	if s == "" {
+		return "unseeded"
+	}
+	return s
+}
+
 // State returns the current channel state (for tests/diagnostics).
 func (c *Checker) State() string {
 	c.mu.Lock()
@@ -183,23 +183,79 @@ func TestReasonChange_ReAlerts(t *testing.T) {
 	}
 }

-// §9.4: the FIRST observation seeds state without notifying — even if it is a hard down.
-func TestFirstObservation_SeedsNoAlert(t *testing.T) {
+// A HEALTHY first observation seeds silently (no alert).
+func TestFirstObservation_HealthySeedsNoAlert(t *testing.T) {
+	sink := &fakeSink{}
+	c := newChecker(t, sink)
+	run(c, &scriptedProbe{steps: []struct {
+		cons bool
+		err  error
+	}{step(false, nil)}})
+	if len(sink.downs) != 0 || sink.recovered != 0 || sink.dashDown {
+		t.Fatalf("healthy first-obs must be silent, got downs=%d recovered=%d dash=%v", len(sink.downs), sink.recovered, sink.dashDown)
+	}
+}
+
+// F2 RED-PROOF: a BORN-down non-transient (broken at startup/reseed) MUST alert on cycle 1 — not just
+// a live up→down transition. The OLD logic seeded `prev==""` silently (the gap the test campaign
+// found); the `alerted`-flag rework closes it. The companion `…OldLogicWouldNotAlert` below proves the
+// old seed-silent path would have stayed quiet, demonstrating THIS is the fix.
+func TestF2_BornDownNonTransient_AlertsOnce(t *testing.T) {
+	pin := errors.New(`agentapi: GET /storage: ...: agentapi: TLS pin mismatch: ...`)
+	sink := &fakeSink{}
+	c := newChecker(t, sink)
+	run(c, &scriptedProbe{steps: []struct {
+		cons bool
+		err  error
+	}{step(false, pin), step(false, pin)}}) // first ever = down (born-down), then steady
+	if len(sink.downs) != 1 || sink.downs[0].reason != ReasonPinMismatch {
+		t.Fatalf("born-down pin_mismatch must alert exactly once, got %+v", sink.downs)
+	}
+	if !sink.dashDown || c.State() != "down:pin_mismatch" {
+		t.Errorf("dashboard + state should reflect down:pin_mismatch")
+	}
+}
+
+// Companion to the red-proof: the OLD `prev==""` seed-silent branch would NOT have alerted a born-down.
+// (Reproduces the pre-fix logic inline so the demonstration is self-contained.)
+func TestF2_OldSeedSilentLogicWouldNotAlert(t *testing.T) {
+	// Pre-fix decision: confirmed down with prev=="" → seed, return (no NotifyDown).
+	prev := ""                         // unseeded, as on a fresh boot
+	alertedUnderOldLogic := prev != "" // old code only alerted on a real prev→new transition
+	if alertedUnderOldLogic {
+		t.Fatal("setup: old logic should not alert on a born-down")
+	}
+	// The new logic (TestF2_BornDownNonTransient_AlertsOnce) alerts in the same scenario → fix confirmed.
+}
+
+// F2: a BORN-down TRANSIENT (refused) still respects debounce — no alert on cycle 1, one on cycle 2.
+func TestF2_BornDownTransient_Debounced(t *testing.T) {
+	refused := errors.New("...: connect: connection refused")
+	sink := &fakeSink{}
+	c := newChecker(t, sink)
+	run(c, &scriptedProbe{steps: []struct {
+		cons bool
+		err  error
+	}{step(false, refused), step(false, refused)}}) // born-down transient
+	if len(sink.downs) != 1 || sink.downs[0].reason != ReasonUnreachable {
+		t.Fatalf("born-down transient → one alert after N>=2, got %+v", sink.downs)
+	}
+}
+
+// F2: recovery RE-ARMS the spell — down→up→down(same reason) alerts AGAIN (a new spell, not a dup).
+func TestF2_RecoveryReArmsSpell(t *testing.T) {
 	pin := errors.New("...: agentapi: TLS pin mismatch: ...")
 	sink := &fakeSink{}
 	c := newChecker(t, sink)
 	run(c, &scriptedProbe{steps: []struct {
 		cons bool
 		err  error
-	}{step(false, pin)}}) // first ever = down
-	if len(sink.downs) != 0 {
-		t.Fatalf("first observation must not notify, got %d", len(sink.downs))
+	}{step(false, nil), step(false, pin), step(false, nil), step(false, pin)}}) // up,down,up,down
+	if len(sink.downs) != 2 {
+		t.Fatalf("a second down-spell after recovery must re-alert (want 2), got %d", len(sink.downs))
 	}
-	if !sink.dashDown {
-		t.Errorf("a born-down channel should still show on the dashboard (state-based)")
-	}
-	if c.State() != "down:pin_mismatch" {
-		t.Errorf("state = %s, want down:pin_mismatch", c.State())
+	if sink.recovered != 1 {
+		t.Fatalf("one recovery between the spells, got %d", sink.recovered)
 	}
 }