From a46707098011606411db2fd6801dcb41e4052cf2 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 25 Mar 2026 17:38:28 +0000 Subject: [PATCH] Normalize stale Patrol acknowledgement state on load The findings store now clears acknowledgement timestamps that predate a recorded regression when persisted findings are loaded, persists the normalized state, and keeps regression lifecycle metadata explicit. --- .../v6/internal/subsystems/ai-runtime.md | 5 ++- internal/ai/findings.go | 20 +++++++++ internal/ai/findings_coverage_test.go | 45 +++++++++++++++++++ 3 files changed, 69 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 fb1074632..4158a9037 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -511,7 +511,10 @@ Patrol actually succeeds or resolves them for a Patrol-owned reason. The shared findings lifecycle must also treat a regressed issue as a new active occurrence. When a resolved finding reappears, `internal/ai/findings.go` must clear any stale acknowledgement timestamp from the prior occurrence instead of -carrying that acknowledgement forward onto the regressed active issue. +carrying that acknowledgement forward onto the regressed active issue. The +same owner must normalize already-persisted active findings on load when a +stored acknowledgement predates the last recorded regression, then persist the +cleaned state back through the canonical findings store. AI chat tool-name labels, pending-tool headers, and assistant status copy now also route through the shared frontend identifier-label helper, so the chat surfaces do not keep their own underscore-stripping behavior separate from diff --git a/internal/ai/findings.go b/internal/ai/findings.go index 47545a152..ceafd53ec 100644 --- a/internal/ai/findings.go +++ b/internal/ai/findings.go @@ -658,6 +658,7 @@ func (s *FindingsStore) SetPersistence(p FindingsPersistence) error { // Reset derived indices/caches before rehydrating. s.byResource = make(map[string][]string) s.activeCounts = make(map[FindingSeverity]int) + normalizedLoadedState := false if rules != nil { s.suppressionRules = make(map[string]*SuppressionRule, len(rules)) for id, r := range rules { @@ -672,6 +673,9 @@ func (s *FindingsStore) SetPersistence(p FindingsPersistence) error { if f == nil { continue } + if normalizeLoadedFinding(f) { + normalizedLoadedState = true + } // Ensure derived fields are consistent after load. f.syncLoopState() s.findings[id] = f @@ -681,6 +685,9 @@ func (s *FindingsStore) SetPersistence(p FindingsPersistence) error { } } s.mu.Unlock() + if normalizedLoadedState { + s.scheduleSave() + } } return nil } @@ -866,6 +873,19 @@ func isAllowedLoopTransition(from, to string) bool { } } +func normalizeLoadedFinding(f *Finding) bool { + if f == nil { + return false + } + + if f.ResolvedAt == nil && f.LastRegressionAt != nil && f.AcknowledgedAt != nil && !f.AcknowledgedAt.After(*f.LastRegressionAt) { + f.AcknowledgedAt = nil + return true + } + + return false +} + // syncLoopStateLocked recomputes loop state and records a lifecycle event if it changed. // Caller must hold s.mu. func (s *FindingsStore) syncLoopStateLocked(f *Finding) { diff --git a/internal/ai/findings_coverage_test.go b/internal/ai/findings_coverage_test.go index a0d109c2c..8ff685b45 100644 --- a/internal/ai/findings_coverage_test.go +++ b/internal/ai/findings_coverage_test.go @@ -78,6 +78,51 @@ func TestFindingsStore_SetPersistence_LoadsFindings(t *testing.T) { } } +func TestFindingsStore_SetPersistence_NormalizesRegressedAcknowledgementState(t *testing.T) { + store := NewFindingsStore() + store.saveDebounce = 5 * time.Millisecond + now := time.Now() + acknowledgedAt := now.Add(-2 * time.Hour) + lastRegressionAt := now.Add(-time.Hour) + saved := make(chan map[string]*Finding, 1) + + p := &recordingPersistence{ + findings: map[string]*Finding{ + "regressed": { + ID: "regressed", + Severity: FindingSeverityWarning, + ResourceID: "res-3", + Title: "Regressed", + LastSeenAt: now, + AcknowledgedAt: &acknowledgedAt, + LastRegressionAt: &lastRegressionAt, + }, + }, + saved: saved, + } + + if err := store.SetPersistence(p); err != nil { + t.Fatalf("SetPersistence failed: %v", err) + } + + got := store.Get("regressed") + if got == nil { + t.Fatal("expected finding to load") + } + if got.AcknowledgedAt != nil { + t.Fatal("expected stale acknowledgement to be cleared on load") + } + + select { + case persisted := <-saved: + if persisted["regressed"].AcknowledgedAt != nil { + t.Fatal("expected normalized acknowledgement state to be persisted") + } + case <-time.After(200 * time.Millisecond): + t.Fatal("timed out waiting for normalized findings save") + } +} + func TestFindingsStore_scheduleSave_NoPersistence(t *testing.T) { store := NewFindingsStore()