From 590671ffbbce71dd7010f14cf350ba6552b45974 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 10 May 2026 21:39:09 +0100 Subject: [PATCH] Retire legacy "Active alert detected" findings on load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit removed the detectAlertSignals path so no NEW alert-mirror findings are emitted, but the findings already persisted from earlier builds stay in the store indefinitely — nothing cleans them up (reconcileStaleFindings is gated on performance/capacity categories, the LLM resolves them just to have them re-detected next run except now the deterministic emitter is gone so re-detection can't happen, but they're left sitting as active findings draining the trust strip and score). FindingsStore.SetPersistence now runs a one-shot retirement pass on load: any active finding with title "Active alert detected", source ai-analysis, and category general is auto-resolved with reason "Patrol no longer mirrors alerts; the Alerts page is the canonical surface for currently-firing alerts." The pass appends an auto_resolved lifecycle event so the retirement is auditable, syncs the loop state to resolved, and schedules a save so the cleanup persists. Idempotent: after the first load with this code, no findings match the signature so the pass is a no-op. Defensive: the signature requires all three fields (title + source + category) to match before retiring, so an operator-authored finding that happens to share the title is left untouched. Test covers the mirror case, the matching-title-but-foreign-source case (must NOT retire), and an unrelated active finding (must NOT retire), plus verifies the retired state persists back through the persistence layer. Updates ai-runtime Current State to record the migration path. --- .../v6/internal/subsystems/ai-runtime.md | 8 +- internal/ai/findings.go | 48 +++++++++ internal/ai/findings_coverage_test.go | 102 ++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletion(-) diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 84db3ca95..96e35d597 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -593,7 +593,13 @@ explicitly resolved the mirrored finding while the underlying alert kept firing. Patrol's job, per its own system prompt, is to surface issues alerts cannot — trends, capacity risks, misconfigurations, reliability gaps, cross-resource correlations. The Alerts page is -the canonical surface for currently-firing alerts. +the canonical surface for currently-firing alerts. To retire the +alert-mirror findings already persisted from an earlier build, +`FindingsStore.SetPersistence` runs a one-shot pass on load that +auto-resolves any active finding matching the legacy signature +(title `"Active alert detected"`, source `ai-analysis`, category +`general`) with a clear retirement reason; the pass is idempotent +and self-cleaning. The overall health score (`calculateOverallHealth` in `internal/ai/intelligence.go`) tiers the "recent Patrol errors" coverage diff --git a/internal/ai/findings.go b/internal/ai/findings.go index 3f68e51d7..ebfa1c4bf 100644 --- a/internal/ai/findings.go +++ b/internal/ai/findings.go @@ -861,6 +861,31 @@ func (s *FindingsStore) SetPersistence(p FindingsPersistence) error { if normalizeLoadedFinding(f) { normalizedLoadedState = true } + // One-shot retirement 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) { + now := time.Now() + prevLoopState := f.LoopState + f.ResolvedAt = &now + f.AutoResolved = true + f.ResolveReason = "Patrol no longer mirrors alerts; the Alerts page is the canonical surface for currently-firing alerts." + f.syncLoopState() + f.Lifecycle = append(f.Lifecycle, FindingLifecycleEvent{ + At: now, + Type: "auto_resolved", + Message: f.ResolveReason, + From: prevLoopState, + To: f.LoopState, + }) + normalizedLoadedState = true + } // Ensure derived fields are consistent after load. f.syncLoopState() s.findings[id] = f @@ -877,6 +902,29 @@ func (s *FindingsStore) SetPersistence(p FindingsPersistence) error { return nil } +// isLegacyAlertMirrorFinding reports whether the finding looks like an +// active "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 { + return false + } + if f.Title != "Active alert detected" { + return false + } + if f.Source != "ai-analysis" { + return false + } + if f.Category != FindingCategoryGeneral { + return false + } + return true +} + // scheduleSave schedules a debounced save operation // This method is lock-safe and can be called without holding the store lock. func (s *FindingsStore) scheduleSave() { diff --git a/internal/ai/findings_coverage_test.go b/internal/ai/findings_coverage_test.go index 8ff685b45..cc8ff3211 100644 --- a/internal/ai/findings_coverage_test.go +++ b/internal/ai/findings_coverage_test.go @@ -123,6 +123,108 @@ func TestFindingsStore_SetPersistence_NormalizesRegressedAcknowledgementState(t } } +func TestFindingsStore_SetPersistence_RetiresLegacyAlertMirrorFindings(t *testing.T) { + store := NewFindingsStore() + store.saveDebounce = 5 * time.Millisecond + now := time.Now() + saved := make(chan map[string]*Finding, 1) + + p := &recordingPersistence{ + findings: map[string]*Finding{ + // Active "Active alert detected" finding from the removed + // SignalActiveAlert path — must be retired on load. + "legacy-mirror": { + ID: "legacy-mirror", + Severity: FindingSeverityWarning, + ResourceID: "vm-100", + Title: "Active alert detected", + Source: "ai-analysis", + Category: FindingCategoryGeneral, + LastSeenAt: now, + }, + // 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). + "foreign-title-match": { + ID: "foreign-title-match", + Severity: FindingSeverityWarning, + ResourceID: "vm-200", + Title: "Active alert detected", + Source: "operator", + Category: FindingCategoryGeneral, + LastSeenAt: now, + }, + // Different finding — must remain active untouched. + "unrelated": { + ID: "unrelated", + Severity: FindingSeverityCritical, + ResourceID: "vm-300", + Title: "Disk nearly full", + Source: "ai-analysis", + Category: FindingCategoryCapacity, + LastSeenAt: now, + }, + }, + saved: saved, + } + + if err := store.SetPersistence(p); err != nil { + t.Fatalf("SetPersistence failed: %v", err) + } + + mirror := store.Get("legacy-mirror") + if mirror == nil { + t.Fatal("expected legacy mirror finding to load") + } + if mirror.ResolvedAt == nil { + t.Fatal("legacy mirror finding must be retired (ResolvedAt set) on load") + } + if !mirror.AutoResolved { + t.Fatal("legacy mirror finding must be marked AutoResolved") + } + if mirror.ResolveReason == "" { + t.Fatal("legacy mirror finding must carry a retirement reason") + } + if mirror.IsActive() { + t.Fatal("legacy mirror finding must not stay active") + } + foundAutoResolvedEvent := false + for _, e := range mirror.Lifecycle { + if e.Type == "auto_resolved" { + foundAutoResolvedEvent = true + break + } + } + if !foundAutoResolvedEvent { + t.Fatal("legacy mirror retirement must append an auto_resolved lifecycle event") + } + + foreign := store.Get("foreign-title-match") + if foreign == nil { + t.Fatal("expected foreign-title-match to load") + } + if foreign.ResolvedAt != nil { + t.Fatalf("foreign-title-match must not be retired by the alert-mirror migration; resolved at %s", foreign.ResolvedAt) + } + + unrelated := store.Get("unrelated") + if unrelated == nil { + t.Fatal("expected unrelated finding to load") + } + if unrelated.ResolvedAt != nil { + t.Fatal("unrelated finding must not be retired") + } + + select { + case persisted := <-saved: + if persisted["legacy-mirror"].ResolvedAt == nil { + t.Fatal("retired mirror state must be persisted back") + } + case <-time.After(500 * time.Millisecond): + t.Fatal("timed out waiting for retirement save") + } +} + func TestFindingsStore_scheduleSave_NoPersistence(t *testing.T) { store := NewFindingsStore()