From 82cdcd8f61e1ac94dc0df1e8d5afa4ea6b59dd69 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:06:55 +0100 Subject: [PATCH] Keep monitor-only alerts out of escalation and replay Treat monitor-only as terminal before quiet-hours replay and flapping, and reject scheduled or queued escalations when the current alert is monitor-only. Align firing, recovery and delivery diagnosis with that policy; cover ordinary and critical-repeat scheduling plus a policy change after callback queueing. Change-source: pulse-maintainer --- .../v6/internal/subsystems/alerts.md | 12 ++ .../alerts/callback_config_coverage_test.go | 118 ++++++++++++++++++ internal/alerts/escalation.go | 7 +- internal/alerts/notification_policy.go | 29 +++-- 4 files changed, 150 insertions(+), 16 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index f6784ff6b..0c5061c42 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -777,6 +777,8 @@ transition references recovery evidence separate from its trigger evidence. 5. Update the event-log schema upgrade and history-projection parity proofs when lifecycle snapshots, occurrence folding, or alert-history authority changes +6. When notification admission changes, verify firing, recovery, escalation, + queued-escalation revalidation, and quiet-hours replay precedence together. ### Attention projection source contract @@ -1408,6 +1410,16 @@ suppression, monitor-only notification suppression, cooldown decisions, and per-alert rate limiting; future notification-gating changes should extend that policy owner rather than burying new checks inside metric or resource-specific evaluators. + +### Monitor-only delivery is terminal + +Monitor-only alerts remain visible, but neither a firing nor a recovery +notification may enter the delivery queue. They must not acquire quiet-hours +replay metadata or schedule escalation levels and critical repeats. An already +queued escalation must be revalidated against the current monitor-only state +before delivery; the read-only delivery diagnosis reports monitor-only +suppression rather than a replayable quiet-hours deferral. + The same policy owner also exposes the read-only alert delivery diagnosis projection used by `/api/alerts/delivery-diagnosis`; that projection may explain current gating state, quiet-hours replay timing, cooldown timing, rate-limit diff --git a/internal/alerts/callback_config_coverage_test.go b/internal/alerts/callback_config_coverage_test.go index ba9490aef..8783fe71c 100644 --- a/internal/alerts/callback_config_coverage_test.go +++ b/internal/alerts/callback_config_coverage_test.go @@ -538,3 +538,121 @@ func TestValidateQuietHoursTimezone(t *testing.T) { } }) } +func TestMonitorOnlyAlertNeverEscalates(t *testing.T) { + m := newTestManager(t) + now := time.Now().UTC() + m.mu.Lock() + m.config.Enabled = true + m.config.ActivationState = ActivationActive + m.config.Schedule.Escalation = EscalationConfig{ + Enabled: true, + Levels: []EscalationLevel{{After: 30, Notify: "email"}}, + RepeatCritical: true, + RepeatEvery: 30, + } + monitorOnly := &Alert{ + ID: "vm-monitor-only", + Type: "cpu", + StartTime: now.Add(-time.Hour), + Metadata: map[string]interface{}{"monitorOnly": true}, + } + repeatingMonitorOnly := &Alert{ + ID: "vm-monitor-only-critical", + Type: "cpu", + Level: AlertLevelCritical, + StartTime: now.Add(-2 * time.Hour), + LastEscalation: 1, + EscalationTimes: []time.Time{now.Add(-time.Hour)}, + Metadata: map[string]interface{}{"monitorOnly": true}, + } + ordinary := &Alert{ID: "vm-ordinary", Type: "cpu", StartTime: now.Add(-time.Hour)} + m.setActiveAlertNoLock(monitorOnly.ID, monitorOnly) + m.setActiveAlertNoLock(repeatingMonitorOnly.ID, repeatingMonitorOnly) + m.setActiveAlertNoLock(ordinary.ID, ordinary) + m.mu.Unlock() + + m.checkEscalations() + if monitorOnly.LastEscalation != 0 || len(monitorOnly.EscalationTimes) != 0 { + t.Fatalf("monitor-only alert scheduled escalation: %+v", monitorOnly) + } + if repeatingMonitorOnly.LastEscalation != 1 || len(repeatingMonitorOnly.EscalationTimes) != 1 { + t.Fatalf("monitor-only critical alert repeated escalation: %+v", repeatingMonitorOnly) + } + if ordinary.LastEscalation != 1 { + t.Fatalf("ordinary alert did not exercise scheduler: %+v", ordinary) + } +} + +func TestMonitorOnlyPolicyChangeRejectsPendingEscalation(t *testing.T) { + m := newTestManager(t) + now := time.Now().UTC() + m.mu.Lock() + m.config.Enabled = true + m.config.ActivationState = ActivationActive + m.config.Schedule.Escalation = EscalationConfig{ + Enabled: true, + Levels: []EscalationLevel{{After: 30, Notify: "email"}}, + } + alert := &Alert{ID: "vm-becomes-monitor-only", Type: "cpu", StartTime: now.Add(-time.Hour)} + m.setActiveAlertNoLock(alert.ID, alert) + snapshot := cloneAlertForOutput(alert) + alert.Metadata = map[string]interface{}{"monitorOnly": true} + m.mu.Unlock() + + if _, _, eligible := m.PrepareEscalationNotification(snapshot, 1); eligible { + t.Fatal("queued escalation remained eligible after alert became monitor-only") + } +} + +func TestMonitorOnlyAlertCannotAcquireQuietHoursReplay(t *testing.T) { + m := newTestManager(t) + now := time.Date(2026, time.April, 12, 12, 0, 0, 0, time.UTC) + m.now = func() time.Time { return now } + m.config.ActivationState = ActivationActive + m.config.Schedule.QuietHours = QuietHours{ + Enabled: true, + Start: "00:00", + End: "23:59", + Timezone: "UTC", + Days: map[string]bool{"sunday": true}, + } + m.SetAlertCallback(func(*Alert) { t.Error("monitor-only firing alert reached callback") }) + alert := &Alert{ + ID: "monitor-only-quiet-hours", + Type: "cpu", + Level: AlertLevelWarning, + StartTime: m.policyNow().Add(-time.Hour), + Metadata: map[string]interface{}{ + "monitorOnly": true, + MetadataQuietHoursReplayAt: m.policyNow().Add(time.Hour).Format(time.RFC3339), + }, + } + if suppressed, _ := m.shouldSuppressNotification(alert); !suppressed { + t.Fatal("fixture did not place alert in quiet hours") + } + m.mu.Lock() + m.setActiveAlertNoLock(alert.ID, alert) + if m.dispatchAlert(alert, false) { + m.mu.Unlock() + t.Fatal("monitor-only firing alert was dispatched") + } + m.mu.Unlock() + if hasQuietHoursNotificationReplay(alert) { + t.Fatalf("monitor-only alert retained replay metadata: %+v", alert.Metadata) + } + if alert.LastNotified != nil { + t.Fatalf("monitor-only alert was marked notified at %s", alert.LastNotified) + } + if !m.ShouldSuppressNotification(alert) || hasQuietHoursNotificationReplay(alert) { + t.Fatalf("public suppression helper deferred monitor-only alert: %+v", alert.Metadata) + } + lastNotified := m.policyNow().Add(-time.Minute) + alert.LastNotified = &lastNotified + if !m.ShouldSuppressResolvedNotification(alert) { + t.Fatal("monitor-only recovery notification was admitted") + } + diagnosis, exists := m.DiagnoseAlertDelivery(alert.ID) + if !exists || diagnosis.Status != AlertDeliveryStatusSuppressed || diagnosis.Reason != AlertDeliveryReasonMonitorOnly || diagnosis.QuietHoursReplayAt != nil { + t.Fatalf("monitor-only delivery diagnosis = %+v, exists=%t", diagnosis, exists) + } +} diff --git a/internal/alerts/escalation.go b/internal/alerts/escalation.go index 5edc3920d..c2385fe16 100644 --- a/internal/alerts/escalation.go +++ b/internal/alerts/escalation.go @@ -44,8 +44,9 @@ func (m *Manager) checkEscalations() { now := m.policyNow() for _, alert := range m.activeAlerts { - // Skip acknowledged alerts - if alert.Acknowledged { + // Monitor-only alerts remain visible but must never schedule a + // notification, including escalation or critical repeats. + if alert == nil || alert.Acknowledged || isMonitorOnlyAlert(alert) { continue } if _, snoozed := alertSnoozeUntil(alert, now); snoozed { @@ -129,7 +130,7 @@ func (m *Manager) PrepareEscalationNotification(snapshot *Alert, level int) (*Al return nil, EscalationLevel{}, false } active, ok := m.getActiveAlertNoLock(snapshot.ID) - if !ok || active == nil || !active.StartTime.Equal(snapshot.StartTime) || active.Acknowledged { + if !ok || active == nil || !active.StartTime.Equal(snapshot.StartTime) || active.Acknowledged || isMonitorOnlyAlert(active) { return nil, EscalationLevel{}, false } if _, snoozed := alertSnoozeUntil(active, m.policyNow()); snoozed { diff --git a/internal/alerts/notification_policy.go b/internal/alerts/notification_policy.go index a1cc40fba..89c88afc1 100644 --- a/internal/alerts/notification_policy.go +++ b/internal/alerts/notification_policy.go @@ -200,6 +200,12 @@ func (m *Manager) dispatchAlert(alert *Alert, async bool) bool { "Notification suppressed by resource monitoring policy.", nil) return false } + if isMonitorOnlyAlert(alert) { + clearQuietHoursNotificationReplay(alert) + m.recordAlertEvent(eventlog.TypeNotificationSuppressed, alert, "", + AlertDeliveryReasonMonitorOnly, "Notification suppressed: alert is monitor-only.", nil) + return false + } trackingKey := canonicalTrackingKeyForAlert(alert) @@ -275,17 +281,6 @@ func (m *Manager) dispatchAlert(alert *Alert, async bool) bool { clearQuietHoursNotificationReplay(alert) } - if isMonitorOnlyAlert(alert) { - log.Info(). - Str("alertID", alert.ID). - Str("resource", alert.ResourceName). - Bool("monitorOnly", true). - Msg("Monitor-only alert detected, skipping alert dispatch") - m.recordAlertEvent(eventlog.TypeNotificationSuppressed, alert, "", - AlertDeliveryReasonMonitorOnly, "Notification suppressed: alert is monitor-only.", nil) - return false - } - // Record metric for fired alert if recordAlertFired != nil { recordAlertFired(alert) @@ -615,6 +610,10 @@ func (m *Manager) ShouldSuppressNotification(alert *Alert) bool { Msg("Notification suppressed by resource monitoring policy") return true } + if isMonitorOnlyAlert(alert) { + clearQuietHoursNotificationReplay(alert) + return true + } suppressed, reason := m.shouldSuppressNotification(alert) if suppressed { @@ -667,6 +666,10 @@ func (m *Manager) ShouldSuppressResolvedNotification(alert *Alert) bool { Msg("Recovery notification suppressed by resource monitoring policy") return true } + if isMonitorOnlyAlert(alert) { + clearQuietHoursNotificationReplay(alert) + return true + } quietHoursReplay := hasQuietHoursNotificationReplay(alert) if alert.LastNotified == nil && !quietHoursReplay { @@ -828,6 +831,8 @@ func (m *Manager) diagnoseActiveAlertLocked(alert *Alert) AlertDeliveryDiagnosis diagnosis.setSuppressed(reason, "Alert notification is suppressed by the resource monitoring policy.") return true }(): + case isMonitorOnlyAlert(alert): + diagnosis.setSuppressed(AlertDeliveryReasonMonitorOnly, "Alert is marked monitor-only, so it stays visible without notification delivery.") case !m.config.Enabled: diagnosis.setSuppressed(AlertDeliveryReasonNotificationsDisabled, "Alert notifications are disabled in the alert configuration.") case m.config.ActivationState != ActivationActive: @@ -863,8 +868,6 @@ func (m *Manager) diagnoseActiveAlertLocked(alert *Alert) AlertDeliveryDiagnosis diagnosis.Message = "Alert delivery is deferred by quiet-hours policy and will be replayed later." return true }(): - case isMonitorOnlyAlert(alert): - diagnosis.setSuppressed(AlertDeliveryReasonMonitorOnly, "Alert is marked monitor-only, so it stays visible without notification delivery.") default: diagnosis.Status = AlertDeliveryStatusWouldSend diagnosis.Reason = AlertDeliveryReasonReady