Reject unsupported pagination params in pull_request_read

The pull_request_read tool exposes two pagination mechanisms:
page/perPage for the REST-backed methods and the after cursor for
get_review_comments. Passing a parameter the selected method does not
use was silently dropped, so callers (especially LLM agents) would
receive the first page again with no signal that anything was ignored.

Validate the pagination parameters against the selected method before
dispatch and return a tool error naming the method and the parameter:

- after is rejected for all methods except get_review_comments
- page is rejected for get_review_comments
- perPage remains accepted for every method

Fixes #3316

Author: muhammadumer-waheed

pkg/github/pullrequests.go

@@ -97,6 +97,9 @@ Possible options:
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
+			if err := validatePaginationParamsForMethod(method, args); err != nil {
+				return utils.NewToolResultError(err.Error()), nil, nil
+			}
 			pagination, err := OptionalPaginationParams(args)
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
@@ -159,6 +162,22 @@ Possible options:
 		})
 }
 
+// validatePaginationParamsForMethod rejects pagination parameters that the
+// selected pull_request_read method does not use. get_review_comments uses
+// cursor-based pagination (perPage, after), while the other methods use
+// page/perPage. perPage applies to every method. Without this check, a
+// mismatched parameter is silently dropped and the caller receives the
+// first page again with no indication that the parameter was ignored.
+func validatePaginationParamsForMethod(method string, args map[string]any) error {
+	if _, hasAfter := args["after"]; hasAfter && method != "get_review_comments" {
+		return fmt.Errorf("method %q uses page/perPage pagination; \"after\" is not supported. Use method %q for cursor-based pagination", method, "get_review_comments")
+	}
+	if _, hasPage := args["page"]; hasPage && method == "get_review_comments" {
+		return fmt.Errorf("method %q uses cursor-based pagination (perPage, after); \"page\" is not supported", method)
+	}
+	return nil
+}
+
 func GetPullRequest(ctx context.Context, client *github.Client, deps ToolDependencies, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) {
 	cache, err := deps.GetRepoAccessCache(ctx)
 	if err != nil {

pkg/github/pullrequests_test.go

@@ -4324,6 +4324,168 @@ func getLatestPendingReviewQuery(p getLatestPendingReviewQueryParams) githubv4mo
 	)
 }
 
+func Test_PullRequestRead_UnsupportedPaginationParams(t *testing.T) {
+	// Verify tool definition once
+	serverTool := PullRequestRead(translations.NullTranslationHelper)
+	tool := serverTool.Tool
+	require.NoError(t, toolsnaps.Test(tool.Name, tool))
+
+	mockFiles := []*github.CommitFile{
+		{
+			Filename:  github.Ptr("file1.go"),
+			Status:    github.Ptr("modified"),
+			Additions: github.Ptr(10),
+			Deletions: github.Ptr(5),
+			Changes:   github.Ptr(15),
+			Patch:     github.Ptr("@@ -1,5 +1,10 @@"),
+		},
+	}
+
+	tests := []struct {
+		name           string
+		mockedClient   *http.Client
+		gqlHTTPClient  *http.Client
+		requestArgs    map[string]any
+		expectError    bool
+		expectedErrMsg string
+	}{
+		{
+			name: "after is rejected for get_files",
+			requestArgs: map[string]any{
+				"method":     "get_files",
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+				"after":      "cursor-page-2",
+			},
+			expectError:    true,
+			expectedErrMsg: `method "get_files" uses page/perPage pagination; "after" is not supported`,
+		},
+		{
+			name: "after is rejected for get",
+			requestArgs: map[string]any{
+				"method":     "get",
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+				"after":      "cursor-page-2",
+			},
+			expectError:    true,
+			expectedErrMsg: `method "get" uses page/perPage pagination; "after" is not supported`,
+		},
+		{
+			name: "page is rejected for get_review_comments",
+			requestArgs: map[string]any{
+				"method":     "get_review_comments",
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+				"page":       float64(2),
+			},
+			expectError:    true,
+			expectedErrMsg: `method "get_review_comments" uses cursor-based pagination (perPage, after); "page" is not supported`,
+		},
+		{
+			name: "page and perPage are accepted for get_files",
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				GetReposPullsFilesByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{
+					"page":     "2",
+					"per_page": "10",
+				}).andThen(
+					mockResponse(t, http.StatusOK, mockFiles),
+				),
+			}),
+			requestArgs: map[string]any{
+				"method":     "get_files",
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+				"page":       float64(2),
+				"perPage":    float64(10),
+			},
+			expectError: false,
+		},
+		{
+			name: "perPage and after are accepted for get_review_comments",
+			gqlHTTPClient: githubv4mock.NewMockedHTTPClient(
+				githubv4mock.NewQueryMatcher(
+					reviewThreadsQuery{},
+					map[string]any{
+						"owner":             githubv4.String("owner"),
+						"repo":              githubv4.String("repo"),
+						"prNum":             githubv4.Int(42),
+						"first":             githubv4.Int(10),
+						"commentsPerThread": githubv4.Int(100),
+						"after":             githubv4.String("cursor-page-2"),
+					},
+					githubv4mock.DataResponse(map[string]any{
+						"repository": map[string]any{
+							"pullRequest": map[string]any{
+								"reviewThreads": map[string]any{
+									"nodes": []map[string]any{},
+									"pageInfo": map[string]any{
+										"hasNextPage":     false,
+										"hasPreviousPage": true,
+										"startCursor":     "cursor3",
+										"endCursor":       "cursor4",
+									},
+									"totalCount": 5,
+								},
+							},
+						},
+					}),
+				),
+			),
+			requestArgs: map[string]any{
+				"method":     "get_review_comments",
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+				"perPage":    float64(10),
+				"after":      "cursor-page-2",
+			},
+			expectError: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			var gqlClient *githubv4.Client
+			if tc.gqlHTTPClient != nil {
+				gqlClient = githubv4.NewClient(tc.gqlHTTPClient)
+			} else {
+				gqlClient = githubv4.NewClient(nil)
+			}
+
+			deps := BaseDeps{
+				Client:          mustNewGHClient(t, tc.mockedClient),
+				GQLClient:       gqlClient,
+				RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
+				Flags:           stubFeatureFlags(nil),
+			}
+			handler := serverTool.Handler(deps)
+
+			// Create call request
+			request := createMCPRequest(tc.requestArgs)
+
+			// Call handler
+			result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+
+			// Verify results
+			if tc.expectError {
+				require.NoError(t, err)
+				require.True(t, result.IsError)
+				errorContent := getErrorResult(t, result)
+				assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
+				return
+			}
+
+			require.NoError(t, err)
+			require.False(t, result.IsError)
+		})
+	}
+}
+
 func TestAddReplyToPullRequestComment(t *testing.T) {
 	t.Parallel()