diff --git a/README.md b/README.md index 6bfc6ab5..811a102a 100644 --- a/README.md +++ b/README.md @@ -302,6 +302,16 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `draft`: Create as draft PR (boolean, optional) - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) +- **request_pull_request_reviewers** - Request reviewers for a pull request + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `pull_number`: Pull request number (number, required) + - `reviewers`: GitHub usernames of reviewers to request (string[], optional*) + - `team_reviewers`: GitHub team names to request review from (string[], optional*) + + - At least one of `reviewers` or `team_reviewers` must be provided + - **add_pull_request_review_comment** - Add a review comment to a pull request or reply to an existing comment - `owner`: Repository owner (string, required) @@ -311,6 +321,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `commit_id`: The SHA of the commit to comment on (string, required unless using in_reply_to) - `path`: The relative path to the file that necessitates a comment (string, required unless using in_reply_to) - `line`: The line of the blob in the pull request diff that the comment applies to (number, optional) + - `side`: The side of the diff to comment on (LEFT or RIGHT) (string, optional) - `start_line`: For multi-line comments, the first line of the range (number, optional) - `start_side`: For multi-line comments, the starting side of the diff (LEFT or RIGHT) (string, optional) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index fd9420d7..e6ebed5d 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1178,3 +1178,92 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu return mcp.NewToolResultText(string(r)), nil } } + +// RequestPullRequestReviewers creates a tool to request reviewers for a pull request. +func RequestPullRequestReviewers(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("request_pull_request_reviewers", + mcp.WithDescription(t("TOOL_REQUEST_PULL_REQUEST_REVIEWERS_DESCRIPTION", "Request reviewers for a pull request")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("pull_number", + mcp.Required(), + mcp.Description("Pull request number"), + ), + mcp.WithArray("reviewers", + mcp.Items(map[string]interface{}{"type": "string"}), + mcp.Description("GitHub usernames of reviewers to request"), + ), + mcp.WithArray("team_reviewers", + mcp.Items(map[string]interface{}{"type": "string"}), + mcp.Description("GitHub team names to request review from"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pullNumber, err := RequiredInt(request, "pull_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Extract reviewers and team reviewers + reviewers, err := OptionalStringArrayParam(request, "reviewers") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + teamReviewers, err := OptionalStringArrayParam(request, "team_reviewers") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Verify at least one reviewer is specified + if len(reviewers) == 0 && len(teamReviewers) == 0 { + return mcp.NewToolResultError("At least one reviewer or team reviewer must be specified"), nil + } + + // Create the request options + reviewersRequest := &github.ReviewersRequest{ + Reviewers: reviewers, + TeamReviewers: teamReviewers, // Reverting to original field name + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, *reviewersRequest) + if err != nil { + return nil, fmt.Errorf("failed to request reviewers: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to request reviewers: %s", string(body))), nil + } + + r, err := json.Marshal(pr) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index bb372624..9998e753 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1916,3 +1916,186 @@ func Test_AddPullRequestReviewComment(t *testing.T) { }) } } + +func Test_RequestPullRequestReviewers(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := RequestPullRequestReviewers(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "request_pull_request_reviewers", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "pull_number") + assert.Contains(t, tool.InputSchema.Properties, "reviewers") + assert.Contains(t, tool.InputSchema.Properties, "team_reviewers") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pull_number"}) + + // Setup mock PR for success case + mockPR := &github.PullRequest{ + Number: github.Ptr(42), + Title: github.Ptr("Test PR"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"), + RequestedReviewers: []*github.User{ + {Login: github.Ptr("reviewer1")}, + {Login: github.Ptr("reviewer2")}, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedPR *github.PullRequest + expectedErrMsg string + }{ + { + name: "successful reviewers request with individual reviewers", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber, + expectRequestBody(t, map[string]interface{}{ + "reviewers": []interface{}{"reviewer1", "reviewer2"}, + }).andThen( + mockResponse(t, http.StatusOK, mockPR), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + "reviewers": []interface{}{"reviewer1", "reviewer2"}, + }, + expectError: false, + expectedPR: mockPR, + }, + { + name: "successful reviewers request with team reviewers", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber, + expectRequestBody(t, map[string]interface{}{ + "team_reviewers": []interface{}{"core-team", "design-team"}, + }).andThen( + mockResponse(t, http.StatusOK, mockPR), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + "team_reviewers": []interface{}{"core-team", "design-team"}, + }, + expectError: false, + expectedPR: mockPR, + }, + { + name: "successful reviewers request with both individual and team reviewers", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber, + expectRequestBody(t, map[string]interface{}{ + "reviewers": []interface{}{"reviewer1"}, + "team_reviewers": []interface{}{"core-team"}, + }).andThen( + mockResponse(t, http.StatusOK, mockPR), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + "reviewers": []interface{}{"reviewer1"}, + "team_reviewers": []interface{}{"core-team"}, + }, + expectError: false, + expectedPR: mockPR, + }, + { + name: "no reviewers specified", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + // No reviewers or team_reviewers provided + }, + expectError: false, + expectedErrMsg: "At least one reviewer or team reviewer must be specified", + }, + { + name: "request reviewers fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Validation failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(999), + "reviewers": []interface{}{"reviewer1"}, + }, + expectError: true, + expectedErrMsg: "failed to request reviewers", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := RequestPullRequestReviewers(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content + textContent := getTextResult(t, result) + + // Check for expected error message within the result text + if tc.expectedErrMsg != "" { + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + // Unmarshal and verify the successful result + var returnedPR github.PullRequest + err = json.Unmarshal([]byte(textContent.Text), &returnedPR) + require.NoError(t, err) + assert.Equal(t, *tc.expectedPR.Number, *returnedPR.Number) + assert.Equal(t, *tc.expectedPR.Title, *returnedPR.Title) + assert.Equal(t, *tc.expectedPR.State, *returnedPR.State) + + // Verify requested reviewers if available + if tc.expectedPR.RequestedReviewers != nil { + require.NotNil(t, returnedPR.RequestedReviewers) + assert.Len(t, returnedPR.RequestedReviewers, len(tc.expectedPR.RequestedReviewers)) + for i, reviewer := range returnedPR.RequestedReviewers { + assert.Equal(t, *tc.expectedPR.RequestedReviewers[i].Login, *reviewer.Login) + } + } + }) + } +} diff --git a/pkg/github/server.go b/pkg/github/server.go index da916b98..b0ab0178 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -64,6 +64,7 @@ func NewServer(getClient GetClientFn, version string, readOnly bool, t translati s.AddTool(CreatePullRequest(getClient, t)) s.AddTool(UpdatePullRequest(getClient, t)) s.AddTool(AddPullRequestReviewComment(getClient, t)) + s.AddTool(RequestPullRequestReviewers(getClient, t)) } // Add GitHub tools - Repositories