diff --git a/README.md b/README.md index 26f47030..352bb50e 100644 --- a/README.md +++ b/README.md @@ -458,6 +458,13 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `base`: New base branch name (string, optional) - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) +- **request_copilot_review** - Request a GitHub Copilot review for a pull request (experimental; subject to GitHub API support) + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `pullNumber`: Pull request number (number, required) + - _Note_: Currently, this tool will only work for github.com + ### Repositories - **create_or_update_file** - Create or update a single file in a repository diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index 489681e9..e3696497 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -772,3 +772,148 @@ func TestDirectoryDeletion(t *testing.T) { require.Equal(t, "test-dir/test-file.txt", trimmedGetCommitText.Files[0].Filename, "expected filename to match") require.Equal(t, 1, trimmedGetCommitText.Files[0].Deletions, "expected one deletion") } + +func TestRequestCopilotReview(t *testing.T) { + t.Parallel() + + mcpClient := setupMCPClient(t) + + ctx := context.Background() + + // First, who am I + getMeRequest := mcp.CallToolRequest{} + getMeRequest.Params.Name = "get_me" + + t.Log("Getting current user...") + resp, err := mcpClient.CallTool(ctx, getMeRequest) + require.NoError(t, err, "expected to call 'get_me' tool successfully") + require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp)) + + require.False(t, resp.IsError, "expected result not to be an error") + require.Len(t, resp.Content, 1, "expected content to have one item") + + textContent, ok := resp.Content[0].(mcp.TextContent) + require.True(t, ok, "expected content to be of type TextContent") + + var trimmedGetMeText struct { + Login string `json:"login"` + } + err = json.Unmarshal([]byte(textContent.Text), &trimmedGetMeText) + require.NoError(t, err, "expected to unmarshal text content successfully") + + currentOwner := trimmedGetMeText.Login + + // Then create a repository with a README (via autoInit) + repoName := fmt.Sprintf("github-mcp-server-e2e-%s-%d", t.Name(), time.Now().UnixMilli()) + createRepoRequest := mcp.CallToolRequest{} + createRepoRequest.Params.Name = "create_repository" + createRepoRequest.Params.Arguments = map[string]any{ + "name": repoName, + "private": true, + "autoInit": true, + } + + t.Logf("Creating repository %s/%s...", currentOwner, repoName) + _, err = mcpClient.CallTool(ctx, createRepoRequest) + require.NoError(t, err, "expected to call 'create_repository' tool successfully") + require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp)) + + // Cleanup the repository after the test + t.Cleanup(func() { + // MCP Server doesn't support deletions, but we can use the GitHub Client + ghClient := gogithub.NewClient(nil).WithAuthToken(getE2EToken(t)) + t.Logf("Deleting repository %s/%s...", currentOwner, repoName) + _, err := ghClient.Repositories.Delete(context.Background(), currentOwner, repoName) + require.NoError(t, err, "expected to delete repository successfully") + }) + + // Create a branch on which to create a new commit + createBranchRequest := mcp.CallToolRequest{} + createBranchRequest.Params.Name = "create_branch" + createBranchRequest.Params.Arguments = map[string]any{ + "owner": currentOwner, + "repo": repoName, + "branch": "test-branch", + "from_branch": "main", + } + + t.Logf("Creating branch in %s/%s...", currentOwner, repoName) + resp, err = mcpClient.CallTool(ctx, createBranchRequest) + require.NoError(t, err, "expected to call 'create_branch' tool successfully") + require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp)) + + // Create a commit with a new file + commitRequest := mcp.CallToolRequest{} + commitRequest.Params.Name = "create_or_update_file" + commitRequest.Params.Arguments = map[string]any{ + "owner": currentOwner, + "repo": repoName, + "path": "test-file.txt", + "content": fmt.Sprintf("Created by e2e test %s", t.Name()), + "message": "Add test file", + "branch": "test-branch", + } + + t.Logf("Creating commit with new file in %s/%s...", currentOwner, repoName) + resp, err = mcpClient.CallTool(ctx, commitRequest) + require.NoError(t, err, "expected to call 'create_or_update_file' tool successfully") + require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp)) + + textContent, ok = resp.Content[0].(mcp.TextContent) + require.True(t, ok, "expected content to be of type TextContent") + + var trimmedCommitText struct { + SHA string `json:"sha"` + } + err = json.Unmarshal([]byte(textContent.Text), &trimmedCommitText) + require.NoError(t, err, "expected to unmarshal text content successfully") + commitId := trimmedCommitText.SHA + + // Create a pull request + prRequest := mcp.CallToolRequest{} + prRequest.Params.Name = "create_pull_request" + prRequest.Params.Arguments = map[string]any{ + "owner": currentOwner, + "repo": repoName, + "title": "Test PR", + "body": "This is a test PR", + "head": "test-branch", + "base": "main", + "commitId": commitId, + } + + t.Logf("Creating pull request in %s/%s...", currentOwner, repoName) + resp, err = mcpClient.CallTool(ctx, prRequest) + require.NoError(t, err, "expected to call 'create_pull_request' tool successfully") + require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp)) + + // Request a copilot review + requestCopilotReviewRequest := mcp.CallToolRequest{} + requestCopilotReviewRequest.Params.Name = "request_copilot_review" + requestCopilotReviewRequest.Params.Arguments = map[string]any{ + "owner": currentOwner, + "repo": repoName, + "pullNumber": 1, + } + + t.Logf("Requesting Copilot review for pull request in %s/%s...", currentOwner, repoName) + resp, err = mcpClient.CallTool(ctx, requestCopilotReviewRequest) + require.NoError(t, err, "expected to call 'request_copilot_review' tool successfully") + require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp)) + + textContent, ok = resp.Content[0].(mcp.TextContent) + require.True(t, ok, "expected content to be of type TextContent") + require.Equal(t, "", textContent.Text, "expected content to be empty") + + // Finally, get requested reviews and see copilot is in there + // MCP Server doesn't support requesting reviews yet, but we can use the GitHub Client + ghClient := gogithub.NewClient(nil).WithAuthToken(getE2EToken(t)) + t.Logf("Getting reviews for pull request in %s/%s...", currentOwner, repoName) + reviewRequests, _, err := ghClient.PullRequests.ListReviewers(context.Background(), currentOwner, repoName, 1, nil) + require.NoError(t, err, "expected to get review requests successfully") + + // Check that there is one review request from copilot + require.Len(t, reviewRequests.Users, 1, "expected to find one review request") + require.Equal(t, "Copilot", *reviewRequests.Users[0].Login, "expected review request to be for Copilot") + require.Equal(t, "Bot", *reviewRequests.Users[0].Type, "expected review request to be for Bot") +} diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index f241d334..3032c938 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -10,6 +10,23 @@ import ( "github.com/stretchr/testify/require" ) +type expectations struct { + path string + queryParams map[string]string + requestBody any +} + +// expect is a helper function to create a partial mock that expects various +// request behaviors, such as path, query parameters, and request body. +func expect(t *testing.T, e expectations) *partialMock { + return &partialMock{ + t: t, + expectedPath: e.path, + expectedQueryParams: e.queryParams, + expectedRequestBody: e.requestBody, + } +} + // expectPath is a helper function to create a partial mock that expects a // request with the given path, with the ability to chain a response handler. func expectPath(t *testing.T, expectedPath string) *partialMock { diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index f4470b7b..f9039d2f 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1246,3 +1246,75 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu return mcp.NewToolResultText(string(r)), nil } } + +// RequestCopilotReview creates a tool to request a Copilot review for a pull request. +// Note that this tool will not work on GHES where this feature is unsupported. In future, we should not expose this +// tool if the configured host does not support it. +func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { + return mcp.NewTool("request_copilot_review", + mcp.WithDescription(t("TOOL_REQUEST_COPILOT_REVIEW_DESCRIPTION", "Request a GitHub Copilot code review for a pull request. Use this for automated feedback on pull requests, usually before requesting a human reviewer.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_REQUEST_COPILOT_REVIEW_USER_TITLE", "Request Copilot review"), + ReadOnlyHint: toBoolPtr(false), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("pullNumber", + mcp.Required(), + mcp.Description("Pull request number"), + ), + ), + 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, "pullNumber") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + _, resp, err := client.PullRequests.RequestReviewers( + ctx, + owner, + repo, + pullNumber, + github.ReviewersRequest{ + // The login name of the copilot reviewer bot + Reviewers: []string{"copilot-pull-request-reviewer[bot]"}, + }, + ) + if err != nil { + return nil, fmt.Errorf("failed to request copilot review: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + 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 copilot review: %s", string(body))), nil + } + + // Return nothing on success, as there's not much value in returning the Pull Request itself + return mcp.NewToolResultText(""), nil + } +} diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index bb372624..fe60e598 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1916,3 +1916,112 @@ func Test_AddPullRequestReviewComment(t *testing.T) { }) } } + +func Test_RequestCopilotReview(t *testing.T) { + t.Parallel() + + mockClient := github.NewClient(nil) + tool, _ := RequestCopilotReview(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "request_copilot_review", 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, "pullNumber") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + + // 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"), + Head: &github.PullRequestBranch{ + SHA: github.Ptr("abcd1234"), + Ref: github.Ptr("feature-branch"), + }, + Base: &github.PullRequestBranch{ + Ref: github.Ptr("main"), + }, + Body: github.Ptr("This is a test PR"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectError bool + expectedErrMsg string + }{ + { + name: "successful request", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber, + expect(t, expectations{ + path: "/repos/owner/repo/pulls/1/requested_reviewers", + requestBody: map[string]any{ + "reviewers": []any{"copilot-pull-request-reviewer[bot]"}, + }, + }).andThen( + mockResponse(t, http.StatusCreated, mockPR), + ), + ), + ), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(1), + }, + expectError: false, + }, + { + name: "request fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(999), + }, + expectError: true, + expectedErrMsg: "failed to request copilot review", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + client := github.NewClient(tc.mockedClient) + _, handler := RequestCopilotReview(stubGetClientFn(client), translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + + result, err := handler(context.Background(), request) + + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + assert.NotNil(t, result) + assert.Len(t, result.Content, 1) + + textContent := getTextResult(t, result) + require.Equal(t, "", textContent.Text) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index faef86ce..b2464b75 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -70,6 +70,7 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, toolsets.NewServerTool(CreatePullRequest(getClient, t)), toolsets.NewServerTool(UpdatePullRequest(getClient, t)), toolsets.NewServerTool(AddPullRequestReviewComment(getClient, t)), + toolsets.NewServerTool(RequestCopilotReview(getClient, t)), ) codeSecurity := toolsets.NewToolset("code_security", "Code security related tools, such as GitHub Code Scanning"). AddReadTools(