Merge pull request #94 from smart-mcp-proxy/fix/oauth-server-auto-connect

Fix: Immediate OAuth server connection and real-time UI updates

Author: Dumbris

frontend/src/components/ServerCard.vue

@@ -3,17 +3,17 @@
     <div class="card-body">
       <!-- Header -->
       <div class="flex justify-between items-start mb-4">
-        <div>
-          <h3 class="card-title text-lg">{{ server.name }}</h3>
-          <p class="text-sm text-base-content/70">
+        <div class="flex-1 min-w-0 mr-2">
+          <h3 class="card-title text-lg truncate">{{ server.name }}</h3>
+          <p class="text-sm text-base-content/70 truncate">
             {{ server.protocol }} • {{ server.url || server.command || 'No endpoint' }}
           </p>
         </div>
 
         <!-- Status indicator -->
         <div
           :class="[
-            'badge',
+            'badge badge-sm flex-shrink-0',
             server.connected ? 'badge-success' :
             server.connecting ? 'badge-warning' :
             'badge-error'
@@ -125,12 +125,27 @@ const systemStore = useSystemStore()
 const loading = ref(false)
 
 const needsOAuth = computed(() => {
-  // Simplified check - in reality, you'd check if the server supports OAuth
-  // and if authentication is required
-  return (props.server.protocol === 'http' || props.server.protocol === 'streamable-http') &&
-         !props.server.connected &&
-         props.server.enabled &&
-         props.server.last_error?.includes('authorization')
+  // Check if server requires OAuth authentication
+  const isHttpProtocol = props.server.protocol === 'http' || props.server.protocol === 'streamable-http'
+  const notConnected = !props.server.connected
+  const isEnabled = props.server.enabled
+
+  // Check for OAuth-related errors in last_error
+  const hasOAuthError = props.server.last_error && (
+    props.server.last_error.includes('authorization') ||
+    props.server.last_error.includes('OAuth') ||
+    props.server.last_error.includes('401') ||
+    props.server.last_error.includes('invalid_token') ||
+    props.server.last_error.includes('Missing or invalid access token')
+  )
+
+  // Check if server has OAuth configuration
+  const hasOAuthConfig = props.server.oauth !== null && props.server.oauth !== undefined
+
+  // Check if server is authenticated
+  const notAuthenticated = !props.server.authenticated
+
+  return isHttpProtocol && notConnected && isEnabled && (hasOAuthError || (hasOAuthConfig && notAuthenticated))
 })
 
 async function toggleEnabled() {

frontend/src/types/api.ts

@@ -15,9 +15,15 @@ export interface Server {
   quarantined: boolean
   connected: boolean
   connecting: boolean
+  authenticated?: boolean
   tool_count: number
   last_error: string
   tool_list_token_size?: number
+  oauth?: {
+    client_id: string
+    auth_url: string
+    token_url: string
+  }
 }
 
 // Tool types

internal/httpapi/server.go

@@ -488,6 +488,30 @@ func (s *Server) handleRestartServer(w http.ResponseWriter, r *http.Request) {
 	select {
 	case err := <-done:
 		if err != nil {
+			// Check if error is OAuth-related (expected state, not a failure)
+			errStr := err.Error()
+			isOAuthError := strings.Contains(errStr, "OAuth authorization") ||
+				strings.Contains(errStr, "oauth") ||
+				strings.Contains(errStr, "authorization required") ||
+				strings.Contains(errStr, "no valid token")
+
+			if isOAuthError {
+				// OAuth required is not a failure - restart succeeded but OAuth is needed
+				s.logger.Info("Server restart completed, OAuth login required",
+					"server", serverID,
+					"error", errStr)
+
+				response := contracts.ServerActionResponse{
+					Server:  serverID,
+					Action:  "restart",
+					Success: true,
+					Async:   false,
+				}
+				s.writeSuccess(w, response)
+				return
+			}
+
+			// Non-OAuth error - treat as failure
 			s.logger.Error("Failed to restart server", "server", serverID, "error", err)
 			s.writeError(w, http.StatusInternalServerError, fmt.Sprintf("Failed to restart server: %v", err))
 			return

internal/runtime/lifecycle.go

@@ -9,6 +9,7 @@ import (
 
 	"mcpproxy-go/internal/config"
 	"mcpproxy-go/internal/runtime/configsvc"
+	"mcpproxy-go/internal/runtime/supervisor"
 )
 
 const connectAttemptTimeout = 45 * time.Second
@@ -43,6 +44,9 @@ func (r *Runtime) StartBackgroundInitialization() {
 			}
 		})
 		r.logger.Info("Reactive tool discovery callback registered")
+
+		// Subscribe to supervisor events and emit servers.changed for Web UI updates
+		go r.supervisorEventForwarder()
 	}
 
 	go r.backgroundInitialization()
@@ -763,3 +767,54 @@ func (r *Runtime) cleanupOrphanedIndexEntries() {
 	r.logger.Debug("Orphaned index cleanup completed",
 		zap.Int("active_servers", len(activeServers)))
 }
+
+// supervisorEventForwarder subscribes to supervisor events and emits runtime events
+// to notify Web UI via SSE when server connection state changes.
+func (r *Runtime) supervisorEventForwarder() {
+	eventCh := r.supervisor.Subscribe()
+	defer r.supervisor.Unsubscribe(eventCh)
+
+	r.logger.Info("Supervisor event forwarder started - will emit servers.changed on connection state changes")
+
+	// Get app context once with proper locking
+	appCtx := r.AppContext()
+
+	for {
+		select {
+		case event, ok := <-eventCh:
+			if !ok {
+				r.logger.Info("Supervisor event channel closed, stopping event forwarder")
+				return
+			}
+
+			// Emit servers.changed event for connection state changes
+			// This triggers Web UI to refresh server list via SSE
+			switch event.Type {
+			case supervisor.EventServerConnected:
+				r.logger.Info("Server connected - emitting servers.changed event",
+					zap.String("server", event.ServerName))
+				r.emitServersChanged("server_connected", map[string]any{
+					"server": event.ServerName,
+				})
+
+			case supervisor.EventServerDisconnected:
+				r.logger.Info("Server disconnected - emitting servers.changed event",
+					zap.String("server", event.ServerName))
+				r.emitServersChanged("server_disconnected", map[string]any{
+					"server": event.ServerName,
+				})
+
+			case supervisor.EventServerStateChanged:
+				r.logger.Debug("Server state changed - emitting servers.changed event",
+					zap.String("server", event.ServerName))
+				r.emitServersChanged("server_state_changed", map[string]any{
+					"server": event.ServerName,
+				})
+			}
+
+		case <-appCtx.Done():
+			r.logger.Info("App context cancelled, stopping supervisor event forwarder")
+			return
+		}
+	}
+}

internal/runtime/supervisor/supervisor.go

@@ -12,6 +12,7 @@ import (
 	"mcpproxy-go/internal/config"
 	"mcpproxy-go/internal/runtime/configsvc"
 	"mcpproxy-go/internal/runtime/stateview"
+	"mcpproxy-go/internal/upstream/types"
 )
 
 // Supervisor manages the desired vs actual state reconciliation for upstream servers.
@@ -487,6 +488,30 @@ func (s *Supervisor) updateStateView(name string, state *ServerState) {
 
 		// Update connection info if available
 		if state.ConnectionInfo != nil {
+			// Extract LastError from ConnectionInfo and convert to string with limit
+			if state.ConnectionInfo.LastError != nil {
+				errorStr := state.ConnectionInfo.LastError.Error()
+				// Limit error string to 500 characters to prevent UI hangs
+				const maxErrorLen = 500
+				if len(errorStr) > maxErrorLen {
+					errorStr = errorStr[:maxErrorLen] + "... (truncated)"
+				}
+				status.LastError = errorStr
+
+				// Set last error time if available
+				if !state.ConnectionInfo.LastRetryTime.IsZero() {
+					t := state.ConnectionInfo.LastRetryTime
+					status.LastErrorTime = &t
+				}
+			} else {
+				status.LastError = ""
+				status.LastErrorTime = nil
+			}
+
+			// Copy retry count
+			status.RetryCount = state.ConnectionInfo.RetryCount
+
+			// Store full connection info in metadata for debugging
 			if status.Metadata == nil {
 				status.Metadata = make(map[string]interface{})
 			}
@@ -628,16 +653,18 @@ func (s *Supervisor) updateSnapshotFromEvent(event Event) {
 			state.LastSeen = event.Timestamp
 
 			// Phase 7.1 FIX: Don't fetch tools on connect events - let background indexing handle it
-			// Just update the cached tool count from the client (non-blocking)
+			// Just update the cached tool count and connection info from the client (non-blocking)
 			var toolCount int
-			if connected {
-				if actualState, err := s.upstream.GetServerState(event.ServerName); err == nil {
-					toolCount = actualState.ToolCount
-				} else {
-					s.logger.Warn("Failed to get server state for tool count",
-						zap.String("server", event.ServerName),
-						zap.Error(err))
-				}
+			var connInfo *types.ConnectionInfo
+			if actualState, err := s.upstream.GetServerState(event.ServerName); err == nil {
+				toolCount = actualState.ToolCount
+				// Update ConnectionInfo for error propagation to UI
+				state.ConnectionInfo = actualState.ConnectionInfo
+				connInfo = actualState.ConnectionInfo
+			} else {
+				s.logger.Warn("Failed to get server state for tool count",
+					zap.String("server", event.ServerName),
+					zap.Error(err))
 			}
 
 			// Update stateview
@@ -661,6 +688,27 @@ func (s *Supervisor) updateSnapshotFromEvent(event Event) {
 					status.Tools = nil // Clear tools on disconnect
 					status.ToolCount = 0
 				}
+
+				// Update ConnectionInfo for immediate error propagation to UI
+				if connInfo != nil {
+					if connInfo.LastError != nil {
+						errorStr := connInfo.LastError.Error()
+						const maxErrorLen = 500
+						if len(errorStr) > maxErrorLen {
+							errorStr = errorStr[:maxErrorLen] + "... (truncated)"
+						}
+						status.LastError = errorStr
+
+						if !connInfo.LastRetryTime.IsZero() {
+							t := connInfo.LastRetryTime
+							status.LastErrorTime = &t
+						}
+					} else {
+						status.LastError = ""
+						status.LastErrorTime = nil
+					}
+					status.RetryCount = connInfo.RetryCount
+				}
 			})
 
 			// Trigger reactive tool discovery when server connects

internal/server/mcp.go

@@ -1430,31 +1430,34 @@ func (p *MCPProxyServer) handleAddUpstream(ctx context.Context, request mcp.Call
 		return mcp.NewToolResultError(fmt.Sprintf("Failed to add upstream: %v", err)), nil
 	}
 
-	// Add to upstream manager and connect
-	var connectionStatus, connectionMessage string
-	if enabled {
-		if err := p.upstreamManager.AddServer(name, serverConfig); err != nil {
-			p.logger.Warn("Failed to add and connect upstream server", zap.String("name", name), zap.Error(err))
-			connectionStatus = statusError
-			connectionMessage = fmt.Sprintf("Failed to add server: %v", err)
-		} else {
-			// Monitor connection status for 1 minute to see final state
-			connectionStatus, connectionMessage = p.monitorConnectionStatus(ctx, name, 1*time.Minute)
-		}
-	} else {
-		connectionStatus = statusDisabled
-		connectionMessage = messageServerDisabled
-	}
-
-	// Trigger configuration save and update
+	// Trigger configuration save which will notify supervisor to reconcile and connect
 	if p.mainServer != nil {
 		// Save configuration first to ensure servers are persisted to config file
+		// This triggers ConfigService update which notifies supervisor to reconcile
+		// Note: SaveConfiguration may fail in test environments without config file - that's OK
 		if err := p.mainServer.SaveConfiguration(); err != nil {
-			p.logger.Error("Failed to save configuration after adding server", zap.Error(err))
+			p.logger.Warn("Failed to save configuration after adding server (may be test environment)",
+				zap.Error(err))
+			// Continue anyway - server is saved to storage, supervisor will reconcile
 		}
 		p.mainServer.OnUpstreamServerChange()
 	}
 
+	// Wait briefly for supervisor to reconcile and connect (if enabled)
+	// This gives us immediate status for the response
+	var connectionStatus, connectionMessage string
+	if enabled {
+		// Give supervisor time to reconcile and attempt connection
+		time.Sleep(2 * time.Second)
+
+		// Monitor connection status for up to 10 seconds to get immediate state
+		// This quickly detects OAuth requirements, connection errors, or success
+		connectionStatus, connectionMessage = p.monitorConnectionStatus(ctx, name, 10*time.Second)
+	} else {
+		connectionStatus = statusDisabled
+		connectionMessage = messageServerDisabled
+	}
+
 	// Check for Docker isolation warnings
 	var dockerWarnings []string
 	if isolationManager := p.getIsolationManager(); isolationManager != nil {

internal/upstream/manager.go

@@ -223,6 +223,22 @@ func (m *Manager) AddServer(id string, serverConfig *config.ServerConfig) error
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 		defer cancel()
 		if err := client.Connect(ctx); err != nil {
+			// Check if this is an OAuth error - don't fail AddServer for OAuth
+			errStr := err.Error()
+			isOAuthError := strings.Contains(errStr, "OAuth") ||
+				strings.Contains(errStr, "oauth") ||
+				strings.Contains(errStr, "authorization required")
+
+			if isOAuthError {
+				m.logger.Info("Server requires OAuth authorization - connection will complete after OAuth login",
+					zap.String("id", id),
+					zap.String("name", serverConfig.Name),
+					zap.Error(err))
+				// Don't return error - server is added successfully, just needs OAuth
+				return nil
+			}
+
+			// For non-OAuth errors, still return error
 			return fmt.Errorf("failed to connect to server %s: %w", serverConfig.Name, err)
 		}
 	} else {