From e22113230a1fe4633eeafc7921ece6b6e6bfe6ff Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 11 May 2026 11:40:07 +0100 Subject: [PATCH] Purge resolved legacy alert-mirror findings on load The previous rip retired only active "Active alert detected" findings from the now-removed detectAlertSignals -> SignalActiveAlert emitter. Resolved instances were left in place, polluting the Resolved tab and inflating the regressed total on the trust strip (a stale 8 resources each marked "regressed 3x" with descriptions like "Active warning alert: Container 'ollama' is powered off"). They have no canonical operator value -- the Alerts surface is the source of truth for currently-firing alerts -- so on load we now purge them entirely rather than keeping them around as Resolved noise. Active mirrors are still retired (auto-resolved with a clear reason) so operators see why the finding closed; resolved mirrors disappear silently because they were already in the terminal state. Idempotent. Extends TestFindingsStore_SetPersistence_RetiresLegacyAlertMirrorFindings with a fixture for the resolved-mirror case and asserts both the in-memory purge and the persisted state no longer carries it. --- internal/ai/findings.go | 47 ++++++++++++++++++--------- internal/ai/findings_coverage_test.go | 28 ++++++++++++++++ 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/internal/ai/findings.go b/internal/ai/findings.go index 04cfded78..4dba20225 100644 --- a/internal/ai/findings.go +++ b/internal/ai/findings.go @@ -861,16 +861,31 @@ func (s *FindingsStore) SetPersistence(p FindingsPersistence) error { if normalizeLoadedFinding(f) { normalizedLoadedState = true } - // One-shot retirement of legacy alert-mirror findings. The + // One-shot cleanup of legacy alert-mirror findings. The // deterministic detectAlertSignals → SignalActiveAlert path - // has been removed; any active "Active alert detected" - // finding still persisted from an earlier build is a stale - // mirror of an alert that already owns its own canonical - // surface. Retire it on load with a clear reason so the - // trust strip, regression counter, and health score stop - // reflecting the duplicate. Idempotent — after the first - // load with this code, no findings match the pattern. - if isLegacyAlertMirrorFinding(f) { + // has been removed; any "Active alert detected" finding + // still persisted from an earlier build is a stale mirror + // of an alert that already owns its own canonical surface. + // + // - Active mirrors are retired (auto-resolved with a + // clear reason) so the trust strip, regression counter, + // and health score stop reflecting the duplicate while + // the operator can still see why the finding closed. + // - Already-resolved mirrors are purged entirely. They + // were never canonical Patrol output and have no + // operator value in the Resolved tab; leaving them in + // place clutters the surface and inflates the + // "regressed" totals on the trust strip. + // + // Idempotent — after the first load with this code, no + // findings match the pattern. + if legacyAlertMirrorShape(f) { + if f.ResolvedAt != nil { + // Purge: do not rehydrate. The deletion is persisted + // by the scheduleSave() call after the load loop. + normalizedLoadedState = true + continue + } now := time.Now() prevLoopState := f.LoopState f.ResolvedAt = &now @@ -978,15 +993,17 @@ func findingHasBogusAutoResolveCycle(f *Finding) bool { return hasAutoResolve } -// isLegacyAlertMirrorFinding reports whether the finding looks like an -// active "Active alert detected" finding produced by the now-removed +// legacyAlertMirrorShape reports whether the finding looks like an +// "Active alert detected" finding produced by the now-removed // detectAlertSignals → SignalActiveAlert deterministic emitter. The title // is the only surface that ever produced exactly this string, so matching // on it is safe even without source/category checks; the additional -// checks tighten the match to avoid retiring a hypothetical operator- -// authored finding with the same name. -func isLegacyAlertMirrorFinding(f *Finding) bool { - if f == nil || f.ResolvedAt != nil { +// checks tighten the match to avoid touching a hypothetical operator- +// authored finding with the same name. The predicate is state-agnostic: +// callers branch on ResolvedAt to choose between active-retirement and +// resolved-purge behavior. +func legacyAlertMirrorShape(f *Finding) bool { + if f == nil { return false } if f.Title != "Active alert detected" { diff --git a/internal/ai/findings_coverage_test.go b/internal/ai/findings_coverage_test.go index 12be7a59b..c938df9ca 100644 --- a/internal/ai/findings_coverage_test.go +++ b/internal/ai/findings_coverage_test.go @@ -129,6 +129,7 @@ func TestFindingsStore_SetPersistence_RetiresLegacyAlertMirrorFindings(t *testin now := time.Now() saved := make(chan map[string]*Finding, 1) + resolvedAt := now.Add(-13 * time.Hour) p := &recordingPersistence{ findings: map[string]*Finding{ // Active "Active alert detected" finding from the removed @@ -142,6 +143,23 @@ func TestFindingsStore_SetPersistence_RetiresLegacyAlertMirrorFindings(t *testin Category: FindingCategoryGeneral, LastSeenAt: now, }, + // Already-resolved legacy mirror finding — must be purged + // from the in-memory store and from disk on next save. + // These have no canonical operator value and clutter the + // Resolved tab / inflate the regressed totals on the trust + // strip if left in place. + "legacy-mirror-resolved": { + ID: "legacy-mirror-resolved", + Severity: FindingSeverityWarning, + ResourceID: "vm-101", + Title: "Active alert detected", + Source: "ai-analysis", + Category: FindingCategoryGeneral, + LastSeenAt: resolvedAt, + ResolvedAt: &resolvedAt, + AutoResolved: true, + RegressionCount: 3, + }, // Distinct finding with matching title but different source — // should NOT be retired (defensive: never retire something we // can't positively identify as the alert-mirror artifact). @@ -199,6 +217,13 @@ func TestFindingsStore_SetPersistence_RetiresLegacyAlertMirrorFindings(t *testin t.Fatal("legacy mirror retirement must append an auto_resolved lifecycle event") } + // Already-resolved legacy mirror must be purged entirely. The + // public Get path applies suppression but Get on a non-existent ID + // returns nil; that's the contract we want. + if got := store.Get("legacy-mirror-resolved"); got != nil { + t.Fatalf("resolved legacy mirror must be purged from the store on load; got %+v", got) + } + foreign := store.Get("foreign-title-match") if foreign == nil { t.Fatal("expected foreign-title-match to load") @@ -220,6 +245,9 @@ func TestFindingsStore_SetPersistence_RetiresLegacyAlertMirrorFindings(t *testin if persisted["legacy-mirror"].ResolvedAt == nil { t.Fatal("retired mirror state must be persisted back") } + if _, stillPersisted := persisted["legacy-mirror-resolved"]; stillPersisted { + t.Fatal("purged resolved legacy mirror must not be persisted back to disk") + } case <-time.After(500 * time.Millisecond): t.Fatal("timed out waiting for retirement save") }