From 44b85f754ba415854d19cc58c9cbb5f893404f43 Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Wed, 26 Aug 2026 23:12:02 +0100
Subject: [PATCH] fix(alerts): re-stamp lifecycle first-matched time when a new
 confirmation run starts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The slice-2 first-matched preservation kept an entry as long as one
existed, but several callers reset the confirmation-count maps directly
without the evaluator path — clearResourceOfflineAlert among them — so
a stale entry from a prior run backdated the next run's alert to the
previous run's first observation. Stamp the first-matched time whenever
the pre-evaluation count is zero, making stale entries harmless at
every reset site. Found by the recovery-gate parity harness.
---
 internal/alerts/canonical_lifecycle.go        |  8 ++-
 .../unified_incident_confirmation_test.go     | 64 +++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/internal/alerts/canonical_lifecycle.go b/internal/alerts/canonical_lifecycle.go
index 6f94602b7..be86323da 100644
--- a/internal/alerts/canonical_lifecycle.go
+++ b/internal/alerts/canonical_lifecycle.go
@@ -390,7 +390,13 @@ func (m *Manager) evaluateCanonicalLifecycleAlert(params canonicalLifecycleAlert
 	if params.Tracking != nil {
 		if result.State.ConsecutiveMatches > 0 {
 			params.Tracking[params.TrackingKey] = result.State.ConsecutiveMatches
-			if _, tracked := m.lifecycleFirstMatched[params.TrackingKey]; !tracked && !result.State.FirstMatchedAt.IsZero() {
+			// Stamp the first-matched time whenever this observation starts
+			// a new confirmation run (the pre-evaluation count was zero).
+			// Several callers reset the count maps directly without going
+			// through this path — clearResourceOfflineAlert among them — so
+			// a keep-if-present guard would let a stale entry from a prior
+			// run backdate the next run's alert.
+			if confirmations == 0 && !result.State.FirstMatchedAt.IsZero() {
 				m.lifecycleFirstMatched[params.TrackingKey] = result.State.FirstMatchedAt
 			}
 		} else {
diff --git a/internal/alerts/unified_incident_confirmation_test.go b/internal/alerts/unified_incident_confirmation_test.go
index 123f64263..2151bc868 100644
--- a/internal/alerts/unified_incident_confirmation_test.go
+++ b/internal/alerts/unified_incident_confirmation_test.go
@@ -226,3 +226,67 @@ func confirmedProviderIncidentResource(observedAt time.Time) unifiedresources.Re
 		}},
 	}
 }
+
+// Regression: several callers reset the confirmation-count maps directly
+// without the evaluator path (clearResourceOfflineAlert among them), which
+// left a stale lifecycleFirstMatched entry that backdated the next run's
+// alert to the previous run's first observation. A new run (pre-evaluation
+// count zero) must re-stamp the first-matched time.
+func TestLifecycleFirstMatchedRestampsAfterDirectCountReset(t *testing.T) {
+	manager := NewManagerWithDataDir(t.TempDir(), WithoutPersistedAlertRestore())
+	t.Cleanup(manager.Stop)
+
+	manager.mu.Lock()
+	manager.config.Enabled = true
+	manager.mu.Unlock()
+
+	epoch := time.Date(2026, 8, 26, 12, 0, 0, 0, time.UTC)
+	observe := func(observed string, at time.Time) {
+		spec, err := buildCanonicalDiscreteStateSpec(
+			"node-9", "node-9", unifiedresources.ResourceTypeAgent,
+			AlertLevelCritical, 3, false, "connectivity", []string{"offline"},
+		)
+		if err != nil {
+			t.Fatalf("build spec: %v", err)
+		}
+		if _, ok := manager.evaluateCanonicalLifecycleAlert(canonicalLifecycleAlertParams{
+			Spec: spec,
+			Evidence: alertspecs.AlertEvidence{
+				ObservedAt:    at,
+				DiscreteState: &alertspecs.DiscreteStateEvidence{StateKey: "connectivity", Observed: observed},
+			},
+			Tracking:     manager.offlineConfirmations,
+			TrackingKey:  "conn:node-9",
+			AlertID:      canonicalDiscreteStateStateID("node-9", "connectivity"),
+			AlertType:    "connectivity",
+			ResourceID:   "node-9",
+			ResourceName: "node-9",
+			Message:      "node offline",
+		}); !ok {
+			t.Fatal("evaluation rejected")
+		}
+	}
+
+	// Two matches begin a run, then the count is reset directly, the way
+	// the healthy-poll paths do — leaving lifecycleFirstMatched stale.
+	observe("offline", epoch)
+	observe("offline", epoch.Add(30*time.Second))
+	manager.mu.Lock()
+	delete(manager.offlineConfirmations, "conn:node-9")
+	manager.mu.Unlock()
+
+	restart := epoch.Add(2 * time.Minute)
+	observe("offline", restart)
+	observe("offline", restart.Add(30*time.Second))
+	observe("offline", restart.Add(60*time.Second))
+
+	manager.mu.Lock()
+	alert, exists := manager.getActiveAlertNoLock(canonicalDiscreteStateStateID("node-9", "connectivity"))
+	manager.mu.Unlock()
+	if !exists || alert == nil {
+		t.Fatal("expected alert to fire after the second run")
+	}
+	if !alert.StartTime.Equal(restart) {
+		t.Fatalf("StartTime = %v, want the second run's first observation %v", alert.StartTime, restart)
+	}
+}