diff --git a/internal/hub/systems/system_manager.go b/internal/hub/systems/system_manager.go index be3a61fa..642b5868 100644 --- a/internal/hub/systems/system_manager.go +++ b/internal/hub/systems/system_manager.go @@ -219,11 +219,15 @@ func (sm *SystemManager) onRecordAfterUpdateSuccess(e *core.RecordEvent) error { // Pause monitoring but keep system in manager for potential resume system.closeSSHConnection() } - _ = deactivateAlerts(e.App, e.Record.Id) + _ = deactivateAlerts(e.App, e.Record.Id, false) sm.hub.CancelPendingStatusAlerts(e.Record.Id) sm.hub.CancelPendingContainerAlerts(e.Record.Id) return e.Next() case pending: + // Keep an active status alert until connectivity is confirmed. This lets + // pending -> up resolve it and send the recovery notification after a + // system address or other connection setting is changed. + _ = deactivateAlerts(e.App, e.Record.Id, true) // Resume monitoring, preferring existing WebSocket connection if ok && system.WsConn != nil { go system.update() @@ -233,7 +237,6 @@ func (sm *SystemManager) onRecordAfterUpdateSuccess(e *core.RecordEvent) error { if err := sm.AddRecord(e.Record, nil); err != nil { e.App.Logger().Error("Error adding record", "err", err) } - _ = deactivateAlerts(e.App, e.Record.Id) return e.Next() case down: // Docker state is unknown while the system is unreachable. Do not let a @@ -256,8 +259,9 @@ func (sm *SystemManager) onRecordAfterUpdateSuccess(e *core.RecordEvent) error { } } - // Trigger status change alerts for up/down transitions - if (newStatus == down && prevStatus == up) || (newStatus == up && prevStatus == down) { + // A connection-setting update moves a down system through pending before it + // comes up, so recover active status alerts on any non-up -> up transition. + if (newStatus == down && prevStatus == up) || (newStatus == up && prevStatus != up) { if err := sm.hub.HandleStatusAlerts(newStatus, e.Record); err != nil { e.App.Logger().Error("Error handling status alerts", "err", err) } @@ -415,10 +419,11 @@ func (sm *SystemManager) createSSHClientConfig() error { return nil } -// deactivateAlerts finds all triggered alerts for a system and sets them to inactive. -// This is called when a system is paused or goes offline to prevent continued alerts. +// deactivateAlerts finds triggered alerts for a system and sets them to inactive. +// Status alerts can be preserved while connection changes are pending so that a +// confirmed recovery still produces an "up" notification. // Monitor incidents remain open: a missing observation does not establish recovery. -func deactivateAlerts(app core.App, systemID string) error { +func deactivateAlerts(app core.App, systemID string, preserveStatusAlert bool) error { // Note: Direct SQL updates don't trigger SSE, so we use the PocketBase API // _, err := app.DB().NewQuery(fmt.Sprintf("UPDATE alerts SET triggered = false WHERE system = '%s'", systemID)).Execute() @@ -428,6 +433,9 @@ func deactivateAlerts(app core.App, systemID string) error { } for _, alert := range alerts { + if preserveStatusAlert && alert.GetString("name") == "Status" { + continue + } alert.Set("triggered", false) if err := app.SaveNoValidate(alert); err != nil { return err diff --git a/internal/hub/systems/systems_test.go b/internal/hub/systems/systems_test.go index 5063b22c..ce87f3a4 100644 --- a/internal/hub/systems/systems_test.go +++ b/internal/hub/systems/systems_test.go @@ -165,6 +165,55 @@ func TestSystemManagerNew(t *testing.T) { }) } +func TestStatusAlertRecoveryAfterPendingTransition(t *testing.T) { + hub, user := tests.GetHubWithUser(t) + defer hub.Cleanup() + + userSettings, err := hub.FindFirstRecordByFilter("user_settings", "user={:user}", map[string]any{"user": user.Id}) + require.NoError(t, err) + userSettings.Set("settings", map[string]any{ + "emails": []string{"test@example.com"}, + "webhooks": []string{}, + }) + require.NoError(t, hub.Save(userSettings)) + + record, err := tests.CreateRecord(hub, "systems", map[string]any{ + "name": "changed-address", + "host": "192.0.2.1", + "port": "33914", + "users": []string{user.Id}, + }) + require.NoError(t, err) + record.Set("status", "down") + require.NoError(t, hub.Save(record)) + + alert, err := tests.CreateRecord(hub, "alerts", map[string]any{ + "name": "Status", + "system": record.Id, + "user": user.Id, + "min": 1, + "triggered": true, + }) + require.NoError(t, err) + initialEmailCount := hub.TestMailer.TotalSend() + + // The edit dialog temporarily moves the system through pending. The active + // status alert must remain active until the new connection is confirmed. + record.Set("host", "192.0.2.2") + record.Set("status", "pending") + require.NoError(t, hub.Save(record)) + alert, err = hub.FindRecordById("alerts", alert.Id) + require.NoError(t, err) + assert.True(t, alert.GetBool("triggered"), "pending connection update should preserve the active status alert") + + record.Set("status", "up") + require.NoError(t, hub.Save(record)) + alert, err = hub.FindRecordById("alerts", alert.Id) + require.NoError(t, err) + assert.False(t, alert.GetBool("triggered"), "pending -> up should resolve the active status alert") + assert.Equal(t, initialEmailCount+1, hub.TestMailer.TotalSend(), "recovery should send an up notification") +} + func testOld(t *testing.T, hub *tests.TestHub) { user, err := tests.CreateUser(hub, "test@testy.com", "testtesttest") require.NoError(t, err)