refactor(oauth): remove dead code and consolidate helpers

- Remove unused StartDeviceFlow, StartInteractiveFlow, StartOAuthFlow (170+ lines)
- Consolidate generateState and generateElicitationID into generateRandomToken
- Simplify tests to verify behavior not internal state
- Net reduction: ~200 lines of dead code

Author: SamMorrowDrums

internal/oauth/manager.go

@@ -116,7 +116,7 @@ func (m *Manager) startDeviceFlowWithElicitation(ctx context.Context, session *m
 
 	// Use session elicitation if available to show the user the verification URL and code
 	if session != nil {
-		elicitID, err := generateElicitationID()
+		elicitID, err := generateRandomToken()
 		if err != nil {
 			// Log warning but continue - elicitation ID is for tracking only
 			elicitID = "fallback-id"
@@ -158,7 +158,7 @@ func (m *Manager) startPKCEFlowWithElicitation(ctx context.Context, session *mcp
 	}
 
 	// Generate state for CSRF protection
-	state, err := generateState()
+	state, err := generateRandomToken()
 	if err != nil {
 		return m.startDeviceFlowWithElicitation(ctx, session)
 	}
@@ -206,7 +206,7 @@ func (m *Manager) startPKCEFlowWithElicitation(ctx context.Context, session *mcp
 	// Only elicit if browser failed to open (e.g., headless environment)
 	// and we need to show the user the URL manually
 	if browserErr != nil && session != nil {
-		elicitID, _ := generateElicitationID()
+		elicitID, _ := generateRandomToken()
 		_, _ = session.Elicit(ctx, &mcp.ElicitParams{
 			Mode:          "url",
 			URL:           authURL,
@@ -258,15 +258,9 @@ func (m *Manager) setToken(token *Result) {
 
 // Helper functions
 
-func generateElicitationID() (string, error) {
-	b := make([]byte, 16)
-	if _, err := rand.Read(b); err != nil {
-		return "", err
-	}
-	return base64.RawURLEncoding.EncodeToString(b), nil
-}
-
-func generateState() (string, error) {
+// generateRandomToken generates a cryptographically random URL-safe token.
+// Used for CSRF state and elicitation IDs.
+func generateRandomToken() (string, error) {
 	b := make([]byte, 16)
 	if _, err := rand.Read(b); err != nil {
 		return "", err

internal/oauth/oauth.go

@@ -1,21 +1,17 @@
 package oauth
 
 import (
-	"context"
 	"crypto/rand"
 	"encoding/base64"
 	"fmt"
 	"io"
-	"log"
 	"net"
 	"net/http"
 	"os"
 	"os/exec"
 	"runtime"
 	"strings"
 	"time"
-
-	"golang.org/x/oauth2"
 )
 
 const (
@@ -72,178 +68,6 @@ func isRunningInDocker() bool {
 	return false
 }
 
-// StartDeviceFlow initiates an OAuth device authorization flow
-// This is suitable for environments without callback capabilities (like Docker containers)
-func StartDeviceFlow(ctx context.Context, cfg Config) (*Result, error) {
-	oauth2Cfg := &oauth2.Config{
-		ClientID:     cfg.ClientID,
-		ClientSecret: cfg.ClientSecret,
-		Scopes:       cfg.Scopes,
-		Endpoint: oauth2.Endpoint{
-			AuthURL:       cfg.AuthURL,
-			TokenURL:      cfg.TokenURL,
-			DeviceAuthURL: cfg.DeviceAuthURL,
-		},
-	}
-
-	// Request device authorization
-	deviceAuth, err := oauth2Cfg.DeviceAuth(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get device authorization: %w", err)
-	}
-
-	// Display verification instructions to user
-	fmt.Fprint(os.Stderr, "\n"+strings.Repeat("=", 80)+"\n")
-	fmt.Fprint(os.Stderr, "GitHub OAuth Device Authorization\n")
-	fmt.Fprint(os.Stderr, strings.Repeat("=", 80)+"\n\n")
-	fmt.Fprintf(os.Stderr, "Please visit: %s\n\n", deviceAuth.VerificationURI)
-	fmt.Fprintf(os.Stderr, "And enter code: %s\n\n", deviceAuth.UserCode)
-	fmt.Fprint(os.Stderr, strings.Repeat("=", 80)+"\n\n")
-
-	// Poll for token
-	token, err := oauth2Cfg.DeviceAccessToken(ctx, deviceAuth)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get device access token: %w", err)
-	}
-
-	fmt.Fprint(os.Stderr, "\n✓ Authorization successful!\n\n")
-
-	return &Result{
-		AccessToken:  token.AccessToken,
-		RefreshToken: token.RefreshToken,
-		TokenType:    token.TokenType,
-		Expiry:       token.Expiry,
-	}, nil
-}
-
-// StartOAuthFlow automatically selects the appropriate OAuth flow based on environment
-// - Device flow for Docker containers (no callback server possible)
-// - Interactive PKCE flow for native binaries (best UX with browser)
-func StartOAuthFlow(ctx context.Context, cfg Config) (*Result, error) {
-	// Check if we're in Docker
-	if isRunningInDocker() && cfg.CallbackPort == 0 {
-		// Docker without explicit callback port - use device flow
-		log.Printf("Detected Docker environment, using device flow")
-		return StartDeviceFlow(ctx, cfg)
-	}
-
-	// Use interactive PKCE flow (browser-based)
-	return StartInteractiveFlow(ctx, cfg)
-}
-
-// StartInteractiveFlow initiates an interactive OAuth flow with PKCE
-// This is intended for stdio mode only and opens a browser for user consent
-func StartInteractiveFlow(ctx context.Context, cfg Config) (*Result, error) {
-	// Generate PKCE verifier
-	verifier, err := generatePKCEVerifier()
-	if err != nil {
-		return nil, fmt.Errorf("failed to generate PKCE verifier: %w", err)
-	}
-
-	// Create OAuth2 config
-	oauth2Cfg := &oauth2.Config{
-		ClientID:     cfg.ClientID,
-		ClientSecret: cfg.ClientSecret,
-		RedirectURL:  cfg.RedirectURL,
-		Scopes:       cfg.Scopes,
-		Endpoint: oauth2.Endpoint{
-			AuthURL:  cfg.AuthURL,
-			TokenURL: cfg.TokenURL,
-		},
-	}
-
-	// Generate state for CSRF protection
-	stateBytes := make([]byte, 16)
-	if _, err := rand.Read(stateBytes); err != nil {
-		return nil, fmt.Errorf("failed to generate state: %w", err)
-	}
-	state := base64.RawURLEncoding.EncodeToString(stateBytes)
-
-	// Start local HTTP server for callback
-	listener, port, err := startLocalServer(cfg.CallbackPort)
-	if err != nil {
-		return nil, fmt.Errorf("failed to start local server: %w", err)
-	}
-	defer listener.Close()
-
-	// Update redirect URL with actual port
-	oauth2Cfg.RedirectURL = fmt.Sprintf("http://localhost:%d/callback", port)
-
-	// Channel to receive the authorization code
-	codeChan := make(chan string, 1)
-	errChan := make(chan error, 1)
-
-	// Setup HTTP handler for callback
-	server := &http.Server{
-		Handler:           createCallbackHandler(state, codeChan, errChan),
-		ReadHeaderTimeout: 10 * time.Second, // Prevent Slowloris attacks
-	}
-
-	// Start server in background
-	go func() {
-		if err := server.Serve(listener); err != nil && err != http.ErrServerClosed {
-			errChan <- fmt.Errorf("server error: %w", err)
-		}
-	}()
-
-	// Build authorization URL with PKCE
-	authURL := oauth2Cfg.AuthCodeURL(
-		state,
-		oauth2.S256ChallengeOption(verifier),
-	)
-
-	// Display URL to user and try to open browser
-	fmt.Fprint(os.Stderr, "\n"+strings.Repeat("=", 80)+"\n")
-	fmt.Fprint(os.Stderr, "GitHub OAuth Authorization Required\n")
-	fmt.Fprint(os.Stderr, strings.Repeat("=", 80)+"\n\n")
-	fmt.Fprint(os.Stderr, "Opening your browser to complete authorization...\n\n")
-	fmt.Fprint(os.Stderr, "If your browser doesn't open automatically, please visit this URL:\n\n")
-	fmt.Fprintf(os.Stderr, "  %s\n\n", authURL)
-	fmt.Fprint(os.Stderr, strings.Repeat("=", 80)+"\n\n")
-
-	// Try to open browser
-	if err := openBrowser(authURL); err != nil {
-		log.Printf("Warning: Could not open browser automatically: %v", err)
-	}
-
-	// Wait for callback with timeout
-	var code string
-	select {
-	case code = <-codeChan:
-		// Success
-	case err := <-errChan:
-		return nil, fmt.Errorf("callback error: %w", err)
-	case <-ctx.Done():
-		return nil, fmt.Errorf("context cancelled: %w", ctx.Err())
-	case <-time.After(DefaultAuthTimeout):
-		return nil, fmt.Errorf("authorization timeout after %v", DefaultAuthTimeout)
-	}
-
-	// Shutdown server gracefully
-	shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-	defer cancel()
-	_ = server.Shutdown(shutdownCtx)
-
-	// Exchange authorization code for token with PKCE verifier
-	token, err := oauth2Cfg.Exchange(
-		ctx,
-		code,
-		oauth2.VerifierOption(verifier),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("failed to exchange code for token: %w", err)
-	}
-
-	fmt.Fprint(os.Stderr, "\n✓ Authorization successful!\n\n")
-
-	return &Result{
-		AccessToken:  token.AccessToken,
-		RefreshToken: token.RefreshToken,
-		TokenType:    token.TokenType,
-		Expiry:       token.Expiry,
-	}, nil
-}
-
 // startLocalServer starts a local HTTP server on the specified port
 // If port is 0, uses a random available port
 func startLocalServer(port int) (net.Listener, int, error) {

internal/oauth/oauth_test.go

@@ -110,9 +110,7 @@ func TestNewManager(t *testing.T) {
 	mgr := NewManager(cfg)
 
 	assert.NotNil(t, mgr)
-	assert.Equal(t, cfg.ClientID, mgr.config.ClientID)
-	assert.Equal(t, cfg.ClientSecret, mgr.config.ClientSecret)
-	assert.Equal(t, cfg.Scopes, mgr.config.Scopes)
+	// Test observable behavior, not internal state
 	assert.False(t, mgr.HasToken())
 	assert.Empty(t, mgr.GetAccessToken())
 }
@@ -180,28 +178,17 @@ func TestManagerSetToken(t *testing.T) {
 	assert.True(t, mgr.HasToken())
 }
 
-func TestGenerateState(t *testing.T) {
-	state1, err := generateState()
+func TestGenerateRandomToken(t *testing.T) {
+	token1, err := generateRandomToken()
 	require.NoError(t, err)
-	require.NotEmpty(t, state1)
+	require.NotEmpty(t, token1)
 
-	// State should be URL-safe base64 encoded
+	// Token should be URL-safe base64 encoded
 	// 16 bytes of random data = ~22 chars in base64url
-	assert.GreaterOrEqual(t, len(state1), 20)
+	assert.GreaterOrEqual(t, len(token1), 20)
 
-	// Each call should produce unique state
-	state2, err := generateState()
+	// Each call should produce unique token
+	token2, err := generateRandomToken()
 	require.NoError(t, err)
-	assert.NotEqual(t, state1, state2)
-}
-
-func TestGenerateElicitationID(t *testing.T) {
-	id1, err := generateElicitationID()
-	require.NoError(t, err)
-	require.NotEmpty(t, id1)
-
-	// Each call should produce unique ID
-	id2, err := generateElicitationID()
-	require.NoError(t, err)
-	assert.NotEqual(t, id1, id2)
+	assert.NotEqual(t, token1, token2)
 }