Skip to content

Add tools for one-off PR comments and replying to PR review comments #143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Apr 12, 2025
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,21 @@ 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)

- **add_pull_request_review_comment** - Add a review comment to a pull request or reply to an existing comment

- `owner`: Repository owner (string, required)
- `repo`: Repository name (string, required)
- `pull_number`: Pull request number (number, required)
- `body`: The text of the review comment (string, required)
- `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)
- `subject_type`: The level at which the comment is targeted (line or file) (string, optional)
- `in_reply_to`: The ID of the review comment to reply to (number, optional). When specified, only body is required and other parameters are ignored.

- **update_pull_request** - Update an existing pull request in a GitHub repository

- `owner`: Repository owner (string, required)
Expand Down
170 changes: 170 additions & 0 deletions pkg/github/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,176 @@ func GetPullRequestComments(getClient GetClientFn, t translations.TranslationHel
}
}

// AddPullRequestReviewComment creates a tool to add a review comment to a pull request.
func AddPullRequestReviewComment(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
return mcp.NewTool("add_pull_request_review_comment",
mcp.WithDescription(t("TOOL_ADD_PULL_REQUEST_COMMENT_DESCRIPTION", "Add a review comment to 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.Required(),
mcp.Description("The text of the review comment"),
),
mcp.WithString("commit_id",
mcp.Description("The SHA of the commit to comment on. Required unless in_reply_to is specified."),
),
mcp.WithString("path",
mcp.Description("The relative path to the file that necessitates a comment. Required unless in_reply_to is specified."),
),
mcp.WithString("subject_type",
mcp.Description("The level at which the comment is targeted, 'line' or 'file'"),
mcp.Enum("line", "file"),
),
mcp.WithNumber("line",
mcp.Description("The line of the blob in the pull request diff that the comment applies to. For multi-line comments, the last line of the range"),
),
mcp.WithString("side",
mcp.Description("The side of the diff to comment on. Can be LEFT or RIGHT"),
mcp.Enum("LEFT", "RIGHT"),
),
mcp.WithNumber("start_line",
mcp.Description("For multi-line comments, the first line of the range that the comment applies to"),
),
mcp.WithString("start_side",
mcp.Description("For multi-line comments, the starting side of the diff that the comment applies to. Can be LEFT or RIGHT"),
mcp.Enum("LEFT", "RIGHT"),
),
mcp.WithNumber("in_reply_to",
mcp.Description("The ID of the review comment to reply to. When specified, only body is required and all other parameters are ignored"),
),
),
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
}
body, err := requiredParam[string](request, "body")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}

client, err := getClient(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
}

// Check if this is a reply to an existing comment
if replyToFloat, ok := request.Params.Arguments["in_reply_to"].(float64); ok {
// Use the specialized method for reply comments due to inconsistency in underlying go-github library: https://github.com/google/go-github/pull/950
commentID := int64(replyToFloat)
createdReply, resp, err := client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, commentID)
if err != nil {
return nil, fmt.Errorf("failed to reply to pull request comment: %w", err)
}
defer func() { _ = resp.Body.Close() }()

if resp.StatusCode != http.StatusCreated {
respBody, 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 reply to pull request comment: %s", string(respBody))), nil
}

r, err := json.Marshal(createdReply)
if err != nil {
return nil, fmt.Errorf("failed to marshal response: %w", err)
}

return mcp.NewToolResultText(string(r)), nil
}

// This is a new comment, not a reply
// Verify required parameters for a new comment
commitID, err := requiredParam[string](request, "commit_id")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
path, err := requiredParam[string](request, "path")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}

comment := &github.PullRequestComment{
Body: github.Ptr(body),
CommitID: github.Ptr(commitID),
Path: github.Ptr(path),
}

subjectType, err := OptionalParam[string](request, "subject_type")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
if subjectType != "file" {
line, lineExists := request.Params.Arguments["line"].(float64)
startLine, startLineExists := request.Params.Arguments["start_line"].(float64)
side, sideExists := request.Params.Arguments["side"].(string)
startSide, startSideExists := request.Params.Arguments["start_side"].(string)

if !lineExists {
return mcp.NewToolResultError("line parameter is required unless using subject_type:file"), nil
}

comment.Line = github.Ptr(int(line))
if sideExists {
comment.Side = github.Ptr(side)
}
if startLineExists {
comment.StartLine = github.Ptr(int(startLine))
}
if startSideExists {
comment.StartSide = github.Ptr(startSide)
}

if startLineExists && !lineExists {
return mcp.NewToolResultError("if start_line is provided, line must also be provided"), nil
}
if startSideExists && !sideExists {
return mcp.NewToolResultError("if start_side is provided, side must also be provided"), nil
}
}

createdComment, resp, err := client.PullRequests.CreateComment(ctx, owner, repo, pullNumber, comment)
if err != nil {
return nil, fmt.Errorf("failed to create pull request comment: %w", err)
}
defer func() { _ = resp.Body.Close() }()

if resp.StatusCode != http.StatusCreated {
respBody, 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 comment: %s", string(respBody))), nil
}

r, err := json.Marshal(createdComment)
if err != nil {
return nil, fmt.Errorf("failed to marshal response: %w", err)
}

return mcp.NewToolResultText(string(r)), nil
}
}

// GetPullRequestReviews creates a tool to get the reviews on a pull request.
func GetPullRequestReviews(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
return mcp.NewTool("get_pull_request_reviews",
Expand Down
197 changes: 197 additions & 0 deletions pkg/github/pullrequests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1719,3 +1719,200 @@ func Test_CreatePullRequest(t *testing.T) {
})
}
}

func Test_AddPullRequestReviewComment(t *testing.T) {
mockClient := github.NewClient(nil)
tool, _ := AddPullRequestReviewComment(stubGetClientFn(mockClient), translations.NullTranslationHelper)

assert.Equal(t, "add_pull_request_review_comment", 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, "commit_id")
assert.Contains(t, tool.InputSchema.Properties, "path")
// Since we've updated commit_id and path to be optional when using in_reply_to
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pull_number", "body"})

mockComment := &github.PullRequestComment{
ID: github.Ptr(int64(123)),
Body: github.Ptr("Great stuff!"),
Path: github.Ptr("file1.txt"),
Line: github.Ptr(2),
Side: github.Ptr("RIGHT"),
}

mockReply := &github.PullRequestComment{
ID: github.Ptr(int64(456)),
Body: github.Ptr("Good point, will fix!"),
}

tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]interface{}
expectError bool
expectedComment *github.PullRequestComment
expectedErrMsg string
}{
{
name: "successful line comment creation",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.PostReposPullsCommentsByOwnerByRepoByPullNumber,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusCreated)
err := json.NewEncoder(w).Encode(mockComment)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
}),
),
),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
"pull_number": float64(1),
"body": "Great stuff!",
"commit_id": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
"path": "file1.txt",
"line": float64(2),
"side": "RIGHT",
},
expectError: false,
expectedComment: mockComment,
},
{
name: "successful reply using in_reply_to",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.PostReposPullsCommentsByOwnerByRepoByPullNumber,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusCreated)
err := json.NewEncoder(w).Encode(mockReply)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
}),
),
),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
"pull_number": float64(1),
"body": "Good point, will fix!",
"in_reply_to": float64(123),
},
expectError: false,
expectedComment: mockReply,
},
{
name: "comment creation fails",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.PostReposPullsCommentsByOwnerByRepoByPullNumber,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnprocessableEntity)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"message": "Validation Failed"}`))
}),
),
),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
"pull_number": float64(1),
"body": "Great stuff!",
"commit_id": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
"path": "file1.txt",
"line": float64(2),
},
expectError: true,
expectedErrMsg: "failed to create pull request comment",
},
{
name: "reply creation fails",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.PostReposPullsCommentsByOwnerByRepoByPullNumber,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"message": "Comment not found"}`))
}),
),
),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
"pull_number": float64(1),
"body": "Good point, will fix!",
"in_reply_to": float64(999),
},
expectError: true,
expectedErrMsg: "failed to reply to pull request comment",
},
{
name: "missing required parameters for comment",
mockedClient: mock.NewMockedHTTPClient(),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
"pull_number": float64(1),
"body": "Great stuff!",
// missing commit_id and path
},
expectError: false,
expectedErrMsg: "missing required parameter: commit_id",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
mockClient := github.NewClient(tc.mockedClient)

_, handler := AddPullRequestReviewComment(stubGetClientFn(mockClient), 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)
require.Len(t, result.Content, 1)

textContent := getTextResult(t, result)
if tc.expectedErrMsg != "" {
assert.Contains(t, textContent.Text, tc.expectedErrMsg)
return
}

var returnedComment github.PullRequestComment
err = json.Unmarshal([]byte(getTextResult(t, result).Text), &returnedComment)
require.NoError(t, err)

assert.Equal(t, *tc.expectedComment.ID, *returnedComment.ID)
assert.Equal(t, *tc.expectedComment.Body, *returnedComment.Body)

// Only check Path, Line, and Side if they exist in the expected comment
if tc.expectedComment.Path != nil {
assert.Equal(t, *tc.expectedComment.Path, *returnedComment.Path)
}
if tc.expectedComment.Line != nil {
assert.Equal(t, *tc.expectedComment.Line, *returnedComment.Line)
}
if tc.expectedComment.Side != nil {
assert.Equal(t, *tc.expectedComment.Side, *returnedComment.Side)
}
})
}
}
Loading