From 3c0b52c11dde8be32bd3558dc6f4bb48e4c9edbe Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 11 May 2026 11:09:03 +0100 Subject: [PATCH] Expose resolved findings to the Patrol Resolved tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trust strip on the Patrol page credits "N auto-resolved" but the Resolved tab next to it sat empty — operators could see the count but not click through to audit which findings had been resolved or by what mechanism. The /api/ai/patrol/findings endpoint only returned active findings, so the frontend filter (status === 'resolved' || 'dismissed' || 'snoozed') had nothing to render. Adds the audit-trail accessor end to end: - PatrolService.GetAllFindingsIncludingResolved returns active + resolved + dismissed + snoozed findings at warning severity or higher, sorted with active first then by severity then recency. Two separate severity orderings — filter (info=0..critical=3, used with >= against the warning floor) and sort (critical=0..info=3, used with < to surface critical first). Conflating them initially let watch findings leak through the warning floor; the test fixture catches that. - HandleGetPatrolFindings honors a new include_resolved=1 query parameter that routes to the new accessor. Default behaviour (active only) is unchanged for clients that just want the live findings list. - Frontend getPatrolFindings accepts an options object with includeResolved and loadPatrolFindings threads it through. - FindingsPanel triggers an includeResolved load whenever the Resolved filter becomes active for the Patrol-source view. Test: TestPatrolService_GetAllFindingsIncludingResolved_IncludesResolvedAndDismissedSortsActiveFirst covers active-first ordering, inclusion of resolved + dismissed, and the warning severity floor (watch-level findings must not leak through). --- frontend-modern/src/api/patrol.ts | 9 +- .../src/components/AI/FindingsPanel.tsx | 11 +++ frontend-modern/src/stores/aiIntelligence.ts | 4 +- internal/ai/patrol_findings.go | 58 +++++++++++ .../ai/patrol_findings_additional_test.go | 95 +++++++++++++++++++ internal/api/ai_handlers.go | 8 ++ 6 files changed, 181 insertions(+), 4 deletions(-) diff --git a/frontend-modern/src/api/patrol.ts b/frontend-modern/src/api/patrol.ts index 314d2229c..bc9c15606 100644 --- a/frontend-modern/src/api/patrol.ts +++ b/frontend-modern/src/api/patrol.ts @@ -236,8 +236,13 @@ export async function getPatrolStatus(): Promise { return apiFetchJSON('/api/ai/patrol/status'); } -export async function getPatrolFindings(): Promise { - const findings = await apiFetchJSON('/api/ai/patrol/findings'); +export async function getPatrolFindings( + options?: { includeResolved?: boolean }, +): Promise { + const path = options?.includeResolved + ? '/api/ai/patrol/findings?include_resolved=1' + : '/api/ai/patrol/findings'; + const findings = await apiFetchJSON(path); return arrayOrEmpty(findings).map((finding) => promoteLegacyAlertIdentifier(finding as Finding & { alert_identifier?: string }), ); diff --git a/frontend-modern/src/components/AI/FindingsPanel.tsx b/frontend-modern/src/components/AI/FindingsPanel.tsx index 525eb5a06..522a7da5e 100644 --- a/frontend-modern/src/components/AI/FindingsPanel.tsx +++ b/frontend-modern/src/components/AI/FindingsPanel.tsx @@ -178,6 +178,17 @@ export const FindingsPanel: Component = (props) => { } }); + // The Patrol findings endpoint defaults to active-only. When the + // operator switches to the Resolved tab, we need to fetch the full + // active+resolved+dismissed+snoozed set so the audit-trail filter has + // data to show. The trust strip credits "N auto-resolved" without + // this load and the Resolved tab is otherwise empty. + createEffect(() => { + if (filter() === 'resolved' && isPatrolFindingsSource()) { + void aiIntelligenceStore.loadPatrolFindings({ includeResolved: true }); + } + }); + // Filter and sort findings const hasUnknownRunSnapshot = createMemo( () => props.runSnapshot !== undefined && props.filterFindingIds === undefined, diff --git a/frontend-modern/src/stores/aiIntelligence.ts b/frontend-modern/src/stores/aiIntelligence.ts index a03c883c3..f59db57ed 100644 --- a/frontend-modern/src/stores/aiIntelligence.ts +++ b/frontend-modern/src/stores/aiIntelligence.ts @@ -442,11 +442,11 @@ export const aiIntelligenceStore = { } }, - async loadPatrolFindings() { + async loadPatrolFindings(options?: { includeResolved?: boolean }) { setPatrolFindingsLoading(true); setPatrolFindingsError(null); try { - const findings = await getPatrolFindings(); + const findings = await getPatrolFindings(options); const now = Date.now(); setPatrolFindings(findings.map((item) => normalizePatrolFindingRecord(item, now))); } catch (e) { diff --git a/internal/ai/patrol_findings.go b/internal/ai/patrol_findings.go index a5308d323..1e55129b2 100644 --- a/internal/ai/patrol_findings.go +++ b/internal/ai/patrol_findings.go @@ -658,6 +658,64 @@ func (p *PatrolService) GetAllFindings() []*Finding { return findings } +// GetAllFindingsIncludingResolved returns active + resolved + dismissed + +// snoozed findings at warning severity or higher, sorted by severity then +// recency. Used by the Resolved tab in the Patrol UI so operators can audit +// the auto_resolved set credited in the trust strip — without this path +// the strip surfaced "8 auto-resolved" but the operator could not click +// through to see what those eight were. +func (p *PatrolService) GetAllFindingsIncludingResolved() []*Finding { + all := p.findings.GetAll(nil) + normalizeFindingResourceTypes(all) + + // Two orderings — they look similar but they're inverse: + // filterOrder is "low-to-high severity" (info=0, ..., critical=3) so + // `>= minOrder` keeps everything at or above the floor. This is the + // same convention FindingsStore.GetActive uses. + // sortOrder is "high-to-low priority" (critical=0, ..., info=3) so a + // `<` comparison surfaces critical first in the result slice. + // Conflating the two earlier let watch-severity findings leak through + // the warning floor in the test, because watch's "sort priority" + // number was higher than warning's even though its severity is lower. + filterOrder := map[FindingSeverity]int{ + FindingSeverityInfo: 0, + FindingSeverityWatch: 1, + FindingSeverityWarning: 2, + FindingSeverityCritical: 3, + } + sortOrder := map[FindingSeverity]int{ + FindingSeverityCritical: 0, + FindingSeverityWarning: 1, + FindingSeverityWatch: 2, + FindingSeverityInfo: 3, + } + minOrder := filterOrder[FindingSeverityWarning] + + filtered := make([]*Finding, 0, len(all)) + for _, f := range all { + if filterOrder[f.Severity] >= minOrder { + filtered = append(filtered, f) + } + } + + sort.Slice(filtered, func(i, j int) bool { + // Active first, then resolved — operator typically wants to review + // the live set before drilling into history. + ai := filtered[i].IsActive() + aj := filtered[j].IsActive() + if ai != aj { + return ai + } + if sortOrder[filtered[i].Severity] != sortOrder[filtered[j].Severity] { + return sortOrder[filtered[i].Severity] < sortOrder[filtered[j].Severity] + } + // Most recent activity first within each bucket. + return filtered[i].LastSeenAt.After(filtered[j].LastSeenAt) + }) + + return filtered +} + func normalizeFindingResourceTypes(findings []*Finding) { for _, f := range findings { if f == nil { diff --git a/internal/ai/patrol_findings_additional_test.go b/internal/ai/patrol_findings_additional_test.go index 23d581823..12e05c5f3 100644 --- a/internal/ai/patrol_findings_additional_test.go +++ b/internal/ai/patrol_findings_additional_test.go @@ -1123,6 +1123,101 @@ func TestPatrolFindingCreatorAdapter_ResolveFinding_RejectsOutOfScopeFinding(t * } } +func TestPatrolService_GetAllFindingsIncludingResolved_IncludesResolvedAndDismissedSortsActiveFirst(t *testing.T) { + // GetAllFindingsIncludingResolved is the audit-trail accessor used by the + // Patrol UI's Resolved tab. Until it landed, the trust strip credited + // "N auto-resolved" but the operator could not click through to see + // what those N findings actually were — the /api/ai/patrol/findings + // endpoint only returned active findings. This test locks in: + // - resolved + dismissed findings are returned (not just active) + // - the warning-severity floor still applies (watch/info filtered) + // - active findings sort before resolved within the result + ps := NewPatrolService(nil, nil) + now := time.Now() + older := now.Add(-2 * time.Hour) + pastResolved := now.Add(-time.Hour) + + active := &Finding{ + ID: "active-warn", + Severity: FindingSeverityWarning, + Category: FindingCategoryReliability, + ResourceID: "vm-1", + ResourceName: "web", + Title: "Active warning", + DetectedAt: older, + LastSeenAt: now, + } + resolved := &Finding{ + ID: "resolved-warn", + Severity: FindingSeverityWarning, + Category: FindingCategoryBackup, + ResourceID: "vm-2", + ResourceName: "db", + Title: "Resolved backup failure", + DetectedAt: older, + LastSeenAt: older, + ResolvedAt: &pastResolved, + AutoResolved: true, + } + dismissed := &Finding{ + ID: "dismissed-warn", + Severity: FindingSeverityWarning, + Category: FindingCategoryGeneral, + ResourceID: "vm-3", + ResourceName: "cache", + Title: "Dismissed as expected", + DetectedAt: older, + LastSeenAt: older, + DismissedReason: "expected_behavior", + } + belowSeverity := &Finding{ + ID: "watch-noise", + Severity: FindingSeverityWatch, + Category: FindingCategoryPerformance, + ResourceID: "vm-4", + ResourceName: "noise", + Title: "Below warning floor", + DetectedAt: older, + LastSeenAt: now, + } + + ps.findings.Add(active) + ps.findings.Add(resolved) + ps.findings.Add(dismissed) + ps.findings.Add(belowSeverity) + + got := ps.GetAllFindingsIncludingResolved() + gotIDs := make([]string, 0, len(got)) + for _, f := range got { + gotIDs = append(gotIDs, f.ID) + } + + // Active-warn must be first (active before resolved/dismissed). + if len(got) == 0 || got[0].ID != "active-warn" { + t.Fatalf("expected active finding first, got order %v", gotIDs) + } + // Resolved and dismissed warnings must be present. + resolvedSeen := false + dismissedSeen := false + for _, f := range got { + if f.ID == "resolved-warn" { + resolvedSeen = true + } + if f.ID == "dismissed-warn" { + dismissedSeen = true + } + if f.ID == "watch-noise" { + t.Errorf("watch-severity finding leaked through warning floor: %v", gotIDs) + } + } + if !resolvedSeen { + t.Errorf("expected resolved finding in result, got %v", gotIDs) + } + if !dismissedSeen { + t.Errorf("expected dismissed finding in result, got %v", gotIDs) + } +} + func TestPatrolService_HasDeterministicVerifierForKey(t *testing.T) { ps := NewPatrolService(nil, nil) diff --git a/internal/api/ai_handlers.go b/internal/api/ai_handlers.go index c3999328b..9f97d6dee 100644 --- a/internal/api/ai_handlers.go +++ b/internal/api/ai_handlers.go @@ -5590,9 +5590,17 @@ func (h *AISettingsHandler) HandleGetPatrolFindings(w http.ResponseWriter, r *ht // Check for resource_id query parameter resourceID := r.URL.Query().Get("resource_id") + // include_resolved=1 returns active + resolved + dismissed + snoozed + // findings, so the Patrol UI's Resolved tab can audit the + // auto_resolved set credited in the trust strip. Default behaviour + // remains active-only for clients that just want to render the + // live findings list. + includeResolved := r.URL.Query().Get("include_resolved") == "1" var findings []*ai.Finding if resourceID != "" { findings = patrol.GetFindingsForResource(resourceID) + } else if includeResolved { + findings = patrol.GetAllFindingsIncludingResolved() } else { findings = patrol.GetAllFindings() }