test: Add tests for GetGuestMetrics, mergeSnapshot, getDockerCommandPayload

- GetGuestMetrics: all metric types, duration filtering, guest not found,
  unknown metric type, empty data (81% -> 100%)
- mergeSnapshot: timestamp merging for LastSuccess/LastError/LastMutated,
  ChangeHash preservation (86.7% -> 100%)
- getDockerCommandPayload: empty hostID, whitespace normalization, host
  not found, expired command cleanup, queued->dispatched marking,
  already-dispatched preservation (91.3% -> 100%)

internal/monitoring/docker_commands_test.go

@@ -719,6 +719,311 @@ func TestQueueDockerStopCommand(t *testing.T) {
 	})
 }
 
+func TestGetDockerCommandPayload(t *testing.T) {
+	t.Parallel()
+
+	t.Run("empty hostID returns nil", func(t *testing.T) {
+		t.Parallel()
+
+		monitor := newTestMonitorForCommands(t)
+
+		payload, status := monitor.getDockerCommandPayload("")
+		if payload != nil {
+			t.Fatalf("expected nil payload for empty hostID, got %v", payload)
+		}
+		if status != nil {
+			t.Fatalf("expected nil status for empty hostID, got %v", status)
+		}
+	})
+
+	t.Run("whitespace-only hostID returns nil", func(t *testing.T) {
+		t.Parallel()
+
+		monitor := newTestMonitorForCommands(t)
+
+		payload, status := monitor.getDockerCommandPayload("   ")
+		if payload != nil {
+			t.Fatalf("expected nil payload for whitespace hostID, got %v", payload)
+		}
+		if status != nil {
+			t.Fatalf("expected nil status for whitespace hostID, got %v", status)
+		}
+	})
+
+	t.Run("host not found in dockerCommands returns nil", func(t *testing.T) {
+		t.Parallel()
+
+		monitor := newTestMonitorForCommands(t)
+
+		payload, status := monitor.getDockerCommandPayload("nonexistent-host")
+		if payload != nil {
+			t.Fatalf("expected nil payload for nonexistent host, got %v", payload)
+		}
+		if status != nil {
+			t.Fatalf("expected nil status for nonexistent host, got %v", status)
+		}
+	})
+
+	t.Run("expired command is cleaned up and returns nil", func(t *testing.T) {
+		t.Parallel()
+
+		monitor := newTestMonitorForCommands(t)
+
+		host := models.DockerHost{
+			ID:          "host-expired",
+			Hostname:    "node-expired",
+			DisplayName: "node-expired",
+			Status:      "online",
+		}
+		monitor.state.UpsertDockerHost(host)
+
+		// Queue a command
+		cmdStatus, err := monitor.queueDockerStopCommand(host.ID)
+		if err != nil {
+			t.Fatalf("queue stop command: %v", err)
+		}
+
+		// Manually expire the command
+		monitor.mu.Lock()
+		cmd := monitor.dockerCommands[host.ID]
+		past := time.Now().Add(-2 * dockerCommandDefaultTTL)
+		cmd.status.ExpiresAt = &past
+		monitor.mu.Unlock()
+
+		// Fetch should return nil and clean up
+		payload, status := monitor.getDockerCommandPayload(host.ID)
+		if payload != nil {
+			t.Fatalf("expected nil payload for expired command, got %v", payload)
+		}
+		if status != nil {
+			t.Fatalf("expected nil status for expired command, got %v", status)
+		}
+
+		// Verify cleanup
+		monitor.mu.Lock()
+		_, commandExists := monitor.dockerCommands[host.ID]
+		_, indexExists := monitor.dockerCommandIndex[cmdStatus.ID]
+		monitor.mu.Unlock()
+
+		if commandExists {
+			t.Fatal("expected expired command to be removed from dockerCommands")
+		}
+		if indexExists {
+			t.Fatal("expected expired command to be removed from dockerCommandIndex")
+		}
+
+		// Verify state was updated with expired status
+		hostState := findDockerHost(t, monitor, host.ID)
+		if hostState.Command == nil {
+			t.Fatal("expected host command to be set in state")
+		}
+		if hostState.Command.Status != DockerCommandStatusExpired {
+			t.Fatalf("expected expired status in state, got %q", hostState.Command.Status)
+		}
+		if hostState.Command.FailureReason == "" {
+			t.Fatal("expected failure reason to be set on expired command")
+		}
+	})
+
+	t.Run("queued command is marked as dispatched", func(t *testing.T) {
+		t.Parallel()
+
+		monitor := newTestMonitorForCommands(t)
+
+		host := models.DockerHost{
+			ID:          "host-queued",
+			Hostname:    "node-queued",
+			DisplayName: "node-queued",
+			Status:      "online",
+		}
+		monitor.state.UpsertDockerHost(host)
+
+		// Queue a command
+		_, err := monitor.queueDockerStopCommand(host.ID)
+		if err != nil {
+			t.Fatalf("queue stop command: %v", err)
+		}
+
+		// Verify command is queued
+		monitor.mu.Lock()
+		cmd := monitor.dockerCommands[host.ID]
+		if cmd.status.Status != DockerCommandStatusQueued {
+			t.Fatalf("expected queued status before fetch, got %q", cmd.status.Status)
+		}
+		monitor.mu.Unlock()
+
+		// Fetch payload - should mark as dispatched
+		payload, status := monitor.getDockerCommandPayload(host.ID)
+
+		// For stop command, payload is nil but status should be returned
+		if status == nil {
+			t.Fatal("expected status to be returned")
+		}
+		if status.Status != DockerCommandStatusDispatched {
+			t.Fatalf("expected dispatched status, got %q", status.Status)
+		}
+		if status.DispatchedAt == nil {
+			t.Fatal("expected DispatchedAt to be set")
+		}
+
+		// Verify state was updated
+		hostState := findDockerHost(t, monitor, host.ID)
+		if hostState.Command == nil {
+			t.Fatal("expected host command to be set in state")
+		}
+		if hostState.Command.Status != DockerCommandStatusDispatched {
+			t.Fatalf("expected dispatched status in state, got %q", hostState.Command.Status)
+		}
+
+		// payload is nil for stop commands (no additional data needed)
+		if payload != nil {
+			t.Fatalf("expected nil payload for stop command, got %v", payload)
+		}
+	})
+
+	t.Run("already dispatched command is not re-marked", func(t *testing.T) {
+		t.Parallel()
+
+		monitor := newTestMonitorForCommands(t)
+
+		host := models.DockerHost{
+			ID:          "host-dispatched",
+			Hostname:    "node-dispatched",
+			DisplayName: "node-dispatched",
+			Status:      "online",
+		}
+		monitor.state.UpsertDockerHost(host)
+
+		// Queue a command
+		_, err := monitor.queueDockerStopCommand(host.ID)
+		if err != nil {
+			t.Fatalf("queue stop command: %v", err)
+		}
+
+		// First fetch - marks as dispatched
+		_, status1 := monitor.getDockerCommandPayload(host.ID)
+		if status1 == nil {
+			t.Fatal("expected status from first fetch")
+		}
+		firstDispatchedAt := status1.DispatchedAt
+		firstUpdatedAt := status1.UpdatedAt
+
+		// Small delay to ensure time would change if re-marked
+		time.Sleep(time.Millisecond)
+
+		// Second fetch - should NOT re-mark
+		_, status2 := monitor.getDockerCommandPayload(host.ID)
+		if status2 == nil {
+			t.Fatal("expected status from second fetch")
+		}
+
+		// DispatchedAt should be the same
+		if status2.DispatchedAt == nil {
+			t.Fatal("expected DispatchedAt to still be set")
+		}
+		if !status2.DispatchedAt.Equal(*firstDispatchedAt) {
+			t.Fatalf("DispatchedAt changed: first=%v, second=%v", *firstDispatchedAt, *status2.DispatchedAt)
+		}
+
+		// UpdatedAt should be the same (not re-updated)
+		if !status2.UpdatedAt.Equal(firstUpdatedAt) {
+			t.Fatalf("UpdatedAt changed: first=%v, second=%v", firstUpdatedAt, status2.UpdatedAt)
+		}
+
+		// Status should still be dispatched
+		if status2.Status != DockerCommandStatusDispatched {
+			t.Fatalf("expected dispatched status, got %q", status2.Status)
+		}
+	})
+
+	t.Run("returns payload and status copy", func(t *testing.T) {
+		t.Parallel()
+
+		monitor := newTestMonitorForCommands(t)
+
+		host := models.DockerHost{
+			ID:          "host-payload",
+			Hostname:    "node-payload",
+			DisplayName: "node-payload",
+			Status:      "online",
+		}
+		monitor.state.UpsertDockerHost(host)
+
+		// Manually create a command with a payload
+		testPayload := map[string]any{
+			"action": "test",
+			"count":  42,
+		}
+		cmd := newDockerHostCommand("test-type", "test message", dockerCommandDefaultTTL, testPayload)
+
+		monitor.mu.Lock()
+		monitor.dockerCommands[host.ID] = &cmd
+		monitor.dockerCommandIndex[cmd.status.ID] = host.ID
+		monitor.mu.Unlock()
+
+		// Fetch payload
+		payload, status := monitor.getDockerCommandPayload(host.ID)
+
+		if payload == nil {
+			t.Fatal("expected payload to be returned")
+		}
+		if payload["action"] != "test" {
+			t.Fatalf("expected action 'test', got %v", payload["action"])
+		}
+		if payload["count"] != 42 {
+			t.Fatalf("expected count 42, got %v", payload["count"])
+		}
+
+		if status == nil {
+			t.Fatal("expected status to be returned")
+		}
+		if status.Type != "test-type" {
+			t.Fatalf("expected type 'test-type', got %q", status.Type)
+		}
+		if status.Message != "test message" {
+			t.Fatalf("expected message 'test message', got %q", status.Message)
+		}
+
+		// Verify it's a copy by modifying the returned status
+		status.Message = "modified"
+		monitor.mu.Lock()
+		originalCmd := monitor.dockerCommands[host.ID]
+		if originalCmd.status.Message == "modified" {
+			t.Fatal("modifying returned status should not affect original")
+		}
+		monitor.mu.Unlock()
+	})
+
+	t.Run("hostID with whitespace is normalized", func(t *testing.T) {
+		t.Parallel()
+
+		monitor := newTestMonitorForCommands(t)
+
+		host := models.DockerHost{
+			ID:          "host-whitespace",
+			Hostname:    "node-whitespace",
+			DisplayName: "node-whitespace",
+			Status:      "online",
+		}
+		monitor.state.UpsertDockerHost(host)
+
+		// Queue a command
+		_, err := monitor.queueDockerStopCommand(host.ID)
+		if err != nil {
+			t.Fatalf("queue stop command: %v", err)
+		}
+
+		// Fetch with whitespace-padded ID
+		_, status := monitor.getDockerCommandPayload("  host-whitespace  ")
+		if status == nil {
+			t.Fatal("expected status to be returned for whitespace-padded hostID")
+		}
+		if status.Status != DockerCommandStatusDispatched {
+			t.Fatalf("expected dispatched status, got %q", status.Status)
+		}
+	})
+}
+
 func TestAcknowledgeDockerCommandErrorPaths(t *testing.T) {
 	t.Parallel()
 

internal/monitoring/metrics_history_test.go

@@ -499,6 +499,190 @@ func TestGetGuestMetrics(t *testing.T) {
 	}
 }
 
+func TestGetGuestMetrics_AllMetricTypes(t *testing.T) {
+	now := time.Now()
+
+	tests := []struct {
+		name       string
+		metricType string
+		value      float64
+	}{
+		{name: "cpu metric", metricType: "cpu", value: 50.5},
+		{name: "memory metric", metricType: "memory", value: 70.0},
+		{name: "disk metric", metricType: "disk", value: 45.0},
+		{name: "diskread metric", metricType: "diskread", value: 1024.0},
+		{name: "diskwrite metric", metricType: "diskwrite", value: 512.0},
+		{name: "netin metric", metricType: "netin", value: 2048.0},
+		{name: "netout metric", metricType: "netout", value: 1536.0},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mh := NewMetricsHistory(100, time.Hour)
+			mh.AddGuestMetric("vm-test", tt.metricType, tt.value, now.Add(-5*time.Minute))
+
+			result := mh.GetGuestMetrics("vm-test", tt.metricType, time.Hour)
+
+			if len(result) != 1 {
+				t.Fatalf("len(result) = %d, want 1", len(result))
+			}
+			if result[0].Value != tt.value {
+				t.Errorf("value = %v, want %v", result[0].Value, tt.value)
+			}
+		})
+	}
+}
+
+func TestGetGuestMetrics_GuestNotFound(t *testing.T) {
+	mh := NewMetricsHistory(100, time.Hour)
+
+	// Add data for one guest
+	mh.AddGuestMetric("vm-100", "cpu", 50.0, time.Now())
+
+	// Query non-existent guest
+	result := mh.GetGuestMetrics("vm-nonexistent", "cpu", time.Hour)
+
+	if len(result) != 0 {
+		t.Errorf("expected empty slice for non-existent guest, got %d elements", len(result))
+	}
+	if result == nil {
+		t.Error("expected empty slice, got nil")
+	}
+}
+
+func TestGetGuestMetrics_UnknownMetricType(t *testing.T) {
+	now := time.Now()
+	mh := NewMetricsHistory(100, time.Hour)
+
+	// Add data for the guest
+	mh.AddGuestMetric("vm-100", "cpu", 50.0, now.Add(-5*time.Minute))
+	mh.AddGuestMetric("vm-100", "memory", 70.0, now.Add(-5*time.Minute))
+
+	unknownTypes := []string{"invalid", "unknown", "foo", "bar", "CPU", "Memory", ""}
+
+	for _, metricType := range unknownTypes {
+		t.Run("type_"+metricType, func(t *testing.T) {
+			result := mh.GetGuestMetrics("vm-100", metricType, time.Hour)
+
+			if len(result) != 0 {
+				t.Errorf("expected empty slice for unknown metric type %q, got %d elements", metricType, len(result))
+			}
+			if result == nil {
+				t.Errorf("expected empty slice for unknown metric type %q, got nil", metricType)
+			}
+		})
+	}
+}
+
+func TestGetGuestMetrics_DurationFiltering(t *testing.T) {
+	now := time.Now()
+	mh := NewMetricsHistory(100, 2*time.Hour)
+
+	// Add points at different times
+	mh.AddGuestMetric("vm-100", "cpu", 10.0, now.Add(-90*time.Minute)) // old
+	mh.AddGuestMetric("vm-100", "cpu", 20.0, now.Add(-60*time.Minute)) // old
+	mh.AddGuestMetric("vm-100", "cpu", 30.0, now.Add(-30*time.Minute)) // recent
+	mh.AddGuestMetric("vm-100", "cpu", 40.0, now.Add(-15*time.Minute)) // recent
+	mh.AddGuestMetric("vm-100", "cpu", 50.0, now.Add(-5*time.Minute))  // recent
+
+	tests := []struct {
+		name      string
+		duration  time.Duration
+		wantLen   int
+		wantFirst float64
+		wantLast  float64
+	}{
+		{
+			name:      "all points within 2 hours",
+			duration:  2 * time.Hour,
+			wantLen:   5,
+			wantFirst: 10.0,
+			wantLast:  50.0,
+		},
+		{
+			name:      "points within 45 minutes",
+			duration:  45 * time.Minute,
+			wantLen:   3,
+			wantFirst: 30.0,
+			wantLast:  50.0,
+		},
+		{
+			name:      "points within 20 minutes",
+			duration:  20 * time.Minute,
+			wantLen:   2,
+			wantFirst: 40.0,
+			wantLast:  50.0,
+		},
+		{
+			name:      "points within 10 minutes",
+			duration:  10 * time.Minute,
+			wantLen:   1,
+			wantFirst: 50.0,
+			wantLast:  50.0,
+		},
+		{
+			name:     "zero duration excludes all",
+			duration: 0,
+			wantLen:  0,
+		},
+		{
+			name:     "very short duration excludes all",
+			duration: 1 * time.Minute,
+			wantLen:  0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := mh.GetGuestMetrics("vm-100", "cpu", tt.duration)
+
+			if len(result) != tt.wantLen {
+				t.Errorf("len(result) = %d, want %d", len(result), tt.wantLen)
+			}
+			if len(result) > 0 {
+				if result[0].Value != tt.wantFirst {
+					t.Errorf("first value = %v, want %v", result[0].Value, tt.wantFirst)
+				}
+				if result[len(result)-1].Value != tt.wantLast {
+					t.Errorf("last value = %v, want %v", result[len(result)-1].Value, tt.wantLast)
+				}
+			}
+		})
+	}
+}
+
+func TestGetGuestMetrics_EmptyMetricsData(t *testing.T) {
+	mh := NewMetricsHistory(100, time.Hour)
+
+	// Directly populate an empty guest metrics entry
+	mh.mu.Lock()
+	mh.guestMetrics["vm-empty"] = &GuestMetrics{
+		CPU:        []MetricPoint{},
+		Memory:     []MetricPoint{},
+		Disk:       []MetricPoint{},
+		DiskRead:   []MetricPoint{},
+		DiskWrite:  []MetricPoint{},
+		NetworkIn:  []MetricPoint{},
+		NetworkOut: []MetricPoint{},
+	}
+	mh.mu.Unlock()
+
+	metricTypes := []string{"cpu", "memory", "disk", "diskread", "diskwrite", "netin", "netout"}
+
+	for _, metricType := range metricTypes {
+		t.Run(metricType, func(t *testing.T) {
+			result := mh.GetGuestMetrics("vm-empty", metricType, time.Hour)
+
+			if len(result) != 0 {
+				t.Errorf("expected empty slice for empty %s metrics, got %d elements", metricType, len(result))
+			}
+			if result == nil {
+				t.Errorf("expected empty slice for empty %s metrics, got nil", metricType)
+			}
+		})
+	}
+}
+
 func TestGetNodeMetrics(t *testing.T) {
 	now := time.Now()
 	mh := NewMetricsHistory(100, time.Hour)

internal/monitoring/staleness_tracker_test.go

@@ -491,3 +491,190 @@ func TestTrackerKey(t *testing.T) {
 		}
 	}
 }
+
+func TestStalenessTracker_MergeSnapshot_TableDriven(t *testing.T) {
+	baseTime := time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC)
+	olderTime := baseTime.Add(-10 * time.Second)
+	newerTime := baseTime.Add(10 * time.Second)
+
+	tests := []struct {
+		name            string
+		existing        *FreshnessSnapshot // nil means no existing entry
+		merge           FreshnessSnapshot
+		wantLastSuccess time.Time
+		wantLastError   time.Time
+		wantLastMutated time.Time
+		wantChangeHash  string
+	}{
+		{
+			name:     "merge into non-existent entry creates new entry",
+			existing: nil,
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "new-instance",
+				LastSuccess:  baseTime,
+				LastError:    baseTime,
+				LastMutated:  baseTime,
+				ChangeHash:   "abc123",
+			},
+			wantLastSuccess: baseTime,
+			wantLastError:   baseTime,
+			wantLastMutated: baseTime,
+			wantChangeHash:  "abc123",
+		},
+		{
+			name: "newer LastSuccess updates existing",
+			existing: &FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastSuccess:  baseTime,
+			},
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastSuccess:  newerTime,
+			},
+			wantLastSuccess: newerTime,
+		},
+		{
+			name: "older LastSuccess does not update existing",
+			existing: &FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastSuccess:  baseTime,
+			},
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastSuccess:  olderTime,
+			},
+			wantLastSuccess: baseTime,
+		},
+		{
+			name: "newer LastError updates existing",
+			existing: &FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastError:    baseTime,
+			},
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastError:    newerTime,
+			},
+			wantLastError: newerTime,
+		},
+		{
+			name: "older LastError does not update existing",
+			existing: &FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastError:    baseTime,
+			},
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastError:    olderTime,
+			},
+			wantLastError: baseTime,
+		},
+		{
+			name: "newer LastMutated updates existing",
+			existing: &FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastMutated:  baseTime,
+			},
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastMutated:  newerTime,
+			},
+			wantLastMutated: newerTime,
+		},
+		{
+			name: "older LastMutated does not update existing",
+			existing: &FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastMutated:  baseTime,
+			},
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				LastMutated:  olderTime,
+			},
+			wantLastMutated: baseTime,
+		},
+		{
+			name: "non-empty ChangeHash updates existing",
+			existing: &FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				ChangeHash:   "old-hash",
+			},
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				ChangeHash:   "new-hash",
+			},
+			wantChangeHash: "new-hash",
+		},
+		{
+			name: "empty ChangeHash does not overwrite existing",
+			existing: &FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				ChangeHash:   "existing-hash",
+			},
+			merge: FreshnessSnapshot{
+				InstanceType: InstanceTypePVE,
+				Instance:     "test",
+				ChangeHash:   "",
+			},
+			wantChangeHash: "existing-hash",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tracker := NewStalenessTracker(nil)
+
+			// Set up existing entry if specified
+			if tt.existing != nil {
+				tracker.setSnapshot(*tt.existing)
+			}
+
+			// Perform merge
+			tracker.mergeSnapshot(tt.merge)
+
+			// Get result
+			snap, ok := tracker.snapshot(tt.merge.InstanceType, tt.merge.Instance)
+			if !ok {
+				t.Fatal("snapshot not found after merge")
+			}
+
+			// Verify instance metadata is always set
+			if snap.InstanceType != tt.merge.InstanceType {
+				t.Errorf("InstanceType = %v, want %v", snap.InstanceType, tt.merge.InstanceType)
+			}
+			if snap.Instance != tt.merge.Instance {
+				t.Errorf("Instance = %q, want %q", snap.Instance, tt.merge.Instance)
+			}
+
+			// Verify timestamps
+			if !tt.wantLastSuccess.IsZero() && !snap.LastSuccess.Equal(tt.wantLastSuccess) {
+				t.Errorf("LastSuccess = %v, want %v", snap.LastSuccess, tt.wantLastSuccess)
+			}
+			if !tt.wantLastError.IsZero() && !snap.LastError.Equal(tt.wantLastError) {
+				t.Errorf("LastError = %v, want %v", snap.LastError, tt.wantLastError)
+			}
+			if !tt.wantLastMutated.IsZero() && !snap.LastMutated.Equal(tt.wantLastMutated) {
+				t.Errorf("LastMutated = %v, want %v", snap.LastMutated, tt.wantLastMutated)
+			}
+			if tt.wantChangeHash != "" && snap.ChangeHash != tt.wantChangeHash {
+				t.Errorf("ChangeHash = %q, want %q", snap.ChangeHash, tt.wantChangeHash)
+			}
+		})
+	}
+}