diff --git a/README.md b/README.md index b18a5964..e1e96efe 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,16 @@ and set it as the GITHUB_PERSONAL_ACCESS_TOKEN environment variable. - `repo`: Repository name (string, required) - `pull_number`: Pull request number (number, required) +- **create_pull_request_review** - Create a review on a pull request review + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `pull_number`: Pull request number (number, required) + - `body`: Review comment text (string, optional) + - `event`: Review action ('APPROVE', 'REQUEST_CHANGES', 'COMMENT') (string, required) + - `commit_id`: SHA of commit to review (string, optional) + - `comments`: Line-specific comments array of objects, each object with path (string), position (number), and body (string) (array, optional) + ### Repositories - **create_or_update_file** - Create or update a single file in a repository @@ -380,7 +390,6 @@ Lots of things! Missing tools: - push_files (files array) -- create_pull_request_review (comments array) Testing diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index ba8b0aef..e0414394 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -494,3 +494,113 @@ func getPullRequestReviews(client *github.Client, t translations.TranslationHelp return mcp.NewToolResultText(string(r)), nil } } + +// createPullRequestReview creates a tool to submit a review on a pull request. +func createPullRequestReview(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("create_pull_request_review", + mcp.WithDescription(t("TOOL_CREATE_PULL_REQUEST_REVIEW_DESCRIPTION", "Create a review on 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.WithString("body", + mcp.Description("Review comment text"), + ), + mcp.WithString("event", + mcp.Required(), + mcp.Description("Review action ('APPROVE', 'REQUEST_CHANGES', 'COMMENT')"), + ), + mcp.WithString("commit_id", + mcp.Description("SHA of commit to review"), + ), + mcp.WithArray("comments", + mcp.Description("Line-specific comments array of objects, each object with path (string), position (number), and body (string)"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner := request.Params.Arguments["owner"].(string) + repo := request.Params.Arguments["repo"].(string) + pullNumber := int(request.Params.Arguments["pull_number"].(float64)) + event := request.Params.Arguments["event"].(string) + + // Create review request + reviewRequest := &github.PullRequestReviewRequest{ + Event: github.Ptr(event), + } + + // Add body if provided + if body, ok := request.Params.Arguments["body"].(string); ok && body != "" { + reviewRequest.Body = github.Ptr(body) + } + + // Add commit ID if provided + if commitID, ok := request.Params.Arguments["commit_id"].(string); ok && commitID != "" { + reviewRequest.CommitID = github.Ptr(commitID) + } + + // Add comments if provided + if commentsObj, ok := request.Params.Arguments["comments"].([]interface{}); ok && len(commentsObj) > 0 { + comments := []*github.DraftReviewComment{} + + for _, c := range commentsObj { + commentMap, ok := c.(map[string]interface{}) + if !ok { + return mcp.NewToolResultError("each comment must be an object with path, position, and body"), nil + } + + path, ok := commentMap["path"].(string) + if !ok || path == "" { + return mcp.NewToolResultError("each comment must have a path"), nil + } + + positionFloat, ok := commentMap["position"].(float64) + if !ok { + return mcp.NewToolResultError("each comment must have a position"), nil + } + position := int(positionFloat) + + body, ok := commentMap["body"].(string) + if !ok || body == "" { + return mcp.NewToolResultError("each comment must have a body"), nil + } + + comments = append(comments, &github.DraftReviewComment{ + Path: github.Ptr(path), + Position: github.Ptr(position), + Body: github.Ptr(body), + }) + } + + reviewRequest.Comments = comments + } + + review, resp, err := client.PullRequests.CreateReview(ctx, owner, repo, pullNumber, reviewRequest) + if err != nil { + return nil, fmt.Errorf("failed to create pull request review: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if 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 create pull request review: %s", string(body))), nil + } + + r, err := json.Marshal(review) + 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 beecb1cc..15c5e070 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -989,3 +989,201 @@ func Test_GetPullRequestReviews(t *testing.T) { }) } } + +func Test_CreatePullRequestReview(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := createPullRequestReview(mockClient, translations.NullTranslationHelper) + + assert.Equal(t, "create_pull_request_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, "pull_number") + assert.Contains(t, tool.InputSchema.Properties, "body") + assert.Contains(t, tool.InputSchema.Properties, "event") + assert.Contains(t, tool.InputSchema.Properties, "commit_id") + assert.Contains(t, tool.InputSchema.Properties, "comments") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pull_number", "event"}) + + // Setup mock review for success case + mockReview := &github.PullRequestReview{ + ID: github.Ptr(int64(301)), + State: github.Ptr("APPROVED"), + Body: github.Ptr("Looks good!"), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42#pullrequestreview-301"), + User: &github.User{ + Login: github.Ptr("reviewer"), + }, + CommitID: github.Ptr("abcdef123456"), + SubmittedAt: &github.Timestamp{Time: time.Now()}, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedReview *github.PullRequestReview + expectedErrMsg string + }{ + { + name: "successful review creation with body only", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.PostReposPullsReviewsByOwnerByRepoByPullNumber, + mockReview, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + "body": "Looks good!", + "event": "APPROVE", + }, + expectError: false, + expectedReview: mockReview, + }, + { + name: "successful review creation with commit_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.PostReposPullsReviewsByOwnerByRepoByPullNumber, + mockReview, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + "body": "Looks good!", + "event": "APPROVE", + "commit_id": "abcdef123456", + }, + expectError: false, + expectedReview: mockReview, + }, + { + name: "successful review creation with comments", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.PostReposPullsReviewsByOwnerByRepoByPullNumber, + mockReview, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + "body": "Some issues to fix", + "event": "REQUEST_CHANGES", + "comments": []interface{}{ + map[string]interface{}{ + "path": "file1.go", + "position": float64(10), + "body": "This needs to be fixed", + }, + map[string]interface{}{ + "path": "file2.go", + "position": float64(20), + "body": "Consider a different approach here", + }, + }, + }, + expectError: false, + expectedReview: mockReview, + }, + { + name: "invalid comment format", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsReviewsByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Invalid comment format"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + "event": "REQUEST_CHANGES", + "comments": []interface{}{ + map[string]interface{}{ + "path": "file1.go", + // missing position + "body": "This needs to be fixed", + }, + }, + }, + expectError: false, + expectedErrMsg: "each comment must have a position", + }, + { + name: "review creation fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsReviewsByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Invalid comment format"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(42), + "body": "Looks good!", + "event": "APPROVE", + }, + expectError: true, + expectedErrMsg: "failed to create pull request review", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := createPullRequestReview(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) + + // For error messages in the result + if tc.expectedErrMsg != "" { + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedReview github.PullRequestReview + err = json.Unmarshal([]byte(textContent.Text), &returnedReview) + require.NoError(t, err) + assert.Equal(t, *tc.expectedReview.ID, *returnedReview.ID) + assert.Equal(t, *tc.expectedReview.State, *returnedReview.State) + assert.Equal(t, *tc.expectedReview.Body, *returnedReview.Body) + assert.Equal(t, *tc.expectedReview.User.Login, *returnedReview.User.Login) + assert.Equal(t, *tc.expectedReview.HTMLURL, *returnedReview.HTMLURL) + }) + } +} diff --git a/pkg/github/server.go b/pkg/github/server.go index 1c0df9a5..75ab0a5f 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -54,6 +54,7 @@ func NewServer(client *github.Client, readOnly bool, t translations.TranslationH if !readOnly { s.AddTool(mergePullRequest(client, t)) s.AddTool(updatePullRequestBranch(client, t)) + s.AddTool(createPullRequestReview(client, t)) } // Add GitHub tools - Repositories