diff --git a/CHANGELOG.md b/CHANGELOG.md index 913dc88..2e021ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,36 @@ +## Unreleased (2026-10-09, kernel night) — check first, stop second (R-921); a household's clear deletes the hub's address (R-922, controller half) — ships with the next controller release + +**MinAgent: 0.131.0** (unchanged — R-921 reads only `GET /backup/tiers` and `GET /backup/status?target=`, both served +since agent v0.97.0; a pre-R-82 agent answers 404 and the pre-check stands aside). **R-922 needs the hub of the same +day** to act on the new field (an older hub ignores an unknown JSON field and keeps the address — today's behaviour). +No version bump; committed, not released. + +- **R-921 — no app is stopped for a tier the agent will refuse because another tier's job is in flight.** Measured + 2026-10-08 night on demo-hp: the off-site window stopped every app, the agent answered „a heavy operation is already + in flight" (`busy=backup:local`, the night OS step after the local copy), and the apps were down about a minute for + no copy. The agent refuses a backup for two reasons; the controller can SEE only one before a stop. New + `quiesce.InFlightProber` (`internal/quiesce/precheck.go`) + `agentapi.Client.BackupJobsInFlight` (tiers → each + tier's job phase, `running`/`snapshotted` = in flight): when a job of ANOTHER tier is in flight the window returns + before the marker and stops nothing; the tier stays due (no breaker, no contention verdict) and is asked again at the + next poll. The window's own tier in flight is left as before (the agent answers its start with the running job). A + pre-check that cannot be answered fails toward backing up (WARN, then today's path). **What it cannot see — the + measured instance:** the agent's host-wide heavy-operation gate (the OS step, a restore-test, fstrim) is served by + no endpoint; there the BUSY answer is the first signal and the apps are resumed AT ONCE — pinned: + `TestR921_BusyRefusalResumesAtOnce` (the next event after the refusal is the first restart, no agent call in + between; with and without the pre-check, i.e. also the race „free at the check, busy at the start"). Closing the + measured case before the stop needs the agent to serve its gate (an additive field) — not built here. Tests + `TestR921_*` (quiesce, 4), `TestBackupJobsInFlight_*` (agentapi, 3); the adapter's wiring is pinned at compile time + (`var _ quiesce.InFlightProber = quiesceBackend{}`). +- **R-922 (controller half, operator ruling 2026-10-09 11:19, option A) — the push says when the household cleared its + address.** `POST /api/v1/preferences` gains `"email_cleared": true` (omitted otherwise). Rule: an empty address + saved where one was stored is a deliberate clear; `settings.NotificationPrefs.EmailCleared` persists it, and every + push carries it while the address stays empty (the dashboard save, the debug resync, and now the startup sync, which + also runs for a cleared box — so a clear the hub missed reaches it later). A box that never had an address never + sends it (the hub's no-clobber guard keeps protecting a seeded address); a new address drops it. Tests + `TestR922_DeliberateClearSendsEmailCleared`, `TestR922_NeverConfiguredBoxSendsNoFlag`, `TestR922_NewAddressDropsFlag` + (web, on the wire against an httptest hub), `TestR922_ClearedByRule`, `TestR922_MarkerPersistsAndDrivesStartupSync` + (settings). The wire-contract gate's `controller -> hub (POST /preferences)` root checks the hub can decode it. + ## v0.304.0 — the operator's decision sheet (D1, D3, D4, D8), a daytime press never cancels the night (R-899), the honest recovery answer (R-304), three dashboard layout fixes (2026-10-09) **MinAgent: 0.131.0** (unchanged). The operator actions (D1) need hub 0.144.0 to arrive; an older hub sends none. The diff --git a/REPORT.md b/REPORT.md index d387946..6f83168 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,12 +1,17 @@ -# REPORT — v0.304.0 released, delivered and proven live (2026-10-09) +# REPORT — 2026-10-09 (afternoon): R-921 pre-check and R-922 `email_cleared` (UNRELEASED) -The 2026-10-08 work (D1 operator actions, D3 dashboard language, D4 sign-ins survive a restart, D8 hold after a failed -off-site restore, R-899, R-304, three layout fixes) released as v0.304.0 (`d7bfdcc`, CI 1577 success), MinAgent 0.131.0 -unchanged. Floors 0.304.0 for demo-hp, demo-felhom, tester-1 (07:05 local); all three run it, healthy. Scratch 9202 set -to 0.304.0 by hand. +Not released, not delivered: both ship with the next controller release, **after** the hub release that decodes +`email_cleared` (felhom.eu `d55c590c`). MinAgent unchanged (0.131.0). -Live proofs: **D1** on Tester 1 — `run_job fill-watch` and `offsite_backup_now` done (box log: „checked 3 -filesystem(s)", „backup OK: 3 app(s)"); **D3** — 9202's health banners English for `lang=en`, Hungarian for `lang=hu`; -**D4** on 9202 — the same cookie works after a restart; after a password change both old cookies are refused, also -after the next restart; password set back. **D8** not measured (needs a scratch off-site repo). -Evidence: `felhom.eu/documentation/audits/release-2026-10-09/proofs/`. +- **R-921 — check first, stop second.** Before a window stops apps, the controller asks the agent which tiers have a + backup job in flight (`GET /backup/status`, per tier). Another tier's job running or snapshotted → no app stops; the + tier stays due. A refusal after the stop still resumes the apps at once (pinned). Red-proof: + `TestR921_NoStopWhileAnotherTiersJobIsInFlight` — before: `3 app(s) stopped for a tier the agent refuses`. + **Not covered:** the agent's host-wide busy lock (night OS step, restore test, fstrim), the case measured on demo-hp + on 2026-10-08 — no agent endpoint serves it; needs an agent field (felhom.eu R-921, LEFT). +- **R-922 — a household's clear reaches the hub.** Saving an empty address where one was stored marks it cleared + (`settings.json`); every preferences push while it stays empty carries `"email_cleared": true` (omitted otherwise); + a never-configured box never sends it; a new address drops it. Red-proof: `TestR922_DeliberateClearSendsEmailCleared` + — before: `the push after a deliberate clear carries no email_cleared:true`. +- Suite: `go test ./... -count=1` rc 0 (35 packages); `controller_gates.py` rc 0. No docker used. +- Built by a helper session under the brief's fences; reviewed and committed by the main session. diff --git a/REUSE.md b/REUSE.md index 4f3727a..f0cf166 100644 --- a/REUSE.md +++ b/REUSE.md @@ -125,7 +125,8 @@ | `infra.SambaContainerName` / `SambaPassdbVolume` / `SambaPassdbMount` | controller/internal/infra/samba.go | consts | single source of truth for the samba container identity | the compose renderer interpolates them; stacks/backup/monitor read them. The CONTAINER name (`felhom-samba`) is NOT the stack name (`samba`) — `EffectiveProtected` needs the container one | | `sambaWriteAtomic` | controller/internal/stacks/samba.go | `(path, data, mode) error` | samba smb.conf/compose writes | tmp+**fsync**+rename (the only one of these that fsyncs). Fourth atomic-write helper in the tree — see §6 | | `Loop.writeMarker` / `Recover` | controller/internal/quiesce/quiesce.go | `(m Marker)` / `()` | Quiesce crash-safety | Marker written BEFORE stopping stacks; Recover restarts stranded stacks at boot | -| `quiesce.TieredBackend` + `Loop.resolveDueTiers` / `quiesceAndPollTiers` | controller/internal/quiesce/tiers.go, quiesce.go | `Tiers/DueFor/StartBackupFor/BackupStatusFor`; `resolveDueTiers(ctx) ([]dueTier,bool,error)` | THE R-82 multi-tier backup schedule — several whole-guest tiers (local daily + PBS weekly) reconciled into ONE quiesce window | **Both tiers due ⇒ ONE stop/start pair**, never two (two = two app outages for one night). Tiers run SEQUENTIALLY (vzdump holds a guest lock) and the app stays down until the LAST tier snapshots — resuming earlier loses app-consistency on the DR tier. Order is fast-first (agent advertises primary first) or downtime blows up. `ErrTiersUnsupported` (route 404) ⇒ pre-R-82 agent ⇒ degrade to the untargeted path and **STILL BACK UP** — never read it as "nothing due". | +| `quiesce.TieredBackend` + `Loop.resolveDueTiers` / `quiesceAndPollTiers` | controller/internal/quiesce/tiers.go, quiesce.go | `Tiers/DueFor/StartBackupFor/BackupStatusFor`; `resolveDueTiers(ctx) ([]dueTier,bool,error)` | THE multi-tier whole-guest backup schedule (local daily + PBS weekly) | **One stop per tier** (`09` §3 decision 156, v0.301.0 — REVERSES R-82's one window for both tiers): a window backs up only the FIRST due tier and resumes the apps at its `snapshotted`; another due tier waits for a later cycle. A tier that refuses to start (BUSY or an error) lets the next tier try in the same window. Order is agent order (primary first). `ErrTiersUnsupported` (route 404) ⇒ pre-R-82 agent ⇒ degrade to the untargeted path and **STILL BACK UP** — never read it as "nothing due". | +| `quiesce.InFlightProber` + `Loop.anotherTierInFlight` · `agentapi.Client.BackupJobsInFlight` (R-921) | controller/internal/quiesce/precheck.go · controller/internal/agentapi/backup_tiers.go | `BackupJobsInFlight(ctx) ([]string, error)` | Check first, stop second: no app is stopped for a tier the agent will refuse because ANOTHER tier's job is in flight | OPTIONAL interface (fakes without it keep the old path). Fails toward backing up on any read error. **It cannot see the agent's host-wide heavy-op gate** (OS step, restore-test, fstrim — no endpoint serves it): that refusal is met by the BUSY path's immediate resume (`TestR921_BusyRefusalResumesAtOnce`). The window's OWN tier in flight is not skipped (the agent answers with the running job) | | `quiesce` whole-guest ledger — `Loop.pressOwesNight` / `recordWholeGuestSuccess`, `WithManualTrigger` / `IsManualTrigger` (R-899) | controller/internal/quiesce/nightowed.go | `pressOwesNight(led, target) bool`; `WithManualTrigger(ctx) ctx` | **A household press never cancels the night's whole-guest backup** (operator ruling 2026-10-08) | The ledger (`whole-guest-ledger.json` beside the marker) records SUCCESSES only, per tier, press vs scheduled — it decides due-ness and is never evidence that a copy exists (the agent's storage answers that). An „owed" tier never fires the window gate's safety valve (`withoutOwed`/`gateAge`). The press mark rides the context to the agent adapter (`agentapi.StartBackupForTrigger` → `trigger=manual`), so the agent runs no OS leg after a press. | | `quiesce.failureBreaker` + `Loop.dropBackedOffTiers` / `noteTierFailure` / `noteTierSuccess` | controller/internal/quiesce/breaker.go, quiesce.go | `blocked/recordFailure/recordSuccess(target, now)`; `backoffFor(n) time.Duration` | **R-88** — a tier whose backups keep failing stops re-quiescing. Backoff 15m→30m→1h→2h→4h (cap), reset on success | **It gates the QUIESCE, not the backup** — the harm was never the failing backup, it was the app outage taken to attempt it, so backed-off tiers are dropped from the due set BEFORE any stack is stopped. **Per TARGET** — a broken offsite tier must never suppress a healthy local one (`TestBreaker_OneFailingTierDoesNotSuppressAHealthyOne`). **Never permanent** — the cap bounds the retry INTERVAL, it never stops retrying; a latched breaker is a silent backup outage, worse than the loop it replaces. **`TriggerNow` is never gated** (it already bypasses due-ness and the window gate), though a manual run still RECORDS its outcome. **`stillRunning` is NOT a failure** — a first full offsite snapshot legitimately runs for hours. State is **in-memory on purpose**: a restart forgets the backoff and re-attempts, which is the cheap direction to fail. Log the deferral ONCE when armed, never per tick. | | `quiesce.TierNotifier` + `Loop.SetTierNotifier` / `noteTierFailure` / `noteTierSuccess` | controller/internal/quiesce/breaker.go, quiesce.go | `BackupFailed(tier,msg,err)` / `BackupRecovered(tier,msg)`; `SetTierNotifier(n)` INIT-ONLY | **R-97a** — the whole-guest backup tier reports its outcome to the hub | A **seam, not an import** — quiesce keeps no dependency on `internal/notify` (same reason `windowStartFn` is injected). Wired by a setter because main.go builds the notifier AFTER the loop; `nil` = unprovisioned guest, not an error. **Edge-triggered:** failure fires only when the breaker ARMS (`n == 1`), never per retry — the cadence is 15m/30m/1h/2h/4h and an event per attempt is an inbox nobody reads. Recovery rides `recordSuccess`'s existing bool. **Event types are OPERATOR-ONLY** (`whole_guest_backup_failed`/`_recovered`, hub >= v0.78.0) — NOT `backup_failed`, which has a customerMessages entry AND sits in live `enabled_events`, so it would email the CUSTOMER about a backup they cannot act on. `WholeGuestBackupDetails.Tier` is load-bearing: the hub keys its per-tier cooldown on it. | diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index f506fac..30c5755 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -1793,8 +1793,9 @@ func main() { if notifier.IsEnabled() { go func() { prefs := sett.GetNotificationPrefs() - if prefs.Email != "" { - if err := notifier.SyncPreferences(prefs.Email, prefs.EnabledEvents, prefs.CooldownHours); err != nil { + // R-922: a household's clear is pushed too, so a clear the hub missed reaches it at the next start. + if prefs.SyncOnStartup() { + if err := notifier.SyncPreferences(prefs.Email, prefs.EnabledEvents, prefs.CooldownHours, prefs.EmailCleared); err != nil { logger.Printf("[WARN] Failed to sync notification preferences on startup: %v", err) } } @@ -3556,6 +3557,15 @@ func (b quiesceBackend) BackupStatusFor(ctx context.Context, target string) (str return r.Phase, err } +// BackupJobsInFlight satisfies quiesce.InFlightProber (R-921): the loop asks it before it stops any app. +// The compile-time assertion below is the wiring pin — a pre-check whose adapter stops implementing the +// interface would silently fall back to "stop, then ask" (a seam built but never wired). +var _ quiesce.InFlightProber = quiesceBackend{} + +func (b quiesceBackend) BackupJobsInFlight(ctx context.Context) ([]string, error) { + return b.c.BackupJobsInFlight(ctx) +} + // quiesceTierNotifier adapts *notify.Notifier to quiesce.TierNotifier (R-97a). // // The whole-guest tier had NO route to the hub at all — `internal/quiesce` did not import diff --git a/controller/internal/agentapi/backup_tiers.go b/controller/internal/agentapi/backup_tiers.go index 31bd080..8ef90e6 100644 --- a/controller/internal/agentapi/backup_tiers.go +++ b/controller/internal/agentapi/backup_tiers.go @@ -127,6 +127,33 @@ func (c *Client) BackupStatusFor(ctx context.Context, target string) (StatusResp return out, nil } +// BackupJobsInFlight (R-921) lists the tiers whose backup job the agent holds in flight right now — phase +// running or snapshotted, the agent's own definition (localapi backupInFlight). A job in flight on another +// tier is the agent's first reason to refuse POST /backup (HTTP 409), so the controller asks this BEFORE it +// stops any app. It reads only GET /backup/tiers and GET /backup/status?target= (both agent >= v0.97.0); a +// pre-R-82 agent (no /backup/tiers) answers nil, nil. Any other error is returned — the caller fails toward +// backing up. It does NOT see the agent's host-wide heavy-operation gate: no endpoint serves it. +func (c *Client) BackupJobsInFlight(ctx context.Context) ([]string, error) { + tiers, err := c.BackupTiers(ctx) + if errors.Is(err, ErrTiersUnsupported) { + return nil, nil + } + if err != nil { + return nil, err + } + var out []string + for _, t := range tiers.Tiers { + st, err := c.BackupStatusFor(ctx, t.Target) + if err != nil { + return nil, err + } + if st.Phase == PhaseRunning || st.Phase == PhaseSnapshotted { + out = append(out, t.Target) + } + } + return out, nil +} + // SetBackupTargetResponse mirrors POST /backup/target (agent >= v0.113.0). type SetBackupTargetResponse struct { Target string `json:"target"` diff --git a/controller/internal/agentapi/client.go b/controller/internal/agentapi/client.go index 5983e50..4821c98 100644 --- a/controller/internal/agentapi/client.go +++ b/controller/internal/agentapi/client.go @@ -255,6 +255,9 @@ const ( PhaseRunning = "running" PhaseDone = "done" PhaseFailed = "failed" + // PhaseSnapshotted (8B.2): the storage snapshot is taken, the upload still runs and still holds the + // guest — the agent counts it as IN FLIGHT (felhom-agent localapi backupInFlight). + PhaseSnapshotted = "snapshotted" ) // BackupDue reports whether a policy-scheduled backup is due for this guest (the quiesce trigger). diff --git a/controller/internal/agentapi/r921_inflight_test.go b/controller/internal/agentapi/r921_inflight_test.go new file mode 100644 index 0000000..88d07c0 --- /dev/null +++ b/controller/internal/agentapi/r921_inflight_test.go @@ -0,0 +1,75 @@ +package agentapi + +import ( + "context" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" +) + +// R-921: the pre-check reads the agent's real wire shapes — GET /backup/tiers, then GET /backup/status per +// target — and reports exactly the tiers whose job is running or snapshotted (the agent's in-flight set). +func TestBackupJobsInFlight_ReportsRunningAndSnapshottedTiers(t *testing.T) { + phases := map[string]string{"local": "done", "felhom-pbs": "snapshotted", "extra": "running"} + mux := http.NewServeMux() + mux.HandleFunc("GET /backup/tiers", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"ok":true,"data":{"vmid":9201,"tiers":[` + + `{"target":"local","cadence_seconds":86400,"primary":true,"storage":"present"},` + + `{"target":"felhom-pbs","cadence_seconds":604800,"primary":false,"storage":"present"},` + + `{"target":"extra","cadence_seconds":604800,"primary":false,"storage":"present"}]}}`)) + }) + mux.HandleFunc("GET /backup/status", func(w http.ResponseWriter, r *http.Request) { + tg := r.URL.Query().Get("target") + ph, ok := phases[tg] + if !ok { + http.Error(w, "unexpected target "+tg, http.StatusBadRequest) + return + } + _, _ = w.Write([]byte(`{"ok":true,"data":{"vmid":9201,"phase":"` + ph + `","target":"` + tg + `"}}`)) + }) + s := httptest.NewTLSServer(mux) + defer s.Close() + c := clientFor(t, s, strings.TrimPrefix(s.URL, "https://")) + + got, err := c.BackupJobsInFlight(context.Background()) + if err != nil { + t.Fatalf("BackupJobsInFlight: %v", err) + } + if want := []string{"felhom-pbs", "extra"}; !reflect.DeepEqual(got, want) { + t.Fatalf("in-flight tiers = %v, want %v (done must not count; running and snapshotted must)", got, want) + } +} + +// A pre-R-82 agent (no /backup/tiers → 404) has no per-tier jobs: nil, nil — never an error that would be +// read as "cannot tell", and never a busy verdict. +func TestBackupJobsInFlight_PreR82AgentIsNotBusy(t *testing.T) { + mux := http.NewServeMux() + s := httptest.NewTLSServer(mux) // every route 404s + defer s.Close() + c := clientFor(t, s, strings.TrimPrefix(s.URL, "https://")) + + got, err := c.BackupJobsInFlight(context.Background()) + if err != nil || got != nil { + t.Fatalf("pre-R-82 agent: got %v, %v — want nil, nil", got, err) + } +} + +// A status read that fails is returned, so the loop can fail toward backing up and say so. +func TestBackupJobsInFlight_StatusErrorIsReturned(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /backup/tiers", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"ok":true,"data":{"vmid":9201,"tiers":[{"target":"local","primary":true}]}}`)) + }) + mux.HandleFunc("GET /backup/status", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "boom", http.StatusInternalServerError) + }) + s := httptest.NewTLSServer(mux) + defer s.Close() + c := clientFor(t, s, strings.TrimPrefix(s.URL, "https://")) + + if _, err := c.BackupJobsInFlight(context.Background()); err == nil { + t.Fatal("a failed status read was swallowed — the caller could not tell it from 'nothing in flight'") + } +} diff --git a/controller/internal/notify/notifier.go b/controller/internal/notify/notifier.go index 8085ae4..e917598 100644 --- a/controller/internal/notify/notifier.go +++ b/controller/internal/notify/notifier.go @@ -1029,11 +1029,16 @@ type preferencesRequest struct { Email string `json:"email"` EnabledEvents []string `json:"enabled_events"` CooldownHours int `json:"cooldown_hours,omitempty"` + // EmailCleared (R-922): the household deliberately cleared its address — the hub deletes the one it + // holds. OMITTED unless true: an empty address without it keeps the hub's no-clobber guard (an + // unconfigured box must never wipe a seeded address). Decoded by the hub's savePreferences. + EmailCleared bool `json:"email_cleared,omitempty"` } // SyncPreferences pushes the current notification preferences to the hub. // Synchronous — returns error for the handler to display to the user. -func (n *Notifier) SyncPreferences(email string, enabledEvents []string, cooldownHours int) error { +// emailCleared (R-922) is settings.NotificationPrefs.EmailCleared — true only after a household's clear. +func (n *Notifier) SyncPreferences(email string, enabledEvents []string, cooldownHours int, emailCleared bool) error { if !n.enabled { return util.MsgError("err.notify.hub_nem_konfiguralt") } @@ -1043,6 +1048,7 @@ func (n *Notifier) SyncPreferences(email string, enabledEvents []string, cooldow Email: email, EnabledEvents: enabledEvents, CooldownHours: cooldownHours, + EmailCleared: emailCleared, } jsonData, err := json.Marshal(payload) @@ -1076,7 +1082,7 @@ func (n *Notifier) SyncPreferences(email string, enabledEvents []string, cooldow if n.debug { n.logger.Printf("[DEBUG] SyncPreferences: response HTTP %d", resp.StatusCode) } - n.logger.Printf("[INFO] Notification preferences synced to hub: email=%s, events=%v, cooldown=%dh", email, enabledEvents, cooldownHours) + n.logger.Printf("[INFO] Notification preferences synced to hub: email=%s, events=%v, cooldown=%dh, email_cleared=%t", email, enabledEvents, cooldownHours, emailCleared) return nil } diff --git a/controller/internal/quiesce/precheck.go b/controller/internal/quiesce/precheck.go new file mode 100644 index 0000000..745a9ce --- /dev/null +++ b/controller/internal/quiesce/precheck.go @@ -0,0 +1,58 @@ +package quiesce + +import "context" + +// R-921 — CHECK FIRST, STOP SECOND. +// +// MEASURED 2026-10-08 night on demo-hp (felhom.eu documentation/audits/dooplex-survival-2026-10-09/partE/ +// R-518.txt): the controller stopped every app for the off-site tier, the agent refused the backup +// („a heavy operation is already in flight", busy=backup:local — the night OS step that follows the local +// copy), and the apps were down about a minute for no copy. +// +// What the agent lets the controller see BEFORE a stop (felhom-agent v0.154.0, read 2026-10-09): the agent +// refuses POST /backup for two reasons. (1) A job of ANOTHER tier of this guest is in flight +// (localapi otherTierInFlight) — that IS readable, per tier, from GET /backup/status?target=…. (2) Its +// host-wide heavy-operation gate is held (backup.InFlight: the OS step after the night's local copy, a +// restore-test, fstrim) — that is served by NO endpoint (InFlight.Busy() exists and nothing exposes it). +// So the pre-check below covers reason (1) only. Reason (2) — the measured instance — is met by the +// BUSY path in quiesceAndPollTiers, which resumes the apps at once (pinned by +// TestR921_BusyRefusalResumesAtOnce); closing it before the stop needs the agent to serve its gate. +// +// The race (free at the check, busy at the start) needs nothing new: it is the BUSY path again. + +// InFlightProber is the OPTIONAL pre-check surface (R-921). An adapter that does not implement it keeps +// the pre-R-921 behaviour exactly: stop, ask, and on refusal resume at once. +type InFlightProber interface { + // BackupJobsInFlight returns the tiers whose backup job the agent holds in flight right now + // (phase running or snapshotted). An agent without per-tier jobs (pre-R-82) answers nil, nil. + BackupJobsInFlight(ctx context.Context) ([]string, error) +} + +// anotherTierInFlight reports whether the agent would refuse the window's first tier because a job of a +// DIFFERENT tier is in flight — so the window must not stop any app. The window's OWN tier in flight is +// not a refusal (the agent answers its start with the running job, 202), and that path is left as it was. +// +// Fail toward backing up: an unanswerable pre-check, an untargeted window (pre-R-82 agent) or an adapter +// without the surface all return false — the window runs as before. +func (l *Loop) anotherTierInFlight(ctx context.Context, first string) bool { + if first == "" { + return false + } + p, ok := l.backend.(InFlightProber) + if !ok { + return false + } + busy, err := p.BackupJobsInFlight(ctx) + if err != nil { + l.logger.Printf("[WARN] [quiesce] pre-check: could not ask the agent which backup jobs are in flight (%v) — stopping the apps and asking as before (R-921)", err) + return false + } + for _, t := range busy { + if t != first { + l.logger.Printf("[INFO] [quiesce] tier %s is due, but the agent still holds a backup job on tier %s — no app is stopped; the tier stays due and is asked again at the next poll (R-921)", + tierLabel(first), tierLabel(t)) + return true + } + } + return false +} diff --git a/controller/internal/quiesce/quiesce.go b/controller/internal/quiesce/quiesce.go index 0bb826a..b59654e 100644 --- a/controller/internal/quiesce/quiesce.go +++ b/controller/internal/quiesce/quiesce.go @@ -556,6 +556,11 @@ func (l *Loop) quiesceAndPollTiers(ctx context.Context, tiers []dueTier) error { if len(tiers) == 0 { return nil } + // R-921: check first, stop second — a tier the agent will refuse (another tier's job in flight) stops + // no app. Pinned by TestR921_NoStopWhileAnotherTiersJobIsInFlight (precheck.go says what it cannot see). + if l.anotherTierInFlight(ctx, tiers[0].target) { + return nil + } running := l.stacks.RunningAppStacks() marker := Marker{Active: true, StartedAt: l.now(), StoppedStacks: running} if err := l.writeMarker(marker); err != nil { diff --git a/controller/internal/quiesce/r921_precheck_test.go b/controller/internal/quiesce/r921_precheck_test.go new file mode 100644 index 0000000..a3f7aab --- /dev/null +++ b/controller/internal/quiesce/r921_precheck_test.go @@ -0,0 +1,286 @@ +package quiesce + +import ( + "context" + "errors" + "io" + "log" + "path/filepath" + "strings" + "sync" + "testing" + "time" +) + +// R-921 (measured 2026-10-08 night on demo-hp): the controller stopped every app for the off-site tier, +// the agent refused the backup (`a heavy operation is already in flight`), and the apps were down about a +// minute for nothing. The rule this file pins: CHECK FIRST, STOP SECOND — and when the check could not +// see the refusal coming, RESUME AT ONCE. +// +// The agent's local API serves no read of its host-wide heavy-operation gate (felhom-agent +// internal/backup.InFlight.Busy() is served by no endpoint). What it DOES serve is each tier's job phase +// (GET /backup/status?target=…), and a job in flight on ANOTHER tier is the agent's first refusal reason +// (localapi handleBackup → otherTierInFlight → HTTP 409). So the pre-check covers that reason; the other +// (the gate held by the night OS step, a restore-test, fstrim) can only be met by the immediate resume. + +// r921Backend is a tiered fake agent that records every call into a shared, ordered event log with the +// fake stacks, so a test can say "nothing happened between the refusal and the first restart". +type r921Backend struct { + mu sync.Mutex + events *[]string + tiers []BackupTier + due map[string]bool + inFlight []string // targets whose job the agent holds in flight (running|snapshotted) + probeErr error + busyOn map[string]bool // StartBackupFor answers ErrTierBusy for these targets + starts []string +} + +func (b *r921Backend) log(e string) { + b.mu.Lock() + defer b.mu.Unlock() + *b.events = append(*b.events, e) +} + +func (b *r921Backend) Tiers(context.Context) ([]BackupTier, error) { return b.tiers, nil } +func (b *r921Backend) DueFor(_ context.Context, target string) (bool, *int64, string, error) { + return b.due[target], nil, "", nil +} +func (b *r921Backend) StartBackupFor(_ context.Context, target string) (string, error) { + b.mu.Lock() + b.starts = append(b.starts, target) + b.mu.Unlock() + if b.busyOn[target] { + b.log("start:" + target + "=BUSY") + return "", busyErr() + } + b.log("start:" + target) + return "job-" + target, nil +} +func (b *r921Backend) BackupStatusFor(_ context.Context, target string) (string, error) { + b.log("status:" + target) + return phaseDone, nil +} +func (b *r921Backend) BackupJobsInFlight(context.Context) ([]string, error) { + b.log("probe") + if b.probeErr != nil { + return nil, b.probeErr + } + return append([]string(nil), b.inFlight...), nil +} +func (b *r921Backend) Due(context.Context) (bool, *int64, error) { return false, nil, nil } +func (b *r921Backend) StartBackup(context.Context) (string, error) { return "", errors.New("unused") } +func (b *r921Backend) BackupStatus(context.Context) (string, error) { + return phaseDone, nil +} + +// r921Stacks logs stops and starts into the same event log. +type r921Stacks struct { + mu sync.Mutex + events *[]string + running []string +} + +func (s *r921Stacks) RunningAppStacks() []string { return append([]string(nil), s.running...) } +func (s *r921Stacks) StopStack(n string) error { + s.mu.Lock() + defer s.mu.Unlock() + *s.events = append(*s.events, "stop:"+n) + return nil +} +func (s *r921Stacks) StartStack(n string) error { + s.mu.Lock() + defer s.mu.Unlock() + *s.events = append(*s.events, "startstack:"+n) + return nil +} + +func r921Setup(t *testing.T) (*r921Backend, *r921Stacks, *Loop, *[]string, *strings.Builder) { + t.Helper() + var events []string + be := &r921Backend{ + events: &events, + tiers: []BackupTier{{Target: "local", Primary: true}, {Target: "felhom-pbs"}}, + due: map[string]bool{}, + busyOn: map[string]bool{}, + } + st := &r921Stacks{events: &events, running: []string{"immich", "paperless-ngx", "vaultwarden"}} + var logs strings.Builder + l := New(Options{ + Backend: be, + Stacks: st, + MarkerPath: filepath.Join(t.TempDir(), "quiesce-state.json"), + StatusPoll: time.Millisecond, + MaxQuiesce: 30 * time.Second, + Logger: log.New(&logs, "", 0), + }) + return be, st, l, &events, &logs +} + +func countPrefix(events []string, prefix string) int { + n := 0 + for _, e := range events { + if strings.HasPrefix(e, prefix) { + n++ + } + } + return n +} + +// RED TEST. Another tier's job is in flight on the agent (a first off-site upload that outlived the +// quiesce bound, or a controller restarted mid-upload) and the local tier is due. The agent WILL refuse +// the local backup (otherTierInFlight → 409). Today's code stops every app, asks, is refused, restarts +// them: a stop for nothing. Asserted on the CONSEQUENCE — no app was stopped and no backup was requested +// — not on the mechanism. It must also leave the tier due (no breaker, no contention verdict): the next +// poll asks again, which costs the household nothing. +func TestR921_NoStopWhileAnotherTiersJobIsInFlight(t *testing.T) { + be, _, l, events, logs := r921Setup(t) + be.due["local"] = true + be.inFlight = []string{"felhom-pbs"} + be.busyOn["local"] = true // what the real agent answers while another tier's job is in flight + + if err := l.runOnce(context.Background()); err != nil { + t.Fatalf("runOnce: %v", err) + } + if n := countPrefix(*events, "stop:"); n != 0 { + t.Fatalf("R-921: %d app(s) stopped for a tier the agent refuses (another tier's job is in flight); events=%v", n, *events) + } + if len(be.starts) != 0 { + t.Fatalf("R-921: a backup was requested although another tier's job is in flight: %v", be.starts) + } + if got := l.breaker.failuresFor("local"); got != 0 { + t.Errorf("the pre-check armed the failure breaker (%d) — nothing failed", got) + } + if _, blocked := l.contention.blocked("local", l.now()); blocked { + t.Errorf("the pre-check set a contention verdict — the tier must simply stay due and be asked again next poll") + } + if !strings.Contains(logs.String(), "felhom-pbs") || !strings.Contains(logs.String(), "R-921") { + t.Errorf("the skip is not explained in the log (want the in-flight tier and R-921): %q", logs.String()) + } + + // The job ends → the next poll backs the local tier up as normal (the skip did not swallow the tier). + be.inFlight = nil + be.busyOn["local"] = false + if err := l.runOnce(context.Background()); err != nil { + t.Fatalf("runOnce 2: %v", err) + } + if len(be.starts) != 1 || be.starts[0] != "local" { + t.Fatalf("after the job ended the local tier was not backed up: starts=%v", be.starts) + } +} + +// The measured instance and the race, both on the BUSY path: the pre-check cannot see the refusal +// coming (the night OS step holds the gate — not readable from the agent — or the agent became busy +// between the check and the start). Then the resume must follow the refusal AT ONCE: the very next +// event after the refused start is the first app's restart — no other agent call, no further tier, no +// wait in between. Measured on the ordered event log, not on a clock. +func TestR921_BusyRefusalResumesAtOnce(t *testing.T) { + for _, tc := range []struct { + name string + due []string + busy []string + probe bool // the pre-check answered "free" (the race) vs. a backend without the pre-check + }{ + {"measured: only the off-site tier due, refused (no pre-check)", []string{"felhom-pbs"}, []string{"felhom-pbs"}, false}, + {"race: pre-check said free, the start was refused", []string{"felhom-pbs"}, []string{"felhom-pbs"}, true}, + } { + t.Run(tc.name, func(t *testing.T) { + be, st, l, events, _ := r921Setup(t) + for _, d := range tc.due { + be.due[d] = true + } + for _, b := range tc.busy { + be.busyOn[b] = true + } + var loop *Loop = l + if !tc.probe { + // A backend WITHOUT the pre-check surface (an adapter that does not implement it). + nb := &noProbeBackend{be} + loop = New(Options{Backend: nb, Stacks: st, MarkerPath: filepath.Join(t.TempDir(), "m.json"), + StatusPoll: time.Millisecond, MaxQuiesce: 30 * time.Second, Logger: log.New(io.Discard, "", 0)}) + } + if err := loop.runOnce(context.Background()); err != nil { + t.Fatalf("runOnce: %v", err) + } + ev := *events + idx := -1 + for i, e := range ev { + if strings.HasSuffix(e, "=BUSY") { + idx = i + } + } + if idx < 0 { + t.Fatalf("no refused start in the events: %v", ev) + } + if idx+1 >= len(ev) || !strings.HasPrefix(ev[idx+1], "startstack:") { + t.Fatalf("the resume did not follow the refusal at once; events after the refusal: %v", ev[idx+1:]) + } + if got, want := countPrefix(ev[idx+1:], "startstack:"), len(st.running); got != want { + t.Fatalf("restarted %d app(s) after the refusal, want all %d; events=%v", got, want, ev) + } + for _, e := range ev[idx+1:] { + if !strings.HasPrefix(e, "startstack:") { + t.Fatalf("an agent call (%q) came after the refusal — the BUSY path must only resume; events=%v", e, ev) + } + } + if _, blocked := loop.contention.blocked("felhom-pbs", loop.now()); !blocked { + t.Errorf("the refused tier was not deferred — the next poll would stop the apps again") + } + }) + } +} + +// A pre-check that cannot be answered must not suppress a backup: fail toward backing up, exactly as +// before R-921 (stop, ask, and on refusal resume at once). +func TestR921_PrecheckErrorFailsTowardBackingUp(t *testing.T) { + be, _, l, events, logs := r921Setup(t) + be.due["local"] = true + be.probeErr = errors.New("agentapi: GET /backup/tiers: connection refused") + + if err := l.runOnce(context.Background()); err != nil { + t.Fatalf("runOnce: %v", err) + } + if len(be.starts) != 1 || be.starts[0] != "local" { + t.Fatalf("an unanswerable pre-check suppressed the backup: starts=%v events=%v", be.starts, *events) + } + if !strings.Contains(logs.String(), "[WARN]") { + t.Errorf("the failed pre-check was not logged at WARN: %q", logs.String()) + } +} + +// The window's OWN tier already in flight is NOT skipped: the agent answers that start with the running +// job (202, idempotent), the loop attaches to it and records its outcome — unchanged behaviour, chosen +// deliberately (only a tier the agent REFUSES is skipped before the stop). +func TestR921_SameTierInFlightIsNotSkipped(t *testing.T) { + be, _, l, _, _ := r921Setup(t) + be.due["felhom-pbs"] = true + be.inFlight = []string{"felhom-pbs"} + + if err := l.runOnce(context.Background()); err != nil { + t.Fatalf("runOnce: %v", err) + } + if len(be.starts) != 1 || be.starts[0] != "felhom-pbs" { + t.Fatalf("the window's own in-flight tier was skipped; starts=%v", be.starts) + } +} + +// noProbeBackend hides BackupJobsInFlight (an adapter without the pre-check surface). +type noProbeBackend struct{ b *r921Backend } + +func (n *noProbeBackend) Tiers(ctx context.Context) ([]BackupTier, error) { return n.b.Tiers(ctx) } +func (n *noProbeBackend) DueFor(ctx context.Context, t string) (bool, *int64, string, error) { + return n.b.DueFor(ctx, t) +} +func (n *noProbeBackend) StartBackupFor(ctx context.Context, t string) (string, error) { + return n.b.StartBackupFor(ctx, t) +} +func (n *noProbeBackend) BackupStatusFor(ctx context.Context, t string) (string, error) { + return n.b.BackupStatusFor(ctx, t) +} +func (n *noProbeBackend) Due(ctx context.Context) (bool, *int64, error) { return n.b.Due(ctx) } +func (n *noProbeBackend) StartBackup(ctx context.Context) (string, error) { + return n.b.StartBackup(ctx) +} +func (n *noProbeBackend) BackupStatus(ctx context.Context) (string, error) { + return n.b.BackupStatus(ctx) +} diff --git a/controller/internal/settings/r922_cleared_test.go b/controller/internal/settings/r922_cleared_test.go new file mode 100644 index 0000000..bc29c61 --- /dev/null +++ b/controller/internal/settings/r922_cleared_test.go @@ -0,0 +1,57 @@ +package settings + +import ( + "io" + "log" + "path/filepath" + "testing" +) + +// R-922: the clear marker's rule as a table — set only by a deliberate clear, kept while the address stays +// empty, dropped by a new address — and it survives a reload from disk (a clear the hub missed is pushed +// again by the next start: SyncOnStartup). +func TestR922_ClearedByRule(t *testing.T) { + for _, tc := range []struct { + name string + stored *NotificationPrefs + newEmail string + want bool + }{ + {"never configured, empty save", &NotificationPrefs{}, "", false}, + {"nil stored prefs", nil, "", false}, + {"had an address, empty save = clear", &NotificationPrefs{Email: "a@example.hu"}, "", true}, + {"already cleared, empty save keeps it", &NotificationPrefs{EmailCleared: true}, "", true}, + {"cleared, new address drops it", &NotificationPrefs{EmailCleared: true}, "b@example.hu", false}, + {"address changed", &NotificationPrefs{Email: "a@example.hu"}, "b@example.hu", false}, + } { + if got := tc.stored.ClearedBy(tc.newEmail); got != tc.want { + t.Errorf("%s: ClearedBy(%q) = %v, want %v", tc.name, tc.newEmail, got, tc.want) + } + } +} + +func TestR922_MarkerPersistsAndDrivesStartupSync(t *testing.T) { + path := filepath.Join(t.TempDir(), "settings.json") + lg := log.New(io.Discard, "", 0) + s, err := Load(path, lg) + if err != nil { + t.Fatal(err) + } + if s.GetNotificationPrefs().SyncOnStartup() { + t.Fatal("a never-configured box would push on startup") + } + if err := s.SetNotificationPrefs(&NotificationPrefs{EmailCleared: true, CooldownHours: 6}); err != nil { + t.Fatal(err) + } + s2, err := Load(path, lg) + if err != nil { + t.Fatal(err) + } + p := s2.GetNotificationPrefs() + if !p.EmailCleared { + t.Fatal("the clear marker did not survive a reload") + } + if !p.SyncOnStartup() { + t.Fatal("a cleared box does not push its clear on startup — a clear the hub missed would never reach it") + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index 5a0df2d..e285907 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -622,6 +622,28 @@ type NotificationPrefs struct { Email string `json:"email,omitempty"` EnabledEvents []string `json:"enabled_events,omitempty"` CooldownHours int `json:"cooldown_hours,omitempty"` // default: 6 + // EmailCleared (R-922, operator ruling 2026-10-09 option A) marks a DELIBERATE clear: the household + // saved an empty address where one was stored. Every hub push carries it as `email_cleared: true` + // while the address stays empty, so the hub deletes the address it holds — and a push the hub missed + // is repaired by the next one. A box that never had an address never sets it (the hub's no-clobber + // guard must keep protecting a seeded address); a new address drops it. Set only by + // web.settingsNotificationsHandler via ClearedBy. Pinned by internal/web TestR922_*. + EmailCleared bool `json:"email_cleared,omitempty"` +} + +// ClearedBy reports whether saving `newEmail` over these stored prefs is (or keeps) a deliberate clear: +// the new address is empty AND the stored one was not — or the stored prefs already record a clear. +func (p *NotificationPrefs) ClearedBy(newEmail string) bool { + if newEmail != "" || p == nil { + return false + } + return p.Email != "" || p.EmailCleared +} + +// SyncOnStartup reports whether the startup push should run: an address to restore after a hub DB +// rebuild, or a household clear the hub may not have received yet (R-922). +func (p *NotificationPrefs) SyncOnStartup() bool { + return p != nil && (p.Email != "" || p.EmailCleared) } // DefaultEnabledEvents are the events enabled by default for new customers. diff --git a/controller/internal/web/handler_debug.go b/controller/internal/web/handler_debug.go index 1f7fba8..96f5d93 100644 --- a/controller/internal/web/handler_debug.go +++ b/controller/internal/web/handler_debug.go @@ -616,7 +616,7 @@ func (s *Server) debugPreferencesSync(w http.ResponseWriter, r *http.Request) { return } prefs := s.settings.GetNotificationPrefs() - if err := s.notifier.SyncPreferences(prefs.Email, prefs.EnabledEvents, prefs.CooldownHours); err != nil { + if err := s.notifier.SyncPreferences(prefs.Email, prefs.EnabledEvents, prefs.CooldownHours, prefs.EmailCleared); err != nil { writeDebugJSON(w, http.StatusOK, false, s.errText(r, err), nil) return } diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index ccfa37e..98e2ed7 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -2957,10 +2957,15 @@ func (s *Server) settingsNotificationsHandler(w http.ResponseWriter, r *http.Req return } + // R-922 (operator ruling 2026-10-09, option A): an empty address where one was stored is the household's + // deliberate clear — persisted, so this push AND every later one carry `email_cleared` while the address + // stays empty, and the hub deletes the address it holds. A box that never had one does not set it. + emailCleared := s.settings.GetNotificationPrefs().ClearedBy(email) prefs := &settings.NotificationPrefs{ Email: email, EnabledEvents: enabledEvents, CooldownHours: cooldownHours, + EmailCleared: emailCleared, } if err := s.settings.SetNotificationPrefs(prefs); err != nil { @@ -2971,13 +2976,13 @@ func (s *Server) settingsNotificationsHandler(w http.ResponseWriter, r *http.Req return } - s.logger.Printf("[INFO] [web] Notification preferences updated: email=%s, events=%v", email, enabledEvents) + s.logger.Printf("[INFO] [web] Notification preferences updated: email=%s, events=%v, email_cleared=%t", email, enabledEvents, emailCleared) s.reportTriggerNow() // v0.139.0: hub reflects the saved prefs in seconds, not next cycle // Sync preferences to hub data := s.notificationsPageData() if s.notifier != nil && s.notifier.IsEnabled() { - if err := s.notifier.SyncPreferences(email, enabledEvents, cooldownHours); err != nil { + if err := s.notifier.SyncPreferences(email, enabledEvents, cooldownHours, emailCleared); err != nil { s.logger.Printf("[WARN] [web] Failed to sync preferences to hub: %v", err) data["NotificationSuccess"] = s.msg(r, "settings.notify_saved_sync_failed", err) } else { diff --git a/controller/internal/web/r922_email_cleared_test.go b/controller/internal/web/r922_email_cleared_test.go new file mode 100644 index 0000000..d04530e --- /dev/null +++ b/controller/internal/web/r922_email_cleared_test.go @@ -0,0 +1,187 @@ +package web + +// R-922 (operator ruling 2026-10-09 11:19, option A): when the household clears its notification address on +// the dashboard, the hub deletes the stored address. The hub cannot tell that apart from an unconfigured box +// pushing an empty address (its no-clobber guard, audit F12) — so the controller SAYS so: the push carries +// `"email_cleared": true`, and ONLY after a deliberate clear. Asserted on the WIRE (a real Notifier against an +// httptest hub), not on a mock's call count. + +import ( + "encoding/json" + "io" + "log" + "net/http" + "net/http/httptest" + "net/url" + "sync" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/notify" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +type prefsHub struct { + mu sync.Mutex + bodies []map[string]json.RawMessage +} + +func (h *prefsHub) last(t *testing.T) map[string]json.RawMessage { + t.Helper() + h.mu.Lock() + defer h.mu.Unlock() + if len(h.bodies) == 0 { + t.Fatal("no preferences push reached the hub") + } + return h.bodies[len(h.bodies)-1] +} + +func (h *prefsHub) count() int { + h.mu.Lock() + defer h.mu.Unlock() + return len(h.bodies) +} + +// r922Server is notifyGuardServer with a REAL notifier aimed at a capturing hub. +func r922Server(t *testing.T) (*Server, *settings.Settings, *prefsHub) { + t.Helper() + s, sett := notifyGuardServer(t) + hub := &prefsHub{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/preferences" { + w.WriteHeader(http.StatusOK) + return + } + raw, _ := io.ReadAll(r.Body) + var m map[string]json.RawMessage + if err := json.Unmarshal(raw, &m); err != nil { + t.Errorf("preferences push is not a JSON object: %v", err) + } + hub.mu.Lock() + hub.bodies = append(hub.bodies, m) + hub.mu.Unlock() + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + s.notifier = notify.New(srv.URL, "test-key", "c1", sett, log.New(io.Discard, "", 0), false) + if !s.notifier.IsEnabled() { + t.Fatal("test notifier not enabled") + } + return s, sett, hub +} + +func clearForm() url.Values { + return url.Values{"notification_email": {""}, "cooldown_hours": {"6"}} // no event_* boxes +} + +func flagOf(t *testing.T, body map[string]json.RawMessage) (present bool, value bool) { + t.Helper() + raw, ok := body["email_cleared"] + if !ok { + return false, false + } + var v bool + if err := json.Unmarshal(raw, &v); err != nil { + t.Fatalf("email_cleared is not a boolean: %s", raw) + } + return true, v +} + +func emailOf(t *testing.T, body map[string]json.RawMessage) string { + t.Helper() + var e string + _ = json.Unmarshal(body["email"], &e) + return e +} + +// RED TEST. A household that had an address and clears it: the push carries email_cleared:true with an empty +// address. Today's push carries no flag, so the hub keeps the old address (seen on Tester 1, 2026-10-09). +// The flag also rides the NEXT push (the debug resync reads the stored prefs) and a second empty save — +// it stays while the address stays empty, so a push the hub missed is repaired by the next one. +func TestR922_DeliberateClearSendsEmailCleared(t *testing.T) { + s, sett, hub := r922Server(t) + if err := sett.SetNotificationPrefs(&settings.NotificationPrefs{ + Email: "household@example.hu", EnabledEvents: []string{"backup_failed"}, CooldownHours: 6, + }); err != nil { + t.Fatal(err) + } + + postNotifications(t, s, clearForm()) + body := hub.last(t) + if present, v := flagOf(t, body); !present || !v { + t.Fatalf("R-922: the push after a deliberate clear carries no email_cleared:true (present=%v value=%v) — the hub keeps the old address; body keys=%v", present, v, keysOf(body)) + } + if e := emailOf(t, body); e != "" { + t.Fatalf("email = %q, want empty beside the clear flag", e) + } + + // The next push from the STORED prefs (the debug resync) carries it too. + before := hub.count() + s.debugPreferencesSync(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/api/debug/preferences/sync", nil)) + if hub.count() != before+1 { + t.Fatalf("the resync did not push") + } + if present, v := flagOf(t, hub.last(t)); !present || !v { + t.Fatalf("the stored clear marker did not ride the next push (present=%v value=%v)", present, v) + } + + // A second empty save keeps it (the stored address is already empty; the household's clear still stands). + postNotifications(t, s, clearForm()) + if present, v := flagOf(t, hub.last(t)); !present || !v { + t.Fatalf("a second empty save dropped the clear flag (present=%v value=%v)", present, v) + } +} + +// A box that never had an address must NOT send the flag: its empty push is exactly the case the hub's +// no-clobber guard protects (a provisioning-seeded address must survive an unconfigured box). +func TestR922_NeverConfiguredBoxSendsNoFlag(t *testing.T) { + s, _, hub := r922Server(t) + + postNotifications(t, s, clearForm()) + if present, _ := flagOf(t, hub.last(t)); present { + t.Fatalf("a never-configured box sent email_cleared — the hub would delete a seeded address; body keys=%v", keysOf(hub.last(t))) + } + s.debugPreferencesSync(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/api/debug/preferences/sync", nil)) + if present, _ := flagOf(t, hub.last(t)); present { + t.Fatalf("the resync of a never-configured box sent email_cleared") + } +} + +// Setting a new address after a clear drops the flag: the push carries the new address and no flag, and the +// later pushes stay clean. +func TestR922_NewAddressDropsFlag(t *testing.T) { + s, sett, hub := r922Server(t) + if err := sett.SetNotificationPrefs(&settings.NotificationPrefs{ + Email: "old@example.hu", EnabledEvents: []string{"backup_failed"}, CooldownHours: 6, + }); err != nil { + t.Fatal(err) + } + postNotifications(t, s, clearForm()) + if present, v := flagOf(t, hub.last(t)); !present || !v { + t.Fatalf("precondition: the clear did not send the flag (present=%v value=%v)", present, v) + } + + postNotifications(t, s, url.Values{ + "notification_email": {"new@example.hu"}, + "event_backup_failed": {"on"}, + "cooldown_hours": {"6"}, + }) + body := hub.last(t) + if present, _ := flagOf(t, body); present { + t.Fatalf("the push with a new address still carries email_cleared; body keys=%v", keysOf(body)) + } + if e := emailOf(t, body); e != "new@example.hu" { + t.Fatalf("email = %q, want new@example.hu", e) + } + s.debugPreferencesSync(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/api/debug/preferences/sync", nil)) + if present, _ := flagOf(t, hub.last(t)); present { + t.Fatalf("the stored marker survived a new address — the next push still carries email_cleared") + } +} + +func keysOf(m map[string]json.RawMessage) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +}