From 58c703bd44c03c08187430eb537b3e70b75ae307 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 4 Aug 2026 18:32:56 +0200 Subject: [PATCH] R-203 Part 2: a run that missed a MANDATORY directory is not a successful run (v0.197.0) The gap was already detected and warned about, in Hungarian, naming the app and the folders -- that warning is what stopped the R-201 drill. The defect was that the run still reported `ok` beside it, and a warning standing beside a success is read as a success. last_status gains "incomplete": minted, because "ok" | "error" | "running" had nothing meaning "it ran, and this app is not fully protected". NOT "error" -- the rest of the run worked and what was captured is real, so SnapshotCount and the LastSuccess anchor still record it. Half a backup is not no backup. The gaps are now recorded STRUCTURALLY (offboxRunResult.mandatoryGaps), not only as prose, so the verdict has something to act on. It reaches the operator through the EXISTING per-run digest (backup_run_failures) rather than a new event type -- a new type is a two-repo change and the hub drops anything outside allowedEventTypes. The stat-filter gains the ClassMandatory check Tier 2 already had. It is a NO-OP today (TierOffsite admits mandatory only), so no customer-visible warning disappears -- demonstrated by widening the tier filter alone and watching the check hold the line. ANTICIPATED: calibre-web on demo-hp has exactly this gap, so its off-site status becomes incomplete the moment this ships. That is correct and is the point. Red-proofs: my first Scenario-C proof PASSED because the test only reached offboxCaptureSet while the mutation lives in runOffboxInternal -- a mutation the test cannot observe is not a red-proof, and the fix was the test. The run-level test now fails under both mutations (unreachable gap recording; unconditional ok). --- CHANGELOG.md | 62 +++++ controller/cmd/controller/main.go | 28 +++ controller/internal/backup/backup.go | 3 + controller/internal/backup/offbox.go | 45 +++- controller/internal/backup/offbox_capture.go | 24 +- .../backup/offbox_verdict_r203_test.go | 218 ++++++++++++++++++ controller/internal/settings/settings.go | 6 +- .../web/templates/backups_remote.html | 4 +- 8 files changed, 375 insertions(+), 15 deletions(-) create mode 100644 controller/internal/backup/offbox_verdict_r203_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d5e24da..aa5402e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,65 @@ +## v0.197.0 — the app and its backup look in the same place, and "ok" means it (2026-08-04, R-203) + +Found when the R-201 drill halted at its pre-wipe backup rather than wiping a machine: the run +reported `ok` with three snapshots and the sentinel file was in none of them. + +### Part 1 — one resolver, five callers + +`appbackup`'s path helpers take a **namespace root**; five call sites passed a bare **drive** path. +On an enrolled drive the two coincide — which is why this survived. On the system-data fallback (a +**supported, named** arrangement: *"the SSD-only system-data fallback"*, `paths.go:26`) they differ by +exactly the `felhom-data` segment, so the app bound `/mnt/sys_drive/userdata/media/books` while the +off-site capture set looked for `/mnt/sys_drive/felhom-data/userdata/media/books`. + +**The rule now has ONE expression** — `appbackup.NamespaceRootFor` / `IsEnrolledDrive`. There were +already **two** copies and they differed: `backup.Manager.namespaceRoot` compared without +`filepath.Clean`, `stacks.Manager.inGuest` with it, so a trailing slash from config would have flipped +the mode in one package and not the other. Both now delegate. + +Routed through it: `stacks/deploy.go` `withPathVars` → `${USERDATA_PATH}` (the live defect); +`appexport/fabplan.go` + `export.go` (via a new `GetStackNamespaceRoot` provider method); +`web/handlers.go` FileBrowser mounts (**latent** — the system drive is deliberately never a +registered `StoragePath`, so it is the identity today); and `stacks/delete.go`'s `ExportDataMounts`. + +`ComputeFabBuckets` now receives the namespace root, which is what `ComputeCaptureSet` has always +received — so the export's classified paths and the backup's capture set describe the same +directories by construction rather than by coincidence. + +**The census found FIVE sites, not the four the spec named.** The fifth is the FileBrowser mount +builder — the customer's own file browser would have shown the wrong directory on a non-enrolled path. + +**`ExportDataMounts` lives in `delete.go` and is NOT a delete path.** Its only production caller is +the `.fab` export adapter; nothing deletes on its result. The delete path's own guard, +`ProtectedHDDPaths`, is layout-agnostic by construction — it protects **both** `/…` and +`/felhom-data/…` — so deletion was never affected. That note is now in the function's doc comment, +and the change shipped as its own commit anyway. + +### Part 2 — a run that missed a mandatory directory is not a successful run + +The gap was already **detected**, and warned about, in Hungarian, naming the app and the folders — +that warning is what stopped the drill. The defect was that the run still reported **`ok`** beside it, +and a warning standing beside a success is read as a success. + +`last_status` gains **`incomplete`**: minted, because the existing vocabulary (`ok` | `error` | +`running`) had nothing meaning *"it ran, and this app is not fully protected"*. **Not `error`** — the +rest of the run worked and the data captured is real, so `SnapshotCount` and the `LastSuccess` anchor +still record it. Half a backup is not no backup, and reporting it as none would be its own lie. + +It reaches the **operator** via the existing per-run digest (`backup_run_failures`), not only the +page: a new event type would be a two-repo change and the hub drops anything outside +`allowedEventTypes`. The Hungarian customer warning is unchanged. + +The stat-filter gains the `ClassMandatory` check Tier 2 already had (*"optional-missing is silent"*). +**It is a no-op today** — `TierOffsite`'s `tierKeeps()` admits mandatory only — so **no customer-visible +warning disappears**. Demonstrated: widening the tier filter alone keeps the tests green *because of +this check*; widening it and removing the check makes an optional gap start reporting. + +### Anticipated live effect + +**`calibre-web` on demo-hp has exactly this gap.** Its off-site status becomes `incomplete` and the +operator digest fires. That is correct — it genuinely is not fully backed up — and it is the fix +telling the truth for the first time, not a regression. + ## v0.196.0 — the recovered key installs itself (2026-08-04, R-200 plumbing half) — MinAgent 0.125.0 `--recover-offsite-install` is the sibling of `--recover-offsite-check`: same fetch → unseal → extract diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 876e320..22558be 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -785,6 +785,34 @@ func main() { backupMgr.OffsiteFailureMessage(err, dur)) } }) + // R-203 (operator half): a completed off-site run could NOT capture a directory an app declares + // MANDATORY. It is not a failed run — what was captured is real and the snapshot count records + // it — but the app is NOT fully protected, and until v0.197.0 that reached nobody: the run said + // ok, the card said ok, the hub said ok, and one WARN line inside the container said otherwise. + // + // It rides the EXISTING per-run digest (backup_run_failures) rather than a new event type: a + // new type is a two-repo change (the hub's allowedEventTypes drops anything unlisted), and the + // digest is already operator-only and already means "this run did not fully do its job". + backupMgr.SetOffboxGapNotify(func(gaps map[string][]string) { + apps := make([]string, 0, len(gaps)) + for app := range gaps { + apps = append(apps, app) + } + sort.Strings(apps) // deterministic message + details + d := notify.BackupRunFailuresDetails{RunKind: "offsite", Attempted: len(apps), Failed: len(apps)} + var parts []string + for _, app := range apps { + sort.Strings(gaps[app]) + reason := "declared MANDATORY data directories not captured: " + strings.Join(gaps[app], ", ") + d.Apps = append(d.Apps, notify.RunFailureDetail{App: app, Leg: "offsite-userdata", Reason: reason}) + parts = append(parts, fmt.Sprintf("%s (%s)", app, strings.Join(gaps[app], ", "))) + } + notifier.NotifyBackupRunFailures(fmt.Sprintf( + "Offsite backup INCOMPLETE: %d app(s) had a directory they declare MANDATORY missing from the snapshot — %s. "+ + "The run itself succeeded and what it captured is real, but these apps are NOT fully protected off-site. "+ + "Check that the declared path exists on disk and that the app's data root resolves where the backup looks (R-203).", + len(apps), strings.Join(parts, "; ")), d) + }) // R-158 / R-167 (D-c, operator half): a per-app Tier-1 recovery-unit capture failed. Until // v0.191.0 this was a `[WARN]` line and nothing else — /backups/apps is the page you open to // ask whether ONE app is backed up, and it was the one page that never said. Fires per app; diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index 7347ef8..2e50c8a 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -93,6 +93,9 @@ type Manager struct { // ("offbox_repo_orphaned" / "offbox_repo_reset"); renamedTo names the move-aside path (reset only). // Wired in main.go to the notifier. Nil-safe. offboxOrphanEvent func(eventType, renamedTo string) + // offboxGapNotify (R-203) fires when a COMPLETED offsite run could not capture a directory an + // app declares MANDATORY — a coverage gap, not a failed run. nil → no signal. + offboxGapNotify func(gaps map[string][]string) // offboxSSH (v0.142.0) is the raw-ssh exec seam for the orphaned-repo move-aside (restic has no // rename); tests inject a fake. Nil → the real ssh invocation (defaultOffboxSSH). offboxSSH func(ctx context.Context, host, user string, port int, keyPath, knownHosts, remoteCmd string) ([]byte, error) diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index 681e205..39114e9 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -54,6 +54,14 @@ func (m *Manager) SetOffboxNotify(fn func(dur time.Duration, snapshots int, err m.offboxNotify = fn } +// SetOffboxGapNotify wires the R-203 operator signal: a run that completed but could NOT capture a +// directory an app declares MANDATORY. Distinct from offboxNotify, which fires only on a hard run +// failure — a coverage gap is not a failed run, and until v0.197.0 it reached nobody at all. +// gaps is app → the relative paths that were missed. nil → no signal (pre-R-203 behaviour). +func (m *Manager) SetOffboxGapNotify(fn func(gaps map[string][]string)) { + m.offboxGapNotify = fn +} + // SetOffboxOrphanEvent wires the offsite-repo continuity event push (main.go → notifier). func (m *Manager) SetOffboxOrphanEvent(fn func(eventType, renamedTo string)) { m.offboxOrphanEvent = fn @@ -854,7 +862,25 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error o.LastError = runErr.Error() o.LastWarning = "" } else { - o.LastStatus = "ok" + // R-203 — THE VERDICT. A run that could not capture a directory the app declares MANDATORY + // is not a successful run. Until v0.197.0 it reported `ok` with a warning beside it, and a + // warning beside a success is read as a success: that is how calibre-web's declared book + // directory stayed out of every off-site snapshot on demo-hp while the card, the counters + // and the hub all said the backup worked. + // + // NOT "error": the rest of the run worked and the data that WAS captured is real. The + // snapshot count and the LastSuccess anchor are deliberately left to record it — half a + // backup is not no backup, and reporting it as none would be its own lie. `incomplete` is + // minted here because the existing vocabulary ("ok" | "error" | "running") has nothing that + // means "it ran, and this app is not fully protected". + if len(runResult.mandatoryGaps) > 0 { + o.LastStatus = "incomplete" + if m.offboxGapNotify != nil { + m.offboxGapNotify(runResult.mandatoryGaps) + } + } else { + o.LastStatus = "ok" + } o.LastError = "" o.SnapshotCount = snapshots o.EnlargedBlocked = blockedNames // replace each run (sorted); empty slice clears it @@ -1027,6 +1053,10 @@ type offboxRunResult struct { // the zero-toggle honesty notice honest: a box with no app toggled but shares in the cloud is NOT // "nothing is covered". sharesBackedUp bool + // mandatoryGaps (R-203) is app → the relative paths of its declared MANDATORY data directories + // that could NOT be captured. It is the STRUCTURED form of the warnings above, and it is what + // decides the run's verdict: a run that dropped a mandatory directory is not a successful run. + mandatoryGaps map[string][]string } // runOffboxInternal does the repo-ensure + per-app DISCOVER → capture-set → gate → multi-path backup + @@ -1051,8 +1081,17 @@ func (m *Manager) runOffboxInternal(ctx context.Context, apps, base, env []strin } // Task 3-core TierOffsite capture set: mandatory userdata paths added to the unit snapshot, // plus loud warnings for structurally-refused / on-disk-missing mandatory paths (SP-3.4). - extra, capWarns := m.offboxCaptureSet(stack) + extra, capWarns, capGaps := m.offboxCaptureSet(stack) res.warns = append(res.warns, capWarns...) + // R-203: the gaps are recorded STRUCTURALLY, not only as prose, because the run's verdict now + // depends on them. A warning standing beside a success is read as a success — which is exactly + // how a customer-declared mandatory directory stayed out of the snapshot while the run said ok. + if len(capGaps) > 0 { + if res.mandatoryGaps == nil { + res.mandatoryGaps = map[string][]string{} + } + res.mandatoryGaps[stack] = append(res.mandatoryGaps[stack], capGaps...) + } // Pre-push enlargement gate (§9): if last-known repo raw-data bytes + the mandatory-set estimate // would cross the soft quota, push UNIT-ONLY (protection never regresses) and record the block. if len(extra) > 0 && t != nil && t.QuotaGB > 0 { @@ -1150,7 +1189,7 @@ type OffboxReportStatus struct { Enabled bool `json:"enabled"` EscrowState string `json:"escrow_state"` LastRun string `json:"last_run,omitempty"` // RFC3339 - LastStatus string `json:"last_status,omitempty"` // "ok" | "error" | "running" + LastStatus string `json:"last_status,omitempty"` // "ok" | "incomplete" (R-203) | "error" | "running" // LastSuccess (R-100) is the last run that SUCCEEDED — the hub's staleness anchor. Absent on a // pre-v0.181.0 controller, which the hub must degrade on explicitly rather than by accident: // treating absence as failure alarms every un-upgraded box, treating it as success keeps the bug. diff --git a/controller/internal/backup/offbox_capture.go b/controller/internal/backup/offbox_capture.go index ce992b9..6bc914b 100644 --- a/controller/internal/backup/offbox_capture.go +++ b/controller/internal/backup/offbox_capture.go @@ -29,13 +29,13 @@ type offboxBlocked struct { // snapshot, plus any Hungarian warnings for capture gaps. It never returns optional/excluded paths // (the TierOffsite filter drops them — §2). Returns (nil, nil) for the legacy / no-provider / no-block // world: offsite stays UNIT-ONLY, byte-identical to pre-v0.134.0 (the SQ5 cost-regression guard). -func (m *Manager) offboxCaptureSet(stack string) (extra []string, warns []string) { +func (m *Manager) offboxCaptureSet(stack string) (extra []string, warns []string, gaps []string) { if m.stackProvider == nil { - return nil, nil // no provider wired → legacy world → unit only + return nil, nil, nil // no provider wired → legacy world → unit only } binds, has := m.stackProvider.GetStackClassifiedBinds(stack) if !has { - return nil, nil // no backup block → legacy → unit only + return nil, nil, nil // no backup block → legacy → unit only } // Resolve against the app's LIVE HDD_PATH (raw — NOT GetAppDrivePath, whose systemDataPath fallback // would resolve userdata onto the wrong drive). Empty ⇒ undeployed / no HDD (decision §2.4): @@ -43,12 +43,11 @@ func (m *Manager) offboxCaptureSet(stack string) (extra []string, warns []string hdd := strings.TrimSpace(m.stackProvider.GetStackHDDPath(stack)) if hdd == "" { m.logger.Printf("[WARN] [offbox] %s: not deployed — offsite push is unit-only (mandatory userdata not resolvable)", stack) - return nil, []string{fmt.Sprintf("Figyelmeztetés: a(z) %s nincs telepítve — csak a mentési egység került a távoli mentésbe.", stack)} + return nil, []string{fmt.Sprintf("Figyelmeztetés: a(z) %s nincs telepítve — csak a mentési egység került a távoli mentésbe.", stack)}, nil } nsRoot := m.namespaceRoot(hdd) cs := appbackup.ComputeCaptureSet(binds, has, appbackup.TierOffsite, nsRoot, m.stackProvider.GetImportRoot()) - var gaps []string // Structurally-refused MANDATORY paths (traversal / bare drive-root / reserved backups/ zone) are // loud ERROR gaps — the path the customer thinks is protected is not in the snapshot. for _, sk := range cs.Skipped { @@ -60,11 +59,18 @@ func (m *Manager) offboxCaptureSet(stack string) (extra []string, warns []string } // Stat-filter (§2.5): a declared mandatory path absent on disk. restic would skip it SILENTLY // (SP-3.4), so drop it from argv AND warn — never a silent "looks backed up but isn't". + // + // R-203: the class check mirrors tier2_capture.go's ("optional-missing is silent"). It is a NO-OP + // today — TierOffsite's tierKeeps() already admits ClassMandatory only, so cs.Paths cannot contain + // an optional path here — and it is written anyway so the two tiers read the same and so the + // verdict below can never be flipped by an unused optional folder if that filter ever widens. for _, p := range cs.Paths { if _, err := os.Stat(p.Abs); err != nil { - m.logger.Printf("[WARN] [offbox] %s: mandatory data path missing on disk, skipped from offsite: %s", stack, p.Abs) - gaps = append(gaps, p.RelPath) - continue + if p.Class == appbackup.ClassMandatory { + m.logger.Printf("[WARN] [offbox] %s: mandatory data path missing on disk, skipped from offsite: %s", stack, p.Abs) + gaps = append(gaps, p.RelPath) + } + continue // optional-missing is silent (not a gap) — parity with Tier 2 } extra = append(extra, p.Abs) } @@ -72,5 +78,5 @@ func (m *Manager) offboxCaptureSet(stack string) (extra []string, warns []string warns = append(warns, fmt.Sprintf("Figyelmeztetés: a(z) %s alkalmazás egyes adatmappái nem kerültek a távoli mentésbe: %s.", stack, strings.Join(gaps, ", "))) } - return extra, warns + return extra, warns, gaps } diff --git a/controller/internal/backup/offbox_verdict_r203_test.go b/controller/internal/backup/offbox_verdict_r203_test.go new file mode 100644 index 0000000..f002b05 --- /dev/null +++ b/controller/internal/backup/offbox_verdict_r203_test.go @@ -0,0 +1,218 @@ +package backup + +import ( + "context" + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" +) + +// R-203 Part 2 — "ok" must mean the mandatory data is in the snapshot. +// +// The defect these pin is NOT that the gap went undetected. It WAS detected, and warned about, in +// Hungarian, naming the app and the folders — that warning is what stopped the drill. The defect is +// that the run reported `ok` beside it, and a warning standing beside a success is read as a success. + +func mandatoryUserdata(rel string) ClassifiedBind { + return ClassifiedBind{ComposeBind: appbackup.ComposeBind{Root: appbackup.RootUserdata, RelPath: rel}, Class: appbackup.ClassMandatory} +} + +// Scenario C — a MANDATORY declared path absent on disk is a STRUCTURAL gap, not just prose. +// +// RED-PROOF: stop recording capGaps into res.mandatoryGaps (or drop the third return) and the verdict +// has nothing to act on — the run reports `ok` over a mandatory gap, which is production behaviour up +// to v0.196.0. +func TestOffboxCaptureSet_MandatoryGapIsStructural(t *testing.T) { + drive := t.TempDir() + m, _, prov := classifiedOffboxManager(t, drive) + prov.hdd["calibre-web"] = drive + prov.binds["calibre-web"] = []ClassifiedBind{mandatoryUserdata("media/books")} + prov.has["calibre-web"] = true + + // The declared directory does not exist on disk — exactly the shape the drill hit. + extra, warns, gaps := m.offboxCaptureSet("calibre-web") + if len(gaps) != 1 || gaps[0] != "media/books" { + t.Fatalf("a missing MANDATORY path must be reported as a structural gap, got %v", gaps) + } + if len(warns) == 0 { + t.Error("the customer-facing Hungarian warning must SURVIVE this change — it is what caught the defect") + } + if len(extra) != 0 { + t.Errorf("a missing path must not be handed to restic, got %v", extra) + } + + // Create it: no gap, no warning, and the path IS captured. + nsRoot := appbackup.NamespaceRootFor(drive, m.systemDataPath) + if err := os.MkdirAll(filepath.Join(appbackup.UserdataDir(nsRoot), "media", "books"), 0o755); err != nil { + t.Fatal(err) + } + extra2, warns2, gaps2 := m.offboxCaptureSet("calibre-web") + if len(gaps2) != 0 || len(warns2) != 0 { + t.Fatalf("a PRESENT mandatory path must be silent, got gaps %v warns %v", gaps2, warns2) + } + if len(extra2) != 1 { + t.Fatalf("a present mandatory path must be handed to restic, got %v", extra2) + } +} + +// Scenario D — an OPTIONAL declared path absent on disk changes nothing. +// +// RED-PROOF: remove the `p.Class == ClassMandatory` check in the stat-filter → an optional gap starts +// being reported, and together with the verdict would flip every app with an unused optional folder +// to not-ok, which is how a status stops being read. +// +// STATED BECAUSE IT CHANGES WHAT THIS PROVES: TierOffsite's tierKeeps() already admits ClassMandatory +// only, so an optional path cannot reach the stat-filter today. The class check is therefore a NO-OP +// and NO customer-visible warning disappears with it. It is written for parity with Tier 2 and so the +// verdict can never be flipped by an optional folder if that tier filter ever widens. +func TestOffboxCaptureSet_OptionalGapIsSilent(t *testing.T) { + drive := t.TempDir() + m, _, prov := classifiedOffboxManager(t, drive) + prov.hdd["komga"] = drive + prov.binds["komga"] = []ClassifiedBind{optionalUserdata("media/comics")} + prov.has["komga"] = true + + extra, warns, gaps := m.offboxCaptureSet("komga") + if len(gaps) != 0 { + t.Fatalf("an absent OPTIONAL path must be silent, got gaps %v", gaps) + } + if len(warns) != 0 { + t.Fatalf("an absent OPTIONAL path must raise no customer warning, got %v", warns) + } + if len(extra) != 0 { + t.Fatalf("an absent path must not be captured, got %v", extra) + } +} + +// A mandatory path that IS present alongside an absent optional one: still silent, still captured. +func TestOffboxCaptureSet_MixedClassesOnlyMandatoryCounts(t *testing.T) { + drive := t.TempDir() + m, _, prov := classifiedOffboxManager(t, drive) + prov.hdd["mixed"] = drive + prov.binds["mixed"] = []ClassifiedBind{mandatoryUserdata("docs"), optionalUserdata("cache")} + prov.has["mixed"] = true + + nsRoot := appbackup.NamespaceRootFor(drive, m.systemDataPath) + if err := os.MkdirAll(filepath.Join(appbackup.UserdataDir(nsRoot), "docs"), 0o755); err != nil { + t.Fatal(err) + } + extra, warns, gaps := m.offboxCaptureSet("mixed") + if len(gaps) != 0 || len(warns) != 0 { + t.Fatalf("a present mandatory + absent optional must be silent, got gaps %v warns %v", gaps, warns) + } + if len(extra) != 1 { + t.Fatalf("the mandatory path must be captured, got %v", extra) + } +} + +// The verdict rule itself, over its inputs. The surrounding run needs a live restic, so the decision +// is asserted where it is made rather than through a fake repository. +func TestMandatoryGapsDecideTheVerdict(t *testing.T) { + verdict := func(gaps map[string][]string) string { + if len(gaps) > 0 { + return "incomplete" + } + return "ok" + } + cases := []struct { + name string + gaps map[string][]string + want string + }{ + {"no gaps", nil, "ok"}, + {"empty map", map[string][]string{}, "ok"}, + {"one app one folder", map[string][]string{"calibre-web": {"media/books"}}, "incomplete"}, + {"two apps", map[string][]string{"a": {"x"}, "b": {"y"}}, "incomplete"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := verdict(tc.gaps) + if got != tc.want { + t.Fatalf("gaps %v → %q, want %q", tc.gaps, got, tc.want) + } + // "incomplete" must be distinct from every value that already existed, so a checker or a + // template matching on those cannot silently treat a coverage gap as one of them. + if got == "ok" && tc.want == "incomplete" { + t.Fatal("a coverage gap must never read as ok") + } + }) + } +} + +// Scenario C, THROUGH THE RUN — the verdict itself, not just the capture set. +// +// The first version of this file tested offboxCaptureSet alone, and its "red-proof" PASSED: the +// mutation (dropping the gap recording) lives in runOffboxInternal, which that test never reaches. +// A mutation that the test cannot observe is not a red-proof, and the fix is the test, not the code. +// +// RED-PROOF (now real): make the gap recording unreachable (`if false && len(capGaps) > 0`) or +// restore `o.LastStatus = "ok"` unconditionally → this FAILS with the run reporting ok over a +// mandatory gap, which is production behaviour up to v0.196.0. +func TestOffboxRun_MandatoryGapMakesTheRunIncomplete(t *testing.T) { + drive := t.TempDir() + m, sett, prov := classifiedOffboxManager(t, drive) + mkUnit(t, drive, "calibre-web") + prov.hdd["calibre-web"] = drive + prov.has["calibre-web"] = true + // Declared MANDATORY and deliberately ABSENT on disk — the drill's shape. + prov.binds["calibre-web"] = []ClassifiedBind{mandatoryUserdata("media/books")} + _ = sett.SetAppOffbox("calibre-web", true) + + var gapNotified map[string][]string + m.SetOffboxGapNotify(func(g map[string][]string) { gapNotified = g }) + + cap := &backupCapture{} + m.SetOffboxRunner(cap.runner()) + if err := m.RunOffboxBackup(context.Background()); err != nil { + t.Fatalf("the run itself must SUCCEED — a coverage gap is not a failed run: %v", err) + } + + got := sett.GetOffboxTarget() + if got.LastStatus != "incomplete" { + t.Fatalf("LastStatus = %q, want \"incomplete\" — a run that dropped a MANDATORY directory is "+ + "not a successful run, and reporting ok beside a warning is how this defect hid", got.LastStatus) + } + // What WAS captured is still recorded — half a backup is not no backup. + if got.LastSuccess == "" { + t.Error("LastSuccess must still record what was captured (§8.5) — suppressing it would be its own lie") + } + if cap.backups != 1 { + t.Errorf("the unit must still be pushed, got %d backup calls", cap.backups) + } + // And the OPERATOR is told, not only the log. + if len(gapNotified) != 1 || len(gapNotified["calibre-web"]) != 1 || gapNotified["calibre-web"][0] != "media/books" { + t.Fatalf("the operator gap signal did not fire with the app and folder, got %v", gapNotified) + } +} + +// The companion: no gap → ok, and no operator signal. Without this, "incomplete" everywhere would +// also pass the test above. +func TestOffboxRun_NoGapStaysOk(t *testing.T) { + drive := t.TempDir() + m, sett, prov := classifiedOffboxManager(t, drive) + mkUnit(t, drive, "calibre-web") + nsRoot := appbackup.NamespaceRootFor(drive, m.systemDataPath) + if err := os.MkdirAll(filepath.Join(appbackup.UserdataDir(nsRoot), "media", "books"), 0o755); err != nil { + t.Fatal(err) + } + prov.hdd["calibre-web"] = drive + prov.has["calibre-web"] = true + prov.binds["calibre-web"] = []ClassifiedBind{mandatoryUserdata("media/books")} + _ = sett.SetAppOffbox("calibre-web", true) + + fired := false + m.SetOffboxGapNotify(func(map[string][]string) { fired = true }) + cap := &backupCapture{} + m.SetOffboxRunner(cap.runner()) + if err := m.RunOffboxBackup(context.Background()); err != nil { + t.Fatalf("run: %v", err) + } + if got := sett.GetOffboxTarget(); got.LastStatus != "ok" { + t.Fatalf("LastStatus = %q, want ok — a complete run must not be downgraded", got.LastStatus) + } + if fired { + t.Error("the operator gap signal must NOT fire when nothing was missed") + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index bb1d475..5ea3108 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -162,7 +162,11 @@ type OffboxTarget struct { // Runtime status (written by the off-box runner; never holds a secret). LastRun string `json:"last_run,omitempty"` // RFC3339 - LastStatus string `json:"last_status,omitempty"` // "ok" | "error" | "running" + // LastStatus — "ok" | "incomplete" | "error" | "running". R-203 added "incomplete": the run + // completed and what it captured is real, but a directory the app declares MANDATORY could not be + // captured, so the app is NOT fully protected. Distinct from "error" (the run failed) on purpose; + // SnapshotCount and LastSuccess still record what WAS captured. + LastStatus string `json:"last_status,omitempty"` // LastSuccess (R-100) is the RFC3339 stamp of the last run that actually SUCCEEDED. // // IT EXISTS BECAUSE LastRun RECORDS AN ATTEMPT, NOT A RESULT. LastRun is written diff --git a/controller/internal/web/templates/backups_remote.html b/controller/internal/web/templates/backups_remote.html index afc7caf..8a7bcab 100644 --- a/controller/internal/web/templates/backups_remote.html +++ b/controller/internal/web/templates/backups_remote.html @@ -31,8 +31,8 @@
{{if .Offbox}}
-
-
{{if eq .Offbox.LastStatus "ok"}}✓ Rendben{{else if eq .Offbox.LastStatus "error"}}✗ Hiba{{else if eq .Offbox.LastStatus "running"}}Fut…{{else}}–{{end}}
+
+
{{if eq .Offbox.LastStatus "ok"}}✓ Rendben{{else if eq .Offbox.LastStatus "incomplete"}}! Hiányos{{else if eq .Offbox.LastStatus "error"}}✗ Hiba{{else if eq .Offbox.LastStatus "running"}}Fut…{{else}}–{{end}}
Utolsó távoli mentés{{if .Offbox.LastRun}}
{{timeAgoStr .Offbox.LastRun}}{{end}}