From e8e9ede8d5745eaed96d5714bbbcc8dcdc43ab99 Mon Sep 17 00:00:00 2001
From: Ashwin Bhat <ashwin@anthropic.com>
Date: Fri, 4 Apr 2025 18:57:02 -0700
Subject: [PATCH 1/3] Add line parameter support to create_pull_request_review
tool
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
- Updated schema to make path and body the only required fields
- Added line parameter as alternative to position for inline comments
- Updated handler to accept either position or line based on GitHub API spec
- Added new test case that verifies line parameter works properly
- Updated error messages for better validation
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
---
README.md | 2 +-
pkg/github/pullrequests.go | 41 +++++++++++++++++++++------------
pkg/github/pullrequests_test.go | 39 ++++++++++++++++++++++++++++++-
3 files changed, 65 insertions(+), 17 deletions(-)
diff --git a/README.md b/README.md
index 9330723cc..3d96bccb5 100644
--- a/README.md
+++ b/README.md
@@ -266,7 +266,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
- `body`: Review comment text (string, optional)
- `event`: Review action ('APPROVE', 'REQUEST_CHANGES', 'COMMENT') (string, required)
- `commitId`: 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)
+ - `comments`: Line-specific comments array of objects, each object with path (string), either position (number) or line (number), and body (string) (array, optional)
- **create_pull_request** - Create a new pull request
diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go
index c02336ca0..3f955a842 100644
--- a/pkg/github/pullrequests.go
+++ b/pkg/github/pullrequests.go
@@ -593,7 +593,7 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
map[string]interface{}{
"type": "object",
"additionalProperties": false,
- "required": []string{"path", "position", "body"},
+ "required": []string{"path", "body"},
"properties": map[string]interface{}{
"path": map[string]interface{}{
"type": "string",
@@ -601,7 +601,11 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
},
"position": map[string]interface{}{
"type": "number",
- "description": "line number in the file",
+ "description": "position of the comment in the diff",
+ },
+ "line": map[string]interface{}{
+ "type": "number",
+ "description": "line number in the file to comment on (alternative to position)",
},
"body": map[string]interface{}{
"type": "string",
@@ -610,7 +614,7 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
},
},
),
- mcp.Description("Line-specific comments array of objects, each object with path (string), position (number), and body (string)"),
+ mcp.Description("Line-specific comments array of objects, each object with path (string), either position (number) or line (number), and body (string)"),
),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
@@ -661,7 +665,7 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
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
+ return mcp.NewToolResultError("each comment must be an object with path and body"), nil
}
path, ok := commentMap["path"].(string)
@@ -669,22 +673,29 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
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),
- })
+ comment := &github.DraftReviewComment{
+ Path: github.Ptr(path),
+ Body: github.Ptr(body),
+ }
+
+ if positionFloat, ok := commentMap["position"].(float64); ok {
+ comment.Position = github.Ptr(int(positionFloat))
+ }
+
+ if lineFloat, ok := commentMap["line"].(float64); ok {
+ comment.Line = github.Ptr(int(lineFloat))
+ }
+
+ if comment.Position == nil && comment.Line == nil {
+ return mcp.NewToolResultError("each comment must have either position or line"), nil
+ }
+
+ comments = append(comments, comment)
}
reviewRequest.Comments = comments
diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go
index b666e8a8b..c3cd261ca 100644
--- a/pkg/github/pullrequests_test.go
+++ b/pkg/github/pullrequests_test.go
@@ -1167,7 +1167,44 @@ func Test_CreatePullRequestReview(t *testing.T) {
},
},
expectError: false,
- expectedErrMsg: "each comment must have a position",
+ expectedErrMsg: "each comment must have either position or line",
+ },
+ {
+ name: "successful review creation with line parameter",
+ mockedClient: mock.NewMockedHTTPClient(
+ mock.WithRequestMatchHandler(
+ mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
+ expectRequestBody(t, map[string]interface{}{
+ "body": "Code review comments",
+ "event": "COMMENT",
+ "comments": []interface{}{
+ map[string]interface{}{
+ "path": "main.go",
+ "line": float64(42),
+ "body": "Consider adding a comment here",
+ },
+ },
+ }).andThen(
+ mockResponse(t, http.StatusOK, mockReview),
+ ),
+ ),
+ ),
+ requestArgs: map[string]interface{}{
+ "owner": "owner",
+ "repo": "repo",
+ "pullNumber": float64(42),
+ "body": "Code review comments",
+ "event": "COMMENT",
+ "comments": []interface{}{
+ map[string]interface{}{
+ "path": "main.go",
+ "line": float64(42),
+ "body": "Consider adding a comment here",
+ },
+ },
+ },
+ expectError: false,
+ expectedReview: mockReview,
},
{
name: "review creation fails",
From 0527bc552591df807ee7be0b3a79f428d2f0ed2c Mon Sep 17 00:00:00 2001
From: Ashwin Bhat <ashwin@anthropic.com>
Date: Sat, 5 Apr 2025 17:22:45 -0700
Subject: [PATCH 2/3] Expand PR review API with multi-line comment support
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
- Added new parameters: line, side, start_line, start_side
- Added proper validation for multi-line comment parameters
- Improved validation logic to handle parameter combinations
- Added test cases for regular and multi-line comments
- Updated schema documentation for better tool discoverability
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
---
README.md | 4 +-
pkg/github/pullrequests.go | 46 +++++++++++---
pkg/github/pullrequests_test.go | 108 ++++++++++++++++++++++++++++++++
3 files changed, 149 insertions(+), 9 deletions(-)
diff --git a/README.md b/README.md
index 3d96bccb5..33db66071 100644
--- a/README.md
+++ b/README.md
@@ -266,7 +266,9 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
- `body`: Review comment text (string, optional)
- `event`: Review action ('APPROVE', 'REQUEST_CHANGES', 'COMMENT') (string, required)
- `commitId`: SHA of commit to review (string, optional)
- - `comments`: Line-specific comments array of objects, each object with path (string), either position (number) or line (number), and body (string) (array, optional)
+ - `comments`: Line-specific comments array of objects to place comments on pull request changes (array, optional)
+ - For inline comments: provide `path`, `position` (or `line`), and `body`
+ - For multi-line comments: provide `path`, `start_line`, `line`, optional `side`/`start_side`, and `body`
- **create_pull_request** - Create a new pull request
diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go
index 3f955a842..a43d5b883 100644
--- a/pkg/github/pullrequests.go
+++ b/pkg/github/pullrequests.go
@@ -605,7 +605,19 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
},
"line": map[string]interface{}{
"type": "number",
- "description": "line number in the file to comment on (alternative to position)",
+ "description": "line number in the file to comment on. For multi-line comments, the end of the line range",
+ },
+ "side": map[string]interface{}{
+ "type": "string",
+ "description": "The side of the diff on which the line resides. For multi-line comments, this is the side for the end of the line range. (LEFT or RIGHT)",
+ },
+ "start_line": map[string]interface{}{
+ "type": "number",
+ "description": "The first line of the range to which the comment refers. Required for multi-line comments.",
+ },
+ "start_side": map[string]interface{}{
+ "type": "string",
+ "description": "The side of the diff on which the start line resides for multi-line comments. (LEFT or RIGHT)",
},
"body": map[string]interface{}{
"type": "string",
@@ -614,7 +626,7 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
},
},
),
- mcp.Description("Line-specific comments array of objects, each object with path (string), either position (number) or line (number), and body (string)"),
+ mcp.Description("Line-specific comments array of objects to place comments on pull request changes. Requires path and body. For line comments use line or position. For multi-line comments use start_line and line with optional side parameters."),
),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
@@ -678,6 +690,21 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
return mcp.NewToolResultError("each comment must have a body"), nil
}
+ _, hasPosition := commentMap["position"].(float64)
+ _, hasLine := commentMap["line"].(float64)
+ _, hasSide := commentMap["side"].(string)
+ _, hasStartLine := commentMap["start_line"].(float64)
+ _, hasStartSide := commentMap["start_side"].(string)
+
+ switch {
+ case !hasPosition && !hasLine:
+ return mcp.NewToolResultError("each comment must have either position or line"), nil
+ case hasPosition && (hasLine || hasSide || hasStartLine || hasStartSide):
+ return mcp.NewToolResultError("position cannot be combined with line, side, start_line, or start_side"), nil
+ case hasStartSide && !hasSide:
+ return mcp.NewToolResultError("if start_side is provided, side must also be provided"), nil
+ }
+
comment := &github.DraftReviewComment{
Path: github.Ptr(path),
Body: github.Ptr(body),
@@ -685,14 +712,17 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
if positionFloat, ok := commentMap["position"].(float64); ok {
comment.Position = github.Ptr(int(positionFloat))
- }
-
- if lineFloat, ok := commentMap["line"].(float64); ok {
+ } else if lineFloat, ok := commentMap["line"].(float64); ok {
comment.Line = github.Ptr(int(lineFloat))
}
-
- if comment.Position == nil && comment.Line == nil {
- return mcp.NewToolResultError("each comment must have either position or line"), nil
+ if side, ok := commentMap["side"].(string); ok {
+ comment.Side = github.Ptr(side)
+ }
+ if startLineFloat, ok := commentMap["start_line"].(float64); ok {
+ comment.StartLine = github.Ptr(int(startLineFloat))
+ }
+ if startSide, ok := commentMap["start_side"].(string); ok {
+ comment.StartSide = github.Ptr(startSide)
}
comments = append(comments, comment)
diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go
index c3cd261ca..3b9d27923 100644
--- a/pkg/github/pullrequests_test.go
+++ b/pkg/github/pullrequests_test.go
@@ -1206,6 +1206,114 @@ func Test_CreatePullRequestReview(t *testing.T) {
expectError: false,
expectedReview: mockReview,
},
+ {
+ name: "successful review creation with multi-line comment",
+ mockedClient: mock.NewMockedHTTPClient(
+ mock.WithRequestMatchHandler(
+ mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
+ expectRequestBody(t, map[string]interface{}{
+ "body": "Multi-line comment review",
+ "event": "COMMENT",
+ "comments": []interface{}{
+ map[string]interface{}{
+ "path": "main.go",
+ "start_line": float64(10),
+ "line": float64(15),
+ "side": "RIGHT",
+ "body": "This entire block needs refactoring",
+ },
+ },
+ }).andThen(
+ mockResponse(t, http.StatusOK, mockReview),
+ ),
+ ),
+ ),
+ requestArgs: map[string]interface{}{
+ "owner": "owner",
+ "repo": "repo",
+ "pullNumber": float64(42),
+ "body": "Multi-line comment review",
+ "event": "COMMENT",
+ "comments": []interface{}{
+ map[string]interface{}{
+ "path": "main.go",
+ "start_line": float64(10),
+ "line": float64(15),
+ "side": "RIGHT",
+ "body": "This entire block needs refactoring",
+ },
+ },
+ },
+ expectError: false,
+ expectedReview: mockReview,
+ },
+ {
+ name: "invalid multi-line comment - missing line parameter",
+ mockedClient: mock.NewMockedHTTPClient(),
+ requestArgs: map[string]interface{}{
+ "owner": "owner",
+ "repo": "repo",
+ "pullNumber": float64(42),
+ "event": "COMMENT",
+ "comments": []interface{}{
+ map[string]interface{}{
+ "path": "main.go",
+ "start_line": float64(10),
+ // missing line parameter
+ "body": "Invalid multi-line comment",
+ },
+ },
+ },
+ expectError: false,
+ expectedErrMsg: "each comment must have either position or line", // Updated error message
+ },
+ {
+ name: "invalid comment - mixing position with line parameters",
+ mockedClient: mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
+ mockReview,
+ ),
+ ),
+ requestArgs: map[string]interface{}{
+ "owner": "owner",
+ "repo": "repo",
+ "pullNumber": float64(42),
+ "event": "COMMENT",
+ "comments": []interface{}{
+ map[string]interface{}{
+ "path": "main.go",
+ "position": float64(5),
+ "line": float64(42),
+ "body": "Invalid parameter combination",
+ },
+ },
+ },
+ expectError: false,
+ expectedErrMsg: "position cannot be combined with line, side, start_line, or start_side",
+ },
+ {
+ name: "invalid multi-line comment - missing side parameter",
+ mockedClient: mock.NewMockedHTTPClient(),
+ requestArgs: map[string]interface{}{
+ "owner": "owner",
+ "repo": "repo",
+ "pullNumber": float64(42),
+ "event": "COMMENT",
+ "comments": []interface{}{
+ map[string]interface{}{
+ "path": "main.go",
+ "start_line": float64(10),
+ "line": float64(15),
+ "start_side": "LEFT",
+ // missing side parameter
+ "body": "Invalid multi-line comment",
+ },
+ },
+ },
+ expectError: false,
+ expectedErrMsg: "if start_side is provided, side must also be provided",
+ },
{
name: "review creation fails",
mockedClient: mock.NewMockedHTTPClient(
From 72d49953b0ac0b9440dd58aa05d3d9b2ce4b3ed1 Mon Sep 17 00:00:00 2001
From: Ashwin Bhat <ashwin@anthropic.com>
Date: Sun, 6 Apr 2025 13:41:05 -0700
Subject: [PATCH 3/3] gofmt
---
pkg/github/pullrequests_test.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go
index 3b9d27923..9e2e9f478 100644
--- a/pkg/github/pullrequests_test.go
+++ b/pkg/github/pullrequests_test.go
@@ -1248,7 +1248,7 @@ func Test_CreatePullRequestReview(t *testing.T) {
expectedReview: mockReview,
},
{
- name: "invalid multi-line comment - missing line parameter",
+ name: "invalid multi-line comment - missing line parameter",
mockedClient: mock.NewMockedHTTPClient(),
requestArgs: map[string]interface{}{
"owner": "owner",
@@ -1293,7 +1293,7 @@ func Test_CreatePullRequestReview(t *testing.T) {
expectedErrMsg: "position cannot be combined with line, side, start_line, or start_side",
},
{
- name: "invalid multi-line comment - missing side parameter",
+ name: "invalid multi-line comment - missing side parameter",
mockedClient: mock.NewMockedHTTPClient(),
requestArgs: map[string]interface{}{
"owner": "owner",