From efd28bf981a33f15323f655411ef5405a26b80e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Mart=C3=ADnez?= Date: Thu, 27 Nov 2025 22:47:51 +0100 Subject: [PATCH] refactor(copilot): address PR review feedback - Simplify error type checking in oauth.go using errors.As directly - Remove redundant errors.As call in GetUserFriendlyMessage - Remove unused CachedAPIToken and TokenManager types (dead code) --- internal/auth/copilot/copilot_auth.go | 71 --------------------------- internal/auth/copilot/errors.go | 17 +++---- internal/auth/copilot/oauth.go | 8 +-- 3 files changed, 10 insertions(+), 86 deletions(-) diff --git a/internal/auth/copilot/copilot_auth.go b/internal/auth/copilot/copilot_auth.go index fbfb1762..e4e5876b 100644 --- a/internal/auth/copilot/copilot_auth.go +++ b/internal/auth/copilot/copilot_auth.go @@ -208,77 +208,6 @@ func (c *CopilotAuth) MakeAuthenticatedRequest(ctx context.Context, method, url return req, nil } -// CachedAPIToken manages caching of Copilot API tokens. -type CachedAPIToken struct { - Token *CopilotAPIToken - ExpiresAt time.Time -} - -// IsExpired checks if the cached token has expired. -func (c *CachedAPIToken) IsExpired() bool { - if c.Token == nil { - return true - } - // Add a 5-minute buffer before expiration - return time.Now().Add(5 * time.Minute).After(c.ExpiresAt) -} - -// TokenManager handles caching and refreshing of Copilot API tokens. -type TokenManager struct { - auth *CopilotAuth - githubToken string - cachedToken *CachedAPIToken -} - -// NewTokenManager creates a new token manager for handling Copilot API tokens. -func NewTokenManager(auth *CopilotAuth, githubToken string) *TokenManager { - return &TokenManager{ - auth: auth, - githubToken: githubToken, - } -} - -// GetToken returns a valid Copilot API token, refreshing if necessary. -func (tm *TokenManager) GetToken(ctx context.Context) (*CopilotAPIToken, error) { - if tm.cachedToken != nil && !tm.cachedToken.IsExpired() { - return tm.cachedToken.Token, nil - } - - // Fetch a new API token - apiToken, err := tm.auth.GetCopilotAPIToken(ctx, tm.githubToken) - if err != nil { - return nil, err - } - - // Cache the token - expiresAt := time.Now().Add(30 * time.Minute) // Default 30 min cache - if apiToken.ExpiresAt > 0 { - expiresAt = time.Unix(apiToken.ExpiresAt, 0) - } - - tm.cachedToken = &CachedAPIToken{ - Token: apiToken, - ExpiresAt: expiresAt, - } - - return apiToken, nil -} - -// GetAuthorizationHeader returns the authorization header value for API requests. -func (tm *TokenManager) GetAuthorizationHeader(ctx context.Context) (string, error) { - token, err := tm.GetToken(ctx) - if err != nil { - return "", err - } - return "Bearer " + token.Token, nil -} - -// UpdateGitHubToken updates the GitHub access token used for getting API tokens. -func (tm *TokenManager) UpdateGitHubToken(githubToken string) { - tm.githubToken = githubToken - tm.cachedToken = nil // Invalidate cache -} - // BuildChatCompletionURL builds the URL for chat completions API. func BuildChatCompletionURL() string { return copilotAPIEndpoint + "/chat/completions" diff --git a/internal/auth/copilot/errors.go b/internal/auth/copilot/errors.go index 01f8e754..dac6ecfa 100644 --- a/internal/auth/copilot/errors.go +++ b/internal/auth/copilot/errors.go @@ -140,10 +140,8 @@ func IsOAuthError(err error) bool { // GetUserFriendlyMessage returns a user-friendly error message based on the error type. func GetUserFriendlyMessage(err error) string { - switch { - case IsAuthenticationError(err): - var authErr *AuthenticationError - errors.As(err, &authErr) + var authErr *AuthenticationError + if errors.As(err, &authErr) { switch authErr.Type { case "device_code_failed": return "Failed to start GitHub authentication. Please check your network connection and try again." @@ -164,9 +162,10 @@ func GetUserFriendlyMessage(err error) string { default: return "Authentication failed. Please try again." } - case IsOAuthError(err): - var oauthErr *OAuthError - errors.As(err, &oauthErr) + } + + var oauthErr *OAuthError + if errors.As(err, &oauthErr) { switch oauthErr.Code { case "access_denied": return "Authentication was cancelled or denied." @@ -177,7 +176,7 @@ func GetUserFriendlyMessage(err error) string { default: return fmt.Sprintf("Authentication failed: %s", oauthErr.Description) } - default: - return "An unexpected error occurred. Please try again." } + + return "An unexpected error occurred. Please try again." } diff --git a/internal/auth/copilot/oauth.go b/internal/auth/copilot/oauth.go index 3f7877b6..1aecf596 100644 --- a/internal/auth/copilot/oauth.go +++ b/internal/auth/copilot/oauth.go @@ -3,6 +3,7 @@ package copilot import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -118,12 +119,7 @@ func (c *DeviceFlowClient) PollForToken(ctx context.Context, deviceCode *DeviceC token, err := c.exchangeDeviceCode(ctx, deviceCode.DeviceCode) if err != nil { var authErr *AuthenticationError - if IsAuthenticationError(err) { - if ok := (err.(*AuthenticationError)); ok != nil { - authErr = ok - } - } - if authErr != nil { + if errors.As(err, &authErr) { switch authErr.Type { case ErrAuthorizationPending.Type: // Continue polling