diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 425f954ef9..fe0928d5dd 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -1653,49 +1653,6 @@ 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), - }, - 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 { @@ -1721,7 +1678,10 @@ func TestGranularAddPullRequestReviewComment(t *testing.T) { ), ) gqlClient := githubv4.NewClient(mockedClient) - deps := BaseDeps{GQLClient: gqlClient} + deps := BaseDeps{ + Client: mockPendingReviewClient(t, "PRR_123"), + GQLClient: gqlClient, + } serverTool := GranularAddPullRequestReviewComment(translations.NullTranslationHelper) handler := serverTool.Handler(deps) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 5cee8b3231..43518fbc59 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1876,31 +1876,38 @@ Available methods: return utils.NewToolResultError(err.Error()), nil, nil } - // Given our owner, repo and PR number, lookup the GQL ID of the PR. - client, err := deps.GetGQLClient(ctx) + gqlClient, err := deps.GetGQLClient(ctx) if err != nil { return utils.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil, nil } switch params.Method { case "create": - result, err := CreatePullRequestReview(ctx, client, params) + result, err := CreatePullRequestReview(ctx, gqlClient, params) return result, nil, err case "submit_pending": - result, err := SubmitPendingPullRequestReview(ctx, client, params) + restClient, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to get GitHub client: %v", err)), nil, nil + } + result, err := SubmitPendingPullRequestReview(ctx, restClient, gqlClient, params) return result, nil, err case "delete_pending": - result, err := DeletePendingPullRequestReview(ctx, client, params) + restClient, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to get GitHub client: %v", err)), nil, nil + } + result, err := DeletePendingPullRequestReview(ctx, restClient, gqlClient, params) return result, nil, err case "resolve_thread": if !withResolutionReason { - result, err := ResolveReviewThread(ctx, client, params.ThreadID, true) + result, err := ResolveReviewThread(ctx, gqlClient, params.ThreadID, true) return result, nil, err } - result, err := ResolveReviewThreadWithReason(ctx, client, params.ThreadID, params.ResolutionReason, true) + result, err := ResolveReviewThreadWithReason(ctx, gqlClient, params.ThreadID, params.ResolutionReason, true) return result, nil, err case "unresolve_thread": - result, err := ResolveReviewThread(ctx, client, params.ThreadID, false) + result, err := ResolveReviewThread(ctx, gqlClient, params.ThreadID, false) return result, nil, err default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", params.Method)), nil, nil @@ -1988,58 +1995,43 @@ func CreatePullRequestReview(ctx context.Context, client *githubv4.Client, param return utils.NewToolResultText("pull request review submitted successfully"), nil } -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 +// GetPendingPullRequestReviewID returns the node ID of the viewer's pending review, +// or a tool error result otherwise. GitHub only includes pending reviews for their +// author in the REST response, including when the viewer is a GitHub App. +func GetPendingPullRequestReviewID(ctx context.Context, client *github.Client, owner, repo string, pullNumber int32) (githubv4.ID, *mcp.CallToolResult) { + opts := &github.ListOptions{PerPage: 100} + for { + reviews, resp, err := client.PullRequests.ListReviews(ctx, owner, repo, int(pullNumber), opts) + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + if err != nil { + return nil, ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list pull request reviews", resp, err) } - } - - 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), - } + for _, review := range reviews { + if review.GetState() != "PENDING" { + continue + } + if review.GetNodeID() == "" { + return nil, utils.NewToolResultError("Pending review did not include a node ID") + } + return githubv4.ID(review.GetNodeID()), nil + } - if err := client.Query(ctx, &getLatestReviewForViewerQuery, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get latest review for current user", - err, - ), nil + if resp == nil || resp.NextPage == 0 { + break + } + opts.Page = resp.NextPage } - // 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 - } + return nil, utils.NewToolResultError("No pending review found for the viewer") +} - 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 +func SubmitPendingPullRequestReview(ctx context.Context, restClient *github.Client, gqlClient *githubv4.Client, params PullRequestReviewWriteParams) (*mcp.CallToolResult, error) { + reviewID, errResult := GetPendingPullRequestReviewID(ctx, restClient, params.Owner, params.Repo, params.PullNumber) + if errResult != nil { + return errResult, nil } // Prepare the mutation @@ -2051,11 +2043,11 @@ func SubmitPendingPullRequestReview(ctx context.Context, client *githubv4.Client } `graphql:"submitPullRequestReview(input: $input)"` } - if err := client.Mutate( + if err := gqlClient.Mutate( ctx, &submitPullRequestReviewMutation, githubv4.SubmitPullRequestReviewInput{ - PullRequestReviewID: &review.ID, + PullRequestReviewID: &reviewID, Event: githubv4.PullRequestReviewEvent(params.Event), Body: newGQLStringlikePtr[githubv4.String](¶ms.Body), }, @@ -2073,58 +2065,10 @@ func SubmitPendingPullRequestReview(ctx context.Context, client *githubv4.Client return utils.NewToolResultText("pending pull request review successfully submitted"), nil } -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 +func DeletePendingPullRequestReview(ctx context.Context, restClient *github.Client, gqlClient *githubv4.Client, params PullRequestReviewWriteParams) (*mcp.CallToolResult, error) { + reviewID, errResult := GetPendingPullRequestReviewID(ctx, restClient, params.Owner, params.Repo, params.PullNumber) + if errResult != nil { + return errResult, nil } // Prepare the mutation @@ -2136,11 +2080,11 @@ func DeletePendingPullRequestReview(ctx context.Context, client *githubv4.Client } `graphql:"deletePullRequestReview(input: $input)"` } - if err := client.Mutate( + if err := gqlClient.Mutate( ctx, &deletePullRequestReviewMutation, githubv4.DeletePullRequestReviewInput{ - PullRequestReviewID: &review.ID, + PullRequestReviewID: &reviewID, }, nil, ); err != nil { @@ -2234,58 +2178,10 @@ 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 +func AddCommentToPendingReviewCall(ctx context.Context, restClient *github.Client, gqlClient *githubv4.Client, params AddCommentToPendingReviewParams) (*mcp.CallToolResult, error) { + reviewID, errResult := GetPendingPullRequestReviewID(ctx, restClient, params.Owner, params.Repo, params.PullNumber) + if errResult != nil { + return errResult, nil } // Create a new review thread comment on the review. @@ -2297,7 +2193,7 @@ func AddCommentToPendingReviewCall(ctx context.Context, client *githubv4.Client, } `graphql:"addPullRequestReviewThread(input: $input)"` } - if err := client.Mutate( + if err := gqlClient.Mutate( ctx, &addPullRequestReviewThreadMutation, githubv4.AddPullRequestReviewThreadInput{ @@ -2308,7 +2204,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: &reviewID, }, nil, ); err != nil { @@ -2435,10 +2331,14 @@ func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.S } startSide, _ := OptionalParam[string](args, "startSide") - client, err := deps.GetGQLClient(ctx) + gqlClient, err := deps.GetGQLClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub GQL client", err), nil, nil } + restClient, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } var linePtr, startLinePtr *int32 if line != 0 { @@ -2457,7 +2357,7 @@ func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.S startSidePtr = &startSide } - result, err := AddCommentToPendingReviewCall(ctx, client, AddCommentToPendingReviewParams{ + result, err := AddCommentToPendingReviewCall(ctx, restClient, gqlClient, AddCommentToPendingReviewParams{ Owner: owner, Repo: repo, PullNumber: int32(pullNumber), // #nosec G115 - PR numbers are always small positive integers diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index e670da9a34..f8b6653ea5 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -489,8 +489,12 @@ func GranularSubmitPendingPullRequestReview(t translations.TranslationHelperFunc if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } + restClient, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } - result, err := SubmitPendingPullRequestReview(ctx, gqlClient, PullRequestReviewWriteParams{ + result, err := SubmitPendingPullRequestReview(ctx, restClient, gqlClient, PullRequestReviewWriteParams{ Owner: owner, Repo: repo, PullNumber: int32(pullNumber), // #nosec G115 - PR numbers are always small positive integers @@ -546,8 +550,12 @@ func GranularDeletePendingPullRequestReview(t translations.TranslationHelperFunc if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } + restClient, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } - result, err := DeletePendingPullRequestReview(ctx, gqlClient, PullRequestReviewWriteParams{ + result, err := DeletePendingPullRequestReview(ctx, restClient, gqlClient, PullRequestReviewWriteParams{ Owner: owner, Repo: repo, PullNumber: int32(pullNumber), // #nosec G115 - PR numbers are always small positive integers @@ -630,6 +638,10 @@ func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) i if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } + restClient, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } // Convert optional int params to *int32 for the helper var linePtr, startLinePtr *int32 @@ -651,7 +663,7 @@ func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) i startSidePtr = &startSide } - result, err := AddCommentToPendingReviewCall(ctx, gqlClient, AddCommentToPendingReviewParams{ + result, err := AddCommentToPendingReviewCall(ctx, restClient, gqlClient, AddCommentToPendingReviewParams{ Owner: owner, Repo: repo, PullNumber: int32(pullNumber), // #nosec G115 - PR numbers are always small positive integers diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index c0e392aea6..84a6942a3d 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -11,6 +11,7 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v89/github" "github.com/google/jsonschema-go/jsonschema" @@ -3681,21 +3682,6 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { "startSide": "RIGHT", }, mockedClient: githubv4mock.NewMockedHTTPClient( - viewerQuery("williammartin"), - getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{ - author: "williammartin", - owner: "owner", - repo: "repo", - prNum: 42, - - reviews: []getLatestPendingReviewQueryReview{ - { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", - }, - }, - }), githubv4mock.NewMutationMatcher( struct { AddPullRequestReviewThread struct { @@ -3740,21 +3726,6 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { "startSide": "RIGHT", }, mockedClient: githubv4mock.NewMockedHTTPClient( - viewerQuery("williammartin"), - getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{ - author: "williammartin", - owner: "owner", - repo: "repo", - prNum: 42, - - reviews: []getLatestPendingReviewQueryReview{ - { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", - }, - }, - }), githubv4mock.NewMutationMatcher( struct { AddPullRequestReviewThread struct { @@ -3821,21 +3792,6 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { "side": "RIGHT", }, mockedClient: githubv4mock.NewMockedHTTPClient( - viewerQuery("williammartin"), - getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{ - author: "williammartin", - owner: "owner", - repo: "repo", - prNum: 42, - - reviews: []getLatestPendingReviewQueryReview{ - { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", - }, - }, - }), githubv4mock.NewMutationMatcher( struct { AddPullRequestReviewThread struct { @@ -3877,6 +3833,7 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { client := githubv4.NewClient(tc.mockedClient) serverTool := AddCommentToPendingReview(translations.NullTranslationHelper) deps := BaseDeps{ + Client: mockPendingReviewClient(t, "PR_kwDODKw3uc6WYN1T"), GQLClient: client, } handler := serverTool.Handler(deps) @@ -3939,21 +3896,6 @@ 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{ - { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", - }, - }, - }), githubv4mock.NewMutationMatcher( struct { SubmitPullRequestReview struct { @@ -3982,6 +3924,7 @@ func TestSubmitPendingPullRequestReview(t *testing.T) { client := githubv4.NewClient(tc.mockedClient) serverTool := PullRequestReviewWrite(translations.NullTranslationHelper) deps := BaseDeps{ + Client: mockPendingReviewClient(t, "PR_kwDODKw3uc6WYN1T"), GQLClient: client, } handler := serverTool.Handler(deps) @@ -4040,21 +3983,6 @@ 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{ - { - id: "PR_kwDODKw3uc6WYN1T", - state: "PENDING", - url: "https://github.com/owner/repo/pull/42", - }, - }, - }), githubv4mock.NewMutationMatcher( struct { DeletePullRequestReview struct { @@ -4081,6 +4009,7 @@ func TestDeletePendingPullRequestReview(t *testing.T) { client := githubv4.NewClient(tc.mockedClient) serverTool := PullRequestReviewWrite(translations.NullTranslationHelper) deps := BaseDeps{ + Client: mockPendingReviewClient(t, "PR_kwDODKw3uc6WYN1T"), GQLClient: client, } handler := serverTool.Handler(deps) @@ -4106,6 +4035,102 @@ func TestDeletePendingPullRequestReview(t *testing.T) { } } +func TestPendingPullRequestReviewLookupErrors(t *testing.T) { + t.Parallel() + + tools := []struct { + name string + tool inventory.ServerTool + method string + }{ + {"comment", AddCommentToPendingReview(translations.NullTranslationHelper), ""}, + {"submit", PullRequestReviewWrite(translations.NullTranslationHelper), "submit_pending"}, + {"delete", PullRequestReviewWrite(translations.NullTranslationHelper), "delete_pending"}, + {"granular comment", GranularAddPullRequestReviewComment(translations.NullTranslationHelper), ""}, + {"granular submit", GranularSubmitPendingPullRequestReview(translations.NullTranslationHelper), ""}, + {"granular delete", GranularDeletePendingPullRequestReview(translations.NullTranslationHelper), ""}, + } + + tests := []struct { + name string + reviews []*github.PullRequestReview + status int + expectedError string + }{ + {name: "no review", status: http.StatusOK, expectedError: "No pending review found for the viewer"}, + { + name: "only submitted reviews", + status: http.StatusOK, + reviews: []*github.PullRequestReview{ + {NodeID: github.Ptr("PRR_123"), State: github.Ptr("COMMENTED")}, + }, + expectedError: "No pending review found for the viewer", + }, + { + name: "pending review without node ID", + status: http.StatusOK, + reviews: []*github.PullRequestReview{ + {State: github.Ptr("PENDING")}, + }, + expectedError: "Pending review did not include a node ID", + }, + {name: "REST failure", status: http.StatusInternalServerError, expectedError: "failed to list pull request reviews"}, + } + + for _, tool := range tools { + for _, tc := range tests { + t.Run(tool.name+"/"+tc.name, func(t *testing.T) { + t.Parallel() + + restClient, err := github.NewClient(github.WithHTTPClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsReviewsByOwnerByRepoByPullNumber: mockResponse(t, tc.status, tc.reviews), + }))) + require.NoError(t, err) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + args := map[string]any{ + "owner": "owner", "repo": "repo", "pullNumber": float64(42), + "event": "COMMENT", "body": "Review comment", + "path": "file.go", "subjectType": "FILE", + } + if tool.method != "" { + args["method"] = tool.method + } + request := createMCPRequest(args) + result, err := tool.tool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, tc.expectedError) + }) + } + } +} + +func TestGetPendingPullRequestReviewIDPaginates(t *testing.T) { + t.Parallel() + + client, err := github.NewClient(github.WithHTTPClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsReviewsByOwnerByRepoByPullNumber: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.URL.Query().Get("page") == "2" { + _ = json.NewEncoder(w).Encode([]*github.PullRequestReview{ + {NodeID: github.Ptr("PRR_pending"), State: github.Ptr("PENDING")}, + }) + return + } + w.Header().Set("Link", `; rel="next"`) + _ = json.NewEncoder(w).Encode([]*github.PullRequestReview{ + {NodeID: github.Ptr("PRR_submitted"), State: github.Ptr("COMMENTED")}, + }) + }, + }))) + require.NoError(t, err) + + reviewID, errResult := GetPendingPullRequestReviewID(context.Background(), client, "owner", "repo", 42) + require.Nil(t, errResult) + assert.Equal(t, githubv4.ID("PRR_pending"), reviewID) +} + func TestGetPullRequestDiff(t *testing.T) { t.Parallel() @@ -4252,76 +4277,15 @@ index 5d6e7b2..8a4f5c3 100644 } } -func viewerQuery(login string) githubv4mock.Matcher { - return githubv4mock.NewQueryMatcher( - struct { - Viewer struct { - Login githubv4.String - } `graphql:"viewer"` - }{}, - map[string]any{}, - githubv4mock.DataResponse(map[string]any{ - "viewer": map[string]any{ - "login": login, - }, +func mockPendingReviewClient(t *testing.T, nodeID string) *github.Client { + t.Helper() + client, err := github.NewClient(github.WithHTTPClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsReviewsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, []*github.PullRequestReview{ + {NodeID: github.Ptr(nodeID), State: github.Ptr("PENDING")}, }), - ) -} - -type getLatestPendingReviewQueryReview struct { - id string - state string - url string -} - -type getLatestPendingReviewQueryParams struct { - author string - owner string - repo string - prNum int32 - - reviews []getLatestPendingReviewQueryReview -} - -func getLatestPendingReviewQuery(p getLatestPendingReviewQueryParams) githubv4mock.Matcher { - return 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(p.author), - "owner": githubv4.String(p.owner), - "name": githubv4.String(p.repo), - "prNum": githubv4.Int(p.prNum), - }, - 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, - }, - }, - }, - }, - }, - }, - ), - ) + }))) + require.NoError(t, err) + return client } func TestAddReplyToPullRequestComment(t *testing.T) {