fix(copilot): keep rate limit denials out of the review permission hint

A 403 carrying X-RateLimit-Remaining: 0, or a secondary rate limit
documentation URL, reached copilotReviewErrMsg as a rate limit error. It
was explained as a missing repository or missing write access, and the
repository read it triggered was refused for the same reason, so the
caller paid an extra call to be told the wrong thing.

Return the base message for both rate limit error types so the rate
limit text stands on its own, and trim the helper and its tests to the
comments the code cannot state.

Co-authored-by: Dylan Pulver <dylanpulver@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: 3 people

pkg/github/copilot.go

@@ -3,6 +3,7 @@ package github
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -900,7 +901,7 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) inventory.Server
 			)
 			if err != nil {
 				return ghErrors.NewGitHubAPIErrorResponse(ctx,
-					copilotReviewErrMsg(ctx, client, "failed to request copilot review", owner, repo, pullNumber, resp),
+					copilotReviewErrMsg(ctx, client, "failed to request copilot review", owner, repo, pullNumber, resp, err),
 					resp,
 					err,
 				), nil, nil
@@ -920,31 +921,32 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) inventory.Server
 		})
 }
 
-// copilotReviewErrMsg explains the opaque failures of the review request
-// endpoint used by request_copilot_review.
-//
-// Requesting a reviewer needs write access to the repository. Being the author
-// of the pull request does not grant it, which is why a fork contributor can be
-// offered a Copilot review by the web UI and still be refused by the API. See
+// copilotReviewErrMsg disambiguates the bare 404 this endpoint returns when the
+// caller lacks write access, which is otherwise indistinguishable from a missing
+// repository or pull request. Authoring the pull request does not grant write
+// access, so fork contributors are refused here even though the website offers
+// them a Copilot review.
 // https://docs.github.com/en/pull-requests/reference/pull-request-reviews#requesting-and-requiring-reviews
-//
-// The endpoint is documented to answer a caller who is not a collaborator with
-// 403 or 422, but in practice it answers with 404 Not Found, which on its own
-// is indistinguishable from a repository or pull request that does not exist.
-// Reading the repository tells the two apart, and only runs once the request
-// has already failed.
-func copilotReviewErrMsg(ctx context.Context, client *github.Client, base, owner, repo string, pullNumber int, resp *github.Response) string {
+func copilotReviewErrMsg(ctx context.Context, client *github.Client, base, owner, repo string, pullNumber int, resp *github.Response, err error) string {
 	if resp == nil || (resp.StatusCode != http.StatusNotFound && resp.StatusCode != http.StatusForbidden) {
 		return base
 	}
 
+	// Rate limiting is also reported as 403, and the read below would be refused
+	// for the same reason, so leave the caller with the rate limit message.
+	var rateLimitErr *github.RateLimitError
+	var abuseErr *github.AbuseRateLimitError
+	if errors.As(err, &rateLimitErr) || errors.As(err, &abuseErr) {
+		return base
+	}
+
 	repository, _, repoErr := client.Repositories.Get(ctx, owner, repo)
 	switch {
 	case repoErr != nil:
-		return fmt.Sprintf("%s. %s/%s could not be read with the current credentials, so either it does not exist or the credentials cannot reach it. "+
-			"GitHub refuses this endpoint the same way when the authenticated user has no write access to the repository.", base, owner, repo)
+		return fmt.Sprintf("%s. %s/%s could not be read with the current credentials, so it may not exist or the credentials may not reach it. "+
+			"Lacking write access is refused with the same status.", base, owner, repo)
 	case !repository.GetPermissions().GetPush():
-		return fmt.Sprintf("%s. The authenticated user has no write access to %s/%s, and GitHub requires write access to request a reviewer even from the author of the pull request. "+
+		return fmt.Sprintf("%s. The authenticated user has no write access to %s/%s, and GitHub requires write access to request a reviewer, even from the author of the pull request. "+
 			"Request the Copilot review from the pull request page on the GitHub website instead, or ask someone with write access to request it.", base, owner, repo)
 	default:
 		return fmt.Sprintf("%s. The authenticated user has write access to %s/%s, so check that pull request #%d exists there and that Copilot code review is available for the repository. "+

pkg/github/copilot_test.go

@@ -5,8 +5,10 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/github/github-mcp-server/internal/githubv4mock"
 	"github.com/github/github-mcp-server/internal/toolsnaps"
@@ -929,9 +931,6 @@ func Test_RequestCopilotReview(t *testing.T) {
 			expectedErrMsg: "failed to request copilot review",
 		},
 		{
-			// The author of a cross-fork pull request has no write access on the
-			// upstream repository, so GitHub refuses the review request with a 404
-			// that says nothing about permissions.
 			name: "pull request author without write access",
 			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
 				PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: mockResponse(t, http.StatusNotFound, map[string]any{"message": "Not Found"}),
@@ -1006,7 +1005,6 @@ func Test_RequestCopilotReview(t *testing.T) {
 			expectedErrMsg: "owner/repo could not be read with the current credentials",
 		},
 		{
-			// A failure that carries no permission signal keeps the original message.
 			name: "server error is not explained as a permission problem",
 			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
 				PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: mockResponse(t, http.StatusInternalServerError, map[string]any{"message": "Internal Server Error"}),
@@ -1020,6 +1018,28 @@ func Test_RequestCopilotReview(t *testing.T) {
 			expectedErrMsg:   "failed to request copilot review",
 			unexpectedErrMsg: "write access",
 		},
+		{
+			name: "rate limited forbidden is not explained as a permission problem",
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
+					w.Header().Set("X-RateLimit-Remaining", "0")
+					w.Header().Set("X-RateLimit-Reset", strconv.FormatInt(time.Now().Add(time.Hour).Unix(), 10))
+					w.WriteHeader(http.StatusForbidden)
+					_, _ = w.Write([]byte(`{"message": "API rate limit exceeded"}`))
+				},
+				GetReposByOwnerByRepo: func(_ http.ResponseWriter, _ *http.Request) {
+					t.Error("repository should not be read while rate limited")
+				},
+			}),
+			requestArgs: map[string]any{
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(1),
+			},
+			expectError:      true,
+			expectedErrMsg:   "rate limit exceeded",
+			unexpectedErrMsg: "write access",
+		},
 	}
 
 	for _, tc := range tests {