From 5429d651ee882ce3354216aa44f4669dce15e30b Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 31 Aug 2026 14:22:32 +0200 Subject: [PATCH] R-403 fix, caught by the live run: the stale flag fired for every healthy app UnitRestoreDate also compared the package's date against the run's and flagged 'older'. A recovery unit is ALWAYS captured shortly before the run that mirrors it, so that comparison is true for every healthy app. Measured on demo-hp: bookstack, kimai, opengist and privatebin all had src and dest manifests at 12:03:49Z against a run at 12:14:24Z - perfectly healthy, and all four would have been told their package was stale. A warning that fires on everything is a warning nobody reads, which costs the same as the comforting lie it was meant to replace. The second return is now UnitLegPreserved and nothing else. TestR403_AHealthyAppIsNeverCalledStale pins it; red-proof: reinstate the comparison -> it fails. --- .../internal/backup/r403_hollow_test.go | 37 +++++++++++++++++++ controller/internal/backup/tier2_restore.go | 19 +++++++--- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/controller/internal/backup/r403_hollow_test.go b/controller/internal/backup/r403_hollow_test.go index c268bd2..6613bb3 100644 --- a/controller/internal/backup/r403_hollow_test.go +++ b/controller/internal/backup/r403_hollow_test.go @@ -132,3 +132,40 @@ func TestR403_SizeIsNeverConsulted(t *testing.T) { t.Error("the predicate ordered these by size rather than by contents") } } + +// TestR403_AHealthyAppIsNeverCalledStale — the bug the LIVE run caught, pinned. +// +// A recovery unit is always captured shortly BEFORE the Tier-2 run that mirrors it, so any test of +// the form "is the package older than the run?" is true for every healthy app. The first draft of +// UnitRestoreDate carried exactly that comparison, and on demo-hp 2026-08-31 four healthy apps +// (bookstack, kimai, opengist, privatebin — manifests at 12:03:49Z, run at 12:14:24Z) would each +// have told their owner the package was stale. Only `UnitLegPreserved` may raise it. +func TestR403_AHealthyAppIsNeverCalledStale(t *testing.T) { + healthy := Tier2Coverage{ + UnitRestorable: true, + CopyLastSuccess: "2026-08-31T12:14:24Z", // the run + CopyLastRun: "2026-08-31T12:14:24Z", + UnitPackageDate: "2026-08-31T12:03:49Z", // the capture, minutes earlier — NORMAL + } + date, preserved := healthy.UnitRestoreDate() + if preserved { + t.Error("a healthy app whose package predates its run was flagged as preserved/stale — " + + "this fires for EVERY app and trains the customer to ignore the warning") + } + if date != "2026-08-31T12:03:49Z" { + t.Errorf("date = %q, want the package's own date", date) + } + + // And the real case still raises it. + preservedCov := healthy + preservedCov.UnitLegPreserved = true + if _, p := preservedCov.UnitRestoreDate(); !p { + t.Error("a genuinely preserved package was NOT flagged") + } + + // No package date recorded → fall back to the copy date, and still only flag on preservation. + noPkg := Tier2Coverage{CopyLastSuccess: "2026-08-31T12:14:24Z"} + if d, p := noPkg.UnitRestoreDate(); d != "2026-08-31T12:14:24Z" || p { + t.Errorf("fallback = (%q,%v), want the copy date and not-preserved", d, p) + } +} diff --git a/controller/internal/backup/tier2_restore.go b/controller/internal/backup/tier2_restore.go index 13c44a6..708d315 100644 --- a/controller/internal/backup/tier2_restore.go +++ b/controller/internal/backup/tier2_restore.go @@ -284,18 +284,27 @@ func (c Tier2Coverage) Tier2CopyDate() (date string, proven bool) { } // UnitRestoreDate returns the date of the PACKAGE the unit restore would actually open, and whether -// that package is OLDER than the copy's newest run. +// the newest run PRESERVED that package rather than refreshing it. // // R-403. `Tier2CopyDate` answers "when was this copy last written to" and is right for the file // restore, whose legs really were refreshed by that run. It is the WRONG answer for the unit restore -// after a preserved leg, because the package is then older than the run that reports success. This +// after a preserved leg, because the package is then from before the run that reports success. This // asks the manifest first and falls back to the copy date only when the package cannot say. -func (c Tier2Coverage) UnitRestoreDate() (date string, olderThanTheRun bool) { - copyDate, _ := c.Tier2CopyDate() +// +// THE SECOND RETURN IS `UnitLegPreserved` AND NOTHING ELSE, and the first draft got this wrong in a +// way only the live run caught. It also compared the package's date against the run's and flagged +// "older" — but a unit is ALWAYS captured shortly before the run that mirrors it, so that comparison +// was true for every healthy app on the box and every one of them rendered the warning. Live on +// demo-hp 2026-08-31: bookstack, kimai, opengist and privatebin all had src and dest manifests at +// `12:03:49Z` against a run at `12:14:24Z` — perfectly healthy, and all four would have been told +// their package was stale. A warning that fires on everything is a warning nobody reads, which costs +// the same as the comforting lie it was meant to replace. +func (c Tier2Coverage) UnitRestoreDate() (date string, preserved bool) { if c.UnitPackageDate == "" { + copyDate, _ := c.Tier2CopyDate() return copyDate, c.UnitLegPreserved } - return c.UnitPackageDate, c.UnitLegPreserved || (copyDate != "" && c.UnitPackageDate < copyDate) + return c.UnitPackageDate, c.UnitLegPreserved } // tier2RecordedCopyDir resolves the RECORDED Tier-2 copy dir for a stack, applying every