Also implement consistency checks in ListWorkloads (#1381)

This was meant to be included in my previous PR.

Author: dmjb

pkg/workloads/statuses/file_status.go

@@ -146,15 +146,22 @@ func (f *fileStatusManager) ListWorkloads(ctx context.Context, listAll bool, lab
 		workloadMap[container.Name] = workload
 	}
 
-	// Then, merge with file workloads, preferring file status
+	// Then, merge with file workloads, validating running workloads
 	for name, fileWorkload := range fileWorkloads {
-		if runtimeWorkload, exists := workloadMap[name]; exists {
-			// Merge: use runtime data but prefer file status
-			merged := runtimeWorkload
-			merged.Status = fileWorkload.Status
-			merged.StatusContext = fileWorkload.StatusContext
-			merged.CreatedAt = fileWorkload.CreatedAt
-			workloadMap[name] = merged
+		if runtimeContainer, exists := runtimeWorkloadMap[name]; exists {
+			// Validate running workloads similar to GetWorkload
+			validatedWorkload, err := f.validateWorkloadInList(ctx, name, fileWorkload, runtimeContainer)
+			if err != nil {
+				logger.Warnf("failed to validate workload %s in list: %v", name, err)
+				// Fall back to basic merge without validation
+				runtimeWorkload := workloadMap[name]
+				runtimeWorkload.Status = fileWorkload.Status
+				runtimeWorkload.StatusContext = fileWorkload.StatusContext
+				runtimeWorkload.CreatedAt = fileWorkload.CreatedAt
+				workloadMap[name] = runtimeWorkload
+			} else {
+				workloadMap[name] = validatedWorkload
+			}
 		} else {
 			// File-only workload (runtime not available)
 			workloadMap[name] = fileWorkload
@@ -506,3 +513,36 @@ func (*fileStatusManager) mergeHealthyWorkloadData(containerInfo rt.ContainerInf
 	runtimeResult.CreatedAt = result.CreatedAt         // Keep the file created time
 	return runtimeResult, nil
 }
+
+// validateWorkloadInList validates a workload during list operations, similar to validateRunningWorkload
+// but with different error handling to avoid disrupting the entire list operation.
+func (f *fileStatusManager) validateWorkloadInList(
+	ctx context.Context, workloadName string, fileWorkload core.Workload, containerInfo rt.ContainerInfo,
+) (core.Workload, error) {
+	// Only validate if file shows running status
+	if fileWorkload.Status != rt.WorkloadStatusRunning {
+		// For non-running workloads, just merge runtime data with file status
+		runtimeWorkload, err := types.WorkloadFromContainerInfo(&containerInfo)
+		if err != nil {
+			return core.Workload{}, err
+		}
+		runtimeWorkload.Status = fileWorkload.Status
+		runtimeWorkload.StatusContext = fileWorkload.StatusContext
+		runtimeWorkload.CreatedAt = fileWorkload.CreatedAt
+		return runtimeWorkload, nil
+	}
+
+	// For running workloads, apply full validation
+	// Check if runtime status matches file status
+	if containerInfo.State != rt.WorkloadStatusRunning {
+		return f.handleRuntimeMismatch(ctx, workloadName, fileWorkload, containerInfo)
+	}
+
+	// Check if proxy process is running when workload is running
+	if unhealthyWorkload, isUnhealthy := f.checkProxyHealth(ctx, workloadName, fileWorkload, containerInfo); isUnhealthy {
+		return unhealthyWorkload, nil
+	}
+
+	// Runtime and proxy confirm workload is healthy - merge runtime data with file status
+	return f.mergeHealthyWorkloadData(containerInfo, fileWorkload)
+}

pkg/workloads/statuses/file_status_test.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -860,3 +861,100 @@ func TestFileStatusManager_GetWorkload_HealthyWithProxy(t *testing.T) {
 	assert.Equal(t, "container started", workload.StatusContext) // Original file context preserved
 	assert.Equal(t, "test-image:latest", workload.Package)
 }
+
+func TestFileStatusManager_ListWorkloads_WithValidation(t *testing.T) {
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	tempDir := t.TempDir()
+	mockRuntime := mocks.NewMockRuntime(ctrl)
+	manager := &fileStatusManager{
+		baseDir: tempDir,
+		runtime: mockRuntime,
+	}
+	ctx := context.Background()
+
+	// Create file workloads - one healthy running, one with runtime mismatch, one with proxy down
+	err := manager.SetWorkloadStatus(ctx, "healthy-workload", rt.WorkloadStatusRunning, "container started")
+	require.NoError(t, err)
+
+	err = manager.SetWorkloadStatus(ctx, "runtime-mismatch", rt.WorkloadStatusRunning, "container started")
+	require.NoError(t, err)
+
+	err = manager.SetWorkloadStatus(ctx, "proxy-down", rt.WorkloadStatusRunning, "container started")
+	require.NoError(t, err)
+
+	// Mock runtime containers
+	runtimeContainers := []rt.ContainerInfo{
+		{
+			Name:   "healthy-workload",
+			Image:  "healthy:latest",
+			Status: "Up 5 minutes",
+			State:  rt.WorkloadStatusRunning,
+			Labels: map[string]string{
+				"toolhive":      "true",
+				"toolhive-name": "healthy-workload",
+			},
+		},
+		{
+			Name:   "runtime-mismatch",
+			Image:  "mismatch:latest",
+			Status: "Exited (0) 1 minute ago",
+			State:  rt.WorkloadStatusStopped, // Runtime says stopped, file says running
+			Labels: map[string]string{
+				"toolhive":      "true",
+				"toolhive-name": "runtime-mismatch",
+			},
+		},
+		{
+			Name:   "proxy-down",
+			Image:  "proxy:latest",
+			Status: "Up 3 minutes",
+			State:  rt.WorkloadStatusRunning,
+			Labels: map[string]string{
+				"toolhive":          "true",
+				"toolhive-name":     "proxy-down",
+				"toolhive-basename": "proxy-down", // This will trigger proxy check
+			},
+		},
+	}
+
+	mockRuntime.EXPECT().ListWorkloads(gomock.Any()).Return(runtimeContainers, nil)
+
+	// List all workloads
+	workloads, err := manager.ListWorkloads(ctx, true, nil)
+	require.NoError(t, err)
+
+	// Should have 3 workloads
+	require.Len(t, workloads, 3)
+
+	// Create a map for easier assertion
+	workloadMap := make(map[string]core.Workload)
+	for _, w := range workloads {
+		workloadMap[w.Name] = w
+	}
+
+	// Verify healthy workload remains running
+	healthyWorkload, exists := workloadMap["healthy-workload"]
+	require.True(t, exists)
+	assert.Equal(t, rt.WorkloadStatusRunning, healthyWorkload.Status)
+
+	// Verify runtime mismatch workload is marked unhealthy (status might be updated async)
+	// We'll check for either unhealthy or the original status with mismatch context
+	runtimeMismatch, exists := workloadMap["runtime-mismatch"]
+	require.True(t, exists)
+	// The workload should either be marked unhealthy or have a status context indicating the issue
+	isValidatedUnhealthy := runtimeMismatch.Status == rt.WorkloadStatusUnhealthy ||
+		strings.Contains(runtimeMismatch.StatusContext, "mismatch")
+	assert.True(t, isValidatedUnhealthy, "Runtime mismatch workload should be detected as unhealthy")
+
+	// Verify proxy down workload is detected (proxy.IsRunning will return false for non-existent proxy)
+	proxyDown, exists := workloadMap["proxy-down"]
+	require.True(t, exists)
+	// Similar check - should be unhealthy or have proxy-related context
+	isProxyValidated := proxyDown.Status == rt.WorkloadStatusUnhealthy ||
+		strings.Contains(proxyDown.StatusContext, "proxy")
+	assert.True(t, isProxyValidated, "Proxy down workload should be detected as unhealthy")
+}