From 29ff3daca443f4f8165b6ffd4aebfb1bed94a96b Mon Sep 17 00:00:00 2001 From: Claude Code Date: Sat, 21 Feb 2026 14:51:18 +0200 Subject: [PATCH] Fix health calculator suggesting 'login' for offline non-OAuth servers (#242) When an HTTP server goes offline, mcp-go wraps the connection error inside "authentication strategies failed", which caused isOAuthRelatedError() to incorrectly classify it as an OAuth issue. The health calculator then suggested "login" instead of "restart". Fix: check for connection error patterns (connection refused, no such host, dial tcp, i/o timeout, etc.) before checking OAuth patterns in both health/calculator.go and upstream/cli/client.go. Closes #242 Co-Authored-By: Claude Opus 4.6 --- internal/health/calculator.go | 20 ++++++++++++ internal/health/calculator_test.go | 50 ++++++++++++++++++++++++++++++ internal/upstream/cli/client.go | 30 +++++++++++++++--- 3 files changed, 95 insertions(+), 5 deletions(-) diff --git a/internal/health/calculator.go b/internal/health/calculator.go index 9a63bc47..fead5c7c 100644 --- a/internal/health/calculator.go +++ b/internal/health/calculator.go @@ -378,10 +378,30 @@ func formatRefreshRetryDetail(retryCount int, nextAttempt *time.Time, lastError } // isOAuthRelatedError checks if the error message indicates an OAuth issue. +// Connection errors take precedence — when a server is simply offline, +// the mcp-go client wraps the connection error inside "authentication strategies failed", +// which would incorrectly trigger OAuth-related detection. func isOAuthRelatedError(err string) bool { if err == "" { return false } + // Connection errors take precedence - these are NOT OAuth issues + // even if the error message also contains OAuth-related text + connectionPatterns := []string{ + "connection refused", + "connection reset", + "no such host", + "network is unreachable", + "dial tcp", + "i/o timeout", + "context deadline exceeded", + "no route to host", + } + for _, pattern := range connectionPatterns { + if stringutil.ContainsIgnoreCase(err, pattern) { + return false + } + } // Check for common OAuth-related error patterns oauthPatterns := []string{ "oauth", diff --git a/internal/health/calculator_test.go b/internal/health/calculator_test.go index 29c8a59b..39943f9f 100644 --- a/internal/health/calculator_test.go +++ b/internal/health/calculator_test.go @@ -712,6 +712,56 @@ func TestCalculateHealth_RefreshStatePriority(t *testing.T) { assert.Equal(t, ActionRestart, result.Action) }) + t.Run("offline HTTP server with OAuth wrapping shows restart not login", func(t *testing.T) { + // Issue #242: When an HTTP server is offline, mcp-go wraps the connection error + // inside "authentication strategies failed", which incorrectly triggers OAuth detection. + // The health calculator should recognize the underlying connection error and suggest restart. + input := HealthCalculatorInput{ + Name: "local-chatgpt-app", + Enabled: true, + State: "error", + OAuthRequired: true, + LastError: "failed to connect: all authentication strategies failed, last error: OAuth authentication required for local-chatgpt-app: dial tcp 127.0.0.1:3000: connect: connection refused", + } + + result := CalculateHealth(input, nil) + + assert.Equal(t, LevelUnhealthy, result.Level) + assert.Equal(t, ActionRestart, result.Action, "Should suggest restart, not login, for connection refused") + assert.NotEqual(t, "Authentication required", result.Summary) + }) + + t.Run("offline HTTP server with no such host shows restart not login", func(t *testing.T) { + input := HealthCalculatorInput{ + Name: "remote-server", + Enabled: true, + State: "disconnected", + OAuthRequired: true, + LastError: "authentication strategies failed: dial tcp: lookup example.invalid: no such host", + } + + result := CalculateHealth(input, nil) + + assert.Equal(t, LevelUnhealthy, result.Level) + assert.Equal(t, ActionRestart, result.Action, "Should suggest restart, not login, for DNS failure") + }) + + t.Run("genuine OAuth error still suggests login", func(t *testing.T) { + input := HealthCalculatorInput{ + Name: "oauth-server", + Enabled: true, + State: "error", + OAuthRequired: true, + LastError: "authentication strategies failed: invalid_grant: token has been revoked", + } + + result := CalculateHealth(input, nil) + + assert.Equal(t, LevelUnhealthy, result.Level) + assert.Equal(t, ActionLogin, result.Action, "Genuine OAuth error should suggest login") + assert.Equal(t, "Authentication required", result.Summary) + }) + t.Run("OAuth expired takes priority over refresh retrying", func(t *testing.T) { input := HealthCalculatorInput{ Name: "test-server", diff --git a/internal/upstream/cli/client.go b/internal/upstream/cli/client.go index c4c969b3..c1552138 100644 --- a/internal/upstream/cli/client.go +++ b/internal/upstream/cli/client.go @@ -382,21 +382,41 @@ func (c *Client) GetOAuthStatus() (string, error) { return "not_required", nil } -// isOAuthRelatedError checks if an error is OAuth-related +// isOAuthRelatedError checks if an error is OAuth-related. +// Connection errors take precedence — when a server is offline, mcp-go may wrap +// the connection error inside "authentication strategies failed". func (c *Client) isOAuthRelatedError(err error) bool { if err == nil { return false } - errStr := err.Error() + errStr := strings.ToLower(err.Error()) + + // Connection errors take precedence - these are NOT OAuth issues + connectionPatterns := []string{ + "connection refused", + "connection reset", + "no such host", + "network is unreachable", + "dial tcp", + "i/o timeout", + "context deadline exceeded", + "no route to host", + } + for _, pattern := range connectionPatterns { + if strings.Contains(errStr, pattern) { + return false + } + } + oauthErrors := []string{ "invalid_token", "invalid_grant", "access_denied", "unauthorized", "401", // HTTP 401 Unauthorized - "Missing or invalid access token", - "OAuth authentication failed", + "missing or invalid access token", + "oauth authentication failed", "oauth timeout", "oauth error", "authorization required", @@ -404,7 +424,7 @@ func (c *Client) isOAuthRelatedError(err error) bool { } for _, oauthErr := range oauthErrors { - if strings.Contains(strings.ToLower(errStr), strings.ToLower(oauthErr)) { + if strings.Contains(errStr, oauthErr) { return true } }