Skip to content

Commit e7ac2c9

Browse files
ashwin-antclaude
ashwin-ant
and
claude
committed
Expand PR review API with multi-line comment support
- 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 <[email protected]>
1 parent e8e9ede commit e7ac2c9

File tree

2 files changed

+146
-8
lines changed

2 files changed

+146
-8
lines changed

‎pkg/github/pullrequests.go‎

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,19 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
605605
},
606606
"line": map[string]interface{}{
607607
"type": "number",
608-
"description": "line number in the file to comment on (alternative to position)",
608+
"description": "line number in the file to comment on. For multi-line comments, the end of the line range",
609+
},
610+
"side": map[string]interface{}{
611+
"type": "string",
612+
"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)",
613+
},
614+
"start_line": map[string]interface{}{
615+
"type": "number",
616+
"description": "The first line of the range to which the comment refers. Required for multi-line comments.",
617+
},
618+
"start_side": map[string]interface{}{
619+
"type": "string",
620+
"description": "The side of the diff on which the start line resides for multi-line comments. (LEFT or RIGHT)",
609621
},
610622
"body": map[string]interface{}{
611623
"type": "string",
@@ -614,7 +626,7 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
614626
},
615627
},
616628
),
617-
mcp.Description("Line-specific comments array of objects, each object with path (string), either position (number) or line (number), and body (string)"),
629+
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."),
618630
),
619631
),
620632
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
@@ -678,21 +690,39 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe
678690
return mcp.NewToolResultError("each comment must have a body"), nil
679691
}
680692

693+
_, hasPosition := commentMap["position"].(float64)
694+
_, hasLine := commentMap["line"].(float64)
695+
_, hasSide := commentMap["side"].(string)
696+
_, hasStartLine := commentMap["start_line"].(float64)
697+
_, hasStartSide := commentMap["start_side"].(string)
698+
699+
switch {
700+
case !hasPosition && !hasLine:
701+
return mcp.NewToolResultError("each comment must have either position or line"), nil
702+
case hasPosition && (hasLine || hasSide || hasStartLine || hasStartSide):
703+
return mcp.NewToolResultError("position cannot be combined with line, side, start_line, or start_side"), nil
704+
case hasStartSide && !hasSide:
705+
return mcp.NewToolResultError("if start_side is provided, side must also be provided"), nil
706+
}
707+
681708
comment := &github.DraftReviewComment{
682709
Path: github.Ptr(path),
683710
Body: github.Ptr(body),
684711
}
685712

686713
if positionFloat, ok := commentMap["position"].(float64); ok {
687714
comment.Position = github.Ptr(int(positionFloat))
688-
}
689-
690-
if lineFloat, ok := commentMap["line"].(float64); ok {
715+
} else if lineFloat, ok := commentMap["line"].(float64); ok {
691716
comment.Line = github.Ptr(int(lineFloat))
692717
}
693-
694-
if comment.Position == nil && comment.Line == nil {
695-
return mcp.NewToolResultError("each comment must have either position or line"), nil
718+
if side, ok := commentMap["side"].(string); ok {
719+
comment.Side = github.Ptr(side)
720+
}
721+
if startLineFloat, ok := commentMap["start_line"].(float64); ok {
722+
comment.StartLine = github.Ptr(int(startLineFloat))
723+
}
724+
if startSide, ok := commentMap["start_side"].(string); ok {
725+
comment.StartSide = github.Ptr(startSide)
696726
}
697727

698728
comments = append(comments, comment)

‎pkg/github/pullrequests_test.go‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,6 +1206,114 @@ func Test_CreatePullRequestReview(t *testing.T) {
12061206
expectError: false,
12071207
expectedReview: mockReview,
12081208
},
1209+
{
1210+
name: "successful review creation with multi-line comment",
1211+
mockedClient: mock.NewMockedHTTPClient(
1212+
mock.WithRequestMatchHandler(
1213+
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
1214+
expectRequestBody(t, map[string]interface{}{
1215+
"body": "Multi-line comment review",
1216+
"event": "COMMENT",
1217+
"comments": []interface{}{
1218+
map[string]interface{}{
1219+
"path": "main.go",
1220+
"start_line": float64(10),
1221+
"line": float64(15),
1222+
"side": "RIGHT",
1223+
"body": "This entire block needs refactoring",
1224+
},
1225+
},
1226+
}).andThen(
1227+
mockResponse(t, http.StatusOK, mockReview),
1228+
),
1229+
),
1230+
),
1231+
requestArgs: map[string]interface{}{
1232+
"owner": "owner",
1233+
"repo": "repo",
1234+
"pullNumber": float64(42),
1235+
"body": "Multi-line comment review",
1236+
"event": "COMMENT",
1237+
"comments": []interface{}{
1238+
map[string]interface{}{
1239+
"path": "main.go",
1240+
"start_line": float64(10),
1241+
"line": float64(15),
1242+
"side": "RIGHT",
1243+
"body": "This entire block needs refactoring",
1244+
},
1245+
},
1246+
},
1247+
expectError: false,
1248+
expectedReview: mockReview,
1249+
},
1250+
{
1251+
name: "invalid multi-line comment - missing line parameter",
1252+
mockedClient: mock.NewMockedHTTPClient(),
1253+
requestArgs: map[string]interface{}{
1254+
"owner": "owner",
1255+
"repo": "repo",
1256+
"pullNumber": float64(42),
1257+
"event": "COMMENT",
1258+
"comments": []interface{}{
1259+
map[string]interface{}{
1260+
"path": "main.go",
1261+
"start_line": float64(10),
1262+
// missing line parameter
1263+
"body": "Invalid multi-line comment",
1264+
},
1265+
},
1266+
},
1267+
expectError: false,
1268+
expectedErrMsg: "each comment must have either position or line", // Updated error message
1269+
},
1270+
{
1271+
name: "invalid comment - mixing position with line parameters",
1272+
mockedClient: mock.NewMockedHTTPClient(
1273+
mock.WithRequestMatch(
1274+
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
1275+
mockReview,
1276+
),
1277+
),
1278+
requestArgs: map[string]interface{}{
1279+
"owner": "owner",
1280+
"repo": "repo",
1281+
"pullNumber": float64(42),
1282+
"event": "COMMENT",
1283+
"comments": []interface{}{
1284+
map[string]interface{}{
1285+
"path": "main.go",
1286+
"position": float64(5),
1287+
"line": float64(42),
1288+
"body": "Invalid parameter combination",
1289+
},
1290+
},
1291+
},
1292+
expectError: false,
1293+
expectedErrMsg: "position cannot be combined with line, side, start_line, or start_side",
1294+
},
1295+
{
1296+
name: "invalid multi-line comment - missing side parameter",
1297+
mockedClient: mock.NewMockedHTTPClient(),
1298+
requestArgs: map[string]interface{}{
1299+
"owner": "owner",
1300+
"repo": "repo",
1301+
"pullNumber": float64(42),
1302+
"event": "COMMENT",
1303+
"comments": []interface{}{
1304+
map[string]interface{}{
1305+
"path": "main.go",
1306+
"start_line": float64(10),
1307+
"line": float64(15),
1308+
"start_side": "LEFT",
1309+
// missing side parameter
1310+
"body": "Invalid multi-line comment",
1311+
},
1312+
},
1313+
},
1314+
expectError: false,
1315+
expectedErrMsg: "if start_side is provided, side must also be provided",
1316+
},
12091317
{
12101318
name: "review creation fails",
12111319
mockedClient: mock.NewMockedHTTPClient(

0 commit comments

Comments
 (0)