From 3de3cd320d30ca33337026bc361d2c42688dbe3f Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:18:40 +0100 Subject: [PATCH] Deliver alerts from first startup reports Wire outbound alert callbacks before a monitor is published so immediately reconnecting agents cannot create active warnings in the constructor-to-Start gap. Add a pre-Start custom-sensor regression and bind the ordering in monitoring and agent lifecycle contracts. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 18 ++++++ .../v6/internal/subsystems/monitoring.md | 18 ++++++ internal/monitoring/monitor.go | 58 +++++++++++++------ .../monitoring/monitor_host_agents_test.go | 42 ++++++++++++++ 4 files changed, 117 insertions(+), 19 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index b3e277a8d..766874135 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -85,6 +85,10 @@ initial notification target into the notification manager. This adjacent alerts/notifications wiring grants no agent enrollment, reporting, removal, profile, update, probe-assignment, or command authority and must not mutate host-agent state. +The same adjacent boundary installs external alert callbacks before the +monitor is exposed to report handlers. This guarantees that the first accepted +agent observation can reach canonical notification delivery without granting +the callback path enrollment, identity, profile, update, or command authority. The shared `internal/api/ai_handlers.go` route may also reopen a dismissed Patrol finding and mirror that state into the unified findings store. This is AI finding-state management only; it grants no agent enrollment, report, @@ -6794,3 +6798,17 @@ agent report or mutate the live agent inventory to make a mock timeline complete. `internal/api/alerting/alerts_test.go` and `internal/mock/alert_incidents_test.go` pin the transport and fixture sides of this separation. + +### First agent reports cannot outrun alert delivery wiring + +The monitoring constructor installs external alert callbacks before it returns +the monitor to API routing. An agent report accepted immediately after process +startup can therefore create a canonical alert and reach notification and push +delivery even if the long-running `Monitor.Start` goroutine has not yet begun. +`Start` may replace those single callback slots with runtime-specific WebSocket +context and add lifecycle projection replay, but it cannot be the first owner +of outbound alert wiring. This ordering changes no enrollment or token +authority; it only prevents the first accepted agent observation from losing +its alert consequence. `TestNewMonitorRoutesStartupCustomSensorWarningBeforeStart` +in `internal/monitoring/monitor_host_agents_test.go` pins the pre-`Start` +warning path. diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 16ac2399e..0b354fc81 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -38,6 +38,10 @@ monitor restart. Monitor construction also applies the persisted grouping enabled flag, window, and node/guest keys as one notification-manager policy, so restart behavior is identical to a live alert-configuration save. +Monitor construction installs external alert callbacks before publishing the +monitor to API handlers. `Monitor.Start` adds runtime WebSocket context and +lifecycle replay, but an immediately reconnecting agent must not create a +canonical warning while notification delivery is still unwired. Monitor construction enables the alerts-owned persistent event log once for each tenant alert manager. This is bootstrap wiring only: monitoring does not own event types, retention, query semantics, or lifecycle/notification truth, @@ -3433,6 +3437,20 @@ must not silently skip a supported destination. Monitoring broadcasts the escalated alert after dispatch but does not reinterpret destination identity, retry semantics, acknowledgement, or the critical-repeat cadence. +### External alert callbacks are ready before monitor publication + +`monitoring.New` installs firing, resolution, AI, and escalation callback slots +before returning a monitor that API handlers can publish. The asynchronous +`Monitor.Start` loop adds lifecycle projection handling and rewires escalation +with its WebSocket hub, but it is not the first notification-delivery boundary. +This closes the startup interval in which an immediately reconnecting agent +could create and persist an active warning while the outbound callback was +still nil. The constructor-time callbacks use the already loaded alert and +destination configuration; runtime rewiring replaces single callback slots and +does not duplicate delivery. `TestNewMonitorRoutesStartupCustomSensorWarningBeforeStart` +in `internal/monitoring/monitor_host_agents_test.go` proves a first custom-sensor +warning reaches the external path before `Start` runs. + ### Proxmox node unavailability is not credential evidence The Proxmox client treats HTTP 595 from a node-scoped API path as a resource diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index b5f06d4f1..50a6a6432 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -1857,6 +1857,13 @@ func New(cfg *config.Config) (*Monitor, error) { Version: "2.0.0-go", } + // Agent endpoints can receive reports as soon as New returns, before the + // monitoring loop goroutine reaches Start. Wire external alert delivery now + // so a first startup observation cannot create an active alert without also + // reaching notifications. Start rewires the escalation callback with its + // WebSocket hub and installs lifecycle projection handling. + m.wireExternalAlertCallbacks(nil) + return m, nil } @@ -1991,25 +1998,7 @@ func (m *Monitor) Start(ctx context.Context, wsHub *websocket.Hub) { // Set up alert callbacks m.alertManager.SubscribeLifecycleCallback(m.handleAlertLifecycleEvent) - m.alertManager.SetAlertCallback(func(alert *alerts.Alert) { - m.handleAlertFired(alert) - }) - // Set up AI analysis callback - this bypasses activation state and other notification suppression - // so AI can analyze alerts even during pending_review setup phase - m.alertManager.SetAlertForAICallback(func(alert *alerts.Alert) { - log.Debug().Str("alertID", alert.ID).Msg("AI alert callback invoked (bypassing notification suppression)") - if m.alertTriggeredAICallback != nil { - m.alertTriggeredAICallback(alert) - } - }) - m.alertManager.SetResolvedCallback(func(alertID string) { - m.handleAlertResolved(alertID) - // Don't broadcast full state here - it causes a cascade with many guests. - // The frontend will get the updated alerts through the regular broadcast ticker. - }) - m.alertManager.SetEscalateCallback(func(alert *alerts.Alert, level int) { - m.handleAlertEscalated(wsHub, alert, level) - }) + m.wireExternalAlertCallbacks(wsHub) m.replayAlertLifecycleProjections() m.reconcileActiveAlertTimelines() m.markDeadManMonitoringProgress(time.Now().UTC()) @@ -2142,6 +2131,37 @@ func (m *Monitor) Start(ctx context.Context, wsHub *websocket.Hub) { } } +// wireExternalAlertCallbacks installs callbacks whose effects leave the alert +// manager. It is safe to call again: these are single callback slots, so Start +// replaces the constructor-time escalation closure with one carrying its hub. +func (m *Monitor) wireExternalAlertCallbacks(wsHub *websocket.Hub) { + if m == nil || m.alertManager == nil { + return + } + m.alertManager.SetAlertCallback(func(alert *alerts.Alert) { + m.handleAlertFired(alert) + }) + // AI analysis bypasses activation and notification suppression so findings + // can be prepared while alert delivery is still pending review. + m.alertManager.SetAlertForAICallback(func(alert *alerts.Alert) { + log.Debug().Str("alertID", alert.ID).Msg("AI alert callback invoked (bypassing notification suppression)") + m.mu.RLock() + callback := m.alertTriggeredAICallback + m.mu.RUnlock() + if callback != nil { + callback(alert) + } + }) + m.alertManager.SetResolvedCallback(func(alertID string) { + m.handleAlertResolved(alertID) + // Don't broadcast full state here - it causes a cascade with many guests. + // The frontend gets updated alerts through the regular broadcast ticker. + }) + m.alertManager.SetEscalateCallback(func(alert *alerts.Alert, level int) { + m.handleAlertEscalated(wsHub, alert, level) + }) +} + // poll fetches data from all configured instances func (m *Monitor) poll(_ context.Context, wsHub *websocket.Hub) { defer recoverFromPanic("poll") diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index 085e787c7..a1e1cc99d 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -24,6 +24,48 @@ import ( "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" ) +func TestNewMonitorRoutesStartupCustomSensorWarningBeforeStart(t *testing.T) { + monitor, err := New(&config.Config{DataPath: t.TempDir()}) + if err != nil { + t.Fatalf("New: %v", err) + } + t.Cleanup(monitor.Stop) + + delivered := make(chan *alerts.Alert, 1) + monitor.SetAlertPushCallback(func(alert *alerts.Alert) { + delivered <- alert + }) + + manager := monitor.GetAlertManager() + alertConfig := manager.GetConfig() + alertConfig.Enabled = true + alertConfig.ActivationState = alerts.ActivationActive + alertConfig.Schedule.QuietHours.Enabled = false + manager.UpdateConfig(alertConfig) + + value := 1.0 + manager.CheckHost(models.Host{ + ID: "startup-warning-host", + Hostname: "startup-warning-host", + Sensors: models.HostSensorSummary{Custom: []models.HostCustomSensorMetric{{ + ID: "apt-upgradable", + Name: "Apt Upgradable", + Value: &value, + Status: "warning", + ObservedAt: time.Now().UTC(), + }}}, + }) + + select { + case alert := <-delivered: + if alert.Type != "custom-sensor" || alert.Level != alerts.AlertLevelWarning { + t.Fatalf("startup alert = %#v, want warning custom-sensor", alert) + } + case <-time.After(time.Second): + t.Fatal("custom sensor warning did not reach external delivery before Monitor.Start") + } +} + func TestHostZFSPoolsFromAgentDisksPreservesDatasetFacts(t *testing.T) { got := hostZFSPoolsFromAgentDisks([]agentshost.Disk{ {Device: "/", Type: "ext4"},