From de489abc65adf172771810951d15f69ac0b534c2 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:07:52 +0100 Subject: [PATCH] Repair confirmation reducer governance proof Record discrete confirmation and first-match timing ownership in the alerts contract, move the manager regression into the recognized incident proof, and restore status revision alignment with the stable source of truth. Change-source: pulse-maintainer --- docs/release-control/v6/internal/status.json | 2 +- .../v6/internal/subsystems/alerts.md | 15 +++- .../alerts/lifecycle_first_matched_test.go | 74 ------------------- .../unified_incident_confirmation_test.go | 62 ++++++++++++++++ 4 files changed, 75 insertions(+), 78 deletions(-) delete mode 100644 internal/alerts/lifecycle_first_matched_test.go diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index e643a61249..5ff53f0461 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -1,6 +1,6 @@ { "version": "6.0", - "updated_at": "2026-08-26", + "updated_at": "2026-08-08", "scope": { "active_repos": [ "pulse", diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index e51382f246..8004459aca 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -642,9 +642,18 @@ models enablement, hysteresis, sustained delay, and warning/critical severity without persistence or notification side effects. The alert manager remains the runtime owner and reference behavior while the parity harness drives both implementations through identical observations; a divergence is a reducer bug -unless a focused manager regression first proves otherwise. Moving additional -alert families or downstream delivery behavior into the reducer requires a -separate contract slice. +unless a focused manager regression first proves otherwise. + +The confirmation/discrete-state slice uses the same boundary for connectivity, +powered-state, and generic discrete matches. It activates after the resolved +number of consecutive matches, clears pending state or firing state on a +non-match at the evaluator layer, and re-derives spec-carried severity on every +firing observation. Both manager and reducer must date first activation at the +first matched observation, not the final confirming poll. Alerts runtime owns +that first-match timestamp alongside the confirmation counter and must clear it +whenever the run resets or its tracking state is removed. Offline recovery +confirmation, recently-resolved re-fire restoration, additional alert families, +and downstream delivery behavior require separate contract slices. That same guest-threshold owner also governs guest-derived lifecycle and posture alerts. Snapshot age, backup age, powered-off state, and configuration-change reevaluation must all construct a canonical lightweight diff --git a/internal/alerts/lifecycle_first_matched_test.go b/internal/alerts/lifecycle_first_matched_test.go deleted file mode 100644 index 14968b96aa..0000000000 --- a/internal/alerts/lifecycle_first_matched_test.go +++ /dev/null @@ -1,74 +0,0 @@ -package alerts - -import ( - "testing" - "time" - - alertspecs "github.com/rcourtman/pulse-go-rewrite/internal/alerts/specs" - "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" -) - -// Regression: the confirmation maps hold only counts, so the reconstructed -// pending state dated the run at the current observation and a -// confirmation-based lifecycle alert stamped StartTime at the final -// confirming poll — understating outage start by the whole confirmation -// window. lifecycleFirstMatched preserves the first matched observation -// (mirroring unifiedIncidentFirstSeen) so activation keeps the true start. -func TestLifecycleAlertStartTimeIsFirstMatchedObservation(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") - } - } - - observe("offline", epoch) - observe("offline", epoch.Add(30*time.Second)) - observe("offline", epoch.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 three confirmations") - } - if !alert.StartTime.Equal(epoch) { - t.Fatalf("StartTime = %v, want first offline observation %v", alert.StartTime, epoch) - } - - // Recovery clears the preserved first-match so a later run re-dates. - observe("online", epoch.Add(90*time.Second)) - manager.mu.Lock() - _, tracked := manager.lifecycleFirstMatched["conn:node-9"] - manager.mu.Unlock() - if tracked { - t.Fatal("first-matched entry should clear with the confirmation run") - } -} diff --git a/internal/alerts/unified_incident_confirmation_test.go b/internal/alerts/unified_incident_confirmation_test.go index 3708824f13..123f642634 100644 --- a/internal/alerts/unified_incident_confirmation_test.go +++ b/internal/alerts/unified_incident_confirmation_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + alertspecs "github.com/rcourtman/pulse-go-rewrite/internal/alerts/specs" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) @@ -143,6 +144,67 @@ func TestUnifiedProviderIncidentGlobalDisableClearsImmediately(t *testing.T) { } } +// Regression: confirmation maps hold only counts, so the lifecycle owner must +// preserve the first matched observation separately and use it as StartTime +// when the final confirmation activates the alert. +func TestLifecycleAlertStartTimeIsFirstMatchedObservation(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") + } + } + + observe("offline", epoch) + observe("offline", epoch.Add(30*time.Second)) + observe("offline", epoch.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 three confirmations") + } + if !alert.StartTime.Equal(epoch) { + t.Fatalf("StartTime = %v, want first offline observation %v", alert.StartTime, epoch) + } + + observe("online", epoch.Add(90*time.Second)) + manager.mu.Lock() + _, tracked := manager.lifecycleFirstMatched["conn:node-9"] + manager.mu.Unlock() + if tracked { + t.Fatal("first-matched entry should clear with the confirmation run") + } +} + func confirmedProviderIncidentResource(observedAt time.Time) unifiedresources.Resource { return unifiedresources.Resource{ ID: "app:media",