From 6a5567e79dcee52aa7dc3de2cf45f5cd35c74e78 Mon Sep 17 00:00:00 2001 From: Larik Date: Mon, 21 Sep 2026 12:32:20 +0300 Subject: [PATCH] fix(pull_request_read): reject pagination parameters the selected method does not support --- pkg/github/pullrequests.go | 40 ++++++++++++++++++++++++ pkg/github/pullrequests_test.go | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 5cee8b3231..bd4c14b335 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -117,6 +117,17 @@ Possible options: return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelRepoUserContent) } + // Each method honours exactly one pagination style: get_review_comments + // takes a GraphQL cursor (`after`), the other list methods take + // `page`/`perPage`. A parameter the selected method cannot use is an + // error rather than a silent no-op, so an LLM caller that passes + // `after` to get_files (or `page` to get_review_comments) learns that + // it did not advance the page instead of receiving the first page + // again as if it had. + if err := rejectUnsupportedPagination(method, args); err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + switch method { case "get": result, err := GetPullRequest(ctx, client, deps, owner, repo, pullNumber) @@ -2502,3 +2513,32 @@ func newGQLIntPtr(i *int32) *githubv4.Int { gi := githubv4.Int(*i) return &gi } + +// rejectUnsupportedPagination returns an error when the caller supplies a +// pagination parameter the selected pull_request_read method does not honour. +// get_review_comments paginates by GraphQL cursor (`after`); the other list +// methods paginate by `page`/`perPage`; `get`, `get_diff` and `get_status` +// return a single object and paginate by nothing. Without this check the +// unsupported parameter is dropped silently and the caller receives the first +// page again, indistinguishable from a successful advance. +func rejectUnsupportedPagination(method string, args map[string]any) error { + _, hasAfter := args["after"] + _, hasPage := args["page"] + _, hasPerPage := args["perPage"] + + switch method { + case "get_review_comments": + if hasPage { + return fmt.Errorf("method %q paginates by cursor (perPage, after); \"page\" is not supported", method) + } + case "get_files", "get_commits", "get_reviews", "get_comments", "get_check_runs": + if hasAfter { + return fmt.Errorf("method %q paginates by page/perPage; \"after\" is not supported", method) + } + case "get", "get_diff", "get_status": + if hasAfter || hasPage || hasPerPage { + return fmt.Errorf("method %q returns a single result and does not accept pagination parameters", method) + } + } + return nil +} diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index c0e392aea6..6753dc2284 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -4877,3 +4877,57 @@ func TestResolveReviewThread(t *testing.T) { }) } } + +func Test_rejectUnsupportedPagination(t *testing.T) { + tests := []struct { + name string + method string + args map[string]any + wantErr string + }{ + { + name: "get_files with page is fine", + method: "get_files", + args: map[string]any{"page": float64(2), "perPage": float64(10)}, + }, + { + name: "get_files with after is rejected", + method: "get_files", + args: map[string]any{"after": "Y3Vyc29y"}, + wantErr: `method "get_files" paginates by page/perPage; "after" is not supported`, + }, + { + name: "get_review_comments with after is fine", + method: "get_review_comments", + args: map[string]any{"after": "Y3Vyc29y", "perPage": float64(10)}, + }, + { + name: "get_review_comments with page is rejected", + method: "get_review_comments", + args: map[string]any{"page": float64(2)}, + wantErr: `method "get_review_comments" paginates by cursor (perPage, after); "page" is not supported`, + }, + { + name: "get with any pagination is rejected", + method: "get", + args: map[string]any{"perPage": float64(10)}, + wantErr: `method "get" returns a single result and does not accept pagination parameters`, + }, + { + name: "get without pagination is fine", + method: "get", + args: map[string]any{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := rejectUnsupportedPagination(tc.method, tc.args) + if tc.wantErr == "" { + require.NoError(t, err) + return + } + require.EqualError(t, err, tc.wantErr) + }) + } +} \ No newline at end of file