fix(health): improve unified health status consistency across interfaces

- Refactor CLI auth status to use unified health status from backend (FR-006, FR-007)
- CLI upstream list now uses health.CalculateHealth() for DRY principle (I-003)
- Add tooltip in ServerCard.vue showing health.detail for context (M-004)
- Add defensive null check in Dashboard.vue for backward compatibility (I-004)
- Include exact token expiration time in Detail field (M-002)
- Add debug logging for health status calculation (M-005)
- Add E2E tests verifying health field structure in MCP responses (FR-017, FR-018)
- Add unit tests ensuring Summary is never empty (FR-004, I-002)
- Extract health status in management service ListServers

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: technicalpickles

cmd/mcpproxy/auth_cmd.go

@@ -228,10 +228,7 @@ func runAuthStatusClientMode(ctx context.Context, dataDir, serverName string, al
 		name, _ := srv["name"].(string)
 		oauth, _ := srv["oauth"].(map[string]interface{})
 		authenticated, _ := srv["authenticated"].(bool)
-		connected, _ := srv["connected"].(bool)
 		lastError, _ := srv["last_error"].(string)
-		enabled, _ := srv["enabled"].(bool)
-		userLoggedOut, _ := srv["user_logged_out"].(bool)
 
 		// Check if this is an OAuth server by:
 		// 1. Has oauth config OR
@@ -247,50 +244,56 @@ func runAuthStatusClientMode(ctx context.Context, dataDir, serverName string, al
 
 		hasOAuthServers = true
 
-		// Determine status emoji and text using oauth_status for accurate state
-		oauthStatus, _ := srv["oauth_status"].(string)
-		var status string
-
-		// Check priority states first
-		if !enabled {
-			// Server is disabled - no reconnection attempts
-			status = "⏸️  Disabled"
-		} else if userLoggedOut {
-			// User explicitly logged out - no auto-reconnection
-			status = "🚪 Logged Out (Login Required)"
-		} else if connected {
-			// Connected states
-			if authenticated {
-				status = "✅ Authenticated & Connected"
-			} else {
-				status = "⚠️  Connected (No OAuth Token)"
-			}
-		} else {
-			// Disconnected states - use oauth_status for clarity
-			switch oauthStatus {
-			case "authenticated":
-				// Token valid but not connected - likely reconnecting
-				status = "⏳ Reconnecting (Token Valid)"
-			case "expired":
-				// Token expired - needs re-authentication
-				status = "⚠️  Token Expired (Login Required)"
-			case "error":
-				status = "❌ Authentication Error"
+		// Use unified health status from backend (FR-006, FR-007)
+		var healthLevel, adminState, healthSummary, healthAction string
+		if health, ok := srv["health"].(map[string]interface{}); ok && health != nil {
+			healthLevel, _ = health["level"].(string)
+			adminState, _ = health["admin_state"].(string)
+			healthSummary, _ = health["summary"].(string)
+			healthAction, _ = health["action"].(string)
+		}
+
+		// Determine status emoji based on admin_state first, then health level
+		var statusEmoji string
+		switch adminState {
+		case "disabled":
+			statusEmoji = "⏸️"
+		case "quarantined":
+			statusEmoji = "🔒"
+		default:
+			// Use health level for enabled servers
+			switch healthLevel {
+			case "healthy":
+				statusEmoji = "✅"
+			case "degraded":
+				statusEmoji = "⚠️"
+			case "unhealthy":
+				statusEmoji = "❌"
 			default:
-				// No token or oauth_status not set
-				if lastError != "" {
-					status = "❌ Authentication Failed"
-				} else if authenticated {
-					// Fallback: has token but no oauth_status
-					status = "⏳ Reconnecting"
-				} else {
-					status = "⏳ Pending Authentication"
-				}
+				statusEmoji = "❓"
 			}
 		}
 
 		fmt.Printf("Server: %s\n", name)
-		fmt.Printf("  Status: %s\n", status)
+		fmt.Printf("  Health: %s %s\n", statusEmoji, healthSummary)
+		if adminState != "" && adminState != "enabled" {
+			fmt.Printf("  Admin State: %s\n", adminState)
+		}
+		// Show action as command hint (FR-007)
+		if healthAction != "" {
+			switch healthAction {
+			case "login":
+				fmt.Printf("  Action: mcpproxy auth login --server=%s\n", name)
+			case "restart":
+				fmt.Printf("  Action: mcpproxy upstream restart %s\n", name)
+			case "enable":
+				fmt.Printf("  Action: mcpproxy upstream enable %s\n", name)
+			case "approve":
+				fmt.Printf("  Action: Approve via Web UI or tray menu\n")
+			case "view_logs":
+				fmt.Printf("  Action: mcpproxy upstream logs %s\n", name)
+			}
+		}
 
 		// Display OAuth configuration details (if available)
 		// Check if this is an autodiscovery server (no explicit OAuth config, but has token)

cmd/mcpproxy/upstream_cmd.go

@@ -18,6 +18,7 @@ import (
 
 	"mcpproxy-go/internal/cliclient"
 	"mcpproxy-go/internal/config"
+	"mcpproxy-go/internal/health"
 	"mcpproxy-go/internal/logs"
 	"mcpproxy-go/internal/reqcontext"
 	"mcpproxy-go/internal/socket"
@@ -177,20 +178,21 @@ func runUpstreamListFromConfig(globalConfig *config.Config) error {
 	// Convert config servers to output format
 	servers := make([]map[string]interface{}, len(globalConfig.Servers))
 	for i, srv := range globalConfig.Servers {
-		// Create health status for config-only mode
-		healthLevel := "unknown"
-		healthAdminState := "enabled"
-		healthSummary := "Daemon not running"
-		healthAction := ""
-
-		if !srv.Enabled {
-			healthAdminState = "disabled"
-			healthSummary = "Disabled"
-			healthAction = "enable"
-		} else if srv.Quarantined {
-			healthAdminState = "quarantined"
-			healthSummary = "Quarantined for review"
-			healthAction = "approve"
+		// I-003: Use health.CalculateHealth() instead of inline logic for DRY principle
+		healthInput := health.HealthCalculatorInput{
+			Name:        srv.Name,
+			Enabled:     srv.Enabled,
+			Quarantined: srv.Quarantined,
+			State:       "disconnected", // Daemon not running
+			Connected:   false,
+			ToolCount:   0,
+		}
+		healthStatus := health.CalculateHealth(healthInput, health.DefaultHealthConfig())
+
+		// Override summary for config-only mode to indicate daemon status
+		summary := healthStatus.Summary
+		if healthStatus.AdminState == health.StateEnabled {
+			summary = "Daemon not running"
 		}
 
 		servers[i] = map[string]interface{}{
@@ -199,12 +201,13 @@ func runUpstreamListFromConfig(globalConfig *config.Config) error {
 			"protocol":   srv.Protocol,
 			"connected":  false,
 			"tool_count": 0,
-			"status":     healthSummary,
+			"status":     summary,
 			"health": map[string]interface{}{
-				"level":       healthLevel,
-				"admin_state": healthAdminState,
-				"summary":     healthSummary,
-				"action":      healthAction,
+				"level":       healthStatus.Level,
+				"admin_state": healthStatus.AdminState,
+				"summary":     summary,
+				"detail":      healthStatus.Detail,
+				"action":      healthStatus.Action,
 			},
 		}
 	}

frontend/src/components/ServerCard.vue

@@ -11,11 +11,14 @@
         </div>
 
         <!-- Status indicator using unified health status -->
+        <!-- M-004: Add tooltip showing health.detail if present -->
         <div
           :class="[
             'badge badge-sm flex-shrink-0',
-            statusBadgeClass
+            statusBadgeClass,
+            statusTooltip ? 'tooltip tooltip-left' : ''
           ]"
+          :data-tip="statusTooltip"
         >
           {{ statusText }}
         </div>
@@ -236,6 +239,15 @@ const statusText = computed(() => {
   return 'Disconnected'
 })
 
+// M-004: Tooltip showing health.detail if present (for additional context)
+const statusTooltip = computed(() => {
+  const health = props.server.health
+  if (health?.detail) {
+    return health.detail
+  }
+  return ''
+})
+
 // Suggested action from health status
 const healthAction = computed(() => {
   return props.server.health?.action || ''

frontend/src/views/Dashboard.vue

@@ -462,12 +462,17 @@ const diagnosticsBadgeClass = computed(() => {
 // Servers needing attention (unhealthy or degraded health level, excluding admin states)
 const serversNeedingAttention = computed(() => {
   return serversStore.servers.filter(server => {
+    // I-004: Defensive null check for backward compatibility
+    if (!server.health) {
+      console.warn(`Server ${server.name} missing health field`)
+      return false
+    }
     // Skip servers with admin states (disabled, quarantined)
-    if (server.health?.admin_state === 'disabled' || server.health?.admin_state === 'quarantined') {
+    if (server.health.admin_state === 'disabled' || server.health.admin_state === 'quarantined') {
       return false
     }
     // Include servers with unhealthy or degraded health level
-    return server.health?.level === 'unhealthy' || server.health?.level === 'degraded'
+    return server.health.level === 'unhealthy' || server.health.level === 'degraded'
   })
 })
 

internal/health/calculator.go

@@ -154,10 +154,12 @@ func CalculateHealth(input HealthCalculatorInput, cfg *HealthCalculatorConfig) *
 					}
 				}
 				// No refresh token - user needs to re-authenticate soon
+				// M-002: Include exact expiration time in Detail field
 				return &contracts.HealthStatus{
 					Level:      LevelDegraded,
 					AdminState: StateEnabled,
 					Summary:    formatExpiringTokenSummary(timeUntilExpiry),
+					Detail:     fmt.Sprintf("Token expires at %s", input.TokenExpiresAt.Format(time.RFC3339)),
 					Action:     ActionLogin,
 				}
 			}

internal/health/calculator_test.go

@@ -374,3 +374,38 @@ func TestDefaultHealthConfig(t *testing.T) {
 	assert.NotNil(t, cfg)
 	assert.Equal(t, time.Hour, cfg.ExpiryWarningDuration)
 }
+
+// I-002: Test FR-004 - All health status responses must include non-empty summary
+func TestCalculateHealth_AlwaysIncludesSummary(t *testing.T) {
+	expiresAt := time.Now().Add(30 * time.Minute)
+
+	testCases := []struct {
+		name  string
+		input HealthCalculatorInput
+	}{
+		{"disabled server", HealthCalculatorInput{Name: "test", Enabled: false}},
+		{"quarantined server", HealthCalculatorInput{Name: "test", Enabled: true, Quarantined: true}},
+		{"error state", HealthCalculatorInput{Name: "test", Enabled: true, State: "error", LastError: "connection refused"}},
+		{"error state no message", HealthCalculatorInput{Name: "test", Enabled: true, State: "error", LastError: ""}},
+		{"disconnected state", HealthCalculatorInput{Name: "test", Enabled: true, State: "disconnected"}},
+		{"connecting state", HealthCalculatorInput{Name: "test", Enabled: true, State: "connecting"}},
+		{"idle state", HealthCalculatorInput{Name: "test", Enabled: true, State: "idle"}},
+		{"connected healthy", HealthCalculatorInput{Name: "test", Enabled: true, State: "connected", Connected: true, ToolCount: 5}},
+		{"connected no tools", HealthCalculatorInput{Name: "test", Enabled: true, State: "connected", Connected: true, ToolCount: 0}},
+		{"oauth expired", HealthCalculatorInput{Name: "test", Enabled: true, State: "connected", Connected: true, OAuthRequired: true, OAuthStatus: "expired"}},
+		{"oauth none", HealthCalculatorInput{Name: "test", Enabled: true, State: "connected", Connected: true, OAuthRequired: true, OAuthStatus: "none"}},
+		{"oauth error", HealthCalculatorInput{Name: "test", Enabled: true, State: "connected", Connected: true, OAuthRequired: true, OAuthStatus: "error"}},
+		{"user logged out", HealthCalculatorInput{Name: "test", Enabled: true, State: "connected", OAuthRequired: true, OAuthStatus: "authenticated", UserLoggedOut: true}},
+		{"token expiring no refresh", HealthCalculatorInput{Name: "test", Enabled: true, State: "connected", Connected: true, OAuthRequired: true, OAuthStatus: "authenticated", TokenExpiresAt: &expiresAt, HasRefreshToken: false}},
+		{"token expiring with refresh", HealthCalculatorInput{Name: "test", Enabled: true, State: "connected", Connected: true, OAuthRequired: true, OAuthStatus: "authenticated", TokenExpiresAt: &expiresAt, HasRefreshToken: true, ToolCount: 5}},
+		{"unknown state", HealthCalculatorInput{Name: "test", Enabled: true, State: "unknown"}},
+		{"empty state", HealthCalculatorInput{Name: "test", Enabled: true, State: ""}},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := CalculateHealth(tc.input, nil)
+			assert.NotEmpty(t, result.Summary, "FR-004: Summary should never be empty for %s", tc.name)
+		})
+	}
+}

internal/management/service.go

@@ -289,6 +289,11 @@ func (s *service) ListServers(ctx context.Context) ([]*contracts.Server, *contra
 			srv.Updated = updated
 		}
 
+		// Extract unified health status
+		if health, ok := srvRaw["health"].(*contracts.HealthStatus); ok {
+			srv.Health = health
+		}
+
 		servers = append(servers, srv)
 
 		// Update stats

internal/runtime/runtime.go

@@ -1679,7 +1679,16 @@ func (r *Runtime) GetAllServers() ([]map[string]interface{}, error) {
 			healthInput.TokenExpiresAt = &tokenExpiresAt
 		}
 
-		serverMap["health"] = health.CalculateHealth(healthInput, healthConfig)
+		healthStatus := health.CalculateHealth(healthInput, healthConfig)
+		serverMap["health"] = healthStatus
+
+		// M-005: Log health status for debugging
+		r.logger.Debug("Server health calculated",
+			zap.String("server", serverStatus.Name),
+			zap.String("level", healthStatus.Level),
+			zap.String("admin_state", healthStatus.AdminState),
+			zap.String("summary", healthStatus.Summary),
+		)
 
 		result = append(result, serverMap)
 	}

internal/server/e2e_mcp_test.go

@@ -141,6 +141,25 @@ func TestMCPProtocolWithBinary(t *testing.T) {
 		assert.Equal(t, "memory", serverMap["name"])
 		assert.Equal(t, "stdio", serverMap["protocol"])
 		assert.Equal(t, true, serverMap["enabled"])
+
+		// I-001: Verify health field is present with expected structure (FR-017, FR-018)
+		healthMap, ok := serverMap["health"].(map[string]interface{})
+		require.True(t, ok, "Server should have health field")
+		assert.NotEmpty(t, healthMap["level"], "Health level should be present")
+		assert.NotEmpty(t, healthMap["admin_state"], "Admin state should be present")
+		assert.NotEmpty(t, healthMap["summary"], "Summary should be present")
+
+		// Verify health level is one of the valid values
+		level, ok := healthMap["level"].(string)
+		require.True(t, ok, "Health level should be a string")
+		validLevels := []string{"healthy", "degraded", "unhealthy"}
+		assert.Contains(t, validLevels, level, "Health level should be valid")
+
+		// Verify admin state is one of the valid values
+		adminState, ok := healthMap["admin_state"].(string)
+		require.True(t, ok, "Admin state should be a string")
+		validStates := []string{"enabled", "disabled", "quarantined"}
+		assert.Contains(t, validStates, adminState, "Admin state should be valid")
 	})
 }