Auto-clear event/persistent Patrol findings on affirmative deterministic verification

The finding lifecycle was asymmetric around the deterministic-resolve
gate: the gate correctly blocks LLM resolves of event/persistent
findings (backup, reliability, security, general) when the verifier
still detects the signal, but nothing ever cleared such a finding when
the underlying issue WAS fixed — absence-based stale auto-resolve only
covers performance/capacity, so a fixed backup stayed an active finding
indefinitely unless the LLM happened to call patrol_resolve_finding.

reconcileStaleFindings now runs the deterministic verifier for seeded,
unreported event/persistent findings whose key has one, and resolves
ONLY on an affirmative "signal gone" verification. Still-present or
inconclusive results fail closed (same standard as the resolve gate);
verifications are capped per run (3) with deferred candidates logged
and retried next run. Test seam: PatrolService.verifyFixResolvedFn.

Two enabling repairs found during verification:
- hasDeterministicVerifierForKey listed 2 of the 7 keys the
  verifyFixDeterministically dispatch handles, so the LLM-resolve gate
  silently skipped existing verifiers for backup-stale and
  guest-unreachable findings. Now aligned; documented as the single
  source of truth for both consumers.
- The finding-key vocabulary was forked: the patrol_report_finding tool
  suggested keys (high-cpu, ...) the verifier switch never matched, so
  verification rarely engaged for new findings. normalizeFindingKey now
  aliases unambiguous directional synonyms onto the canonical verifier
  vocabulary (high-cpu -> cpu-high etc.; pbs-job-failed and node-offline
  deliberately NOT aliased — different resource models), and the tool
  description teaches the canonical keys.

NOT changed: the synthetic ai-patrol-error finding — verification showed
the reported "accumulation" is a non-problem (deterministic ID, store
merges repeats as heartbeats, successful runs auto-resolve it).

Teeth: 8 new tests in patrol_reconcile_test.go including the
idempotence invariant (repeat reconcile over unchanged state produces
zero resolves and zero lifecycle growth) and the cap-defers case.
Contract: verified stale-resolve clause added beside the
deterministic-resolve-gate in ai-runtime.md. Full internal/ai tree green.
This commit is contained in:
rcourtman 2026-06-09 21:47:14 +01:00
parent b707512e38
commit ab9552716c
9 changed files with 311 additions and 21 deletions

File diff suppressed because one or more lines are too long

View file

@ -36,6 +36,7 @@
package ai
import (
"context"
"encoding/json"
"strings"
"sync"
@ -453,6 +454,11 @@ type PatrolService struct {
incidentStore *memory.IncidentStore // For incident timeline capture
alertResolver AlertResolver // For AI-based alert resolution
// verifyFixResolvedFn is the deterministic-verification seam used by the
// stale-finding reconcile pass. Tests inject a stub; nil means use the
// real VerifyFixResolved.
verifyFixResolvedFn func(ctx context.Context, resourceID, resourceType, findingKey, findingID string) (bool, error)
// Unified resource provider — reads physical disks, Ceph, etc. from canonical model
unifiedResourceProvider UnifiedResourceProvider

View file

@ -3547,6 +3547,35 @@ func seedFormatTimeAgo(now, t time.Time) string {
// a security vulnerability does not mean the issue has been addressed, and silent
// auto-resolve there produced bogus auto_resolved → re-detected → regressed
// cycles that inflated the trust strip and the regression counter.
//
// maxVerifiedStaleResolvesPerRun bounds how many deterministic verifications
// one reconcile pass may run. Verifications hit live state (and for
// smart-failure/backup-failed, governed tool calls), so a pathological store
// must not turn the end-of-run reconcile into a verification storm. Deferred
// candidates are logged and retried on the next successful full patrol.
const maxVerifiedStaleResolvesPerRun = 3
// verifiedStaleResolveReason is the resolve reason for event/persistent
// findings cleared by an affirmative deterministic verification (as opposed
// to the absence-based "No longer detected by patrol" used for
// performance/capacity findings).
const verifiedStaleResolveReason = "Deterministic verification confirmed the issue cleared"
// verifyStaleFindingResolved runs the deterministic verifier for a finding's
// key with the same timeout the LLM-resolve gate uses. The injectable seam
// (verifyFixResolvedFn) exists for tests; production uses VerifyFixResolved.
func (p *PatrolService) verifyStaleFindingResolved(finding *Finding) (bool, error) {
p.mu.RLock()
verify := p.verifyFixResolvedFn
p.mu.RUnlock()
if verify == nil {
verify = p.VerifyFixResolved
}
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
return verify(ctx, finding.ResourceID, finding.ResourceType, finding.Key, finding.ID)
}
func (p *PatrolService) reconcileStaleFindings(reportedIDs, resolvedIDs, seededFindingIDs []string, runHadErrors bool) int {
if runHadErrors {
return 0
@ -3569,18 +3598,65 @@ func (p *PatrolService) reconcileStaleFindings(reportedIDs, resolvedIDs, seededF
}
count := 0
verifyAttempts := 0
for _, id := range seededFindingIDs {
if reported[id] || resolved[id] {
continue
}
// Category gate: only continuous current-state categories are safe to
// auto-resolve from absence. Event/persistent categories stay active
// until explicitly resolved.
// until explicitly resolved — unless a deterministic verifier can
// affirmatively confirm the underlying signal is gone.
finding := p.findings.Get(id)
if finding == nil {
continue
}
if !CategorySupportsStaleAutoResolve(finding.Category) {
// Absence of a re-report is not evidence for these categories,
// but an affirmative deterministic verification is — the same
// standard the LLM-resolve gate demands before honoring a
// patrol_resolve_finding call. Without this pass, a fixed backup
// or recovered service stays an active finding forever unless
// the LLM happens to call resolve: the asymmetric half of the
// "Backup failed" flap. Resolve only on a positive "signal gone"
// verification; on "still present" or inconclusive, fail closed
// and leave the finding active.
if p.hasDeterministicVerifierForKey(finding.Key) {
if verifyAttempts >= maxVerifiedStaleResolvesPerRun {
log.Info().
Str("finding_id", id).
Str("key", finding.Key).
Msg("AI Patrol: deferring verified stale-finding check — per-run verification cap reached, will retry next run")
continue
}
verifyAttempts++
verified, verifyErr := p.verifyStaleFindingResolved(finding)
if verifyErr == nil && verified {
if ok := p.findings.ResolveWithReason(id, verifiedStaleResolveReason); ok {
count++
p.mu.RLock()
resolveUnified := p.unifiedFindingResolver
p.mu.RUnlock()
if resolveUnified != nil {
resolveUnified(id)
}
log.Info().
Str("finding_id", id).
Str("category", string(finding.Category)).
Str("key", finding.Key).
Msg("AI Patrol: Auto-resolved event/persistent finding — deterministic verification confirmed the issue cleared")
}
continue
}
log.Debug().
Err(verifyErr).
Str("finding_id", id).
Str("category", string(finding.Category)).
Str("key", finding.Key).
Bool("still_present", verifyErr == nil && !verified).
Msg("AI Patrol: keeping event/persistent finding active — deterministic verification did not confirm resolution (fail closed)")
continue
}
log.Debug().
Str("finding_id", id).
Str("category", string(finding.Category)).

View file

@ -938,6 +938,18 @@ func (a *patrolFindingCreatorAdapter) getResolvedIDs() []string {
return result
}
// findingKeyAliases maps directional synonyms the LLM plausibly assigns onto
// the canonical verifier vocabulary (the verifyFixDeterministically switch),
// so deduplication and deterministic verification meet on one key. Only
// unambiguous aliases belong here — node-offline is NOT guest-unreachable
// and pbs-job-failed is NOT backup-failed; mapping those would point the
// verifier at the wrong resource model.
var findingKeyAliases = map[string]string{
"high-cpu": "cpu-high",
"high-memory": "memory-high",
"high-disk": "disk-high",
}
func normalizeFindingKey(key string) string {
if key == "" {
return ""
@ -954,7 +966,11 @@ func normalizeFindingKey(key string) string {
b.WriteRune(r)
}
}
return strings.Trim(b.String(), "-")
normalized := strings.Trim(b.String(), "-")
if canonical, ok := findingKeyAliases[normalized]; ok {
return canonical
}
return normalized
}
// recoverStuckInvestigations detects findings stuck in "running" state for longer than
@ -1365,13 +1381,18 @@ func (p *PatrolService) verifyFixDeterministically(
}
// hasDeterministicVerifierForKey reports whether a deterministic
// verifier exists for the given finding key. Used by ResolveFinding to
// decide whether an LLM-driven resolve attempt on an event/persistent
// category finding can be grounded in deterministic evidence. Keep this
// list aligned with the switch in verifyFixDeterministically.
// verifier exists for the given finding key. It is the single source of
// truth consulted by both the LLM-resolve gate (ResolveFinding) and the
// verified stale-finding reconcile pass (reconcileStaleFindings), and it
// must stay aligned with the dispatch switch in verifyFixDeterministically.
// It previously listed only smart-failure and backup-failed despite the
// dispatch handling seven keys, which made the resolve gate silently skip
// verification that existed (trust-the-LLM fallback) for backup-stale and
// guest-unreachable findings.
func (p *PatrolService) hasDeterministicVerifierForKey(key string) bool {
switch key {
case "smart-failure", "backup-failed":
switch normalizeFindingKey(key) {
case "backup-stale", "cpu-high", "memory-high", "disk-high",
"guest-unreachable", "smart-failure", "backup-failed":
return true
default:
return false

View file

@ -1175,16 +1175,22 @@ func TestPatrolService_GetAllFindingsIncludingResolved_IncludesResolvedAndDismis
func TestPatrolService_HasDeterministicVerifierForKey(t *testing.T) {
ps := NewPatrolService(nil, nil)
// Keys with deterministic verifiers (mirrors the switch in
// verifyFixDeterministically). Keep this test in lockstep with that
// switch — the gate in ResolveFinding depends on the two staying
// aligned.
for _, key := range []string{"smart-failure", "backup-failed"} {
// Keys with deterministic verifiers (mirrors the FULL dispatch switch in
// verifyFixDeterministically — hasDeterministicVerifierForKey is the
// single source of truth for both the LLM-resolve gate and the verified
// stale-finding reconcile pass; it previously listed only the two
// tool-based keys and silently skipped verification that existed for
// the state-only verifiers). Alias keys normalize onto canonical keys.
for _, key := range []string{
"backup-stale", "cpu-high", "memory-high", "disk-high",
"guest-unreachable", "smart-failure", "backup-failed",
"high-cpu", "high-memory", "high-disk",
} {
if !ps.hasDeterministicVerifierForKey(key) {
t.Errorf("hasDeterministicVerifierForKey(%q) = false, want true", key)
}
}
for _, key := range []string{"", "unknown-key", "cpu-high", "node-offline", "service-restart"} {
for _, key := range []string{"", "unknown-key", "node-offline", "service-restart", "pbs-job-failed", "restart-loop"} {
if ps.hasDeterministicVerifierForKey(key) {
t.Errorf("hasDeterministicVerifierForKey(%q) = true, want false", key)
}

View file

@ -138,17 +138,19 @@ func TestIntegration_PatrolRunCreatesFindings(t *testing.T) {
time.Sleep(20 * time.Millisecond)
}
// Verify finding was created
// Verify finding was created. The LLM stub reports key "high-cpu",
// which normalizes onto the canonical verifier vocabulary ("cpu-high")
// at creation so dedup and deterministic verification share one key.
allFindings := ps.findings.GetActive(FindingSeverityInfo)
found := false
for _, f := range allFindings {
if f.ResourceID == "vm-100" && f.Key == "high-cpu" {
if f.ResourceID == "vm-100" && f.Key == "cpu-high" {
found = true
break
}
}
if !found {
t.Fatalf("expected finding for vm-100 with key 'high-cpu', got %d active findings", len(allFindings))
t.Fatalf("expected finding for vm-100 with key 'cpu-high', got %d active findings", len(allFindings))
}
// Verify investigation was triggered (check with timeout due to async goroutine)

View file

@ -1,6 +1,8 @@
package ai
import (
"context"
"fmt"
"testing"
"time"
)
@ -284,3 +286,178 @@ func TestCategorySupportsStaleAutoResolve(t *testing.T) {
})
}
}
// --- Verified auto-clear of event/persistent findings ---------------------
//
// Event/persistent categories must not auto-resolve from absence, but an
// affirmative deterministic verification ("the failure signal is gone") is
// stronger evidence than absence — the same standard the LLM-resolve gate
// demands. These tests pin the asymmetric half of the "Backup failed" flap
// fix: a fixed issue clears once the verifier confirms it, and anything
// short of an affirmative confirmation fails closed.
func newVerifierFinding(id, key string) *Finding {
return &Finding{
ID: id,
Key: key,
Severity: FindingSeverityWarning,
Category: FindingCategoryBackup,
ResourceID: "vm-100",
ResourceName: "web",
Title: "Backup failed",
DetectedAt: time.Now().Add(-1 * time.Hour),
}
}
func TestReconcileStaleFindings_VerifiedAutoClearResolvesFixedFinding(t *testing.T) {
f := newVerifierFinding("find-backup", "backup-failed")
p := newTestPatrolWithFindings([]*Finding{f})
p.verifyFixResolvedFn = func(ctx context.Context, resourceID, resourceType, findingKey, findingID string) (bool, error) {
if resourceID != "vm-100" || findingKey != "backup-failed" || findingID != "find-backup" {
t.Fatalf("verifier called with wrong identity: %s/%s/%s", resourceID, findingKey, findingID)
}
return true, nil // signal affirmatively gone
}
resolved := p.reconcileStaleFindings(nil, nil, []string{f.ID}, false)
if resolved != 1 {
t.Fatalf("expected 1 verified auto-resolve, got %d", resolved)
}
stored := p.findings.Get(f.ID)
if stored == nil || stored.ResolvedAt == nil {
t.Fatal("finding should be resolved after affirmative verification")
}
if !stored.AutoResolved {
t.Fatal("verified clear should be marked auto-resolved")
}
if stored.ResolveReason != verifiedStaleResolveReason {
t.Fatalf("unexpected resolve reason: %s", stored.ResolveReason)
}
}
func TestReconcileStaleFindings_VerifierStillDetectsKeepsActive(t *testing.T) {
f := newVerifierFinding("find-backup", "backup-failed")
p := newTestPatrolWithFindings([]*Finding{f})
p.verifyFixResolvedFn = func(ctx context.Context, resourceID, resourceType, findingKey, findingID string) (bool, error) {
return false, nil // failure signal still present
}
if resolved := p.reconcileStaleFindings(nil, nil, []string{f.ID}, false); resolved != 0 {
t.Fatalf("expected 0 resolves while signal still present, got %d", resolved)
}
if stored := p.findings.Get(f.ID); stored == nil || stored.ResolvedAt != nil {
t.Fatal("finding must stay active while the failure signal is still present")
}
}
func TestReconcileStaleFindings_VerifierInconclusiveFailsClosed(t *testing.T) {
f := newVerifierFinding("find-backup", "backup-failed")
p := newTestPatrolWithFindings([]*Finding{f})
p.verifyFixResolvedFn = func(ctx context.Context, resourceID, resourceType, findingKey, findingID string) (bool, error) {
return false, context.DeadlineExceeded // inconclusive
}
if resolved := p.reconcileStaleFindings(nil, nil, []string{f.ID}, false); resolved != 0 {
t.Fatalf("expected 0 resolves on inconclusive verification, got %d", resolved)
}
if stored := p.findings.Get(f.ID); stored == nil || stored.ResolvedAt != nil {
t.Fatal("finding must stay active when verification is inconclusive (fail closed)")
}
}
func TestReconcileStaleFindings_NoVerifierEventFindingStaysActive(t *testing.T) {
// pbs-job-failed has no deterministic verifier and must not be mapped
// onto backup-failed; the finding stays active until LLM/operator resolve.
f := newVerifierFinding("find-pbs", "pbs-job-failed")
p := newTestPatrolWithFindings([]*Finding{f})
p.verifyFixResolvedFn = func(ctx context.Context, resourceID, resourceType, findingKey, findingID string) (bool, error) {
t.Fatal("verifier must not be called for keys without a deterministic verifier")
return false, nil
}
if resolved := p.reconcileStaleFindings(nil, nil, []string{f.ID}, false); resolved != 0 {
t.Fatalf("expected 0 resolves for verifier-less event finding, got %d", resolved)
}
if stored := p.findings.Get(f.ID); stored == nil || stored.ResolvedAt != nil {
t.Fatal("verifier-less event finding must stay active")
}
}
func TestReconcileStaleFindings_VerifiedAutoClearIsIdempotent(t *testing.T) {
// The no-noise invariant: repeating reconcile over unchanged state must
// not produce new resolves, lifecycle events, or resolve/re-detect cycles.
f := newVerifierFinding("find-backup", "backup-failed")
p := newTestPatrolWithFindings([]*Finding{f})
p.verifyFixResolvedFn = func(ctx context.Context, resourceID, resourceType, findingKey, findingID string) (bool, error) {
return true, nil
}
if first := p.reconcileStaleFindings(nil, nil, []string{f.ID}, false); first != 1 {
t.Fatalf("expected first pass to resolve 1, got %d", first)
}
stored := p.findings.Get(f.ID)
resolvedAt := *stored.ResolvedAt
lifecycleLen := len(stored.Lifecycle)
if second := p.reconcileStaleFindings(nil, nil, []string{f.ID}, false); second != 0 {
t.Fatalf("expected second pass over unchanged state to resolve 0, got %d", second)
}
stored = p.findings.Get(f.ID)
if stored.ResolvedAt == nil || !stored.ResolvedAt.Equal(resolvedAt) {
t.Fatal("second pass must not touch the resolution")
}
if len(stored.Lifecycle) != lifecycleLen {
t.Fatalf("second pass must not append lifecycle events: %d -> %d", lifecycleLen, len(stored.Lifecycle))
}
}
func TestReconcileStaleFindings_VerificationCapDefersExcessCandidates(t *testing.T) {
findings := []*Finding{
newVerifierFinding("find-1", "backup-failed"),
newVerifierFinding("find-2", "backup-failed"),
newVerifierFinding("find-3", "backup-failed"),
newVerifierFinding("find-4", "backup-failed"),
}
// Distinct resources so the store keeps all four.
for i, f := range findings {
f.ResourceID = fmt.Sprintf("vm-%d", 100+i)
}
p := newTestPatrolWithFindings(findings)
calls := 0
p.verifyFixResolvedFn = func(ctx context.Context, resourceID, resourceType, findingKey, findingID string) (bool, error) {
calls++
return true, nil
}
ids := []string{"find-1", "find-2", "find-3", "find-4"}
resolved := p.reconcileStaleFindings(nil, nil, ids, false)
if calls != maxVerifiedStaleResolvesPerRun {
t.Fatalf("expected verification calls capped at %d, got %d", maxVerifiedStaleResolvesPerRun, calls)
}
if resolved != maxVerifiedStaleResolvesPerRun {
t.Fatalf("expected %d resolves this run, got %d", maxVerifiedStaleResolvesPerRun, resolved)
}
deferred := p.findings.Get("find-4")
if deferred == nil || deferred.ResolvedAt != nil {
t.Fatal("candidate beyond the cap must stay active and retry next run")
}
}
func TestNormalizeFindingKey_CanonicalAliases(t *testing.T) {
cases := map[string]string{
"high-cpu": "cpu-high",
"High_Memory": "memory-high",
"high disk": "disk-high",
"cpu-high": "cpu-high",
"backup-stale": "backup-stale",
// Non-aliased keys pass through normalization unchanged.
"pbs-job-failed": "pbs-job-failed",
}
for in, want := range cases {
if got := normalizeFindingKey(in); got != want {
t.Fatalf("normalizeFindingKey(%q) = %q, want %q", in, got, want)
}
}
}

View file

@ -1164,9 +1164,11 @@ func TestNormalizeFindingKey(t *testing.T) {
expected: "cpu100warning",
},
{
// Trim applies before alias lookup, so the trimmed key still
// canonicalizes onto the verifier vocabulary (high-cpu → cpu-high).
name: "leading/trailing whitespace",
input: " high-cpu ",
expected: "high-cpu",
expected: "cpu-high",
},
{
name: "with numbers",
@ -1176,7 +1178,7 @@ func TestNormalizeFindingKey(t *testing.T) {
{
name: "leading/trailing dashes trimmed",
input: "-high-cpu-",
expected: "high-cpu",
expected: "cpu-high",
},
}

View file

@ -25,7 +25,7 @@ Returns: {"ok": true, "finding_id": "...", "is_new": true/false} on success.`,
Properties: map[string]PropertySchema{
"key": {
Type: "string",
Description: "Stable issue key for deduplication (e.g. high-cpu, high-memory, high-disk, backup-stale, backup-never, storage-high-usage, node-offline, restart-loop, pbs-job-failed)",
Description: "Stable issue key for deduplication. Use the canonical key when one fits — cpu-high, memory-high, disk-high, backup-stale, backup-failed, smart-failure, guest-unreachable (these have deterministic verifiers that ground resolution) — otherwise a stable kebab-case key like backup-never, storage-high-usage, node-offline, restart-loop, pbs-job-failed.",
},
"severity": {
Type: "string",