From 52edbd4e78b17afeca67850fa42345efd54de0d5 Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Tue, 7 Oct 2025 20:31:00 +0000
Subject: [PATCH] Fix Docker alert cleanup and email provider UI

- Add HandleDockerHostRemoved to properly clean up all alerts and tracking
  when Docker hosts are removed from config
- Implement pruneStaleDockerAlerts to automatically clear orphaned Docker
  alerts during sync cycles
- Clean up restart tracking and exit code maps in addition to state confirmations
- Add comprehensive test coverage for Docker host removal cleanup
- Improve email provider select layout with consistent styling and instruction boxes
- Fix RemoveDockerHost to handle missing hosts gracefully and still clear alerts
---
 .../components/Alerts/EmailProviderSelect.tsx | 15 +--
 internal/alerts/alerts.go                     | 94 ++++++++++++-------
 internal/alerts/alerts_test.go                | 47 +++++++++-
 internal/monitoring/monitor.go                | 89 +++++++++++++++++-
 4 files changed, 199 insertions(+), 46 deletions(-)

diff --git a/frontend-modern/src/components/Alerts/EmailProviderSelect.tsx b/frontend-modern/src/components/Alerts/EmailProviderSelect.tsx
index 86ef48d3d..7e4c004b6 100644
--- a/frontend-modern/src/components/Alerts/EmailProviderSelect.tsx
+++ b/frontend-modern/src/components/Alerts/EmailProviderSelect.tsx
@@ -86,16 +86,17 @@ export function EmailProviderSelect(props: EmailProviderSelectProps) {
   };
 
   const currentProvider = () => providers().find((p) => p.name === props.config.provider);
+  const instructionBoxClass = "mt-2 rounded border border-blue-200 bg-blue-50 px-3 py-2 text-xs leading-relaxed text-blue-900 dark:border-blue-700 dark:bg-blue-900/20 dark:text-blue-200";
 
   return (
     <div class="space-y-4 text-sm overflow-hidden">
-      <div class="grid w-full gap-2 sm:grid-cols-[150px_1fr] sm:items-center">
-        <label class={`${labelClass()} sm:text-right`}>Email provider</label>
+      <div class={formField}>
+        <label class={labelClass()}>Email provider</label>
         <div class="flex w-full flex-wrap items-center gap-2 sm:flex-nowrap">
           <select
             value={props.config.provider}
             onChange={(e) => handleProviderChange(e.currentTarget.value)}
-            class={`${controlClass('px-2 py-1.5')} sm:w-auto sm:min-w-[160px]`}
+            class={`${controlClass('px-2 py-1.5')} sm:w-auto sm:min-w-[180px]`}
           >
             <option value="">Manual configuration</option>
             <For each={providers()}>
@@ -131,13 +132,15 @@ export function EmailProviderSelect(props: EmailProviderSelectProps) {
             {showInstructions() ? 'Hide setup instructions' : 'Show setup instructions'}
           </button>
           <Show when={showInstructions()}>
-            <div class="mt-2 rounded border border-blue-200 bg-blue-50 px-3 py-2 text-xs leading-relaxed text-blue-900 dark:border-blue-700 dark:bg-blue-900/20 dark:text-blue-200">
+            <div class={instructionBoxClass}>
               {currentProvider()!.instructions}
             </div>
           </Show>
         </div>
-        <div class="hidden w-full sm:block sm:border-l-2 sm:border-blue-300 sm:pl-3 sm:text-xs sm:leading-relaxed sm:text-blue-800 dark:sm:border-blue-700 dark:sm:text-blue-200">
-          {currentProvider()!.instructions}
+        <div class="hidden w-full sm:block">
+          <div class={instructionBoxClass}>
+            {currentProvider()!.instructions}
+          </div>
         </div>
       </Show>
 
diff --git a/internal/alerts/alerts.go b/internal/alerts/alerts.go
index d7feee536..905aeba89 100644
--- a/internal/alerts/alerts.go
+++ b/internal/alerts/alerts.go
@@ -295,12 +295,12 @@ type Manager struct {
 	// Time threshold tracking
 	pendingAlerts map[string]time.Time // Track when thresholds were first exceeded
 	// Offline confirmation tracking
-	nodeOfflineCount     map[string]int // Track consecutive offline counts for nodes (legacy)
-	offlineConfirmations map[string]int // Track consecutive offline counts for all resources
-	dockerOfflineCount   map[string]int // Track consecutive offline counts for Docker hosts
-	dockerStateConfirm   map[string]int // Track consecutive state confirmations for Docker containers
+	nodeOfflineCount      map[string]int                  // Track consecutive offline counts for nodes (legacy)
+	offlineConfirmations  map[string]int                  // Track consecutive offline counts for all resources
+	dockerOfflineCount    map[string]int                  // Track consecutive offline counts for Docker hosts
+	dockerStateConfirm    map[string]int                  // Track consecutive state confirmations for Docker containers
 	dockerRestartTracking map[string]*dockerRestartRecord // Track restart counts and times for restart loop detection
-	dockerLastExitCode    map[string]int // Track last exit code for OOM detection
+	dockerLastExitCode    map[string]int                  // Track last exit code for OOM detection
 	// Persistent acknowledgement state so quick alert rebuilds keep user acknowledgements
 	ackState map[string]ackRecord
 }
@@ -322,16 +322,16 @@ type dockerRestartRecord struct {
 func NewManager() *Manager {
 	alertsDir := filepath.Join(utils.GetDataDir(), "alerts")
 	m := &Manager{
-		activeAlerts:         make(map[string]*Alert),
-		historyManager:       NewHistoryManager(alertsDir),
-		escalationStop:       make(chan struct{}),
-		alertRateLimit:       make(map[string][]time.Time),
-		recentAlerts:         make(map[string]*Alert),
-		suppressedUntil:      make(map[string]time.Time),
-		recentlyResolved:     make(map[string]*ResolvedAlert),
-		pendingAlerts:        make(map[string]time.Time),
-		nodeOfflineCount:     make(map[string]int),
-		offlineConfirmations: make(map[string]int),
+		activeAlerts:          make(map[string]*Alert),
+		historyManager:        NewHistoryManager(alertsDir),
+		escalationStop:        make(chan struct{}),
+		alertRateLimit:        make(map[string][]time.Time),
+		recentAlerts:          make(map[string]*Alert),
+		suppressedUntil:       make(map[string]time.Time),
+		recentlyResolved:      make(map[string]*ResolvedAlert),
+		pendingAlerts:         make(map[string]time.Time),
+		nodeOfflineCount:      make(map[string]int),
+		offlineConfirmations:  make(map[string]int),
 		dockerOfflineCount:    make(map[string]int),
 		dockerStateConfirm:    make(map[string]int),
 		dockerRestartTracking: make(map[string]*dockerRestartRecord),
@@ -354,14 +354,14 @@ func NewManager() *Manager {
 				Disk:        &HysteresisThreshold{Trigger: 90, Clear: 85},
 				Temperature: &HysteresisThreshold{Trigger: 80, Clear: 75}, // Warning at 80°C, clear at 75°C
 			},
-		DockerDefaults: DockerThresholdConfig{
-			CPU:               HysteresisThreshold{Trigger: 80, Clear: 75},
-			Memory:            HysteresisThreshold{Trigger: 85, Clear: 80},
-			RestartCount:      3,
-			RestartWindow:     300, // 5 minutes
-			MemoryWarnPct:     90,
-			MemoryCriticalPct: 95,
-		},
+			DockerDefaults: DockerThresholdConfig{
+				CPU:               HysteresisThreshold{Trigger: 80, Clear: 75},
+				Memory:            HysteresisThreshold{Trigger: 85, Clear: 80},
+				RestartCount:      3,
+				RestartWindow:     300, // 5 minutes
+				MemoryWarnPct:     90,
+				MemoryCriticalPct: 95,
+			},
 			StorageDefault:    HysteresisThreshold{Trigger: 85, Clear: 80},
 			MinimumDelta:      2.0, // 2% minimum change
 			SuppressionWindow: 5,   // 5 minutes
@@ -565,11 +565,11 @@ func (m *Manager) reevaluateActiveAlertsLocked() {
 		}
 
 		if alert.Type == "docker-host-offline" ||
-		   strings.HasPrefix(alertID, "docker-container-health-") ||
-		   strings.HasPrefix(alertID, "docker-container-state-") ||
-		   strings.HasPrefix(alertID, "docker-container-restart-loop-") ||
-		   strings.HasPrefix(alertID, "docker-container-oom-") ||
-		   strings.HasPrefix(alertID, "docker-container-memory-limit-") {
+			strings.HasPrefix(alertID, "docker-container-health-") ||
+			strings.HasPrefix(alertID, "docker-container-state-") ||
+			strings.HasPrefix(alertID, "docker-container-restart-loop-") ||
+			strings.HasPrefix(alertID, "docker-container-oom-") ||
+			strings.HasPrefix(alertID, "docker-container-memory-limit-") {
 			// Non-metric Docker alerts are not governed by thresholds
 			continue
 		}
@@ -1437,6 +1437,18 @@ func (m *Manager) HandleDockerHostOnline(host models.DockerHost) {
 	}
 }
 
+// HandleDockerHostRemoved clears all alerts and tracking when a Docker host is deleted.
+func (m *Manager) HandleDockerHostRemoved(host models.DockerHost) {
+	if host.ID == "" {
+		return
+	}
+
+	// Reuse the online handler to clear offline alerts and tracking.
+	m.HandleDockerHostOnline(host)
+	// Drop any container alerts and host-scoped tracking entries.
+	m.clearDockerHostContainerAlerts(host.ID)
+}
+
 // HandleDockerHostOffline raises an alert when a Docker host stops reporting.
 func (m *Manager) HandleDockerHostOffline(host models.DockerHost) {
 	if host.ID == "" {
@@ -1750,14 +1762,14 @@ func (m *Manager) checkDockerContainerRestartLoop(host models.DockerHost, contai
 			StartTime:    now,
 			LastSeen:     now,
 			Metadata: map[string]interface{}{
-				"hostId":        host.ID,
-				"hostName":      host.DisplayName,
-				"containerId":   container.ID,
-				"containerName": containerName,
-				"image":         container.Image,
-				"state":         container.State,
-				"status":        container.Status,
-				"restartCount":  container.RestartCount,
+				"hostId":         host.ID,
+				"hostName":       host.DisplayName,
+				"containerId":    container.ID,
+				"containerName":  containerName,
+				"image":          container.Image,
+				"state":          container.State,
+				"status":         container.Status,
+				"restartCount":   container.RestartCount,
 				"recentRestarts": recentCount,
 			},
 		}
@@ -1995,6 +2007,16 @@ func (m *Manager) clearDockerHostContainerAlerts(hostID string) {
 			delete(m.dockerStateConfirm, resourceID)
 		}
 	}
+	for resourceID := range m.dockerRestartTracking {
+		if strings.HasPrefix(resourceID, prefix) {
+			delete(m.dockerRestartTracking, resourceID)
+		}
+	}
+	for resourceID := range m.dockerLastExitCode {
+		if strings.HasPrefix(resourceID, prefix) {
+			delete(m.dockerLastExitCode, resourceID)
+		}
+	}
 	m.mu.Unlock()
 
 	for _, alertID := range toClear {
diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go
index ef1ce9759..f48fa670e 100644
--- a/internal/alerts/alerts_test.go
+++ b/internal/alerts/alerts_test.go
@@ -1,6 +1,10 @@
 package alerts
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/models"
+)
 
 func TestAcknowledgePersistsThroughCheckMetric(t *testing.T) {
 	m := NewManager()
@@ -35,3 +39,44 @@ func TestAcknowledgePersistsThroughCheckMetric(t *testing.T) {
 		t.Fatalf("acknowledged flag lost after update")
 	}
 }
+
+func TestHandleDockerHostRemovedClearsAlertsAndTracking(t *testing.T) {
+	m := NewManager()
+	host := models.DockerHost{ID: "host1", DisplayName: "Host One", Hostname: "host-one"}
+	containerResourceID := "docker:host1/container1"
+	containerAlertID := "docker-container-state-" + containerResourceID
+	hostAlertID := "docker-host-offline-host1"
+
+	m.mu.Lock()
+	m.activeAlerts[hostAlertID] = &Alert{ID: hostAlertID, ResourceID: "docker:host1"}
+	m.activeAlerts[containerAlertID] = &Alert{ID: containerAlertID, ResourceID: containerResourceID}
+	m.dockerOfflineCount[host.ID] = 2
+	m.dockerStateConfirm[containerResourceID] = 1
+	m.dockerRestartTracking[containerResourceID] = &dockerRestartRecord{}
+	m.dockerLastExitCode[containerResourceID] = 137
+	m.mu.Unlock()
+
+	m.HandleDockerHostRemoved(host)
+
+	m.mu.RLock()
+	defer m.mu.RUnlock()
+
+	if _, exists := m.activeAlerts[containerAlertID]; exists {
+		t.Fatalf("expected container alerts to be cleared")
+	}
+	if _, exists := m.activeAlerts[hostAlertID]; exists {
+		t.Fatalf("expected host offline alert to be cleared")
+	}
+	if _, exists := m.dockerOfflineCount[host.ID]; exists {
+		t.Fatalf("expected offline tracking to be cleared")
+	}
+	if _, exists := m.dockerStateConfirm[containerResourceID]; exists {
+		t.Fatalf("expected state confirmation to be cleared")
+	}
+	if _, exists := m.dockerRestartTracking[containerResourceID]; exists {
+		t.Fatalf("expected restart tracking to be cleared")
+	}
+	if _, exists := m.dockerLastExitCode[containerResourceID]; exists {
+		t.Fatalf("expected last exit code tracking to be cleared")
+	}
+}
diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go
index c0ecde460..1afacae0c 100644
--- a/internal/monitoring/monitor.go
+++ b/internal/monitoring/monitor.go
@@ -148,18 +148,25 @@ func (m *Monitor) RemoveDockerHost(hostID string) (models.DockerHost, error) {
 
 	host, removed := m.state.RemoveDockerHost(hostID)
 	if !removed {
-		return models.DockerHost{}, fmt.Errorf("docker host %s not found", hostID)
+		log.Debug().Str("dockerHostID", hostID).Msg("Docker host not present in state during removal; proceeding to clear alerts")
+		host = models.DockerHost{
+			ID:          hostID,
+			Hostname:    hostID,
+			DisplayName: hostID,
+		}
 	}
 
 	m.state.RemoveConnectionHealth(dockerConnectionPrefix + hostID)
 	if m.alertManager != nil {
-		m.alertManager.HandleDockerHostOnline(host)
+		m.alertManager.HandleDockerHostRemoved(host)
+		m.SyncAlertState()
 	}
 
 	log.Info().
 		Str("dockerHost", host.Hostname).
 		Str("dockerHostID", hostID).
-		Msg("Docker host removed from state")
+		Bool("removed", removed).
+		Msg("Docker host removed and alerts cleared")
 
 	return host, nil
 }
@@ -1203,6 +1210,10 @@ func (m *Monitor) poll(ctx context.Context, wsHub *websocket.Hub) {
 // syncAlertsToState copies the latest alert manager data into the shared state snapshot.
 // This keeps WebSocket broadcasts aligned with in-memory acknowledgement updates.
 func (m *Monitor) syncAlertsToState() {
+	if m.pruneStaleDockerAlerts() {
+		log.Debug().Msg("Pruned stale docker alerts during sync")
+	}
+
 	activeAlerts := m.alertManager.GetActiveAlerts()
 	modelAlerts := make([]models.Alert, 0, len(activeAlerts))
 	for _, alert := range activeAlerts {
@@ -1240,6 +1251,78 @@ func (m *Monitor) SyncAlertState() {
 	m.syncAlertsToState()
 }
 
+// pruneStaleDockerAlerts removes docker alerts that reference hosts no longer present in state.
+func (m *Monitor) pruneStaleDockerAlerts() bool {
+	if m.alertManager == nil {
+		return false
+	}
+
+	hosts := m.state.GetDockerHosts()
+	knownHosts := make(map[string]struct{}, len(hosts))
+	for _, host := range hosts {
+		id := strings.TrimSpace(host.ID)
+		if id != "" {
+			knownHosts[id] = struct{}{}
+		}
+	}
+
+	if len(knownHosts) == 0 {
+		// Still allow stale entries to be cleared if no hosts remain.
+	}
+
+	active := m.alertManager.GetActiveAlerts()
+	processed := make(map[string]struct{})
+	cleared := false
+
+	for _, alert := range active {
+		var hostID string
+
+		switch {
+		case alert.Type == "docker-host-offline":
+			hostID = strings.TrimPrefix(alert.ID, "docker-host-offline-")
+		case strings.HasPrefix(alert.ResourceID, "docker:"):
+			resource := strings.TrimPrefix(alert.ResourceID, "docker:")
+			if idx := strings.Index(resource, "/"); idx >= 0 {
+				hostID = resource[:idx]
+			} else {
+				hostID = resource
+			}
+		default:
+			continue
+		}
+
+		hostID = strings.TrimSpace(hostID)
+		if hostID == "" {
+			continue
+		}
+
+		if _, known := knownHosts[hostID]; known {
+			continue
+		}
+		if _, alreadyCleared := processed[hostID]; alreadyCleared {
+			continue
+		}
+
+		host := models.DockerHost{
+			ID:          hostID,
+			DisplayName: alert.ResourceName,
+			Hostname:    alert.Node,
+		}
+		if host.DisplayName == "" {
+			host.DisplayName = hostID
+		}
+		if host.Hostname == "" {
+			host.Hostname = hostID
+		}
+
+		m.alertManager.HandleDockerHostRemoved(host)
+		processed[hostID] = struct{}{}
+		cleared = true
+	}
+
+	return cleared
+}
+
 // pollConcurrent polls all instances concurrently
 func (m *Monitor) pollConcurrent(ctx context.Context) {
 	var wg sync.WaitGroup