diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index febd974..d57fa3e 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,3 +1,40 @@ +## v0.90.0 — a dropped notification leaves a trace, and the backup digest arrives (2026-08-03, R-182) + +**The smallest change on the board with the largest effect on trust: `processOperator`'s cooldown no +longer returns bare.** It used to drop the event *before* `LogNotification`, so a suppressed operator +alert and an event that never happened were indistinguishable — from the operator's side **and from +the hub's own records**. Measured 2026-08-03: nine `recovery_unit_capture_failed` events arrived, two +were mailed, and **seven left no row anywhere**. That is why the defect took a day to get the right +way round: there was nothing to read. + +A suppressed operator event now writes a `suppressed` row carrying the message and **the key that +suppressed it**, so the collision is readable without reading code. This applies to **every** operator +event, not only the one that exposed it. It deliberately does **not** change the cooldown's duration +or semantics — it makes the drop visible, not absent. + +**`backup_run_failures` — the per-run digest.** One operator mail at the end of a backup run listing +every app that failed, its leg and its reason, with the counts and the target filesystem's free +space. Added to `allowedEventTypes` **and** to `operatorOnlyEvents` — allowlisting alone does not make +an event operator-only, and `FormatCustomerEmail` falls back to the raw English message rather than +blocking. A test demonstrates a customer with the type in their enabled list receiving nothing. + +**`recordOnlyEvents` — a third routing class.** Types that are STORED and RECORDED but never mailed. +`recovery_unit_capture_failed` moves here: it is the durable per-failure record, and the digest is the +notification. Deliberately a register rather than downgrading the severity to `info`, which would have +had the same routing effect while relabelling a genuine failure as informational in the events table, +the operator UI and every historical query. + +**`cooldownRunSuffix` — the run discriminator.** A sibling of `cooldownTierSuffix` rather than a +branch inside it, so `tier` keeps byte-identical semantics and R-97a's tests are untouched. It makes +the cooldown effectively inert for the digest, **which is the intent**: a digest is already +rate-limited by construction, one per run and only when something failed, so there is nothing for a +timer to collapse — while the periodic refresh sweep sends **no** `run_id` and therefore stays under +the ordinary hourly cooldown. + +**The e-mail is rendered as a list, not a JSON blob** — the one operator mail with a variable-length +payload, and a dozen apps on one line is unreadable on a phone at 07:00. An absent space reading +renders as *unavailable*, never as zeros. + ## v0.89.0 — the two halves of decision D-c (2026-08-02, R-167 · R-158) **Decision D-c routes two new signals to two different audiences, and the hub is where that routing diff --git a/hub/internal/api/backup_run_digest_event_test.go b/hub/internal/api/backup_run_digest_event_test.go new file mode 100644 index 0000000..8154eee --- /dev/null +++ b/hub/internal/api/backup_run_digest_event_test.go @@ -0,0 +1,40 @@ +package api + +import "testing" + +// R-182 — the backup run digest needs the allowlist half, and NOT the customerMessages half. +// +// A new event type is a pair of register entries, and each half fails differently. For this type the +// pair is unusual and that is the point: +// +// - missing from `allowedEventTypes` → POST /event returns 400 and the digest VANISHES, +// which would rebuild the exact silence R-182 exists to end; +// - PRESENT in `customerMessages` → the customer would be e-mailed, in Hungarian, a +// list of which apps' backups failed and why — operator detail they can take no action on. So +// this type must be allowlisted and must NOT have a customer message. +// +// The customer-facing half of decision D-c is the FILL WARNING, which fires before this and is +// actionable (free space, delete files, add a drive). This is the operator's half. +// +// Operator-only routing itself is enforced by `notify.operatorOnlyEvents`, NOT by the absence of a +// customerMessages entry — that assumption shipped in v0.78.0 and was wrong, because +// FormatCustomerEmail falls back to the raw message. It is pinned in +// `internal/notify/backup_run_digest_test.go`, which demonstrates a customer with the type in their +// enabled list receiving nothing. +func TestBackupRunDigestIsAllowlisted(t *testing.T) { + if !allowedEventTypes["backup_run_failures"] { + t.Fatal("backup_run_failures must be in allowedEventTypes, or POST /event 400s and the " + + "whole run digest is dropped at the door — the silence R-182 was filed against") + } +} + +// The per-app event is the RECORD and must not be removed while the digest is the notification. +// Deleting it would make the digest the only trace, and a digest that fails to send would then take +// the record with it — the coupling R-182's fix exists to break. +func TestPerAppCaptureEventStaysAllowlisted(t *testing.T) { + if !allowedEventTypes["recovery_unit_capture_failed"] { + t.Fatal("recovery_unit_capture_failed was removed from allowedEventTypes — it is the " + + "durable per-failure RECORD, and the digest is only the notification; the operator " + + "register and every historical query depend on it") + } +} diff --git a/hub/internal/api/handler.go b/hub/internal/api/handler.go index 01c6d93..4f92a94 100644 --- a/hub/internal/api/handler.go +++ b/hub/internal/api/handler.go @@ -1586,6 +1586,11 @@ var allowedEventTypes = map[string]bool{ // OPERATOR-ONLY IS ENFORCED BY `notify.operatorOnlyEvents` — see the paragraph above. This entry // alone does NOT make it operator-only. "recovery_unit_capture_failed": true, + // R-182. The per-run backup digest: one event at the end of a run, listing every app whose + // backup failed or was refused. Allowlisting it is NOT what keeps it away from customers — + // `notify.operatorOnlyEvents` is (see the comment there); both entries ship together and + // `backup_run_digest_event_test.go` pins the pair. + "backup_run_failures": true, // Controller-pushed events "controller_started": true, diff --git a/hub/internal/notify/backup_run_digest_test.go b/hub/internal/notify/backup_run_digest_test.go new file mode 100644 index 0000000..ea5729a --- /dev/null +++ b/hub/internal/notify/backup_run_digest_test.go @@ -0,0 +1,266 @@ +package notify + +import ( + "io" + "log" + "strings" + "testing" +) + +// R-182 — one e-mail per backup run, and nothing dropped without a trace. +// +// MEASURED, NOT SUPPOSED. On 2026-08-03 nine `recovery_unit_capture_failed` events reached the hub +// and TWO operator e-mails went out. The operator cooldown key is +// `customerID + ":" + eventType + cooldownTierSuffix(details)`, that event carries `app` but no +// `tier`, so the key held no app identifier: the first refused app took the hour's slot and every +// other app's failure was discarded — **before `LogNotification`**, so it left no row on any channel +// and could not be found afterwards. +// +// The operator ruled against the obvious fix (putting `app` in the key), because on a full disk that +// is one e-mail per app. These pin the shape that replaced it. + +// ── Scenario D — a suppressed operator event leaves a trace ─────────────────────────────────────── + +// The bare `return` at the cooldown is the whole reason this defect took a day to get the right way +// round: there was nothing to read. A drop must be as visible in the record as a send. +// +// DELIBERATELY EXERCISED ON A DIFFERENT EVENT TYPE than the one that exposed the defect. +// `recovery_unit_capture_failed` is now record-only and never reaches the cooldown at all, so using +// it here would prove nothing. `whole_guest_backup_failed` is an ordinary operator event, and using +// it pins §2.1's actual claim: the suppression row applies to EVERY operator event, not only the one +// that happened to be measured. +func TestSuppressedOperatorEvent_LeavesARow(t *testing.T) { + st := newDispStore(t) + d := NewDispatcher(st, "test-key", "from@felhom.eu", "op@felhom.eu", true, log.New(io.Discard, "", 0)) + sent := captureSeam(d) + + // Two events of the SAME type with no discriminator — the second must be suppressed. + d.ProcessEvent("c1", "whole_guest_backup_failed", "error", "opengist failed", `{"app":"opengist"}`, "controller") + d.ProcessEvent("c1", "whole_guest_backup_failed", "error", "privatebin failed", `{"app":"privatebin"}`, "controller") + + if got := len(mailsFor(*sent, "op@felhom.eu")); got != 1 { + t.Fatalf("operator mails = %d, want 1 — the premise of this test is that the second IS suppressed", got) + } + + rows, err := st.GetRecentNotifications("c1", 50) + if err != nil { + t.Fatal(err) + } + var sentRows, suppressed []store2Row + for _, r := range rows { + if r.Channel != "operator" || r.EventType != "whole_guest_backup_failed" { + continue + } + switch r.Status { + case "sent": + sentRows = append(sentRows, store2Row{r.Status, r.Message, r.ErrorMessage}) + case "suppressed": + suppressed = append(suppressed, store2Row{r.Status, r.Message, r.ErrorMessage}) + } + } + if len(sentRows) != 1 { + t.Fatalf("want 1 'sent' operator row, got %d", len(sentRows)) + } + if len(suppressed) != 1 { + t.Fatalf("want 1 'suppressed' operator row, got %d — a cooldown drop that writes NOTHING is "+ + "indistinguishable from an event that never happened, which is exactly how seven "+ + "failures went missing on 2026-08-03", len(suppressed)) + } + // The row must name the app that was dropped, or it records that something was suppressed + // without recording WHAT — half a fix. + if !strings.Contains(suppressed[0].message, "privatebin") { + t.Fatalf("the suppressed row does not name the dropped event: %q", suppressed[0].message) + } + // And it must carry the key, so the reason it collided is readable without reading code. + if !strings.Contains(suppressed[0].errMsg, "key=") { + t.Fatalf("the suppressed row does not carry the cooldown key: %q", suppressed[0].errMsg) + } +} + +type store2Row struct{ status, message, errMsg string } + +// ── Scenario E — two runs in a day each report ─────────────────────────────────────────────────── + +// The operator ruled explicitly on this: someone pressing the backup button is actively trying to +// get a backup, and finding out tomorrow would be worse than an extra mail in a rare case. +func TestTwoRunsInAnHour_BothReport(t *testing.T) { + st := newDispStore(t) + d := NewDispatcher(st, "test-key", "from@felhom.eu", "op@felhom.eu", true, log.New(io.Discard, "", 0)) + sent := captureSeam(d) + + nightly := `{"run_id":"run-a","run_kind":"nightly","failed":2,"attempted":5,"apps":[{"app":"opengist","leg":"volume dump","reason":"reserve"},{"app":"privatebin","leg":"volume dump","reason":"reserve"}]}` + manual := `{"run_id":"run-b","run_kind":"manual","failed":2,"attempted":5,"apps":[{"app":"opengist","leg":"volume dump","reason":"reserve"},{"app":"privatebin","leg":"volume dump","reason":"reserve"}]}` + + d.ProcessEvent("c1", "backup_run_failures", "error", "2 of 5 apps failed", nightly, "controller") + d.ProcessEvent("c1", "backup_run_failures", "error", "2 of 5 apps failed", manual, "controller") + + if got := len(mailsFor(*sent, "op@felhom.eu")); got != 2 { + t.Fatalf("operator mails = %d, want 2 — the 1-hour cooldown swallowed the manual run's "+ + "digest, which is the fix reappearing one level up: press the button, the run fails, "+ + "and you are told nothing because the machine already wrote this hour", got) + } +} + +// The run discriminator must be NARROW, exactly like its `tier` sibling — empty unless the producer +// opts in — or every existing event type's cooldown silently stops collapsing anything. +func TestCooldownRunSuffix_EmptyForEverythingElse(t *testing.T) { + cases := []struct{ name, details string }{ + {"no details", ""}, + {"details without run_id", `{"app":"opengist","error":"boom"}`}, + {"empty run_id value", `{"run_id":""}`}, + {"malformed json", `{{{nope`}, + {"run_id mentioned in a STRING, not as a key", `{"error":"the run_id: abc failed"}`}, + {"null details", `null`}, + } + for _, c := range cases { + if got := cooldownRunSuffix(c.details); got != "" { + t.Errorf("%s: suffix must be EMPTY so every other type's cooldown is unchanged, got %q", c.name, got) + } + } + if got := cooldownRunSuffix(`{"run_id":"run-a"}`); got != ":run-a" { + t.Fatalf("suffix should be the run id, got %q", got) + } + // The two suffixes must not interfere: a tier event still keys on its tier and nothing else. + if got := cooldownTierSuffix(`{"tier":"felhom-pbs"}`) + cooldownRunSuffix(`{"tier":"felhom-pbs"}`); got != ":felhom-pbs" { + t.Fatalf("a tier-only event's key changed to %q — R-97a's behaviour must be byte-identical", got) + } +} + +// ── Scenario G — the customer never receives the digest ────────────────────────────────────────── + +// v0.78.0 asserted in a COMMENT that a type with no `customerMessages` entry structurally cannot +// reach a customer. It can: FormatCustomerEmail falls back to the raw English message and the only +// customer gate is configuration. So this is demonstrated, not argued. +func TestDigest_IsOperatorOnly_EvenWithAWideEnabledList(t *testing.T) { + st := newDispStore(t) + // A customer who has enabled EVERYTHING, including this type by name. + if err := st.SaveNotificationPrefs("c1", "cust@example.com", + []string{"backup_run_failures", "node_down", "disk_warning"}, 6); err != nil { + t.Fatal(err) + } + d := NewDispatcher(st, "test-key", "from@felhom.eu", "op@felhom.eu", true, log.New(io.Discard, "", 0)) + sent := captureSeam(d) + + details := `{"run_id":"run-a","run_kind":"nightly","failed":1,"attempted":3,"apps":[{"app":"opengist","leg":"volume dump","reason":"reserve"}]}` + d.ProcessEvent("c1", "backup_run_failures", "error", "1 of 3 apps failed", details, "controller") + + if got := mailsFor(*sent, "cust@example.com"); len(got) != 0 { + t.Fatalf("the CUSTOMER received an operator digest (%d mails) — a list of which apps' "+ + "backups failed is not something they can act on, and the raw body is English", len(got)) + } + if got := len(mailsFor(*sent, "op@felhom.eu")); got != 1 { + t.Fatalf("operator mails = %d, want 1", got) + } + if !operatorOnlyEvents["backup_run_failures"] { + t.Fatal("backup_run_failures is not in operatorOnlyEvents — allowlisting alone does NOT " + + "keep it from a customer; that assumption shipped once and was wrong (v0.78.0)") + } +} + +// ── Part 3 — the e-mail a person actually reads ────────────────────────────────────────────────── + +func TestDigestEmail_ListsAppsLegsAndReasons(t *testing.T) { + details := `{"run_id":"run-a","run_kind":"nightly","failed":3,"attempted":40,` + + `"target_path":"/mnt/sys_drive","used_gb":64.3,"avail_gb":0.9,"total_gb":68.7,` + + `"used_percent":94,"space_known":true,"apps":[` + + `{"app":"opengist","leg":"volume dump","reason":"refused: below the reserve (headroom)"},` + + `{"app":"privatebin","leg":"volume dump","reason":"refused: below the reserve (headroom)"},` + + `{"app":"immich","leg":"database dump","reason":"pg_dump: connection refused"}]}` + + subject, body := FormatOperatorEmail("demo-hp", "backup_run_failures", "error", + "3 of 40 apps failed to back up", details) + + // The subject must carry the counts: the operator's first decision is made from it alone. + for _, want := range []string{"demo-hp", "3 of 40", "nightly"} { + if !strings.Contains(subject, want) { + t.Errorf("subject %q missing %q", subject, want) + } + } + // Every app, its leg and its reason. + for _, want := range []string{ + "opengist", "privatebin", "immich", + "volume dump", "database dump", + "below the reserve", "pg_dump: connection refused", + } { + if !strings.Contains(body, want) { + t.Errorf("body missing %q:\n%s", want, body) + } + } + // The counts and the free space, so "one broken app" and "a full disk" read differently. + if !strings.Contains(body, "3 of 40") { + t.Errorf("body does not carry the failed-of-attempted count:\n%s", body) + } + if !strings.Contains(body, "0.9 GB free") { + t.Errorf("body does not carry the free space:\n%s", body) + } + // It must NOT be a JSON blob. + if strings.Contains(body, `"apps":[`) { + t.Errorf("the digest rendered as raw JSON — unreadable on a phone at 07:00:\n%s", body) + } +} + +// An absent space reading must render as unavailable, never as zeros: "0 GB free" and "we could not +// look" are opposite diagnoses, and the operator acts differently on each. +func TestDigestEmail_UnknownSpaceIsNotZero(t *testing.T) { + details := `{"run_id":"r","run_kind":"nightly","failed":1,"attempted":2,"target_path":"/mnt/x",` + + `"space_known":false,"apps":[{"app":"a","leg":"capture","reason":"boom"}]}` + _, body := FormatOperatorEmail("demo-hp", "backup_run_failures", "error", "1 of 2 failed", details) + if strings.Contains(body, "0.0 GB free") { + t.Fatalf("an unreadable filesystem rendered as zeros:\n%s", body) + } + if !strings.Contains(body, "unavailable") { + t.Fatalf("an unreadable filesystem must say so:\n%s", body) + } +} + +// A payload that cannot be parsed must still produce a mail — degraded, never swallowed. +func TestDigestEmail_UnparseableDetailsStillMails(t *testing.T) { + _, body := FormatOperatorEmail("demo-hp", "backup_run_failures", "error", "something failed", `{{{`) + if body == "" || !strings.Contains(body, "something failed") { + t.Fatalf("an unparseable digest lost the mail entirely:\n%s", body) + } +} + +// ── Scenario C — every failure is RECORDED, e-mailed or not ────────────────────────────────────── + +// The per-app event is the record; the digest is the notification. The record must not inherit the +// notification's conditions — no cooldown, no preferences, no dependence on a mail going out. +func TestPerAppFailure_IsRecordedButNotMailed(t *testing.T) { + st := newDispStore(t) + d := NewDispatcher(st, "test-key", "from@felhom.eu", "op@felhom.eu", true, log.New(io.Discard, "", 0)) + sent := captureSeam(d) + + apps := []string{"opengist", "privatebin", "immich", "homebox", "nextcloud"} + for _, a := range apps { + d.ProcessEvent("c1", "recovery_unit_capture_failed", "error", + "Recovery unit capture FAILED for \""+a+"\"", `{"app":"`+a+`"}`, "controller") + } + + // NOT mailed — the digest is the notification. + if got := len(*sent); got != 0 { + t.Fatalf("%d mail(s) sent for per-app failures — they are the RECORD; one mail per app on a "+ + "full disk is the volume problem wearing the correctness problem's clothes, which is "+ + "exactly what the operator ruled against", got) + } + + // But ALL FIVE recorded — this is the assertion yesterday's defect would have failed: nine + // arrived, two were mailed, seven left no row anywhere. + rows, err := st.GetRecentNotifications("c1", 50) + if err != nil { + t.Fatal(err) + } + seen := map[string]bool{} + for _, r := range rows { + if r.EventType == "recovery_unit_capture_failed" && r.Status == "recorded" { + for _, a := range apps { + if strings.Contains(r.Message, a) { + seen[a] = true + } + } + } + } + if len(seen) != len(apps) { + t.Fatalf("only %d of %d per-app failures were recorded (%v) — a failure that produced no row "+ + "anywhere is the measured defect of 2026-08-03", len(seen), len(apps), seen) + } +} diff --git a/hub/internal/notify/dispatcher.go b/hub/internal/notify/dispatcher.go index 19d035e..e26f41e 100644 --- a/hub/internal/notify/dispatcher.go +++ b/hub/internal/notify/dispatcher.go @@ -116,6 +116,18 @@ func (d *Dispatcher) ProcessEvent(customerID, eventType, severity, message, deta return } + // R-182: record-only types are written down and never mailed. Placed BEFORE the severity gate + // so the row is written whatever the severity — the record must not inherit the notification's + // conditions, which is the coupling this whole finding is about. + if recordOnlyEvents[eventType] { + if err := d.store.LogNotification(customerID, eventType, severity, message, "recorded", + "record-only: the per-run digest (backup_run_failures) carries the notification", "operator"); err != nil { + d.logger.Printf("[WARN] Failed to record %s for %s: %v", eventType, customerID, err) + } + d.logger.Printf("[INFO] Recorded (not mailed) %s for %s — the run digest is the notification", eventType, customerID) + return + } + // 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). @@ -260,15 +272,67 @@ func cooldownTierSuffix(detailsJSON string) string { return ":" + d.Tier } +// cooldownRunSuffix returns ":"+run_id when the event's details carry a non-empty `run_id`, else "". +// +// R-182. `cooldownTierSuffix`'s sibling, and deliberately a SEPARATE function rather than an extra +// branch inside it: `tier` keeps byte-identical semantics for every type that uses it, so R-97a's +// behaviour and its tests are untouched by this. +// +// WHY A BACKUP RUN NEEDS ONE. The run digest describes ONE RUN, and a box can have two in a day — +// the nightly one and a manual one the operator triggered *because* something looked wrong. With no +// run-scoped discriminator the 1-hour cooldown would swallow the second, which is the failure this +// row exists to fix, reappearing one level up: the operator presses the button, the run fails, and +// they are told nothing because the machine already wrote that hour. +// +// IT MAKES THE COOLDOWN EFFECTIVELY INERT FOR THIS TYPE, AND THAT IS THE INTENT, NOT AN OVERSIGHT. +// A digest is already rate-limited by construction — one per run, emitted only when something +// failed — so there is nothing for a timer to collapse. The cooldown protects against a repeating +// identical alert; a digest cannot repeat, because each run is a different run. +// +// NARROW, LIKE ITS SIBLING: empty unless the producer opts in by sending a `run_id`, so no existing +// event type's cooldown behaviour changes. +func cooldownRunSuffix(detailsJSON string) string { + if detailsJSON == "" || !strings.Contains(detailsJSON, "\"run_id\"") { + return "" + } + var d struct { + RunID string `json:"run_id"` + } + if err := json.Unmarshal([]byte(detailsJSON), &d); err != nil || d.RunID == "" { + return "" + } + return ":" + d.RunID +} + func (d *Dispatcher) processOperator(customerID, eventType, severity, message, detailsJSON, source string) { if !d.operatorOn || d.operatorEmail == "" { return } - cooldownKey := customerID + ":" + eventType + cooldownTierSuffix(detailsJSON) + cooldownKey := customerID + ":" + eventType + cooldownTierSuffix(detailsJSON) + cooldownRunSuffix(detailsJSON) d.mu.Lock() if last, ok := d.opCooldowns[cooldownKey]; ok && time.Since(last) < 1*time.Hour { d.mu.Unlock() + // R-182: RECORD THE SUPPRESSION. This used to be a bare `return` — the event was dropped + // before any LogNotification, so a cooldown drop and an event that never happened were + // indistinguishable from the operator's side AND from the hub's own records. + // + // Measured 2026-08-03: nine `recovery_unit_capture_failed` events arrived, two emails were + // sent, and the other seven left NO ROW ON ANY CHANNEL. The defect that hid was serious — + // the cooldown key carries no app identifier, so the first refused app took the slot and + // every other app's failure that hour was discarded — but the reason it took a day to find + // the right way round is this line: there was nothing to read. + // + // "We chose not to e-mail you" and "nothing happened" must never look identical. This + // applies to EVERY operator event, not only the one that exposed it. It makes the drop + // visible; it deliberately does NOT change the cooldown's duration or semantics. + if err := d.store.LogNotification(customerID, eventType, severity, message, + "suppressed", "operator cooldown 1h, key="+cooldownKey, "operator"); err != nil { + d.logger.Printf("[WARN] Failed to record suppressed operator notification for %s/%s: %v", + customerID, eventType, err) + } + d.logger.Printf("[INFO] Operator email suppressed for %s/%s — cooldown (key=%s)", + customerID, eventType, cooldownKey) return } d.opCooldowns[cooldownKey] = time.Now() @@ -285,6 +349,33 @@ func (d *Dispatcher) processOperator(customerID, eventType, severity, message, d d.store.LogNotification(customerID, eventType, severity, message, "sent", "", "operator") } +// recordOnlyEvents are STORED and RECORDED but never e-mailed, on either channel. +// +// R-182. The distinction this register exists to make is the whole of that finding: **the record and +// the notification are different things.** A per-app backup failure must always be written down — +// every time, unconditionally, regardless of cooldowns, preferences or whether any mail went out — +// and it must NOT compete for an e-mail slot, because the per-run digest +// (`backup_run_failures`) is what a person is meant to read. +// +// Before this, `recovery_unit_capture_failed` was both at once, and it did neither well: on +// 2026-08-03 nine of them arrived, two were e-mailed, and the other seven were dropped by the +// 1-hour cooldown BEFORE anything was written down. So the operator was told about one app, the +// other apps' failures were discarded, and nothing anywhere recorded that a choice had been made. +// +// WHY A REGISTER AND NOT severity "info". Downgrading the severity would have the same routing +// effect — `severityNotifies` drops info — but it would also relabel a genuine failure as +// informational in the events table, the operator UI and every historical query, and it would +// silently drop the X-Priority handling if the type were ever promoted back. This says what it +// means: not silent, not urgent, RECORDED. +// +// IT IS NOT A WAY TO MUTE THINGS. A type belongs here only when something else carries its +// notification. Adding one with no digest behind it rebuilds the silence R-182 was filed against. +var recordOnlyEvents = map[string]bool{ + // The per-app Tier-1 capture/refusal failure. Its notification is the run digest, which lists + // every failed app in one mail; this row is the durable per-failure record behind it. + "recovery_unit_capture_failed": true, +} + // operatorOnlyEvents are event types that must NEVER reach a customer, whatever their preferences say. // // R-97c. This register exists because the guarantee it provides was previously ASSERTED IN A COMMENT @@ -321,6 +412,15 @@ var operatorOnlyEvents = map[string]bool{ // 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, + // R-182. The per-run backup digest. It is the same class as the line above and for the same + // reason — a customer can act on a full disk (that is the fill warning, which fires first and + // IS customer-facing) but not on a list of which apps' backups failed and why. It also carries + // operator-grade detail: per-app leg names, raw refusal reasons and byte figures. + // + // Listed here rather than relying on the absence of a `customerMessages` entry, which is NOT a + // block — `FormatCustomerEmail` falls back to the raw English message. That mistake shipped + // once (v0.78.0) and the comment above records it. + "backup_run_failures": true, } // IsOperatorOnly reports whether an event type is barred from customer dispatch. Exported so the diff --git a/hub/internal/notify/templates.go b/hub/internal/notify/templates.go index b15428e..38fe3a1 100644 --- a/hub/internal/notify/templates.go +++ b/hub/internal/notify/templates.go @@ -1,6 +1,7 @@ package notify import ( + "encoding/json" "fmt" "strings" "time" @@ -41,6 +42,17 @@ Severity: %s Time: %s Message: %s`, customerID, eventType, severity, now, message) + // R-182: the backup run digest gets a rendered list instead of a raw JSON blob. It is the one + // operator mail that carries a VARIABLE-LENGTH payload, and a dozen apps as one line of JSON is + // unreadable on a phone at 07:00, which is the only time it matters. + if eventType == "backup_run_failures" { + if rendered, sub, ok := renderBackupRunFailures(customerID, detailsJSON); ok { + return sub, body + rendered + fmt.Sprintf("\n\nDashboard: https://hub.felhom.eu/customers/%s", customerID) + } + // Unparseable details fall through to the raw form below rather than losing the mail. A + // digest that renders badly still tells the operator something; a swallowed one does not. + } + if detailsJSON != "" && detailsJSON != "{}" { body += fmt.Sprintf("\nDetails: %s", detailsJSON) } @@ -282,3 +294,91 @@ Ha nem te kérted ezt, hagyd figyelmen kívül ezt az e-mailt. Felhom.eu`, link) return subject, body } + +// ────────────────────────────────────────────────────────────────────── +// R-182 — the backup run digest +// ────────────────────────────────────────────────────────────────────── + +// backupRunFailure is one app's failed leg within a run. +type backupRunFailure struct { + App string `json:"app"` + Leg string `json:"leg"` + Reason string `json:"reason"` +} + +// backupRunDetails is the digest payload the controller sends. +type backupRunDetails struct { + RunID string `json:"run_id"` + RunKind string `json:"run_kind"` + Failed int `json:"failed"` + Attempted int `json:"attempted"` + TargetPath string `json:"target_path"` + UsedGB float64 `json:"used_gb"` + AvailGB float64 `json:"avail_gb"` + TotalGB float64 `json:"total_gb"` + UsedPercent float64 `json:"used_percent"` + SpaceKnown bool `json:"space_known"` + Apps []backupRunFailure `json:"apps"` +} + +// renderBackupRunFailures turns the digest details into an operator-readable block and a subject +// that says the count without being opened. Returns ok=false when the payload cannot be parsed or +// names no apps, so the caller can fall back to the raw rendering rather than mail an empty list. +// +// THE SUCCESS COUNT IS NOT DECORATION. "3 of 4 apps failed" is a catastrophe and "3 of 40" is a bad +// night; the list alone cannot tell them apart, and the operator's first decision — get up now, or +// look after coffee — depends entirely on which it is. +func renderBackupRunFailures(customerID, detailsJSON string) (string, string, bool) { + if detailsJSON == "" { + return "", "", false + } + var d backupRunDetails + if err := json.Unmarshal([]byte(detailsJSON), &d); err != nil || len(d.Apps) == 0 { + return "", "", false + } + + kind := d.RunKind + if kind == "" { + kind = "backup" + } + subject := fmt.Sprintf("[Felhom] 🔴 %s: %d of %d apps failed to back up (%s run)", + customerID, d.Failed, d.Attempted, kind) + + // Column-align the app names so the leg and reason line up and the block scans vertically. + width := 0 + for _, a := range d.Apps { + if len(a.App) > width { + width = len(a.App) + } + } + legWidth := 0 + for _, a := range d.Apps { + if len(a.Leg) > legWidth { + legWidth = len(a.Leg) + } + } + + 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 + if reason == "" { + reason = "(no reason recorded)" + } + fmt.Fprintf(&b, " %-*s %-*s %s\n", width, a.App, legWidth, a.Leg, reason) + } + + // The space figures answer "is this one broken app or a full disk" before the reasons are read. + // An absent reading renders as unavailable, never as zeros — "0 GB free" and "we could not look" + // are opposite diagnoses (the UnitSpace rule, same reasoning, other side of the wire). + if d.SpaceKnown { + fmt.Fprintf(&b, "\nFilesystem: %s — %.1f/%.1f GB used (%.0f%%), %.1f GB free\n", + d.TargetPath, d.UsedGB, d.TotalGB, d.UsedPercent, d.AvailGB) + } else { + fmt.Fprintf(&b, "\nFilesystem: %s — usage unavailable (the filesystem could not be read)\n", d.TargetPath) + } + + b.WriteString("\nEvery failure above is also recorded individually in the notification log,\n") + b.WriteString("whether or not this mail was sent.") + return b.String(), subject, true +} diff --git a/hub/internal/store/store.go b/hub/internal/store/store.go index bc63235..24bea1a 100644 --- a/hub/internal/store/store.go +++ b/hub/internal/store/store.go @@ -815,7 +815,7 @@ type NotificationLogEntry struct { EventType string Severity string Message string - Status string // "sent", "skipped", "failed" + Status string // "sent", "skipped", "failed", "suppressed" (R-182: a cooldown drop, recorded rather than silent) ErrorMessage string Channel string // "operator" or "customer" CreatedAt time.Time diff --git a/manifests/hub.yaml b/manifests/hub.yaml index 4ac963a..573fc2b 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.89.0 + image: gitea.dooplex.hu/admin/felhom-hub:0.90.0 ports: - containerPort: 8080 name: http