diff --git a/CHANGELOG.md b/CHANGELOG.md index 312e7dc..5e616ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,26 @@ +## v0.124.1 — the repair record must survive the probe that did NOT feed the hub (2026-08-04, R-190) + +**v0.124.0's transition record did not reach the hub, and the live run is what showed it.** The +capability reported degraded for "one cycle" — meaning the probe call that performed the repair. But +`probeAll` is invoked **independently** by the periodic self-check log and by the collector building a +host-report. On the demo box the repairing call was the log's (`09:39:34`, journal shows +`GRANT WAS MISSING AND HAS BEEN SELF-REPAIRED` and `degraded=1`), and the host-report built three +seconds later found the grant present and sent **`ok`**. The agent's journal had the record; the hub +had nothing; the operator would have learned nothing. + +That is the exact silence R-190 is about, re-created inside its own mitigation — and every unit test +passed while it was true. + +**The fix is a latch on TIME rather than on call count.** A confirmed repair is reported for +`storeGrantRepairReportWindow` (20 minutes), which comfortably exceeds the 900 s host-report interval, +so at least one report must carry the transition. It clears on its own — a permanently degraded +capability would be its own false alarm — and it is per tier. + +**Two hollow tests were caught and fixed on the way**, both the same shape this repo keeps finding: a +test asserting a value it constructed itself, and a test asserting the latch HELPER rather than the +path that consumes it — whose red-proof duly passed. The decisions now live in +`storeGrantHealthyVerdict` and `storeGrantRepairedVerdict`, and the tests call those. + ## v0.124.0 — a lost storage grant repairs itself, and says that it was lost (2026-08-04, R-190) **R-190 is a grant that demonstrably worked at 04:44 on 2026-08-03 and was gone by 09:24** — with a diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 57ab88b..f0b2b27 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -451,6 +451,21 @@ func storeGrantStatuses(ctx context.Context, px *proxmox.Client, cfg config.Conf return out } +// storeGrantRepairReportWindow is how long after a repair the capability keeps reporting the +// transition. It MUST exceed the hub report interval, or the record never reaches the operator. +// +// FOUND BY THE LIVE RUN, NOT BY THE TESTS (2026-08-04). The first implementation reported degraded +// for exactly "one cycle" — the probe call that did the repair. But `probeAll` is invoked +// INDEPENDENTLY by the startup/periodic self-check log and by the collector building a host report, +// so the repairing call was the LOG's, and the report built three seconds later found the grant +// present and reported `ok`. The agent's journal had the record; the hub had nothing; the operator +// would have learned nothing. That is precisely the silence R-190 is about, re-created inside its own +// mitigation. +// +// A latch on TIME rather than on call count fixes it: 20 minutes comfortably exceeds the 900 s report +// interval, so at least one host-report must carry the transition, and it still clears on its own. +const storeGrantRepairReportWindow = 20 * time.Minute + // storeGrantRepairMinInterval bounds how often a single tier's grant may be re-granted (Scenario F). // // A storage can be unreadable for reasons an ACL cannot fix — the storage is gone, PVE is wedged, @@ -464,10 +479,37 @@ const storeGrantRepairMinInterval = time.Hour // restart re-arms the repair, which is correct — a restart is exactly when a box should re-check // everything it depends on. type storeGrantRepairer struct { - run func(ctx context.Context, name string, args ...string) ([]byte, []byte, error) - log *slog.Logger - mu sync.Mutex - last map[string]time.Time // target id → last ATTEMPT (success or failure) + run func(ctx context.Context, name string, args ...string) ([]byte, []byte, error) + log *slog.Logger + mu sync.Mutex + last map[string]time.Time // target id → last ATTEMPT (success or failure) + repaired map[string]time.Time // target id → last CONFIRMED repair (drives the report latch) +} + +// noteRepaired latches a confirmed repair so it is reported for storeGrantRepairReportWindow. +func (r *storeGrantRepairer) noteRepaired(target string, now time.Time) { + if r == nil { + return + } + r.mu.Lock() + defer r.mu.Unlock() + if r.repaired == nil { + r.repaired = map[string]time.Time{} + } + r.repaired[target] = now +} + +// recentlyRepaired reports whether a confirmed repair is still inside its report window — the latch +// that guarantees a host-report carries the transition even though the probe that repaired may have +// been a log-only one. +func (r *storeGrantRepairer) recentlyRepaired(target string, now time.Time) bool { + if r == nil { + return false + } + r.mu.Lock() + defer r.mu.Unlock() + t, ok := r.repaired[target] + return ok && now.Sub(t) < storeGrantRepairReportWindow } // mayAttempt reports whether a repair may run now for this target, and records the attempt if so. @@ -544,9 +586,16 @@ func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string, defer cancel() privs, err := px.Permissions(pctx, "/storage/"+targetID) s = storeGrantVerdict(targetID, critical, privs, err) - if err != nil || s.Status != capability.StatusDegraded { + if err != nil { return s } + if s.Status != capability.StatusDegraded { + // Healthy — but if this tier was repaired moments ago, keep REPORTING the transition until a + // host-report has certainly carried it. Without this latch the repairing probe may be a + // log-only one and the hub never learns anything happened (measured live, see the window's + // comment). + return storeGrantHealthyVerdict(targetID, critical, s, repair.recentlyRepaired(targetID, time.Now())) + } // ── R-190 mitigation: the grant is missing — repair it, and SAY that it was missing ────────── // @@ -593,6 +642,7 @@ func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string, // produces exactly one alert pair and the operator learns of it. NOTHING NEW WAS BUILT: no wire // change, no hub change, no new event type. The `Feature` text carries the explanation because // that is the field the hub puts in the operator's e-mail (the Reason does not travel). + repair.noteRepaired(targetID, time.Now()) s = storeGrantRepairedVerdict(targetID, critical) repairLogger(repair).Error("store-grant: GRANT WAS MISSING AND HAS BEEN SELF-REPAIRED — investigate the loss (R-190)", "target", targetID, "privilege", storeGrantRequiredPriv, @@ -600,6 +650,22 @@ func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string, return s } +// storeGrantHealthyVerdict decides what a HEALTHY probe reports — which is not always "ok". +// +// Split out so the tests exercise this decision rather than a copy of it. An earlier version of this +// guard lived inline and its red-proof PASSED, because the test asserted the latch helper instead of +// the path that consumes it — the same hollow shape this file has now caught twice. +// +// If the tier was repaired inside the report window, the transition is reported even though the grant +// is present: the probe that repaired may have been a log-only one, and without this the host-report +// carries `ok` and the operator never learns the permission vanished (measured live 2026-08-04). +func storeGrantHealthyVerdict(targetID string, critical bool, healthy capability.Status, repairedRecently bool) capability.Status { + if repairedRecently { + return storeGrantRepairedVerdict(targetID, critical) + } + return healthy +} + // storeGrantRepairedVerdict is the post-repair verdict — the RECORD half of R-190, split out so the // tests exercise the real thing rather than a copy of it (yesterday's hollow-test lesson). // diff --git a/cmd/felhom-agent/storegrant_test.go b/cmd/felhom-agent/storegrant_test.go index 1998d75..cf6e0f5 100644 --- a/cmd/felhom-agent/storegrant_test.go +++ b/cmd/felhom-agent/storegrant_test.go @@ -353,3 +353,65 @@ func TestMainWiresTheGrantRepair(t *testing.T) { "leave it, which is v0.123.0's behaviour and not R-190's mitigation") } } + +// The transition must survive a probe that is NOT the one feeding the hub. +// +// MEASURED LIVE 2026-08-04, and this test exists because the first implementation failed it in +// production while every unit test passed: `probeAll` is called independently by the self-check LOG +// and by the collector building a host-report. The repairing call was the log's; the report three +// seconds later found the grant present and reported `ok`. The agent's journal had the record and the +// hub had nothing — the exact silence R-190 is about, re-created inside its own mitigation. +// +// COMPANION RED-PROOF (observed): delete the `recentlyRepaired` branch from the healthy path → +// +// --- FAIL: TestGrantRepair_TransitionSurvivesALaterProbe +// storegrant_test.go: a probe AFTER the repair must still report the transition; got "ok" — +// the host-report would carry ok and the operator would never learn the grant vanished +// +// Restored. +func TestGrantRepair_TransitionSurvivesALaterProbe(t *testing.T) { + r := newRepairer(&fakeRepairRunner{}) + // Jittered, never landing on the window boundary. + repairedAt := time.Date(2026, 8, 4, 9, 39, 34, 0, time.UTC) + r.noteRepaired("felhom-backup", repairedAt) + + // The DECISION a later probe makes — the production function, not the helper it calls. An + // earlier draft asserted `recentlyRepaired` directly and its red-proof PASSED, because removing + // the latch's USE left the helper untouched. + healthy := probeWith(permGranted, "felhom-backup", true) + if healthy.Status != capability.StatusOK { + t.Fatalf("precondition: a granted tier is ok; got %q", healthy.Status) + } + got := storeGrantHealthyVerdict("felhom-backup", true, + healthy, r.recentlyRepaired("felhom-backup", repairedAt.Add(3*time.Second))) + if got.Status != capability.StatusDegraded { + t.Fatalf("a probe AFTER the repair must still report the transition; got %q — the host-report "+ + "would carry ok and the operator would never learn the grant vanished", got.Status) + } + if !strings.Contains(got.Feature, "RESTORED") { + t.Fatalf("the later probe must carry the explanation into the hub's e-mail; got: %s", got.Feature) + } + // Outside the window it reports plain ok again. + late := storeGrantHealthyVerdict("felhom-backup", true, + healthy, r.recentlyRepaired("felhom-backup", repairedAt.Add(storeGrantRepairReportWindow+time.Minute))) + if late.Status != capability.StatusOK { + t.Fatalf("outside the window a healthy tier reports ok; got %q — a permanent degraded state "+ + "would be its own false alarm", late.Status) + } + if !r.recentlyRepaired("felhom-backup", repairedAt.Add(14*time.Minute+37*time.Second)) { + t.Fatal("the latch must outlast the 900s hub report interval, or the record never reaches the hub") + } + // ...and it clears on its own rather than latching a box degraded forever. + if r.recentlyRepaired("felhom-backup", repairedAt.Add(storeGrantRepairReportWindow+time.Minute+7*time.Second)) { + t.Fatal("the latch must clear — a permanent degraded state would be its own false alarm") + } + // It is per tier. + if r.recentlyRepaired("felhom-pbs", repairedAt.Add(time.Second)) { + t.Fatal("one tier's repair must not latch another tier's status") + } + // The window MUST exceed the report interval — the property, asserted rather than assumed. + if storeGrantRepairReportWindow <= 15*time.Minute { + t.Fatalf("the report window (%s) must exceed the 900s hub report interval, or a transition can "+ + "be missed entirely", storeGrantRepairReportWindow) + } +}