diff --git a/hub/internal/notify/backup_run_digest_test.go b/hub/internal/notify/backup_run_digest_test.go index ea5729a..fe01497 100644 --- a/hub/internal/notify/backup_run_digest_test.go +++ b/hub/internal/notify/backup_run_digest_test.go @@ -264,3 +264,38 @@ func TestPerAppFailure_IsRecordedButNotMailed(t *testing.T) { "anywhere is the measured defect of 2026-08-03", len(seen), len(apps), seen) } } + +// The per-app reason must not repeat the filesystem figures the digest already prints once. Reviewed +// as copy against the first real digest, not designed in the abstract. +func TestDigestEmail_ReasonDoesNotRepeatTheUsageLine(t *testing.T) { + reason := "refused: below the reserve (reserve: 97% used or 1.0 GiB free) — /mnt/sys_drive: 65.0/68.7 GB used (95%), 0.2 GB free" + details := `{"run_id":"r","run_kind":"nightly","failed":1,"attempted":2,"target_path":"/mnt/sys_drive",` + + `"used_gb":65,"avail_gb":0.2,"total_gb":68.7,"used_percent":95,"space_known":true,` + + `"apps":[{"app":"opengist","leg":"whole app","reason":"` + reason + `"}]}` + _, body := FormatOperatorEmail("demo-hp", "backup_run_failures", "error", "1 of 2 failed", details) + + // The figures appear ONCE, on the Filesystem line — not again on every app row. + if strings.Count(body, "65.0/68.7 GB used") != 1 { + t.Fatalf("the usage clause appears %d times; it must appear once, on its own line — repeated "+ + "down a list of a dozen apps it pushes the part that DIFFERS off a phone screen:\n%s", + strings.Count(body, "65.0/68.7 GB used"), body) + } + // But the reason itself survives — trimming must not eat the diagnosis. + if !strings.Contains(body, "below the reserve") { + t.Fatalf("the reason was trimmed away entirely:\n%s", body) + } +} + +// A reason naming a DIFFERENT path, or none, must be left completely alone. +func TestTrimRepeatedUsage_LeavesUnrelatedReasonsAlone(t *testing.T) { + for _, c := range []struct{ reason, target string }{ + {"pg_dump: connection refused", "/mnt/sys_drive"}, + {"tar failed — /mnt/other: 1/2 GB used (50%), 1 GB free", "/mnt/sys_drive"}, + {"boom", ""}, + {"", "/mnt/sys_drive"}, + } { + if got := trimRepeatedUsage(c.reason, c.target); got != c.reason { + t.Errorf("reason %q (target %q) was altered to %q", c.reason, c.target, got) + } + } +} diff --git a/hub/internal/notify/recovery_unit_dispatch_test.go b/hub/internal/notify/recovery_unit_dispatch_test.go index 0dd4573..e70fd4a 100644 --- a/hub/internal/notify/recovery_unit_dispatch_test.go +++ b/hub/internal/notify/recovery_unit_dispatch_test.go @@ -39,33 +39,45 @@ func TestRecoveryUnitCaptureFailed_NeverReachesTheCustomer(t *testing.T) { } } - // The operator must still get it: the register mutes the customer channel, not the signal. - gotOperator := false + // R-182 CHANGED WHAT THIS ASSERTS, DELIBERATELY, AND THE OLD ASSERTION IS WORTH KEEPING IN VIEW. + // + // Until 2026-08-03 this test required the OPERATOR to be e-mailed here, on the grounds that "the + // alert is the whole point of R-158". That was right when this event was the only signal, and it + // is wrong now: measured, nine of these arrived at the hub and two were mailed, because the + // operator cooldown key carries no app identifier — so as an alarm it told the operator about one + // app and threw the rest away. + // + // The type is now RECORD-ONLY: written down every time, never mailed. R-158's guarantee — the + // operator learns WHICH app failed and WHY — is not weakened, it MOVED: the per-run digest + // `backup_run_failures` carries every failed app in one mail, and is pinned by + // backup_run_digest_test.go. The customer safety claim below is untouched and is the reason this + // test still exists. for _, to := range rec.to { if to == "operator@felhom.eu" { - gotOperator = true + t.Fatal("the operator was e-mailed a PER-APP capture failure — this type is the record " + + "now, not the alarm. One mail per failing app on a full disk is a dozen mails, which " + + "is the volume problem the operator ruled against; the digest is the notification") } } - 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). + // The RECORD must exist, always. It is what makes the digest trustworthy: if the digest is ever + // lost, delayed or suppressed, the failures are still individually written down. An absent row is + // equally consistent with "correctly not mailed" 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") { + if l.EventType == "recovery_unit_capture_failed" && l.Status == "recorded" && + strings.Contains(l.ErrorMessage, "record-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)) + t.Fatalf("the per-app failure left no 'recorded' row — a failure that is neither mailed nor "+ + "written down is exactly the 2026-08-03 defect, rebuilt; got %d row(s)", len(logs)) } } diff --git a/hub/internal/notify/templates.go b/hub/internal/notify/templates.go index 38fe3a1..e413e83 100644 --- a/hub/internal/notify/templates.go +++ b/hub/internal/notify/templates.go @@ -361,7 +361,7 @@ func renderBackupRunFailures(customerID, detailsJSON string) (string, string, bo var b strings.Builder fmt.Fprintf(&b, "\n\nFAILED: %d of %d apps attempted in this %s run.\n\n", d.Failed, d.Attempted, kind) for _, a := range d.Apps { - reason := a.Reason + reason := trimRepeatedUsage(a.Reason, d.TargetPath) if reason == "" { reason = "(no reason recorded)" } @@ -382,3 +382,27 @@ func renderBackupRunFailures(customerID, detailsJSON string) (string, string, bo b.WriteString("whether or not this mail was sent.") return b.String(), subject, true } + +// trimRepeatedUsage strips the trailing "— /path: X/Y GB used (Z%), W GB free" clause from a per-app +// reason, because the digest prints those figures ONCE for the whole run on its own line. +// +// This is a copy fix, and it was made after reading the first real digest rather than from the +// design. The reserve's refusal message is authored for a single-app alert, where naming the +// filesystem is exactly right; repeated down a list of a dozen apps it is the same forty characters +// twelve times, and it pushes the part that differs off the right-hand edge of a phone screen at +// 07:00 — which is the only moment this mail has to work. +// +// It trims ONLY an exact "— :" suffix, so a reason that mentions a different path, or +// none, is left completely alone. A reason that is nothing but the usage clause is left alone too: +// removing everything would turn a bad line into an empty one. +func trimRepeatedUsage(reason, targetPath string) string { + if reason == "" || targetPath == "" { + return reason + } + marker := " — " + targetPath + ":" + i := strings.LastIndex(reason, marker) + if i <= 0 { + return reason + } + return strings.TrimSpace(reason[:i]) +} diff --git a/manifests/hub.yaml b/manifests/hub.yaml index 573fc2b..224f9a3 100644 --- a/manifests/hub.yaml +++ b/manifests/hub.yaml @@ -125,7 +125,7 @@ spec: spec: containers: - name: hub - image: gitea.dooplex.hu/admin/felhom-hub:0.90.0 + image: gitea.dooplex.hu/admin/felhom-hub:0.90.1 ports: - containerPort: 8080 name: http