Skip to content

Commit b1d6fa7

Browse files
juruenjuruen
authored
add support for the create_pull_request_review_tool (github#33)
1 parent 20d97c7 commit b1d6fa7

File tree

4 files changed

+319
-1
lines changed

4 files changed

+319
-1
lines changed

‎README.md‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,16 @@ and set it as the GITHUB_PERSONAL_ACCESS_TOKEN environment variable.
126126
- `repo`: Repository name (string, required)
127127
- `pull_number`: Pull request number (number, required)
128128

129+
- **create_pull_request_review** - Create a review on a pull request review
130+
131+
- `owner`: Repository owner (string, required)
132+
- `repo`: Repository name (string, required)
133+
- `pull_number`: Pull request number (number, required)
134+
- `body`: Review comment text (string, optional)
135+
- `event`: Review action ('APPROVE', 'REQUEST_CHANGES', 'COMMENT') (string, required)
136+
- `commit_id`: SHA of commit to review (string, optional)
137+
- `comments`: Line-specific comments array of objects, each object with path (string), position (number), and body (string) (array, optional)
138+
129139
### Repositories
130140

131141
- **create_or_update_file** - Create or update a single file in a repository
@@ -380,7 +390,6 @@ Lots of things!
380390
Missing tools:
381391
382392
- push_files (files array)
383-
- create_pull_request_review (comments array)
384393
385394
Testing
386395

‎pkg/github/pullrequests.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,3 +494,113 @@ func getPullRequestReviews(client *github.Client, t translations.TranslationHelp
494494
return mcp.NewToolResultText(string(r)), nil
495495
}
496496
}
497+
498+
// createPullRequestReview creates a tool to submit a review on a pull request.
499+
func createPullRequestReview(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
500+
return mcp.NewTool("create_pull_request_review",
501+
mcp.WithDescription(t("TOOL_CREATE_PULL_REQUEST_REVIEW_DESCRIPTION", "Create a review on a pull request")),
502+
mcp.WithString("owner",
503+
mcp.Required(),
504+
mcp.Description("Repository owner"),
505+
),
506+
mcp.WithString("repo",
507+
mcp.Required(),
508+
mcp.Description("Repository name"),
509+
),
510+
mcp.WithNumber("pull_number",
511+
mcp.Required(),
512+
mcp.Description("Pull request number"),
513+
),
514+
mcp.WithString("body",
515+
mcp.Description("Review comment text"),
516+
),
517+
mcp.WithString("event",
518+
mcp.Required(),
519+
mcp.Description("Review action ('APPROVE', 'REQUEST_CHANGES', 'COMMENT')"),
520+
),
521+
mcp.WithString("commit_id",
522+
mcp.Description("SHA of commit to review"),
523+
),
524+
mcp.WithArray("comments",
525+
mcp.Description("Line-specific comments array of objects, each object with path (string), position (number), and body (string)"),
526+
),
527+
),
528+
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
529+
owner := request.Params.Arguments["owner"].(string)
530+
repo := request.Params.Arguments["repo"].(string)
531+
pullNumber := int(request.Params.Arguments["pull_number"].(float64))
532+
event := request.Params.Arguments["event"].(string)
533+
534+
// Create review request
535+
reviewRequest := &github.PullRequestReviewRequest{
536+
Event: github.Ptr(event),
537+
}
538+
539+
// Add body if provided
540+
if body, ok := request.Params.Arguments["body"].(string); ok && body != "" {
541+
reviewRequest.Body = github.Ptr(body)
542+
}
543+
544+
// Add commit ID if provided
545+
if commitID, ok := request.Params.Arguments["commit_id"].(string); ok && commitID != "" {
546+
reviewRequest.CommitID = github.Ptr(commitID)
547+
}
548+
549+
// Add comments if provided
550+
if commentsObj, ok := request.Params.Arguments["comments"].([]interface{}); ok && len(commentsObj) > 0 {
551+
comments := []*github.DraftReviewComment{}
552+
553+
for _, c := range commentsObj {
554+
commentMap, ok := c.(map[string]interface{})
555+
if !ok {
556+
return mcp.NewToolResultError("each comment must be an object with path, position, and body"), nil
557+
}
558+
559+
path, ok := commentMap["path"].(string)
560+
if !ok || path == "" {
561+
return mcp.NewToolResultError("each comment must have a path"), nil
562+
}
563+
564+
positionFloat, ok := commentMap["position"].(float64)
565+
if !ok {
566+
return mcp.NewToolResultError("each comment must have a position"), nil
567+
}
568+
position := int(positionFloat)
569+
570+
body, ok := commentMap["body"].(string)
571+
if !ok || body == "" {
572+
return mcp.NewToolResultError("each comment must have a body"), nil
573+
}
574+
575+
comments = append(comments, &github.DraftReviewComment{
576+
Path: github.Ptr(path),
577+
Position: github.Ptr(position),
578+
Body: github.Ptr(body),
579+
})
580+
}
581+
582+
reviewRequest.Comments = comments
583+
}
584+
585+
review, resp, err := client.PullRequests.CreateReview(ctx, owner, repo, pullNumber, reviewRequest)
586+
if err != nil {
587+
return nil, fmt.Errorf("failed to create pull request review: %w", err)
588+
}
589+
defer func() { _ = resp.Body.Close() }()
590+
591+
if resp.StatusCode != http.StatusOK {
592+
body, err := io.ReadAll(resp.Body)
593+
if err != nil {
594+
return nil, fmt.Errorf("failed to read response body: %w", err)
595+
}
596+
return mcp.NewToolResultError(fmt.Sprintf("failed to create pull request review: %s", string(body))), nil
597+
}
598+
599+
r, err := json.Marshal(review)
600+
if err != nil {
601+
return nil, fmt.Errorf("failed to marshal response: %w", err)
602+
}
603+
604+
return mcp.NewToolResultText(string(r)), nil
605+
}
606+
}

‎pkg/github/pullrequests_test.go‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -989,3 +989,201 @@ func Test_GetPullRequestReviews(t *testing.T) {
989989
})
990990
}
991991
}
992+
993+
func Test_CreatePullRequestReview(t *testing.T) {
994+
// Verify tool definition once
995+
mockClient := github.NewClient(nil)
996+
tool, _ := createPullRequestReview(mockClient, translations.NullTranslationHelper)
997+
998+
assert.Equal(t, "create_pull_request_review", tool.Name)
999+
assert.NotEmpty(t, tool.Description)
1000+
assert.Contains(t, tool.InputSchema.Properties, "owner")
1001+
assert.Contains(t, tool.InputSchema.Properties, "repo")
1002+
assert.Contains(t, tool.InputSchema.Properties, "pull_number")
1003+
assert.Contains(t, tool.InputSchema.Properties, "body")
1004+
assert.Contains(t, tool.InputSchema.Properties, "event")
1005+
assert.Contains(t, tool.InputSchema.Properties, "commit_id")
1006+
assert.Contains(t, tool.InputSchema.Properties, "comments")
1007+
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pull_number", "event"})
1008+
1009+
// Setup mock review for success case
1010+
mockReview := &github.PullRequestReview{
1011+
ID: github.Ptr(int64(301)),
1012+
State: github.Ptr("APPROVED"),
1013+
Body: github.Ptr("Looks good!"),
1014+
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42#pullrequestreview-301"),
1015+
User: &github.User{
1016+
Login: github.Ptr("reviewer"),
1017+
},
1018+
CommitID: github.Ptr("abcdef123456"),
1019+
SubmittedAt: &github.Timestamp{Time: time.Now()},
1020+
}
1021+
1022+
tests := []struct {
1023+
name string
1024+
mockedClient *http.Client
1025+
requestArgs map[string]interface{}
1026+
expectError bool
1027+
expectedReview *github.PullRequestReview
1028+
expectedErrMsg string
1029+
}{
1030+
{
1031+
name: "successful review creation with body only",
1032+
mockedClient: mock.NewMockedHTTPClient(
1033+
mock.WithRequestMatch(
1034+
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
1035+
mockReview,
1036+
),
1037+
),
1038+
requestArgs: map[string]interface{}{
1039+
"owner": "owner",
1040+
"repo": "repo",
1041+
"pull_number": float64(42),
1042+
"body": "Looks good!",
1043+
"event": "APPROVE",
1044+
},
1045+
expectError: false,
1046+
expectedReview: mockReview,
1047+
},
1048+
{
1049+
name: "successful review creation with commit_id",
1050+
mockedClient: mock.NewMockedHTTPClient(
1051+
mock.WithRequestMatch(
1052+
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
1053+
mockReview,
1054+
),
1055+
),
1056+
requestArgs: map[string]interface{}{
1057+
"owner": "owner",
1058+
"repo": "repo",
1059+
"pull_number": float64(42),
1060+
"body": "Looks good!",
1061+
"event": "APPROVE",
1062+
"commit_id": "abcdef123456",
1063+
},
1064+
expectError: false,
1065+
expectedReview: mockReview,
1066+
},
1067+
{
1068+
name: "successful review creation with comments",
1069+
mockedClient: mock.NewMockedHTTPClient(
1070+
mock.WithRequestMatch(
1071+
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
1072+
mockReview,
1073+
),
1074+
),
1075+
requestArgs: map[string]interface{}{
1076+
"owner": "owner",
1077+
"repo": "repo",
1078+
"pull_number": float64(42),
1079+
"body": "Some issues to fix",
1080+
"event": "REQUEST_CHANGES",
1081+
"comments": []interface{}{
1082+
map[string]interface{}{
1083+
"path": "file1.go",
1084+
"position": float64(10),
1085+
"body": "This needs to be fixed",
1086+
},
1087+
map[string]interface{}{
1088+
"path": "file2.go",
1089+
"position": float64(20),
1090+
"body": "Consider a different approach here",
1091+
},
1092+
},
1093+
},
1094+
expectError: false,
1095+
expectedReview: mockReview,
1096+
},
1097+
{
1098+
name: "invalid comment format",
1099+
mockedClient: mock.NewMockedHTTPClient(
1100+
mock.WithRequestMatchHandler(
1101+
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
1102+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1103+
w.WriteHeader(http.StatusUnprocessableEntity)
1104+
_, _ = w.Write([]byte(`{"message": "Invalid comment format"}`))
1105+
}),
1106+
),
1107+
),
1108+
requestArgs: map[string]interface{}{
1109+
"owner": "owner",
1110+
"repo": "repo",
1111+
"pull_number": float64(42),
1112+
"event": "REQUEST_CHANGES",
1113+
"comments": []interface{}{
1114+
map[string]interface{}{
1115+
"path": "file1.go",
1116+
// missing position
1117+
"body": "This needs to be fixed",
1118+
},
1119+
},
1120+
},
1121+
expectError: false,
1122+
expectedErrMsg: "each comment must have a position",
1123+
},
1124+
{
1125+
name: "review creation fails",
1126+
mockedClient: mock.NewMockedHTTPClient(
1127+
mock.WithRequestMatchHandler(
1128+
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
1129+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1130+
w.WriteHeader(http.StatusUnprocessableEntity)
1131+
_, _ = w.Write([]byte(`{"message": "Invalid comment format"}`))
1132+
}),
1133+
),
1134+
),
1135+
requestArgs: map[string]interface{}{
1136+
"owner": "owner",
1137+
"repo": "repo",
1138+
"pull_number": float64(42),
1139+
"body": "Looks good!",
1140+
"event": "APPROVE",
1141+
},
1142+
expectError: true,
1143+
expectedErrMsg: "failed to create pull request review",
1144+
},
1145+
}
1146+
1147+
for _, tc := range tests {
1148+
t.Run(tc.name, func(t *testing.T) {
1149+
// Setup client with mock
1150+
client := github.NewClient(tc.mockedClient)
1151+
_, handler := createPullRequestReview(client, translations.NullTranslationHelper)
1152+
1153+
// Create call request
1154+
request := createMCPRequest(tc.requestArgs)
1155+
1156+
// Call handler
1157+
result, err := handler(context.Background(), request)
1158+
1159+
// Verify results
1160+
if tc.expectError {
1161+
require.Error(t, err)
1162+
assert.Contains(t, err.Error(), tc.expectedErrMsg)
1163+
return
1164+
}
1165+
1166+
require.NoError(t, err)
1167+
1168+
// For error messages in the result
1169+
if tc.expectedErrMsg != "" {
1170+
textContent := getTextResult(t, result)
1171+
assert.Contains(t, textContent.Text, tc.expectedErrMsg)
1172+
return
1173+
}
1174+
1175+
// Parse the result and get the text content if no error
1176+
textContent := getTextResult(t, result)
1177+
1178+
// Unmarshal and verify the result
1179+
var returnedReview github.PullRequestReview
1180+
err = json.Unmarshal([]byte(textContent.Text), &returnedReview)
1181+
require.NoError(t, err)
1182+
assert.Equal(t, *tc.expectedReview.ID, *returnedReview.ID)
1183+
assert.Equal(t, *tc.expectedReview.State, *returnedReview.State)
1184+
assert.Equal(t, *tc.expectedReview.Body, *returnedReview.Body)
1185+
assert.Equal(t, *tc.expectedReview.User.Login, *returnedReview.User.Login)
1186+
assert.Equal(t, *tc.expectedReview.HTMLURL, *returnedReview.HTMLURL)
1187+
})
1188+
}
1189+
}

‎pkg/github/server.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ func NewServer(client *github.Client, readOnly bool, t translations.TranslationH
5454
if !readOnly {
5555
s.AddTool(mergePullRequest(client, t))
5656
s.AddTool(updatePullRequestBranch(client, t))
57+
s.AddTool(createPullRequestReview(client, t))
5758
}
5859

5960
// Add GitHub tools - Repositories

0 commit comments

Comments
 (0)