Retire legacy "Active alert detected" findings on load

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.
This commit is contained in:
rcourtman
2026-05-10 21:39:09 +01:00
parent 271d12ecab
commit 590671ffbb
3 changed files with 157 additions and 1 deletions
@@ -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
+48
View File
@@ -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() {
+102
View File
@@ -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()