Skip to content

Commit da260e1

Browse files
Fix final code review issues
- Clarify string index boundary check logic (idx > 0 is correct) - Update error message to show both flag and env var options - Add comment explaining the boundary check reasoning Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
1 parent f87313f commit da260e1

File tree

2 files changed

+4
-2
lines changed

2 files changed

+4
-2
lines changed

cmd/github-mcp-server/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ var (
5454

5555
token = result.AccessToken
5656
} else {
57-
return errors.New("GITHUB_PERSONAL_ACCESS_TOKEN not set and OAuth not configured (set GITHUB_OAUTH_CLIENT_ID)")
57+
return errors.New("GITHUB_PERSONAL_ACCESS_TOKEN not set and OAuth not configured (set --oauth-client-id or GITHUB_OAUTH_CLIENT_ID)")
5858
}
5959
}
6060

internal/oauth/oauth.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,9 @@ func getOAuthEndpoints(host string) (authURL, tokenURL string) {
302302
}
303303

304304
// Remove any trailing slashes or paths
305-
if idx := strings.Index(hostname, "/"); idx >= 0 {
305+
// strings.Index returns -1 if not found, and we want to keep everything if there's no slash
306+
// If slash is at index 0, that would be invalid (e.g., "/example"), so we check > 0
307+
if idx := strings.Index(hostname, "/"); idx > 0 {
306308
hostname = hostname[:idx]
307309
}
308310

0 commit comments

Comments
 (0)