Conversation
- Add provider name constants to replace string literals in switch dispatch - Store API URLs in struct field for all OAuth providers consistently - Extract TruncateString utility to deduplicate body-preview truncation - Unmarshal JSON response once instead of twice on error paths - Handle io.ReadAll error in fetchJSON instead of silently ignoring it Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the OAuth provider implementation to reduce stringly-typed branching and centralize provider API endpoints, while also deduplicating HTTP error body preview truncation across auth/token HTTP API providers.
Changes:
- Introduces exported OAuth provider name constants and uses them in provider constructors and switch statements.
- Standardizes OAuth provider user-info endpoints via an
apiURLfield (now set consistently for GitHub/Microsoft as well). - Extracts
util.TruncateStringand uses it for bounded response-body previews; simplifiesauth/http_api.goby unmarshalling the JSON body only once, and improvesfetchJSONerror handling on non-200 responses.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/util/string.go | Adds TruncateString helper used to truncate HTTP body previews. |
| internal/util/string_test.go | Adds unit tests for TruncateString. |
| internal/token/http_api.go | Replaces inline body-preview truncation with util.TruncateString. |
| internal/auth/http_api.go | Unmarshals once and uses util.TruncateString for non-JSON/unknown error bodies. |
| internal/auth/oauth_provider.go | Adds provider constants, sets apiURL for GitHub/Microsoft, and handles io.ReadAll errors on non-OK responses. |
| internal/auth/oauth_github.go | Uses p.apiURL for GitHub API calls (including /emails). |
| internal/auth/oauth_microsoft.go | Uses p.apiURL for Microsoft Graph /me call. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/util/string.go
Outdated
Comment on lines
+3
to
+8
| // TruncateString truncates s to maxLen characters and appends "..." if truncated. | ||
| func TruncateString(s string, maxLen int) string { | ||
| if len(s) <= maxLen { | ||
| return s | ||
| } | ||
| return s[:maxLen] + "..." |
There was a problem hiding this comment.
TruncateString will panic when maxLen is negative (s[:maxLen]). Since this is an exported util, either guard maxLen <= 0 (and define the behavior) or explicitly document/enforce that maxLen must be non-negative to avoid runtime panics.
- Guard against negative or zero maxLen to prevent runtime panic - Use rune-based slicing instead of byte-based to avoid splitting multibyte UTF-8 characters - Add tests for negative maxLen, multibyte runes, and mixed-character strings - Update inaccurate apiURL struct comment in OAuthProvider Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apiURLstruct field for all OAuth providers (GitHub, Microsoft now consistent with Gitea, GitLab)util.TruncateStringto deduplicate body-preview truncation inauth/http_api.goandtoken/http_api.goio.ReadAllerror infetchJSONTest plan
go test ./internal/auth/... ./internal/token/... ./internal/util/...)TestTruncateStringcovers edge cases (empty, exact length, truncation, zero max)🤖 Generated with Claude Code