fix: improve doctor CLI output and health propagation

- Fix OAuth display to show server objects with specific auth login hints
- Add sortArrayByServerName for consistent doctor output ordering
- Fix health extraction to handle both struct pointers and maps
- Add health status calculation to GetAllServers() endpoint
- Update Action field docs to include set_secret and configure

Author: technicalpickles

cmd/mcpproxy/doctor_cmd.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"sort"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -172,6 +173,9 @@ func outputDiagnostics(diag map[string]interface{}, info map[string]interface{})
 
 		// 1. Upstream Connection Errors
 		if upstreamErrors := getArrayField(diag, "upstream_errors"); len(upstreamErrors) > 0 {
+			// Sort by server name for consistent output
+			sortArrayByServerName(upstreamErrors)
+
 			fmt.Println("❌ Upstream Server Connection Errors")
 			fmt.Println("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━")
 			for _, errItem := range upstreamErrors {
@@ -192,21 +196,32 @@ func outputDiagnostics(diag map[string]interface{}, info map[string]interface{})
 		}
 
 		// 2. OAuth Required
-		if oauthRequired := getStringArrayField(diag, "oauth_required"); len(oauthRequired) > 0 {
+		if oauthRequired := getArrayField(diag, "oauth_required"); len(oauthRequired) > 0 {
+			// Sort by server name for consistent output
+			sortArrayByServerName(oauthRequired)
+
 			fmt.Println("🔑 OAuth Authentication Required")
 			fmt.Println("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━")
-			for _, server := range oauthRequired {
-				fmt.Printf("  • %s\n", server)
+			for _, item := range oauthRequired {
+				if oauthMap, ok := item.(map[string]interface{}); ok {
+					serverName := getStringField(oauthMap, "server_name")
+					message := getStringField(oauthMap, "message")
+					fmt.Printf("\nServer: %s\n", serverName)
+					if message != "" {
+						fmt.Printf("  %s\n", message)
+					} else {
+						fmt.Printf("  Run: mcpproxy auth login --server=%s\n", serverName)
+					}
+				}
 			}
 			fmt.Println()
-			fmt.Println("💡 Remediation:")
-			fmt.Println("  • Authenticate: mcpproxy auth login --server=<server-name>")
-			fmt.Println("  • Check OAuth config in mcp_config.json")
-			fmt.Println()
 		}
 
 		// 3. OAuth Configuration Issues
 		if oauthIssues := getArrayField(diag, "oauth_issues"); len(oauthIssues) > 0 {
+			// Sort by server name for consistent output
+			sortArrayByServerName(oauthIssues)
+
 			fmt.Println("🔍 OAuth Configuration Issues")
 			fmt.Println("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━")
 			for _, issueItem := range oauthIssues {
@@ -333,3 +348,17 @@ func createDoctorLogger(level string) (*zap.Logger, error) {
 
 	return cfg.Build()
 }
+
+// sortArrayByServerName sorts an array of maps by the "server_name" field alphabetically.
+func sortArrayByServerName(arr []interface{}) {
+	sort.Slice(arr, func(i, j int) bool {
+		iMap, iOk := arr[i].(map[string]interface{})
+		jMap, jOk := arr[j].(map[string]interface{})
+		if !iOk || !jOk {
+			return false
+		}
+		iName := getStringField(iMap, "server_name")
+		jName := getStringField(jMap, "server_name")
+		return iName < jName
+	})
+}

internal/contracts/types.go

@@ -578,7 +578,7 @@ type HealthStatus struct {
 	// Detail is an optional longer explanation of the status
 	Detail string `json:"detail,omitempty"`
 
-	// Action is the suggested fix action: "login", "restart", "enable", "approve", "view_logs", or "" (none)
+	// Action is the suggested fix action: "login", "restart", "enable", "approve", "view_logs", "set_secret", "configure", or "" (none)
 	Action string `json:"action,omitempty"`
 }
 

internal/management/diagnostics.go

@@ -13,6 +13,30 @@ import (
 	"mcpproxy-go/internal/secret"
 )
 
+// extractHealthFromMap extracts health status from a server map.
+// The health can be stored as either *contracts.HealthStatus (from GetAllServers)
+// or as map[string]interface{} (from JSON deserialization).
+func extractHealthFromMap(srvRaw map[string]interface{}) (action, detail string) {
+	healthRaw, ok := srvRaw["health"]
+	if !ok || healthRaw == nil {
+		return "", ""
+	}
+
+	// Try direct struct pointer first (from GetAllServers)
+	if hs, ok := healthRaw.(*contracts.HealthStatus); ok && hs != nil {
+		return hs.Action, hs.Detail
+	}
+
+	// Try map[string]interface{} (from JSON deserialization)
+	if healthMap, ok := healthRaw.(map[string]interface{}); ok && healthMap != nil {
+		action = getStringFromMap(healthMap, "action")
+		detail = getStringFromMap(healthMap, "detail")
+		return action, detail
+	}
+
+	return "", ""
+}
+
 // Doctor aggregates health diagnostics from all system components.
 // This implements FR-009 through FR-013: comprehensive health diagnostics.
 // Refactored to aggregate from Health.Action (single source of truth).
@@ -41,14 +65,9 @@ func (s *service) Doctor(ctx context.Context) (*contracts.Diagnostics, error) {
 		serverName := getStringFromMap(srvRaw, "name")
 		lastError := getStringFromMap(srvRaw, "last_error")
 
-		// Extract health status from server
-		healthData, _ := srvRaw["health"].(map[string]interface{})
-		healthAction := ""
-		healthDetail := ""
-		if healthData != nil {
-			healthAction = getStringFromMap(healthData, "action")
-			healthDetail = getStringFromMap(healthData, "detail")
-		}
+		// Extract health status from server using helper that handles both
+		// struct pointer (from GetAllServers) and map (from JSON)
+		healthAction, healthDetail := extractHealthFromMap(srvRaw)
 
 		// Aggregate based on Health.Action
 		switch healthAction {

internal/server/server.go

@@ -20,6 +20,7 @@ import (
 
 	"mcpproxy-go/internal/config"
 	"mcpproxy-go/internal/contracts"
+	"mcpproxy-go/internal/health"
 	"mcpproxy-go/internal/httpapi"
 	"mcpproxy-go/internal/logs"
 	"mcpproxy-go/internal/management"
@@ -580,6 +581,27 @@ func (s *Server) GetAllServers() ([]map[string]interface{}, error) {
 			protocol = serverStatus.Config.Protocol
 		}
 
+		// Calculate unified health status (Spec 013: Health is single source of truth)
+		healthInput := health.HealthCalculatorInput{
+			Name:        serverStatus.Name,
+			Enabled:     serverStatus.Enabled,
+			Quarantined: serverStatus.Quarantined,
+			State:       status,
+			Connected:   connected,
+			LastError:   serverStatus.LastError,
+			ToolCount:   serverStatus.ToolCount,
+			// Extract missing secret and OAuth config error from last error
+			MissingSecret:  health.ExtractMissingSecret(serverStatus.LastError),
+			OAuthConfigErr: health.ExtractOAuthConfigError(serverStatus.LastError),
+		}
+
+		// Check if OAuth is required for this server
+		if serverStatus.Config != nil && serverStatus.Config.OAuth != nil {
+			healthInput.OAuthRequired = true
+		}
+
+		healthStatus := health.CalculateHealth(healthInput, health.DefaultHealthConfig())
+
 		result = append(result, map[string]interface{}{
 			"name":            serverStatus.Name,
 			"url":             url,
@@ -595,7 +617,8 @@ func (s *Server) GetAllServers() ([]map[string]interface{}, error) {
 			"status":          status,
 			"should_retry":    false, // Managed by Actor internally now
 			"retry_count":     serverStatus.RetryCount,
-			"last_retry_time": nil, // Actor tracks this internally
+			"last_retry_time": nil,    // Actor tracks this internally
+			"health":          healthStatus, // Spec 013: Health is source of truth
 		})
 	}
 

oas/docs.go

@@ -124,6 +124,45 @@
 
 ---
 
+## Phase 7: Follow-up Fixes (Identified Gaps)
+
+**Purpose**: Address gaps discovered during output review vs spec/plan
+
+### Gap 1: `mcpproxy doctor` Shows OAuth Issues as Generic Connection Errors
+
+**Problem**: Servers needing OAuth login (health action=`login`) appear under "Upstream Server Connection Errors" with generic remediation hints, not under "OAuth Authentication Required" with specific `auth login` hints.
+
+**Root Cause**: The `doctor_cmd.go` CLI doesn't properly display the `OAuthRequired` array populated by `Doctor()`. The servers with `login` action get routed to `OAuthRequired` in `diagnostics.go` (lines 68-73), but the CLI only shows them if parsed as a string array (line 195: `getStringArrayField`), not the actual `OAuthRequirement` struct array.
+
+- [x] T027 [US2] Fix doctor_cmd.go to display OAuthRequired array as objects (server_name, message) not string array
+- [x] T028 [US2] Update doctor remediation for OAuth to show server-specific auth login commands
+
+### Gap 2: `upstream list` ACTION Column Shows Incomplete Command
+
+**Problem**: The ACTION column shows `auth login --server=gcal` but this isn't a runnable command - users need to know which binary to run (e.g., `./mcpproxy auth login --server=gcal` or `mcpproxy auth login --server=gcal`).
+
+**Spec Reference**: The spec table shows "CLI Hint" as `auth login --server=X`, but users expect copy-paste-able commands.
+
+- [x] T029 [US2] Update upstream_cmd.go to show full runnable command (matches spec format: `auth login --server=X`)
+
+### Gap 3: `mcpproxy doctor` Remediation Not Action-Specific
+
+**Problem**: The "Remediation" section shows generic hints for all upstream errors:
+```
+💡 Remediation:
+  • Check server configuration in mcp_config.json
+  • View detailed logs: mcpproxy upstream logs <server-name>
+  • Restart server: mcpproxy upstream restart <server-name>
+```
+
+But spec says remediation should be derived from Health.Action. For `login` action servers, it should say "Run: mcpproxy auth login --server=<name>".
+
+- [x] T030 [US2] Fixed by properly routing servers to correct diagnostic categories (OAuth→oauth_required, errors→upstream_errors)
+
+**Checkpoint**: All gaps addressed ✅
+
+---
+
 ## Dependencies & Execution Order
 
 ### Phase Dependencies