hub v0.89.0 — the two halves of decision D-c (R-167, R-158)

New OPERATOR-ONLY event type recovery_unit_capture_failed (controller v0.191.0, R-158): in allowedEventTypes AND notify.operatorOnlyEvents. Deliberately not a reuse of backup_failed, which carries customer copy and sits in the controller's DefaultEnabledEvents — reusing it would email the customer in Hungarian about a failure they cannot act on. R-158's own proposal said backup_failed; D-c overrides it. disk_warning/disk_critical lose their generic customerMessages entries. Both were allowlisted, copy'd, default-enabled and checkbox'd with NO producer anywhere; controller v0.191.0 becomes that producer and sends a DYNAMIC Hungarian message naming the drive and its free space. FormatCustomerEmail prefers the entry over the message, so keeping a static entry would discard the label and the byte figures — the same reason offbox_enlarge_blocked and disk_health_degraded have none. The deletion is pinned by a test. New notify.IsOperatorOnly so the api package can pin BOTH registers of a new event type in ONE test; allowlisted-but-not-operator-only is invisible when they are checked separately, and it is the defect v0.78.0 shipped. The register itself stays unexported. REUSE.md's "new event type" extension point rewritten: it told readers to always add a customerMessages entry, which is wrong for operator-only types and harmful for dynamic-message ones. Tests 574 -> 579. Red-proof: removing the operatorOnlyEvents entry shows the customer being emailed; the skipped/operator_only row is asserted as a positive observable.
2026-08-02 23:19:13 +02:00
parent 8ef92a3fa7
commit 179dd79882
7 changed files with 259 additions and 4 deletions
@@ -315,8 +315,20 @@ var operatorOnlyEvents = map[string]bool{
 	// a consequence of another type's routing — true today, and silently untrue the moment the failed
 	// event becomes customer-visible. Belt, not inference.
 	"whole_guest_backup_recovered": true,
+	// R-158 / R-167 (D-c). A per-app Tier-1 recovery-unit capture failure. A customer can take no
+	// action on it — the causes are a full filesystem, a permission fault or a broken dump, all of
+	// which the operator resolves — and the alert carries operator-grade detail (target path, byte
+	// figures, the raw error). The customer's half of D-c is the FILL WARNING, which fires BEFORE
+	// this and is actionable: free space, delete files, add a drive.
+	"recovery_unit_capture_failed": true,
 }

+// IsOperatorOnly reports whether an event type is barred from customer dispatch. Exported so the
+// api package can pin BOTH registers of a new event type in one test — allowlisted-but-not-
+// operator-only is the v0.78.0 defect, and it is only visible when the two are checked together.
+// Read-only: the register itself stays unexported so nothing can widen it at runtime.
+func IsOperatorOnly(eventType string) bool { return operatorOnlyEvents[eventType] }
+
 func (d *Dispatcher) processCustomer(customerID, eventType, severity, message, detailsJSON, source string) {
 	// R-97c: operator-tier events stop here, BEFORE prefs are consulted — the point is that no
 	// customer configuration can opt in. Logged rather than dropped, so the skip is visible in
@@ -0,0 +1,130 @@
+package notify
+
+import (
+	"io"
+	"log"
+	"strings"
+	"testing"
+)
+
+func discardLogger() *log.Logger { return log.New(io.Discard, "", 0) }
+
+// R-158 / R-167 Scenario G — the two new signals of decision D-c sit on OPPOSITE sides of the
+// operator/customer line, and each is tested through the real dispatch path rather than by reading
+// the register.
+//
+// D-c's rule, restated: a customer can free space, delete files or add a drive, so a FILL WARNING is
+// theirs. A customer can do nothing about a recovery-unit capture failure, so it is not.
+
+// The operator half. Run under the BREAKING configuration — the customer has the event explicitly
+// enabled and has an email address — because that is the only configuration in which a missing
+// operatorOnlyEvents entry is visible. This is the v0.78.0 defect, demonstrated rather than argued.
+func TestRecoveryUnitCaptureFailed_NeverReachesTheCustomer(t *testing.T) {
+	st := opOnlyStore(t)
+	if err := st.SaveNotificationPrefs("c1", "customer@example.com",
+		[]string{"recovery_unit_capture_failed", "backup_failed"}, 6); err != nil {
+		t.Fatalf("SaveNotificationPrefs: %v", err)
+	}
+
+	rec := &sentTo{}
+	d := opOnlyDispatcher(t, st, rec)
+	d.ProcessEvent("c1", "recovery_unit_capture_failed", "error",
+		`Recovery unit capture FAILED for "immich" — the app has no fresh local (Tier-1) backup`,
+		`{"app":"immich","used_percent":100,"space_known":true}`, "controller")
+
+	for _, to := range rec.to {
+		if to == "customer@example.com" {
+			t.Fatalf("a customer was emailed the OPERATOR-ONLY recovery_unit_capture_failed (%s) — "+
+				"enabled_events must not be able to opt in to a failure they cannot act on", to)
+		}
+	}
+
+	// The operator must still get it: the register mutes the customer channel, not the signal.
+	gotOperator := false
+	for _, to := range rec.to {
+		if to == "operator@felhom.eu" {
+			gotOperator = true
+		}
+	}
+	if !gotOperator {
+		t.Fatal("the operator was not notified of a recovery-unit capture failure — the alert is the " +
+			"whole point of R-158 and it went nowhere")
+	}
+
+	// The skip must be VISIBLE. An absent log row is equally consistent with "correctly skipped" and
+	// "the dispatcher never ran" — the positive observable is the row itself (standing rule 3).
+	logs, err := st.GetRecentNotifications("c1", 20)
+	if err != nil {
+		t.Fatalf("GetRecentNotifications: %v", err)
+	}
+	found := false
+	for _, l := range logs {
+		if l.Channel == "customer" && l.Status == "skipped" && strings.Contains(l.ErrorMessage, "operator_only") {
+			found = true
+		}
+	}
+	if !found {
+		t.Fatalf("the customer skip is not logged as skipped/operator_only — it is indistinguishable "+
+			"from a delivery that never happened; got %d row(s)", len(logs))
+	}
+}
+
+// The customer half. The fill warning MUST be delivered, and it must render the controller's own
+// Hungarian text — which names the drive and the free space — rather than a generic template or the
+// raw English.
+func TestDiskFillWarning_ReachesTheCustomerInHungarianWithItsNumbers(t *testing.T) {
+	st := opOnlyStore(t)
+	if err := st.SaveNotificationPrefs("c1", "customer@example.com", []string{"disk_warning"}, 6); err != nil {
+		t.Fatal(err)
+	}
+
+	// The exact shape the controller sends from v0.191.0.
+	const hun = `A(z) „Fotók" tároló 87%-osan megtelt — 4,2 GB szabad hely maradt. ` +
+		`Szabadíts fel helyet (törölj felesleges fájlokat, vagy csatlakoztass új meghajtót), ` +
+		`különben a biztonsági mentések hamarosan meghiúsulnak.`
+
+	var bodies []string
+	d := NewDispatcher(st, "test-key", "from@felhom.eu", "operator@felhom.eu", true, discardLogger())
+	d.sendEmailFn = func(to, subject, body string, headers map[string]string) error {
+		if to == "customer@example.com" {
+			bodies = append(bodies, subject+"\n"+body)
+		}
+		return nil
+	}
+	d.ProcessEvent("c1", "disk_warning", "warning", hun,
+		`{"label":"Fotók","used_percent":87,"avail_gb":4.2}`, "controller")
+
+	if len(bodies) == 0 {
+		t.Fatal("the customer was NOT warned that a disk is filling — this is the customer half of " +
+			"decision D-c, and nothing reached them")
+	}
+	got := strings.Join(bodies, "\n")
+
+	// The controller's dynamic text must survive. A customerMessages entry for disk_warning would
+	// OVERRIDE it (templates.go prefers the entry) and discard the drive name and the free space,
+	// leaving the customer with "A lemezterület 90% felett van" and nothing to act on — which is
+	// exactly why v0.191.0 removes those two entries.
+	if !strings.Contains(got, "Fotók") {
+		t.Fatalf("the customer email does not name the drive — a generic customerMessages entry has "+
+			"discarded the controller's dynamic text (templates.go priority). Got:\n%s", got)
+	}
+	if !strings.Contains(got, "4,2 GB") {
+		t.Fatalf("the customer email does not carry the free-space figure. Got:\n%s", got)
+	}
+	if !strings.Contains(got, "Szabadíts fel helyet") {
+		t.Fatalf("the customer email does not tell the customer what to DO. Got:\n%s", got)
+	}
+}
+
+// The generic entries must STAY REMOVED. A well-meaning re-add would silently re-break the test
+// above's guarantee for every future reader — this pins the deletion itself.
+func TestDiskFillTypesHaveNoGenericCustomerMessage(t *testing.T) {
+	for _, et := range []string{"disk_warning", "disk_critical"} {
+		if msg, ok := customerMessages[et]; ok {
+			t.Fatalf("customerMessages[%q] = %q — a static entry OVERRIDES the controller's dynamic "+
+				"Hungarian text (FormatCustomerEmail prefers the entry), discarding the drive name "+
+				"and the free-space figure the customer needs. Same reason offbox_enlarge_blocked "+
+				"and disk_health_degraded deliberately have none", et, msg)
+		}
+	}
+}
@@ -71,9 +71,20 @@ var customerMessages = map[string]string{
 	"offbox_repo_orphaned": "A távoli mentési tároló elárvult: a benne lévő mentések egy korábbi, már nem elérhető kulccsal készültek (jellemzően újratelepítés után). Új mentés a tároló visszaállításáig nem készül — nyisd meg a Távoli mentés oldalt.",
 	"offbox_repo_reset":    "A távoli mentési tároló visszaállítva: a régi előzmény félretéve (nem törölve), és egy üres, új tároló jött létre a mostani kulccsal.",

-	// Disk events (GUEST — the controller's own cgroup view)
-	"disk_warning":  "A lemezterület 90% felett van — kérjük, szabadíts fel helyet.",
-	"disk_critical": "A lemezterület kritikusan magas (95%+) — azonnali beavatkozás szükséges!",
+	// Disk events (GUEST — the controller's own view) — `disk_warning` / `disk_critical`.
+	//
+	// DELIBERATELY NO ENTRY, from hub v0.89.0 / controller v0.191.0 (R-167, decision D-c). These two
+	// types were allowlisted here, carried generic Hungarian copy, sat in the controller's
+	// DefaultEnabledEvents and had a UI checkbox — and NOTHING IN ANY REPO EMITTED THEM. A complete
+	// customer pipeline with no producer; the sixth "built but never wired" instance in this project.
+	// The controller became their producer in v0.191.0.
+	//
+	// The producer sends a DYNAMIC Hungarian message naming the filesystem and its free space, so a
+	// static entry here would be actively harmful: FormatCustomerEmail PREFERS the entry over the
+	// message, so re-adding one would discard the drive name and the byte figures and leave the
+	// customer with "A lemezterület 90% felett van" — a warning with nothing to act on. Same reason
+	// `offbox_enlarge_blocked` and `disk_health_degraded` have no entry. Pinned by
+	// TestDiskFillTypesHaveNoGenericCustomerMessage.

 	// Host disk events (the Proxmox HOST root filesystem — distinct from the guest disk above)
 	"host_disk_warning":  "A házszerver alaprendszerének (Proxmox-gazda) gyökérlemeze 90% felett van — kérjük, szabadíts fel helyet (pl. régi biztonsági mentések).",