feat: implement OAuth extra parameters workaround for RFC 8707 Runlayer integration

Author: technicalpickles

docs/plans/2025-11-27-oauth-extra-params.md

@@ -1,7 +1,8 @@
 # Implementation Plan: OAuth Extra Parameters Support
 
-**Status**: Proposed
+**Status**: Partially Implemented (Workaround Active)
 **Created**: 2025-11-27
+**Updated**: 2025-12-01
 **Priority**: High (blocks Runlayer integration)
 **Related**: docs/runlayer-oauth-investigation.md
 
@@ -837,6 +838,177 @@ Initial deployment with feature flag disabled by default:
 4. How to handle parameter conflicts with discovered metadata?
    - **Decision**: User-specified extra_params take precedence (explicit override)
 
+## Current Implementation Status (2025-12-01)
+
+### ✅ Implemented: Simple Workaround
+
+A **15-line workaround** has been successfully implemented that injects `extraParams` into the OAuth authorization URL after mcp-go constructs it:
+
+**Location**: `internal/upstream/core/connection.go:1845-1865`
+
+**Implementation**:
+```go
+// Add extra OAuth parameters if configured (workaround until mcp-go supports this natively)
+if len(extraParams) > 0 {
+    u, err := url.Parse(authURL)
+    if err != nil {
+        return fmt.Errorf("failed to parse authorization URL: %w", err)
+    }
+
+    q := u.Query()
+    for k, v := range extraParams {
+        q.Set(k, v)
+    }
+    u.RawQuery = q.Encode()
+    authURL = u.String()
+
+    c.logger.Info("✨ Added extra OAuth parameters to authorization URL",
+        zap.String("server", c.config.Name),
+        zap.Any("extra_params", extraParams))
+}
+```
+
+**Results**:
+- ✅ Slack (Runlayer) OAuth working with `resource` parameter
+- ✅ Glean (Runlayer) OAuth working with `resource` parameter
+- ✅ Zero-configuration for Runlayer servers (auto-detected from `.well-known/oauth-authorization-server`)
+- ✅ 14 Slack tools and 7 Glean tools accessible
+
+**Testing**:
+```bash
+./mcpproxy auth login --server=slack
+# ✅ OAuth succeeds with resource parameter injected
+# ✅ Authorization URL includes: &resource=https%3A%2F%2F...%2Fmcp
+
+./mcpproxy tools list --server=slack
+# ✅ 14 tools available (search_messages, post_message, etc.)
+```
+
+### ❌ UX Gap: OAuth Pending State Appears as Error
+
+**Problem**: When OAuth-required servers connect during startup, they show as "failed" even though they just need user authentication.
+
+**Current User Experience**:
+1. User adds Runlayer Slack server to config
+2. MCPProxy starts, attempts automatic connection
+3. Logs show: `"failed to connect: OAuth authorization required - deferred for background processing"`
+4. Server state: `Error` (❌)
+5. User sees error and thinks something is broken
+6. User must discover tray menu or `auth login` command manually
+
+**Code Location**: `internal/upstream/core/connection.go:1164`
+```go
+return fmt.Errorf("OAuth authorization required - deferred for background processing")
+```
+
+**Impact**:
+- 🚨 **Confusing**: Appears as an error when it's really a pending state
+- 🚨 **Poor Discoverability**: No clear indication that user needs to take action
+- 🚨 **Misleading Status**: Server shows "Error" instead of "Pending Auth"
+- 🚨 **Hidden Solution**: OAuth login action not prominently surfaced
+
+**Desired User Experience**:
+1. User adds Runlayer Slack server to config
+2. MCPProxy starts, detects OAuth requirement
+3. Logs show: `"⏳ Slack requires authentication - login via tray menu or CLI"`
+4. Server state: `PendingAuth` (⏳)
+5. Tray shows: "🔐 Slack - Click to authenticate"
+6. User clicks tray menu → OAuth flow starts immediately
+
+**Proposed Solutions**:
+
+#### Option A: New Server State (Recommended)
+Add a `PendingAuth` state separate from `Error`:
+```go
+// internal/upstream/manager.go
+const (
+    StateDisconnected = "Disconnected"
+    StateConnecting   = "Connecting"
+    StatePendingAuth  = "PendingAuth"  // NEW
+    StateReady        = "Ready"
+    StateError        = "Error"
+)
+```
+
+**Changes Required**:
+1. Add `PendingAuth` state to state machine
+2. Return special error type for OAuth deferral: `ErrOAuthPending`
+3. Supervisor recognizes `ErrOAuthPending` → transition to `PendingAuth`
+4. Tray UI shows ⏳ icon with "Click to authenticate" tooltip
+5. `upstream list` shows: `slack    ⏳ Pending Auth    Login required`
+
+#### Option B: Proactive OAuth Notification
+Show system notification when OAuth-required server is detected:
+```go
+if isDeferOAuthForTray() {
+    notification.Show("Slack requires authentication", "Click here to login")
+    // Auto-highlight tray menu item
+}
+```
+
+#### Option C: Auto-trigger OAuth Flow
+Automatically open OAuth flow on first connection attempt (with user consent):
+```go
+if firstConnectionAttempt && requiresOAuth && !userOptedOut {
+    // Start OAuth flow immediately instead of deferring
+    handleOAuthAuthorization(ctx, err, oauthConfig, extraParams)
+}
+```
+
+**Recommendation**: Implement **Option A (New State)** + **Option B (Notification)** for best UX.
+
+### 🔧 Required Implementation Work
+
+To properly address the UX gap:
+
+#### 1. State Machine Changes
+- Add `PendingAuth` state to `internal/upstream/manager.go`
+- Create `ErrOAuthPending` error type
+- Update state transition logic to handle OAuth deferral
+
+#### 2. Connection Layer Changes
+- Return `ErrOAuthPending` instead of generic error (line 1164)
+- Add metadata: OAuth URL, server name, instructions
+
+#### 3. UI/UX Changes
+- Tray: Show ⏳ icon for `PendingAuth` servers with "Authenticate" action
+- CLI: `upstream list` displays "Pending Auth" with clear instructions
+- Logs: Use INFO level instead of ERROR for OAuth deferral
+- Notification: Optional system notification for new OAuth requirements
+
+#### 4. Testing
+- Unit tests for `PendingAuth` state transitions
+- E2E test: Add OAuth server, verify state is `PendingAuth` not `Error`
+- UX test: Verify tray menu shows authentication action
+
+#### 5. Documentation
+- Update user guide: "Understanding OAuth Server States"
+- CLI help text: Explain `PendingAuth` status
+- Troubleshooting: "Server shows as pending - what to do?"
+
+### Timeline Estimate
+
+| Task | Effort | Priority |
+|------|--------|----------|
+| State machine refactor | 2-3 hours | High |
+| Connection error handling | 1-2 hours | High |
+| Tray UI updates | 2-3 hours | High |
+| CLI display updates | 1 hour | Medium |
+| System notifications | 2 hours | Low |
+| Testing | 2-3 hours | High |
+| Documentation | 1-2 hours | Medium |
+| **Total** | **11-16 hours** | **~2 days** |
+
+### Success Criteria
+
+- ✅ OAuth-required servers never show state as `Error` before user authentication
+- ✅ Server state shows `PendingAuth` with clear action required
+- ✅ Tray menu prominently displays "Authenticate" action for pending servers
+- ✅ `upstream list` output clearly distinguishes pending auth from actual errors
+- ✅ Logs use INFO level for OAuth deferral, not ERROR
+- ✅ Optional: System notification alerts user to authentication requirement
+- ✅ User can complete authentication within 30 seconds of seeing notification
+
 ## References
 
 - RFC 8707: Resource Indicators - https://www.rfc-editor.org/rfc/rfc8707.html

internal/upstream/core/connection.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"os"
 	"os/exec"
 	"reflect"
@@ -1101,7 +1102,7 @@ func (c *Client) tryOAuthAuth(ctx context.Context) error {
 				zap.String("server", c.config.Name))
 
 			// Handle OAuth authorization manually using the example pattern
-			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig); oauthErr != nil {
+			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig, extraParams); oauthErr != nil {
 				c.clearOAuthState() // Clear state on OAuth failure
 				return fmt.Errorf("OAuth authorization failed: %w", oauthErr)
 			}
@@ -1167,7 +1168,7 @@ func (c *Client) tryOAuthAuth(ctx context.Context) error {
 			c.clearOAuthState()
 
 			// Handle OAuth authorization manually using the example pattern
-			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig); oauthErr != nil {
+			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig, extraParams); oauthErr != nil {
 				c.clearOAuthState() // Clear state on OAuth failure
 				return fmt.Errorf("OAuth authorization during MCP init failed: %w", oauthErr)
 			}
@@ -1312,7 +1313,7 @@ func (c *Client) trySSEOAuthAuth(ctx context.Context) error {
 		zap.String("strategy", "SSE OAuth"))
 
 	// Create OAuth config using the oauth package
-	oauthConfig, _ := oauth.CreateOAuthConfig(c.config, c.storage)
+	oauthConfig, extraParams := oauth.CreateOAuthConfig(c.config, c.storage)
 	if oauthConfig == nil {
 		return fmt.Errorf("failed to create OAuth config")
 	}
@@ -1420,7 +1421,7 @@ func (c *Client) trySSEOAuthAuth(ctx context.Context) error {
 				zap.String("server", c.config.Name))
 
 			// Handle OAuth authorization manually using the example pattern
-			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig); oauthErr != nil {
+			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig, extraParams); oauthErr != nil {
 				c.clearOAuthState() // Clear state on OAuth failure
 				return fmt.Errorf("SSE OAuth authorization failed: %w", oauthErr)
 			}
@@ -1721,7 +1722,7 @@ func (c *Client) DisconnectWithContext(_ context.Context) error {
 }
 
 // handleOAuthAuthorization handles the manual OAuth flow following the mcp-go example pattern
-func (c *Client) handleOAuthAuthorization(ctx context.Context, authErr error, oauthConfig *client.OAuthConfig) error {
+func (c *Client) handleOAuthAuthorization(ctx context.Context, authErr error, oauthConfig *client.OAuthConfig, extraParams map[string]string) error {
 	// Check if OAuth is already in progress to prevent duplicate flows (CRITICAL FIX for Phase 1)
 	if c.isOAuthInProgress() {
 		c.logger.Warn("⚠️ OAuth authorization already in progress, skipping duplicate attempt",
@@ -1842,6 +1843,28 @@ func (c *Client) handleOAuthAuthorization(ctx context.Context, authErr error, oa
 		return fmt.Errorf("failed to get authorization URL: %w", authURLErr)
 	}
 
+	// Add extra OAuth parameters if configured (workaround until mcp-go supports this natively)
+	if len(extraParams) > 0 {
+		u, err := url.Parse(authURL)
+		if err != nil {
+			c.logger.Error("Failed to parse OAuth authorization URL for extra params injection",
+				zap.String("server", c.config.Name),
+				zap.Error(err))
+			return fmt.Errorf("failed to parse authorization URL: %w", err)
+		}
+
+		q := u.Query()
+		for k, v := range extraParams {
+			q.Set(k, v)
+		}
+		u.RawQuery = q.Encode()
+		authURL = u.String()
+
+		c.logger.Info("✨ Added extra OAuth parameters to authorization URL",
+			zap.String("server", c.config.Name),
+			zap.Any("extra_params", extraParams))
+	}
+
 	// Always log the computed authorization URL so users can copy/paste if auto-launch fails.
 	c.logger.Info("OAuth authorization URL ready",
 		zap.String("server", c.config.Name),
@@ -2074,7 +2097,7 @@ func (c *Client) ForceOAuthFlow(ctx context.Context) error {
 // forceHTTPOAuthFlow forces OAuth flow for HTTP transport
 func (c *Client) forceHTTPOAuthFlow(ctx context.Context) error {
 	// Create OAuth config
-	oauthConfig, _ := oauth.CreateOAuthConfig(c.config, c.storage)
+	oauthConfig, extraParams := oauth.CreateOAuthConfig(c.config, c.storage)
 	if oauthConfig == nil {
 		return fmt.Errorf("failed to create OAuth config - server may not support OAuth")
 	}
@@ -2111,7 +2134,7 @@ func (c *Client) forceHTTPOAuthFlow(ctx context.Context) error {
 			c.logger.Info("✅ OAuth authorization requirement triggered - starting manual OAuth flow")
 
 			// Handle OAuth authorization manually using the example pattern
-			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig); oauthErr != nil {
+			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig, extraParams); oauthErr != nil {
 				return fmt.Errorf("OAuth authorization failed: %w", oauthErr)
 			}
 
@@ -2133,7 +2156,7 @@ func (c *Client) forceHTTPOAuthFlow(ctx context.Context) error {
 // forceSSEOAuthFlow forces OAuth flow for SSE transport
 func (c *Client) forceSSEOAuthFlow(ctx context.Context) error {
 	// Create OAuth config
-	oauthConfig, _ := oauth.CreateOAuthConfig(c.config, c.storage)
+	oauthConfig, extraParams := oauth.CreateOAuthConfig(c.config, c.storage)
 	if oauthConfig == nil {
 		return fmt.Errorf("failed to create OAuth config - server may not support OAuth")
 	}
@@ -2163,7 +2186,7 @@ func (c *Client) forceSSEOAuthFlow(ctx context.Context) error {
 			c.logger.Info("✅ OAuth authorization required from SSE Start() - triggering manual OAuth flow")
 
 			// Handle OAuth authorization manually
-			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig); oauthErr != nil {
+			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig, extraParams); oauthErr != nil {
 				return fmt.Errorf("OAuth authorization failed: %w", oauthErr)
 			}
 
@@ -2187,7 +2210,7 @@ func (c *Client) forceSSEOAuthFlow(ctx context.Context) error {
 			c.logger.Info("✅ OAuth authorization requirement from initialize - starting manual OAuth flow")
 
 			// Handle OAuth authorization manually using the example pattern
-			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig); oauthErr != nil {
+			if oauthErr := c.handleOAuthAuthorization(ctx, err, oauthConfig, extraParams); oauthErr != nil {
 				return fmt.Errorf("OAuth authorization failed: %w", oauthErr)
 			}