From a3c2e90b6817a266aa38516e54e58c8d207bc704 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:25:32 +0000 Subject: [PATCH 1/3] Fix pending review lookup for masked logins Use stable GraphQL node IDs instead of viewer login values when selecting the current user's pending pull request review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/granular_tools_test.go | 50 ++----- pkg/github/pullrequests.go | 231 ++++++++++-------------------- pkg/github/pullrequests_test.go | 184 ++++++++++++------------ 3 files changed, 178 insertions(+), 287 deletions(-) diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 425f954ef9..0f03e3c0ba 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -1653,49 +1653,15 @@ func TestGranularUpdatePullRequestDraftState(t *testing.T) { func TestGranularAddPullRequestReviewComment(t *testing.T) { mockedClient := githubv4mock.NewMockedHTTPClient( - githubv4mock.NewQueryMatcher( - struct { - Viewer struct { - Login githubv4.String - } - }{}, - nil, - githubv4mock.DataResponse(map[string]any{ - "viewer": map[string]any{"login": "testuser"}, - }), - ), - githubv4mock.NewQueryMatcher( - struct { - Repository struct { - PullRequest struct { - Reviews struct { - Nodes []struct { - ID githubv4.ID - State githubv4.PullRequestReviewState - URL githubv4.URI - } - } `graphql:"reviews(first: 1, author: $author)"` - } `graphql:"pullRequest(number: $prNum)"` - } `graphql:"repository(owner: $owner, name: $name)"` - }{}, - map[string]any{ - "author": githubv4.String("testuser"), - "owner": githubv4.String("owner"), - "name": githubv4.String("repo"), - "prNum": githubv4.Int(1), + viewerIDQuery("U_testuser"), + getPendingReviewsQuery(getPendingReviewsQueryParams{ + owner: "owner", + repo: "repo", + prNum: 1, + reviews: []pendingReviewQueryReview{ + {id: "PRR_123", authorID: "U_testuser"}, }, - githubv4mock.DataResponse(map[string]any{ - "repository": map[string]any{ - "pullRequest": map[string]any{ - "reviews": map[string]any{ - "nodes": []map[string]any{ - {"id": "PRR_123", "state": "PENDING", "url": "https://github.com/owner/repo/pull/1#pullrequestreview-123"}, - }, - }, - }, - }, - }), - ), + }), githubv4mock.NewMutationMatcher( struct { AddPullRequestReviewThread struct { diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 5cee8b3231..c267b6e4ca 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1989,57 +1989,9 @@ func CreatePullRequestReview(ctx context.Context, client *githubv4.Client, param } func SubmitPendingPullRequestReview(ctx context.Context, client *githubv4.Client, params PullRequestReviewWriteParams) (*mcp.CallToolResult, error) { - // First we'll get the current user - var getViewerQuery struct { - Viewer struct { - Login githubv4.String - } - } - - if err := client.Query(ctx, &getViewerQuery, nil); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get current user", - err, - ), nil - } - - var getLatestReviewForViewerQuery struct { - Repository struct { - PullRequest struct { - Reviews struct { - Nodes []struct { - ID githubv4.ID - State githubv4.PullRequestReviewState - URL githubv4.URI - } - } `graphql:"reviews(first: 1, author: $author)"` - } `graphql:"pullRequest(number: $prNum)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "author": githubv4.String(getViewerQuery.Viewer.Login), - "owner": githubv4.String(params.Owner), - "name": githubv4.String(params.Repo), - "prNum": githubv4.Int(params.PullNumber), - } - - if err := client.Query(ctx, &getLatestReviewForViewerQuery, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get latest review for current user", - err, - ), nil - } - - // Validate there is one review and the state is pending - if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return utils.NewToolResultError("No pending review found for the viewer"), nil - } - - review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] - if review.State != githubv4.PullRequestReviewStatePending { - errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) - return utils.NewToolResultError(errText), nil + review, result := getPendingPullRequestReviewForViewer(ctx, client, params.Owner, params.Repo, params.PullNumber) + if result != nil { + return result, nil } // Prepare the mutation @@ -2055,7 +2007,7 @@ func SubmitPendingPullRequestReview(ctx context.Context, client *githubv4.Client ctx, &submitPullRequestReviewMutation, githubv4.SubmitPullRequestReviewInput{ - PullRequestReviewID: &review.ID, + PullRequestReviewID: review, Event: githubv4.PullRequestReviewEvent(params.Event), Body: newGQLStringlikePtr[githubv4.String](¶ms.Body), }, @@ -2074,57 +2026,9 @@ func SubmitPendingPullRequestReview(ctx context.Context, client *githubv4.Client } func DeletePendingPullRequestReview(ctx context.Context, client *githubv4.Client, params PullRequestReviewWriteParams) (*mcp.CallToolResult, error) { - // First we'll get the current user - var getViewerQuery struct { - Viewer struct { - Login githubv4.String - } - } - - if err := client.Query(ctx, &getViewerQuery, nil); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get current user", - err, - ), nil - } - - var getLatestReviewForViewerQuery struct { - Repository struct { - PullRequest struct { - Reviews struct { - Nodes []struct { - ID githubv4.ID - State githubv4.PullRequestReviewState - URL githubv4.URI - } - } `graphql:"reviews(first: 1, author: $author)"` - } `graphql:"pullRequest(number: $prNum)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "author": githubv4.String(getViewerQuery.Viewer.Login), - "owner": githubv4.String(params.Owner), - "name": githubv4.String(params.Repo), - "prNum": githubv4.Int(params.PullNumber), - } - - if err := client.Query(ctx, &getLatestReviewForViewerQuery, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get latest review for current user", - err, - ), nil - } - - // Validate there is one review and the state is pending - if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return utils.NewToolResultError("No pending review found for the viewer"), nil - } - - review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] - if review.State != githubv4.PullRequestReviewStatePending { - errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) - return utils.NewToolResultError(errText), nil + review, result := getPendingPullRequestReviewForViewer(ctx, client, params.Owner, params.Repo, params.PullNumber) + if result != nil { + return result, nil } // Prepare the mutation @@ -2140,7 +2044,7 @@ func DeletePendingPullRequestReview(ctx context.Context, client *githubv4.Client ctx, &deletePullRequestReviewMutation, githubv4.DeletePullRequestReviewInput{ - PullRequestReviewID: &review.ID, + PullRequestReviewID: review, }, nil, ); err != nil { @@ -2235,57 +2139,9 @@ type AddCommentToPendingReviewParams struct { // AddCommentToPendingReviewCall adds a review comment to the viewer's pending pull request review. func AddCommentToPendingReviewCall(ctx context.Context, client *githubv4.Client, params AddCommentToPendingReviewParams) (*mcp.CallToolResult, error) { - // Get the current user - var getViewerQuery struct { - Viewer struct { - Login githubv4.String - } - } - - if err := client.Query(ctx, &getViewerQuery, nil); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get current user", - err, - ), nil - } - - var getLatestReviewForViewerQuery struct { - Repository struct { - PullRequest struct { - Reviews struct { - Nodes []struct { - ID githubv4.ID - State githubv4.PullRequestReviewState - URL githubv4.URI - } - } `graphql:"reviews(first: 1, author: $author)"` - } `graphql:"pullRequest(number: $prNum)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "author": githubv4.String(getViewerQuery.Viewer.Login), - "owner": githubv4.String(params.Owner), - "name": githubv4.String(params.Repo), - "prNum": githubv4.Int(params.PullNumber), - } - - if err := client.Query(ctx, &getLatestReviewForViewerQuery, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get latest review for current user", - err, - ), nil - } - - // Validate there is one review and the state is pending - if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return utils.NewToolResultError("No pending review found for the viewer"), nil - } - - review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] - if review.State != githubv4.PullRequestReviewStatePending { - errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) - return utils.NewToolResultError(errText), nil + review, result := getPendingPullRequestReviewForViewer(ctx, client, params.Owner, params.Repo, params.PullNumber) + if result != nil { + return result, nil } // Create a new review thread comment on the review. @@ -2308,7 +2164,7 @@ func AddCommentToPendingReviewCall(ctx context.Context, client *githubv4.Client, Side: newGQLStringlikePtr[githubv4.DiffSide](params.Side), StartLine: newGQLIntPtr(params.StartLine), StartSide: newGQLStringlikePtr[githubv4.DiffSide](params.StartSide), - PullRequestReviewID: &review.ID, + PullRequestReviewID: review, }, nil, ); err != nil { @@ -2326,6 +2182,69 @@ func AddCommentToPendingReviewCall(ctx context.Context, client *githubv4.Client, return utils.NewToolResultText("pull request review comment successfully added to pending review"), nil } +func getPendingPullRequestReviewForViewer(ctx context.Context, client *githubv4.Client, owner, repo string, pullNumber int32) (*githubv4.ID, *mcp.CallToolResult) { + var getViewerQuery struct { + Viewer struct { + ID githubv4.ID + } + } + + if err := client.Query(ctx, &getViewerQuery, nil); err != nil { + return nil, ghErrors.NewGitHubGraphQLErrorResponse(ctx, + "failed to get current user", + err, + ) + } + + vars := map[string]any{ + "after": (*githubv4.String)(nil), + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + "prNum": githubv4.Int(pullNumber), + "states": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStatePending}, + } + + for { + var getPendingReviewsQuery struct { + Repository struct { + PullRequest struct { + Reviews struct { + Nodes []struct { + ID githubv4.ID + Author struct { + ID githubv4.ID + } + } + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String + } + } `graphql:"reviews(first: 100, after: $after, states: $states)"` + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + if err := client.Query(ctx, &getPendingReviewsQuery, vars); err != nil { + return nil, ghErrors.NewGitHubGraphQLErrorResponse(ctx, + "failed to get pending pull request reviews", + err, + ) + } + + for _, review := range getPendingReviewsQuery.Repository.PullRequest.Reviews.Nodes { + if review.Author.ID == getViewerQuery.Viewer.ID { + reviewID := review.ID + return &reviewID, nil + } + } + + if !getPendingReviewsQuery.Repository.PullRequest.Reviews.PageInfo.HasNextPage { + return nil, utils.NewToolResultError("No pending review found for the viewer") + } + vars["after"] = githubv4.NewString(getPendingReviewsQuery.Repository.PullRequest.Reviews.PageInfo.EndCursor) + } +} + // AddCommentToPendingReview creates a tool to add a comment to a pull request review. func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index c0e392aea6..36d2915a26 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -3667,7 +3667,7 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { expectedToolErrMsg string }{ { - name: "successful line comment addition", + name: "selects viewer pending review by ID", requestArgs: map[string]any{ "owner": "owner", "repo": "repo", @@ -3681,18 +3681,20 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { "startSide": "RIGHT", }, mockedClient: githubv4mock.NewMockedHTTPClient( - viewerQuery("williammartin"), - getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{ - author: "williammartin", - owner: "owner", - repo: "repo", - prNum: 42, - - reviews: []getLatestPendingReviewQueryReview{ + viewerIDQuery("U_viewer"), + getPendingReviewsQuery(getPendingReviewsQueryParams{ + owner: "owner", + repo: "repo", + prNum: 42, + + reviews: []pendingReviewQueryReview{ + { + id: "PR_other", + authorID: "U_other", + }, { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", + id: "PR_kwDODKw3uc6WYN1T", + authorID: "U_viewer", }, }, }), @@ -3740,18 +3742,16 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { "startSide": "RIGHT", }, mockedClient: githubv4mock.NewMockedHTTPClient( - viewerQuery("williammartin"), - getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{ - author: "williammartin", - owner: "owner", - repo: "repo", - prNum: 42, - - reviews: []getLatestPendingReviewQueryReview{ + viewerIDQuery("U_viewer"), + getPendingReviewsQuery(getPendingReviewsQueryParams{ + owner: "owner", + repo: "repo", + prNum: 42, + + reviews: []pendingReviewQueryReview{ { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", + id: "PR_kwDODKw3uc6WYN1T", + authorID: "U_viewer", }, }, }), @@ -3821,18 +3821,16 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { "side": "RIGHT", }, mockedClient: githubv4mock.NewMockedHTTPClient( - viewerQuery("williammartin"), - getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{ - author: "williammartin", - owner: "owner", - repo: "repo", - prNum: 42, - - reviews: []getLatestPendingReviewQueryReview{ + viewerIDQuery("U_viewer"), + getPendingReviewsQuery(getPendingReviewsQueryParams{ + owner: "owner", + repo: "repo", + prNum: 42, + + reviews: []pendingReviewQueryReview{ { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", + id: "PR_kwDODKw3uc6WYN1T", + authorID: "U_viewer", }, }, }), @@ -3939,18 +3937,16 @@ func TestSubmitPendingPullRequestReview(t *testing.T) { "body": "This is a test review", }, mockedClient: githubv4mock.NewMockedHTTPClient( - viewerQuery("williammartin"), - getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{ - author: "williammartin", - owner: "owner", - repo: "repo", - prNum: 42, - - reviews: []getLatestPendingReviewQueryReview{ + viewerIDQuery("U_viewer"), + getPendingReviewsQuery(getPendingReviewsQueryParams{ + owner: "owner", + repo: "repo", + prNum: 42, + + reviews: []pendingReviewQueryReview{ { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", + id: "PR_kwDODKw3uc6WYN1T", + authorID: "U_viewer", }, }, }), @@ -4040,18 +4036,16 @@ func TestDeletePendingPullRequestReview(t *testing.T) { "pullNumber": float64(42), }, mockedClient: githubv4mock.NewMockedHTTPClient( - viewerQuery("williammartin"), - getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{ - author: "williammartin", - owner: "owner", - repo: "repo", - prNum: 42, - - reviews: []getLatestPendingReviewQueryReview{ + viewerIDQuery("U_viewer"), + getPendingReviewsQuery(getPendingReviewsQueryParams{ + owner: "owner", + repo: "repo", + prNum: 42, + + reviews: []pendingReviewQueryReview{ { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", + id: "PR_kwDODKw3uc6WYN1T", + authorID: "U_viewer", }, }, }), @@ -4252,76 +4246,88 @@ index 5d6e7b2..8a4f5c3 100644 } } -func viewerQuery(login string) githubv4mock.Matcher { +func viewerIDQuery(id string) githubv4mock.Matcher { return githubv4mock.NewQueryMatcher( struct { Viewer struct { - Login githubv4.String + ID githubv4.ID } `graphql:"viewer"` }{}, map[string]any{}, githubv4mock.DataResponse(map[string]any{ "viewer": map[string]any{ - "login": login, + "id": id, }, }), ) } -type getLatestPendingReviewQueryReview struct { - id string - state string - url string +type pendingReviewQueryReview struct { + id string + authorID string } -type getLatestPendingReviewQueryParams struct { - author string - owner string - repo string - prNum int32 +type getPendingReviewsQueryParams struct { + owner string + repo string + prNum int32 - reviews []getLatestPendingReviewQueryReview + reviews []pendingReviewQueryReview } -func getLatestPendingReviewQuery(p getLatestPendingReviewQueryParams) githubv4mock.Matcher { - return githubv4mock.NewQueryMatcher( +func getPendingReviewsQuery(p getPendingReviewsQueryParams) githubv4mock.Matcher { + reviews := make([]any, 0, len(p.reviews)) + for _, review := range p.reviews { + reviews = append(reviews, map[string]any{ + "id": review.id, + "author": map[string]any{ + "id": review.authorID, + }, + }) + } + + matcher := githubv4mock.NewQueryMatcher( struct { Repository struct { PullRequest struct { Reviews struct { Nodes []struct { - ID githubv4.ID - State githubv4.PullRequestReviewState - URL githubv4.URI + ID githubv4.ID + Author struct { + ID githubv4.ID + } + } + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String } - } `graphql:"reviews(first: 1, author: $author)"` + } `graphql:"reviews(first: 100, after: $after, states: $states)"` } `graphql:"pullRequest(number: $prNum)"` } `graphql:"repository(owner: $owner, name: $name)"` }{}, map[string]any{ - "author": githubv4.String(p.author), + "after": (*githubv4.String)(nil), "owner": githubv4.String(p.owner), "name": githubv4.String(p.repo), "prNum": githubv4.Int(p.prNum), + "states": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStatePending}, }, - githubv4mock.DataResponse( - map[string]any{ - "repository": map[string]any{ - "pullRequest": map[string]any{ - "reviews": map[string]any{ - "nodes": []any{ - map[string]any{ - "id": p.reviews[0].id, - "state": p.reviews[0].state, - "url": p.reviews[0].url, - }, - }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "pullRequest": map[string]any{ + "reviews": map[string]any{ + "nodes": reviews, + "pageInfo": map[string]any{ + "hasNextPage": false, + "endCursor": "", }, }, }, }, - ), + }), ) + matcher.Variables["states"] = []any{"PENDING"} + return matcher } func TestAddReplyToPullRequestComment(t *testing.T) { From 3e7802c5555673e1577caaddc25a73760b5a56da Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:34:19 +0000 Subject: [PATCH 2/3] Handle concrete review author types Select actor node IDs through GraphQL fragments because the Actor interface does not expose id directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/pullrequests.go | 39 +++++++++++++++++++++++++++++---- pkg/github/pullrequests_test.go | 20 ++++++++++------- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index c267b6e4ca..19885caa56 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -2182,6 +2182,39 @@ func AddCommentToPendingReviewCall(ctx context.Context, client *githubv4.Client, return utils.NewToolResultText("pull request review comment successfully added to pending review"), nil } +type pendingReviewAuthor struct { + Bot struct { + ID githubv4.ID `graphql:"botId: id"` + } `graphql:"... on Bot"` + EnterpriseUserAccount struct { + ID githubv4.ID `graphql:"enterpriseUserAccountId: id"` + } `graphql:"... on EnterpriseUserAccount"` + Mannequin struct { + ID githubv4.ID `graphql:"mannequinId: id"` + } `graphql:"... on Mannequin"` + Organization struct { + ID githubv4.ID `graphql:"organizationId: id"` + } `graphql:"... on Organization"` + User struct { + ID githubv4.ID `graphql:"userId: id"` + } `graphql:"... on User"` +} + +func (a pendingReviewAuthor) id() githubv4.ID { + for _, id := range []githubv4.ID{ + a.Bot.ID, + a.EnterpriseUserAccount.ID, + a.Mannequin.ID, + a.Organization.ID, + a.User.ID, + } { + if id != nil { + return id + } + } + return nil +} + func getPendingPullRequestReviewForViewer(ctx context.Context, client *githubv4.Client, owner, repo string, pullNumber int32) (*githubv4.ID, *mcp.CallToolResult) { var getViewerQuery struct { Viewer struct { @@ -2211,9 +2244,7 @@ func getPendingPullRequestReviewForViewer(ctx context.Context, client *githubv4. Reviews struct { Nodes []struct { ID githubv4.ID - Author struct { - ID githubv4.ID - } + Author pendingReviewAuthor } PageInfo struct { HasNextPage githubv4.Boolean @@ -2232,7 +2263,7 @@ func getPendingPullRequestReviewForViewer(ctx context.Context, client *githubv4. } for _, review := range getPendingReviewsQuery.Repository.PullRequest.Reviews.Nodes { - if review.Author.ID == getViewerQuery.Viewer.ID { + if review.Author.id() == getViewerQuery.Viewer.ID { reviewID := review.ID return &reviewID, nil } diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 36d2915a26..b2de748987 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -3693,8 +3693,9 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { authorID: "U_other", }, { - id: "PR_kwDODKw3uc6WYN1T", - authorID: "U_viewer", + id: "PR_kwDODKw3uc6WYN1T", + authorID: "U_viewer", + authorField: "botId", }, }, }), @@ -4263,8 +4264,9 @@ func viewerIDQuery(id string) githubv4mock.Matcher { } type pendingReviewQueryReview struct { - id string - authorID string + id string + authorID string + authorField string } type getPendingReviewsQueryParams struct { @@ -4278,10 +4280,14 @@ type getPendingReviewsQueryParams struct { func getPendingReviewsQuery(p getPendingReviewsQueryParams) githubv4mock.Matcher { reviews := make([]any, 0, len(p.reviews)) for _, review := range p.reviews { + authorField := review.authorField + if authorField == "" { + authorField = "userId" + } reviews = append(reviews, map[string]any{ "id": review.id, "author": map[string]any{ - "id": review.authorID, + authorField: review.authorID, }, }) } @@ -4293,9 +4299,7 @@ func getPendingReviewsQuery(p getPendingReviewsQueryParams) githubv4mock.Matcher Reviews struct { Nodes []struct { ID githubv4.ID - Author struct { - ID githubv4.ID - } + Author pendingReviewAuthor } PageInfo struct { HasNextPage githubv4.Boolean From 8fb90cdc0149a754c577896d2f6696001d72be51 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:18:37 +0000 Subject: [PATCH 3/3] Test pending review lookup pagination Cover cursor propagation and viewer review selection on a second page. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/pullrequests_test.go | 113 +++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 25 deletions(-) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index b2de748987..1fe90feeb2 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -4101,6 +4101,57 @@ func TestDeletePendingPullRequestReview(t *testing.T) { } } +func TestGetPendingPullRequestReviewForViewerPaginates(t *testing.T) { + t.Parallel() + + var reviewQueries atomic.Int32 + transport := NewMockRoundTripper().OnRequest(http.MethodPost, "/graphql", func(w http.ResponseWriter, r *http.Request) { + var request struct { + Query string `json:"query"` + Variables map[string]any `json:"variables"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + + w.Header().Set("Content-Type", "application/json") + switch { + case strings.Contains(request.Query, "viewer"): + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{ + "viewer": map[string]any{"id": "U_viewer"}, + }, + })) + case strings.Contains(request.Query, "reviews"): + queryNumber := reviewQueries.Add(1) + if queryNumber == 1 { + assert.Nil(t, request.Variables["after"]) + require.NoError(t, json.NewEncoder(w).Encode(pendingReviewsResponse( + []pendingReviewQueryReview{{id: "PR_other", authorID: "U_other"}}, + true, + "cursor-1", + ))) + return + } + + assert.Equal(t, "cursor-1", request.Variables["after"]) + require.NoError(t, json.NewEncoder(w).Encode(pendingReviewsResponse( + []pendingReviewQueryReview{{id: "PR_viewer", authorID: "U_viewer"}}, + false, + "", + ))) + default: + t.Fatalf("unexpected GraphQL query: %s", request.Query) + } + }) + + client := githubv4.NewClient(&http.Client{Transport: transport}) + reviewID, result := getPendingPullRequestReviewForViewer(context.Background(), client, "owner", "repo", 42) + + require.Nil(t, result) + require.NotNil(t, reviewID) + assert.Equal(t, githubv4.ID("PR_viewer"), *reviewID) + assert.Equal(t, int32(2), reviewQueries.Load()) +} + func TestGetPullRequestDiff(t *testing.T) { t.Parallel() @@ -4278,20 +4329,6 @@ type getPendingReviewsQueryParams struct { } func getPendingReviewsQuery(p getPendingReviewsQueryParams) githubv4mock.Matcher { - reviews := make([]any, 0, len(p.reviews)) - for _, review := range p.reviews { - authorField := review.authorField - if authorField == "" { - authorField = "userId" - } - reviews = append(reviews, map[string]any{ - "id": review.id, - "author": map[string]any{ - authorField: review.authorID, - }, - }) - } - matcher := githubv4mock.NewQueryMatcher( struct { Repository struct { @@ -4317,23 +4354,49 @@ func getPendingReviewsQuery(p getPendingReviewsQueryParams) githubv4mock.Matcher "states": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStatePending}, }, githubv4mock.DataResponse(map[string]any{ - "repository": map[string]any{ - "pullRequest": map[string]any{ - "reviews": map[string]any{ - "nodes": reviews, - "pageInfo": map[string]any{ - "hasNextPage": false, - "endCursor": "", - }, - }, - }, - }, + "repository": pendingReviewsData(p.reviews, false, "")["repository"], }), ) matcher.Variables["states"] = []any{"PENDING"} return matcher } +func pendingReviewsResponse(reviews []pendingReviewQueryReview, hasNextPage bool, endCursor string) map[string]any { + return map[string]any{ + "data": pendingReviewsData(reviews, hasNextPage, endCursor), + } +} + +func pendingReviewsData(reviews []pendingReviewQueryReview, hasNextPage bool, endCursor string) map[string]any { + nodes := make([]any, 0, len(reviews)) + for _, review := range reviews { + authorField := review.authorField + if authorField == "" { + authorField = "userId" + } + nodes = append(nodes, map[string]any{ + "id": review.id, + "author": map[string]any{ + authorField: review.authorID, + }, + }) + } + + return map[string]any{ + "repository": map[string]any{ + "pullRequest": map[string]any{ + "reviews": map[string]any{ + "nodes": nodes, + "pageInfo": map[string]any{ + "hasNextPage": hasNextPage, + "endCursor": endCursor, + }, + }, + }, + }, + } +} + func TestAddReplyToPullRequestComment(t *testing.T) { t.Parallel()