From bafcaf57c322d374299f54aa8b64eb4022694701 Mon Sep 17 00:00:00 2001 From: John CSA <103165870+jluocsa@users.noreply.github.com> Date: Tue, 19 May 2026 10:40:19 -0700 Subject: [PATCH] fix(pull_request_read): expose `after` cursor parameter in input schema (#2489) The `pull_request_read` tool description tells clients that `get_review_comments` uses cursor-based pagination (`perPage`, `after`), and the handler does plumb `after` through to the GraphQL query, but the input schema only declared `page` and `perPage` (via `WithPagination`). Because `after` was not advertised in `inputSchema`, MCP clients had no way to request it, leaving cursor pagination effectively broken: `perPage: 1` returned only the first thread with no way to advance, and `page` was silently ignored by the GraphQL path. This change adds `after` to the schema (string, optional) with a description making clear it only applies to `get_review_comments`. All other methods continue to ignore it. No handler behavior is changed. - Add `after` schema property after `WithPagination` in `PullRequestRead` - Regenerate `__toolsnaps__/pull_request_read.snap` and update README - Add a regression test asserting `after` is in the schema and a new table-driven case verifying the cursor is forwarded to the GraphQL query Fixes #2122 (for the `get_review_comments` pagination part). The remaining concerns in #2122 about unbounded response sizes for `get`, `get_diff`, and `get_reviews` are deferred to follow-up design. Co-authored-by: Sam Morrow --- README.md | 1 + .../__toolsnaps__/pull_request_read.snap | 4 ++ pkg/github/pullrequests.go | 7 +++ pkg/github/pullrequests_test.go | 53 +++++++++++++++++++ 4 files changed, 65 insertions(+) diff --git a/README.md b/README.md index 1030f83ca0..b291af0e66 100644 --- a/README.md +++ b/README.md @@ -1111,6 +1111,7 @@ The following sets of tools are available: - **pull_request_read** - Get details for a single pull request - **Required OAuth Scopes**: `repo` + - `after`: Cursor for pagination, used only by the get_review_comments method. Pass the endCursor from the previous page's PageInfo to fetch the next page. (string, optional) - `method`: Action to specify what pull request data needs to be retrieved from GitHub. Possible options: 1. get - Get details of a specific pull request. diff --git a/pkg/github/__toolsnaps__/pull_request_read.snap b/pkg/github/__toolsnaps__/pull_request_read.snap index 26b4f14ca9..d70f77e1e0 100644 --- a/pkg/github/__toolsnaps__/pull_request_read.snap +++ b/pkg/github/__toolsnaps__/pull_request_read.snap @@ -6,6 +6,10 @@ "description": "Get information on a specific pull request in GitHub repository.", "inputSchema": { "properties": { + "after": { + "description": "Cursor for pagination, used only by the get_review_comments method. Pass the endCursor from the previous page's PageInfo to fetch the next page.", + "type": "string" + }, "method": { "description": "Action to specify what pull request data needs to be retrieved from GitHub. \nPossible options: \n 1. get - Get details of a specific pull request.\n 2. get_diff - Get the diff of a pull request.\n 3. get_status - Get combined commit status of a head commit in a pull request.\n 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.\n 5. get_review_comments - Get review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns threads with metadata (isResolved, isOutdated, isCollapsed) and their associated comments. Use cursor-based pagination (perPage, after) to control results.\n 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned.\n 7. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned.\n 8. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR.\n", "enum": [ diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 3653c906ba..9672f85244 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -58,6 +58,13 @@ Possible options: Required: []string{"method", "owner", "repo", "pullNumber"}, } WithPagination(schema) + // get_review_comments uses GraphQL cursor-based pagination and accepts the + // `after` cursor. Other methods rely on the `page`/`perPage` parameters + // added by WithPagination and ignore `after`. + schema.Properties["after"] = &jsonschema.Schema{ + Type: "string", + Description: "Cursor for pagination, used only by the get_review_comments method. Pass the endCursor from the previous page's PageInfo to fetch the next page.", + } return NewTool( ToolsetMetadataPullRequests, diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 29339ee7db..a73ba2e173 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1687,6 +1687,11 @@ func Test_GetPullRequestComments(t *testing.T) { assert.Contains(t, schema.Properties, "owner") assert.Contains(t, schema.Properties, "repo") assert.Contains(t, schema.Properties, "pullNumber") + // `after` is required for cursor-based pagination on get_review_comments + // to be reachable from MCP clients; without it in the schema, callers + // cannot advance past the first page (issue #2122). + assert.Contains(t, schema.Properties, "after") + assert.Equal(t, "string", schema.Properties["after"].Type) assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"}) tests := []struct { @@ -1804,6 +1809,54 @@ func Test_GetPullRequestComments(t *testing.T) { assert.Equal(t, 1, result.TotalCount) }, }, + { + name: "after cursor is forwarded to GraphQL query", + gqlHTTPClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + reviewThreadsQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "prNum": githubv4.Int(42), + "first": githubv4.Int(30), + "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), + "after": "cursor-page-2", + }, + expectError: false, + validateResult: func(t *testing.T, textContent string) { + var result MinimalReviewThreadsResponse + err := json.Unmarshal([]byte(textContent), &result) + require.NoError(t, err) + assert.Len(t, result.ReviewThreads, 0) + assert.Equal(t, true, result.PageInfo.HasPreviousPage) + assert.Equal(t, "cursor4", result.PageInfo.EndCursor) + }, + }, { name: "review threads fetch fails", gqlHTTPClient: githubv4mock.NewMockedHTTPClient(