hub v0.24.0: dispatcher routes critical severity (+ nil-prefs crash guard)

ProcessEvent routed only warning/error; a critical-severity event was silently dropped.
Now routes warning/error/critical, logs unrecognized severities, and guards a nil
GetNotificationPrefs (which would panic/crash the hub). host_disk_critical emits its
natural critical severity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxLA1mZurFq9kt8hneFeCs

hub/CHANGELOG.md

@@ -1,5 +1,26 @@
 # Felhom Hub — Changelog
 
+## v0.24.0 — dispatcher routes `critical` severity (+ nil-prefs crash guard) (2026-06-30)
+
+NAS Part A2's "Part 0": close the dispatcher's silent drop of `critical`-severity events.
+
+- **`internal/notify/dispatcher.go` `ProcessEvent`:** the severity gate was `severity != "warning" &&
+  severity != "error"` → a `critical` event was **silently dropped** (never emailed). Now routes
+  **warning / error / critical** (`severityNotifies`); `info` stays an intentional non-notify; any
+  **unrecognized** severity is **logged** (`[WARN] Dispatcher: unrecognized severity …`), never silently
+  dropped. Verified safe first: no controller event emits `critical` (all are info/warning/error) and the
+  hub's only would-be `critical` emitter is `host_disk` — so no surprise alert volume.
+- **`internal/monitor/host_disk.go`:** `host_disk_critical` now emits its **natural `critical` severity**
+  (was forced to `error` to survive the old gate); `FormatOperatorEmail` styles `critical` 🔴 like `error`.
+- **Latent crash guard:** `processCustomer` dereferenced `GetNotificationPrefs`, which returns `(nil, nil)`
+  for a customer with no notification row — an event for such a customer would **panic the dispatcher
+  goroutine and crash the hub**. Now guards `prefs == nil` before use.
+- **Seam:** `sendEmailFn` field (defaults to the Resend sender) so routing is unit-tested without real HTTP.
+- Tests: `severityNotifies` (warning/error/critical notify; info/unknown don't) + **companion red-proof**
+  (the pre-fix `warning||error` predicate drops `critical`); ProcessEvent routes `critical` to the operator;
+  an unknown severity is logged not dropped; `info` is silent and not mis-logged. `go build/vet/test ./...`
+  green.
+
 ## v0.23.0 — host root-disk pressure monitoring + alert (2026-06-30)
 
 Closes the silent-failure gap behind the felhom-pve incident: a Proxmox host root fs filling up (vzdump

hub/internal/monitor/host_disk.go

@@ -167,9 +167,9 @@ func (dc *HostDiskChecker) emit(row store.HostDiskRow, oldBand, newBand string)
 	switch newBand {
 	case bandCritical:
 		eventType = "host_disk_critical"
-		// NB: the dispatcher only routes severity "warning"/"error" — a "critical" severity would be
-		// silently dropped — so the critical BAND maps to severity "error" (and the operator email's 🔴).
-		severity = "error"
+		// Natural "critical" severity (hub v0.24.0 routes it; the operator email styles it 🔴). Before
+		// v0.24.0 the dispatcher silently dropped "critical", so this had to be "error" — now it is honest.
+		severity = "critical"
 		message = fmt.Sprintf("Host %s: root filesystem CRITICALLY full at %.0f%% (threshold %.0f%%) — PVE/logging/agent writes may start failing; free space (e.g. old vzdump backups) immediately", row.HostID, row.DiskPercent, dc.crit)
 	case bandWarning:
 		eventType = "host_disk_warning"

hub/internal/monitor/host_disk_test.go

@@ -117,8 +117,8 @@ func TestHostDiskChecker_Severity(t *testing.T) {
 	dc.Check()
 	saveDiskReport(t, st, 97)
 	dc.Check()
-	if len(sev) != 2 || sev[0] != "warning" || sev[1] != "error" {
-		t.Fatalf("severities = %v, want [warning error] (critical band must be 'error' so the dispatcher routes it)", sev)
+	if len(sev) != 2 || sev[0] != "warning" || sev[1] != "critical" {
+		t.Fatalf("severities = %v, want [warning critical] (hub v0.24.0 routes the natural critical severity)", sev)
 	}
 }
 

hub/internal/notify/dispatcher.go

@@ -27,11 +27,15 @@ type Dispatcher struct {
 	mu            sync.Mutex
 	opCooldowns   map[string]time.Time // "customerID:eventType" → last operator notify
 	custCooldowns map[string]time.Time // "customerID:eventType" → last customer notify
+
+	// sendEmailFn is the email sender, seam-injected so tests exercise routing without real HTTP.
+	// Defaults to (*Dispatcher).sendEmail (Resend) in NewDispatcher.
+	sendEmailFn func(to, subject, textBody string) error
 }
 
 // NewDispatcher creates a new notification dispatcher.
 func NewDispatcher(s *store.Store, resendAPIKey, fromEmail, operatorEmail string, operatorOn bool, logger *log.Logger) *Dispatcher {
-	return &Dispatcher{
+	d := &Dispatcher{
 		store:         s,
 		resendAPIKey:  resendAPIKey,
 		fromEmail:     fromEmail,
@@ -42,6 +46,20 @@ func NewDispatcher(s *store.Store, resendAPIKey, fromEmail, operatorEmail string
 		opCooldowns:   make(map[string]time.Time),
 		custCooldowns: make(map[string]time.Time),
 	}
+	d.sendEmailFn = d.sendEmail
+	return d
+}
+
+// severityNotifies reports whether a severity triggers email notifications. warning / error / critical
+// notify; everything else (info, recovery/status, or an unrecognized value) does not. Pure → unit-tested.
+// (Before v0.24.0 a "critical" severity was silently dropped here — the host_disk-class bug.)
+func severityNotifies(severity string) bool {
+	switch severity {
+	case "warning", "error", "critical":
+		return true
+	default:
+		return false
+	}
 }
 
 // ProcessEvent evaluates an event and sends notifications as appropriate.
@@ -57,8 +75,13 @@ func (d *Dispatcher) ProcessEvent(customerID, eventType, severity, message, deta
 		return
 	}
 
-	// Only warning and error severity trigger notifications
-	if severity != "warning" && severity != "error" {
+	// warning / error / critical trigger notifications. "info" is an intentional non-notify (status/
+	// recovery events). Anything else is UNRECOGNIZED — log it (don't silently drop), so a bad severity
+	// surfaces instead of vanishing (the felhom-pve-class lesson: a critical event must never be lost).
+	if !severityNotifies(severity) {
+		if severity != "info" {
+			d.logger.Printf("[WARN] Dispatcher: unrecognized severity %q for %s/%s — not routing", severity, customerID, eventType)
+		}
 		return
 	}
 
@@ -79,7 +102,7 @@ func (d *Dispatcher) sendTestEmail(customerID string) {
 	subject := "[Felhom] Teszt értesítés"
 	body := "Kedves Ügyfél!\n\nEz egy teszt értesítés a Felhom monitoring rendszerből.\nAz értesítések megfelelően működnek.\n\nÜdvözlettel,\nFelhom.eu monitoring"
 
-	if err := d.sendEmail(prefs.Email, subject, body); err != nil {
+	if err := d.sendEmailFn(prefs.Email, subject, body); err != nil {
 		d.logger.Printf("[ERROR] Test email to %s failed: %v", prefs.Email, err)
 		d.store.LogNotification(customerID, "test", "info", "Teszt értesítés", "failed", err.Error(), "customer")
 		return
@@ -104,7 +127,7 @@ func (d *Dispatcher) processOperator(customerID, eventType, severity, message, d
 
 	subject, body := FormatOperatorEmail(customerID, eventType, severity, message, detailsJSON)
 
-	if err := d.sendEmail(d.operatorEmail, subject, body); err != nil {
+	if err := d.sendEmailFn(d.operatorEmail, subject, body); err != nil {
 		d.logger.Printf("[ERROR] Operator email failed for %s/%s: %v", customerID, eventType, err)
 		d.store.LogNotification(customerID, eventType, severity, message, "failed", err.Error(), "operator")
 		return
@@ -119,9 +142,11 @@ func (d *Dispatcher) processCustomer(customerID, eventType, severity, message, d
 		return
 	}
 
-	// Load preferences
+	// Load preferences. GetNotificationPrefs returns (nil, nil) for a customer with no notification row —
+	// guard the nil BEFORE dereferencing (else an event for such a customer panics the dispatcher
+	// goroutine and crashes the hub). No prefs / no email → no customer notification.
 	prefs, err := d.store.GetNotificationPrefs(customerID)
-	if err != nil || prefs.Email == "" {
+	if err != nil || prefs == nil || prefs.Email == "" {
 		return
 	}
 
@@ -148,7 +173,7 @@ func (d *Dispatcher) processCustomer(customerID, eventType, severity, message, d
 
 	subject, body := FormatCustomerEmail(customerID, eventType, severity, message, detailsJSON)
 
-	if err := d.sendEmail(prefs.Email, subject, body); err != nil {
+	if err := d.sendEmailFn(prefs.Email, subject, body); err != nil {
 		d.logger.Printf("[ERROR] Customer email failed for %s/%s: %v", customerID, eventType, err)
 		d.store.LogNotification(customerID, eventType, severity, message, "failed", err.Error(), "customer")
 		return

hub/internal/notify/dispatcher_test.go

@@ -0,0 +1,99 @@
+package notify
+
+import (
+	"bytes"
+	"io"
+	"log"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-hub/internal/store"
+	_ "modernc.org/sqlite"
+)
+
+func newDispStore(t *testing.T) *store.Store {
+	t.Helper()
+	st, err := store.New(filepath.Join(t.TempDir(), "d.db"), log.New(io.Discard, "", 0))
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { st.Close() })
+	st.SaveCustomerConfig(&store.CustomerConfig{CustomerID: "c1", APIKey: "k", RetrievalPassword: "p"})
+	return st
+}
+
+// TestSeverityNotifies is the Part-0 contract + companion red-proof: warning/error/critical notify; info
+// and unknown values do not. The companion models the PRE-FIX predicate (warning||error) that silently
+// dropped "critical" — the bug this fixes.
+func TestSeverityNotifies(t *testing.T) {
+	for _, s := range []string{"warning", "error", "critical"} {
+		if !severityNotifies(s) {
+			t.Errorf("%q must notify", s)
+		}
+	}
+	for _, s := range []string{"info", "", "frobnicate", "debug"} {
+		if severityNotifies(s) {
+			t.Errorf("%q must NOT notify", s)
+		}
+	}
+	// COMPANION RED-PROOF: the pre-fix predicate dropped "critical". The fix routes it.
+	preFix := func(sev string) bool { return sev == "warning" || sev == "error" }
+	if preFix("critical") {
+		t.Fatal("control: the pre-fix predicate should not match critical")
+	}
+	if !severityNotifies("critical") {
+		t.Fatal("the fix MUST route critical where the old (warning||error) predicate dropped it")
+	}
+}
+
+// TestProcessEvent_CriticalRoutes: a critical-severity event reaches the operator channel (Scenario E).
+func TestProcessEvent_CriticalRoutes(t *testing.T) {
+	st := newDispStore(t)
+	d := NewDispatcher(st, "test-key", "from@felhom.eu", "op@felhom.eu", true, log.New(io.Discard, "", 0))
+	var mu sync.Mutex
+	var sent []string
+	d.sendEmailFn = func(to, _, _ string) error { mu.Lock(); defer mu.Unlock(); sent = append(sent, to); return nil }
+
+	d.ProcessEvent("c1", "host_disk_critical", "critical", "root full", "{}", "hub")
+	if len(sent) != 1 || sent[0] != "op@felhom.eu" {
+		t.Fatalf("critical must route to the operator, sent=%v", sent)
+	}
+}
+
+// TestProcessEvent_UnknownSeverityLogged: an unrecognized severity is LOGGED (not silently dropped) and
+// not routed.
+func TestProcessEvent_UnknownSeverityLogged(t *testing.T) {
+	st := newDispStore(t)
+	var buf bytes.Buffer
+	d := NewDispatcher(st, "test-key", "from", "op@felhom.eu", true, log.New(&buf, "", 0))
+	sent := 0
+	d.sendEmailFn = func(_, _, _ string) error { sent++; return nil }
+
+	d.ProcessEvent("c1", "weird_event", "frobnicate", "msg", "{}", "hub")
+	if sent != 0 {
+		t.Fatal("an unknown severity must not route")
+	}
+	if !strings.Contains(buf.String(), "unrecognized severity") {
+		t.Fatalf("an unknown severity must be logged, got: %q", buf.String())
+	}
+}
+
+// TestProcessEvent_InfoSilent: "info" is an intentional non-notify — not routed AND not logged as
+// unrecognized (it's a known, deliberate non-alert).
+func TestProcessEvent_InfoSilent(t *testing.T) {
+	st := newDispStore(t)
+	var buf bytes.Buffer
+	d := NewDispatcher(st, "test-key", "from", "op@felhom.eu", true, log.New(&buf, "", 0))
+	sent := 0
+	d.sendEmailFn = func(_, _, _ string) error { sent++; return nil }
+
+	d.ProcessEvent("c1", "controller_started", "info", "msg", "{}", "hub")
+	if sent != 0 {
+		t.Fatal("info must not route")
+	}
+	if strings.Contains(buf.String(), "unrecognized") {
+		t.Fatal("info is an intentional non-notify — it must NOT be logged as unrecognized")
+	}
+}

hub/internal/notify/templates.go

@@ -23,7 +23,7 @@ func init() {
 // FormatOperatorEmail returns (subject, textBody) for the operator channel.
 func FormatOperatorEmail(customerID, eventType, severity, message, detailsJSON string) (string, string) {
 	icon := "⚠️"
-	if severity == "error" {
+	if severity == "error" || severity == "critical" {
 		icon = "🔴"
 	}