Skip to content

Commit c193d38

Browse files
williammartinwilliammartin
committed
Support requesting copilot as a reviewer
1 parent dfce03d commit c193d38

File tree

6 files changed

+306
-22
lines changed

6 files changed

+306
-22
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
462462

463463
- `owner`: Repository owner (string, required)
464464
- `repo`: Repository name (string, required)
465-
- `pull_number`: Pull request number (number, required)
465+
- `pullNumber`: Pull request number (number, required)
466466
- _Note: As of now, requesting a Copilot review programmatically is not supported by the GitHub API. This tool will return an error until GitHub exposes this functionality._
467467

468468
### Repositories

‎e2e/e2e_test.go

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,3 +369,148 @@ func TestTags(t *testing.T) {
369369
require.Equal(t, "v0.0.1", trimmedTag[0].Name, "expected tag name to match")
370370
require.Equal(t, *ref.Object.SHA, trimmedTag[0].Commit.SHA, "expected tag SHA to match")
371371
}
372+
373+
func TestRequestCopilotReview(t *testing.T) {
374+
t.Parallel()
375+
376+
mcpClient := setupMCPClient(t)
377+
378+
ctx := context.Background()
379+
380+
// First, who am I
381+
getMeRequest := mcp.CallToolRequest{}
382+
getMeRequest.Params.Name = "get_me"
383+
384+
t.Log("Getting current user...")
385+
resp, err := mcpClient.CallTool(ctx, getMeRequest)
386+
require.NoError(t, err, "expected to call 'get_me' tool successfully")
387+
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
388+
389+
require.False(t, resp.IsError, "expected result not to be an error")
390+
require.Len(t, resp.Content, 1, "expected content to have one item")
391+
392+
textContent, ok := resp.Content[0].(mcp.TextContent)
393+
require.True(t, ok, "expected content to be of type TextContent")
394+
395+
var trimmedGetMeText struct {
396+
Login string `json:"login"`
397+
}
398+
err = json.Unmarshal([]byte(textContent.Text), &trimmedGetMeText)
399+
require.NoError(t, err, "expected to unmarshal text content successfully")
400+
401+
currentOwner := trimmedGetMeText.Login
402+
403+
// Then create a repository with a README (via autoInit)
404+
repoName := fmt.Sprintf("github-mcp-server-e2e-%s-%d", t.Name(), time.Now().UnixMilli())
405+
createRepoRequest := mcp.CallToolRequest{}
406+
createRepoRequest.Params.Name = "create_repository"
407+
createRepoRequest.Params.Arguments = map[string]any{
408+
"name": repoName,
409+
"private": true,
410+
"autoInit": true,
411+
}
412+
413+
t.Logf("Creating repository %s/%s...", currentOwner, repoName)
414+
_, err = mcpClient.CallTool(ctx, createRepoRequest)
415+
require.NoError(t, err, "expected to call 'create_repository' tool successfully")
416+
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
417+
418+
// Cleanup the repository after the test
419+
t.Cleanup(func() {
420+
// MCP Server doesn't support deletions, but we can use the GitHub Client
421+
ghClient := gogithub.NewClient(nil).WithAuthToken(getE2EToken(t))
422+
t.Logf("Deleting repository %s/%s...", currentOwner, repoName)
423+
_, err := ghClient.Repositories.Delete(context.Background(), currentOwner, repoName)
424+
require.NoError(t, err, "expected to delete repository successfully")
425+
})
426+
427+
// Create a branch on which to create a new commit
428+
createBranchRequest := mcp.CallToolRequest{}
429+
createBranchRequest.Params.Name = "create_branch"
430+
createBranchRequest.Params.Arguments = map[string]any{
431+
"owner": currentOwner,
432+
"repo": repoName,
433+
"branch": "test-branch",
434+
"from_branch": "main",
435+
}
436+
437+
t.Logf("Creating branch in %s/%s...", currentOwner, repoName)
438+
resp, err = mcpClient.CallTool(ctx, createBranchRequest)
439+
require.NoError(t, err, "expected to call 'create_branch' tool successfully")
440+
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
441+
442+
// Create a commit with a new file
443+
commitRequest := mcp.CallToolRequest{}
444+
commitRequest.Params.Name = "create_or_update_file"
445+
commitRequest.Params.Arguments = map[string]any{
446+
"owner": currentOwner,
447+
"repo": repoName,
448+
"path": "test-file.txt",
449+
"content": fmt.Sprintf("Created by e2e test %s", t.Name()),
450+
"message": "Add test file",
451+
"branch": "test-branch",
452+
}
453+
454+
t.Logf("Creating commit with new file in %s/%s...", currentOwner, repoName)
455+
resp, err = mcpClient.CallTool(ctx, commitRequest)
456+
require.NoError(t, err, "expected to call 'create_or_update_file' tool successfully")
457+
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
458+
459+
textContent, ok = resp.Content[0].(mcp.TextContent)
460+
require.True(t, ok, "expected content to be of type TextContent")
461+
462+
var trimmedCommitText struct {
463+
SHA string `json:"sha"`
464+
}
465+
err = json.Unmarshal([]byte(textContent.Text), &trimmedCommitText)
466+
require.NoError(t, err, "expected to unmarshal text content successfully")
467+
commitId := trimmedCommitText.SHA
468+
469+
// Create a pull request
470+
prRequest := mcp.CallToolRequest{}
471+
prRequest.Params.Name = "create_pull_request"
472+
prRequest.Params.Arguments = map[string]any{
473+
"owner": currentOwner,
474+
"repo": repoName,
475+
"title": "Test PR",
476+
"body": "This is a test PR",
477+
"head": "test-branch",
478+
"base": "main",
479+
"commitId": commitId,
480+
}
481+
482+
t.Logf("Creating pull request in %s/%s...", currentOwner, repoName)
483+
resp, err = mcpClient.CallTool(ctx, prRequest)
484+
require.NoError(t, err, "expected to call 'create_pull_request' tool successfully")
485+
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
486+
487+
// Request a copilot review
488+
requestCopilotReviewRequest := mcp.CallToolRequest{}
489+
requestCopilotReviewRequest.Params.Name = "request_copilot_review"
490+
requestCopilotReviewRequest.Params.Arguments = map[string]any{
491+
"owner": currentOwner,
492+
"repo": repoName,
493+
"pullNumber": 1,
494+
}
495+
496+
t.Logf("Requesting Copilot review for pull request in %s/%s...", currentOwner, repoName)
497+
resp, err = mcpClient.CallTool(ctx, requestCopilotReviewRequest)
498+
require.NoError(t, err, "expected to call 'request_copilot_review' tool successfully")
499+
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
500+
501+
textContent, ok = resp.Content[0].(mcp.TextContent)
502+
require.True(t, ok, "expected content to be of type TextContent")
503+
require.Equal(t, "", textContent.Text, "expected content to be empty")
504+
505+
// Finally, get requested reviews and see copilot is in there
506+
// MCP Server doesn't support requesting reviews yet, but we can use the GitHub Client
507+
ghClient := gogithub.NewClient(nil).WithAuthToken(getE2EToken(t))
508+
t.Logf("Getting reviews for pull request in %s/%s...", currentOwner, repoName)
509+
reviewRequests, _, err := ghClient.PullRequests.ListReviewers(context.Background(), currentOwner, repoName, 1, nil)
510+
require.NoError(t, err, "expected to get review requests successfully")
511+
512+
// Check that there is one review request from copilot
513+
require.Len(t, reviewRequests.Users, 1, "expected to find one review request")
514+
require.Equal(t, "Copilot", *reviewRequests.Users[0].Login, "expected review request to be for Copilot")
515+
require.Equal(t, "Bot", *reviewRequests.Users[0].Type, "expected review request to be for Bot")
516+
}

‎pkg/github/helper_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,23 @@ import (
1010
"github.com/stretchr/testify/require"
1111
)
1212

13+
type expectations struct {
14+
path string
15+
queryParams map[string]string
16+
requestBody any
17+
}
18+
19+
// expect is a helper function to create a partial mock that expects various
20+
// request behaviors, such as path, query parameters, and request body.
21+
func expect(t *testing.T, e expectations) *partialMock {
22+
return &partialMock{
23+
t: t,
24+
expectedPath: e.path,
25+
expectedQueryParams: e.queryParams,
26+
expectedRequestBody: e.requestBody,
27+
}
28+
}
29+
1330
// expectPath is a helper function to create a partial mock that expects a
1431
// request with the given path, with the ability to chain a response handler.
1532
func expectPath(t *testing.T, expectedPath string) *partialMock {

‎pkg/github/pullrequests.go

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,9 +1248,15 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu
12481248
}
12491249

12501250
// RequestCopilotReview creates a tool to request a Copilot review for a pull request.
1251-
func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
1251+
// Note that this tool will not work on GHES where this feature is unsupported. In future, we should not expose this
1252+
// tool if the configured host does not support it.
1253+
func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) {
12521254
return mcp.NewTool("request_copilot_review",
12531255
mcp.WithDescription(t("TOOL_REQUEST_COPILOT_REVIEW_DESCRIPTION", "Request a GitHub Copilot review for a pull request. Note: This feature depends on GitHub API support and may not be available for all users.")),
1256+
mcp.WithToolAnnotation(mcp.ToolAnnotation{
1257+
Title: t("TOOL_REQUEST_COPILOT_REVIEW_USER_TITLE", "Request Copilot review"),
1258+
ReadOnlyHint: toBoolPtr(false),
1259+
}),
12541260
mcp.WithString("owner",
12551261
mcp.Required(),
12561262
mcp.Description("Repository owner"),
@@ -1259,7 +1265,7 @@ func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelpe
12591265
mcp.Required(),
12601266
mcp.Description("Repository name"),
12611267
),
1262-
mcp.WithNumber("pull_number",
1268+
mcp.WithNumber("pullNumber",
12631269
mcp.Required(),
12641270
mcp.Description("Pull request number"),
12651271
),
@@ -1269,17 +1275,46 @@ func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelpe
12691275
if err != nil {
12701276
return mcp.NewToolResultError(err.Error()), nil
12711277
}
1278+
12721279
repo, err := requiredParam[string](request, "repo")
12731280
if err != nil {
12741281
return mcp.NewToolResultError(err.Error()), nil
12751282
}
1276-
pullNumber, err := RequiredInt(request, "pull_number")
1283+
1284+
pullNumber, err := RequiredInt(request, "pullNumber")
1285+
if err != nil {
1286+
return mcp.NewToolResultError(err.Error()), nil
1287+
}
1288+
1289+
client, err := getClient(ctx)
12771290
if err != nil {
12781291
return mcp.NewToolResultError(err.Error()), nil
12791292
}
12801293

1281-
// As of now, GitHub API does not support Copilot as a reviewer programmatically.
1282-
// This is a placeholder for future support.
1283-
return mcp.NewToolResultError(fmt.Sprintf("Requesting a Copilot review for PR #%d in %s/%s is not currently supported by the GitHub API. Please request a Copilot review via the GitHub UI.", pullNumber, owner, repo)), nil
1294+
_, resp, err := client.PullRequests.RequestReviewers(
1295+
ctx,
1296+
owner,
1297+
repo,
1298+
pullNumber,
1299+
github.ReviewersRequest{
1300+
// The login name of the copilot reviewer bot
1301+
Reviewers: []string{"copilot-pull-request-reviewer[bot]"},
1302+
},
1303+
)
1304+
if err != nil {
1305+
return nil, fmt.Errorf("failed to request copilot review: %w", err)
1306+
}
1307+
defer func() { _ = resp.Body.Close() }()
1308+
1309+
if resp.StatusCode != http.StatusCreated {
1310+
body, err := io.ReadAll(resp.Body)
1311+
if err != nil {
1312+
return nil, fmt.Errorf("failed to read response body: %w", err)
1313+
}
1314+
return mcp.NewToolResultError(fmt.Sprintf("failed to request copilot review: %s", string(body))), nil
1315+
}
1316+
1317+
// Return nothing on success, as there's not much value in returning the Pull Request itself
1318+
return mcp.NewToolResultText(""), nil
12841319
}
12851320
}

‎pkg/github/pullrequests_test.go

Lines changed: 100 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1918,25 +1918,110 @@ func Test_AddPullRequestReviewComment(t *testing.T) {
19181918
}
19191919

19201920
func Test_RequestCopilotReview(t *testing.T) {
1921+
t.Parallel()
1922+
19211923
mockClient := github.NewClient(nil)
1922-
tool, handler := RequestCopilotReview(stubGetClientFn(mockClient), translations.NullTranslationHelper)
1924+
tool, _ := RequestCopilotReview(stubGetClientFn(mockClient), translations.NullTranslationHelper)
19231925

19241926
assert.Equal(t, "request_copilot_review", tool.Name)
19251927
assert.NotEmpty(t, tool.Description)
19261928
assert.Contains(t, tool.InputSchema.Properties, "owner")
19271929
assert.Contains(t, tool.InputSchema.Properties, "repo")
1928-
assert.Contains(t, tool.InputSchema.Properties, "pull_number")
1929-
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pull_number"})
1930-
1931-
request := createMCPRequest(map[string]interface{}{
1932-
"owner": "owner",
1933-
"repo": "repo",
1934-
"pull_number": float64(42),
1935-
})
1936-
1937-
result, err := handler(context.Background(), request)
1938-
assert.NoError(t, err)
1939-
assert.NotNil(t, result)
1940-
textContent := getTextResult(t, result)
1941-
assert.Contains(t, textContent.Text, "not currently supported by the GitHub API")
1930+
assert.Contains(t, tool.InputSchema.Properties, "pullNumber")
1931+
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"})
1932+
1933+
// Setup mock PR for success case
1934+
mockPR := &github.PullRequest{
1935+
Number: github.Ptr(42),
1936+
Title: github.Ptr("Test PR"),
1937+
State: github.Ptr("open"),
1938+
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
1939+
Head: &github.PullRequestBranch{
1940+
SHA: github.Ptr("abcd1234"),
1941+
Ref: github.Ptr("feature-branch"),
1942+
},
1943+
Base: &github.PullRequestBranch{
1944+
Ref: github.Ptr("main"),
1945+
},
1946+
Body: github.Ptr("This is a test PR"),
1947+
User: &github.User{
1948+
Login: github.Ptr("testuser"),
1949+
},
1950+
}
1951+
1952+
tests := []struct {
1953+
name string
1954+
mockedClient *http.Client
1955+
requestArgs map[string]any
1956+
expectError bool
1957+
expectedErrMsg string
1958+
}{
1959+
{
1960+
name: "successful request",
1961+
mockedClient: mock.NewMockedHTTPClient(
1962+
mock.WithRequestMatchHandler(
1963+
mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber,
1964+
expect(t, expectations{
1965+
path: "/repos/owner/repo/pulls/1/requested_reviewers",
1966+
requestBody: map[string]any{
1967+
"reviewers": []any{"copilot-pull-request-reviewer[bot]"},
1968+
},
1969+
}).andThen(
1970+
mockResponse(t, http.StatusCreated, mockPR),
1971+
),
1972+
),
1973+
),
1974+
requestArgs: map[string]any{
1975+
"owner": "owner",
1976+
"repo": "repo",
1977+
"pullNumber": float64(1),
1978+
},
1979+
expectError: false,
1980+
},
1981+
{
1982+
name: "request fails",
1983+
mockedClient: mock.NewMockedHTTPClient(
1984+
mock.WithRequestMatchHandler(
1985+
mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber,
1986+
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
1987+
w.WriteHeader(http.StatusNotFound)
1988+
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
1989+
}),
1990+
),
1991+
),
1992+
requestArgs: map[string]any{
1993+
"owner": "owner",
1994+
"repo": "repo",
1995+
"pullNumber": float64(999),
1996+
},
1997+
expectError: true,
1998+
expectedErrMsg: "failed to request copilot review",
1999+
},
2000+
}
2001+
2002+
for _, tc := range tests {
2003+
t.Run(tc.name, func(t *testing.T) {
2004+
t.Parallel()
2005+
2006+
client := github.NewClient(tc.mockedClient)
2007+
_, handler := RequestCopilotReview(stubGetClientFn(client), translations.NullTranslationHelper)
2008+
2009+
request := createMCPRequest(tc.requestArgs)
2010+
2011+
result, err := handler(context.Background(), request)
2012+
2013+
if tc.expectError {
2014+
require.Error(t, err)
2015+
assert.Contains(t, err.Error(), tc.expectedErrMsg)
2016+
return
2017+
}
2018+
2019+
require.NoError(t, err)
2020+
assert.NotNil(t, result)
2021+
assert.Len(t, result.Content, 1)
2022+
2023+
textContent := getTextResult(t, result)
2024+
require.Equal(t, "", textContent.Text)
2025+
})
2026+
}
19422027
}

‎pkg/github/tools.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn,
6969
toolsets.NewServerTool(CreatePullRequest(getClient, t)),
7070
toolsets.NewServerTool(UpdatePullRequest(getClient, t)),
7171
toolsets.NewServerTool(AddPullRequestReviewComment(getClient, t)),
72+
73+
toolsets.NewServerTool(RequestCopilotReview(getClient, t)),
7274
)
7375
codeSecurity := toolsets.NewToolset("code_security", "Code security related tools, such as GitHub Code Scanning").
7476
AddReadTools(

0 commit comments

Comments
 (0)