From 8b41e0890b1a086c88bccc580425a298e4f58b49 Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Fri, 18 Apr 2025 20:02:16 -0700 Subject: [PATCH 1/3] feat: add UpdatePullRequestComment tool to edit PR review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- pkg/github/pullrequests.go | 70 ++++++++++++++++++++ pkg/github/pullrequests_test.go | 110 ++++++++++++++++++++++++++++++++ pkg/github/tools.go | 2 + 3 files changed, 182 insertions(+) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index abdf6448..dc65bbd9 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -959,6 +959,76 @@ func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translation } } +// UpdatePullRequestComment creates a tool to update a review comment on a pull request. +func UpdatePullRequestComment(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("update_pull_request_comment", + mcp.WithDescription(t("TOOL_UPDATE_PULL_REQUEST_COMMENT_DESCRIPTION", "Update a review comment on a pull request")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("commentId", + mcp.Required(), + mcp.Description("Comment ID to update"), + ), + mcp.WithString("body", + mcp.Required(), + mcp.Description("The new text for the comment"), + ), + ), + 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 + } + commentID, err := RequiredInt(request, "commentId") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + body, err := requiredParam[string](request, "body") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + comment := &github.PullRequestComment{ + Body: github.Ptr(body), + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + updatedComment, resp, err := client.PullRequests.EditComment(ctx, owner, repo, int64(commentID), comment) + if err != nil { + return nil, fmt.Errorf("failed to update pull request comment: %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 update pull request comment: %s", string(body))), nil + } + + r, err := json.Marshal(updatedComment) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // CreatePendingPullRequestReview creates a tool to create a pending review on a pull request. func CreatePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("create_pending_pull_request_review", diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 6202ec16..88110e35 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1655,6 +1655,116 @@ func Test_RequestCopilotReview(t *testing.T) { } } +func Test_UpdatePullRequestComment(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := UpdatePullRequestComment(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "update_pull_request_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, "commentId") + assert.Contains(t, tool.InputSchema.Properties, "body") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "commentId", "body"}) + + // Setup mock comment for success case + mockUpdatedComment := &github.PullRequestComment{ + ID: github.Ptr(int64(456)), + Body: github.Ptr("Updated comment text here"), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/1#discussion_r456"), + Path: github.Ptr("file1.txt"), + UpdatedAt: &github.Timestamp{Time: time.Now()}, + User: &github.User{ + Login: github.Ptr("testuser"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedComment *github.PullRequestComment + expectedErrMsg string + }{ + { + name: "successful update", + mockedClient: httpMock( + NewJSONResponder(200, mockUpdatedComment), + ), + requestArgs: map[string]interface{}{ + "owner": "testowner", + "repo": "testrepo", + "commentId": float64(456), + "body": "Updated comment text here", + }, + expectError: false, + expectedComment: mockUpdatedComment, + }, + { + name: "missing required parameters", + mockedClient: httpMock( + NewJSONResponder(200, mockUpdatedComment), + ), + requestArgs: map[string]interface{}{ + "owner": "testowner", + "repo": "testrepo", + // Missing commentId and body + }, + expectError: true, + expectedErrMsg: "commentId is required", + }, + { + name: "http error", + mockedClient: httpMock( + NewStringResponder(400, "Bad Request"), + ), + requestArgs: map[string]interface{}{ + "owner": "testowner", + "repo": "testrepo", + "commentId": float64(456), + "body": "Invalid body", // Changed this to a non-empty string + }, + expectError: true, + expectedErrMsg: "failed to update pull request comment", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + _, handler := UpdatePullRequestComment(stubGetClientFn(client), translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + + if tc.expectError { + require.True(t, result.IsError) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + // Parse the result for success case + require.False(t, result.IsError) + + var returnedComment *github.PullRequestComment + err = json.Unmarshal([]byte(textContent.Text), &returnedComment) + require.NoError(t, err) + + // Verify comment details + assert.Equal(t, *tc.expectedComment.ID, *returnedComment.ID) + assert.Equal(t, *tc.expectedComment.Body, *returnedComment.Body) + assert.Equal(t, *tc.expectedComment.HTMLURL, *returnedComment.HTMLURL) + }) + } +} + func TestCreatePendingPullRequestReview(t *testing.T) { t.Parallel() diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 9c1ab34a..50310649 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -73,6 +73,8 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, toolsets.NewServerTool(CreatePullRequest(getClient, t)), toolsets.NewServerTool(UpdatePullRequest(getClient, t)), toolsets.NewServerTool(RequestCopilotReview(getClient, t)), + toolsets.NewServerTool(AddPullRequestReviewComment(getClient, t)), + toolsets.NewServerTool(UpdatePullRequestComment(getClient, t)), // Reviews toolsets.NewServerTool(CreateAndSubmitPullRequestReview(getGQLClient, t)), From d875c744ffc10d1e2b9b348eb3ee3d5edb956edc Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Fri, 18 Apr 2025 20:11:35 -0700 Subject: [PATCH 2/3] feat: add UpdateIssueComment tool to edit issue comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- pkg/github/issues.go | 70 +++++++++++++++++ pkg/github/issues_test.go | 135 +++++++++++++++++++++++++++++++- pkg/github/pullrequests_test.go | 49 +++++++++--- pkg/github/tools.go | 2 +- 4 files changed, 242 insertions(+), 14 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 07c76078..ae66c944 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -153,6 +153,76 @@ func AddIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc } } +// UpdateIssueComment creates a tool to update a comment on an issue. +func UpdateIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("update_issue_comment", + mcp.WithDescription(t("TOOL_UPDATE_ISSUE_COMMENT_DESCRIPTION", "Update a comment on an issue")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("commentId", + mcp.Required(), + mcp.Description("Comment ID to update"), + ), + mcp.WithString("body", + mcp.Required(), + mcp.Description("The new text for the comment"), + ), + ), + 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 + } + commentID, err := RequiredInt(request, "commentId") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + body, err := requiredParam[string](request, "body") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + comment := &github.IssueComment{ + Body: github.Ptr(body), + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + updatedComment, resp, err := client.Issues.EditComment(ctx, owner, repo, int64(commentID), comment) + if err != nil { + return nil, fmt.Errorf("failed to update issue comment: %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 update issue comment: %s", string(body))), nil + } + + r, err := json.Marshal(updatedComment) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // SearchIssues creates a tool to search for issues and pull requests. func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_issues", diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index cd715de6..2620a209 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1123,6 +1123,140 @@ func Test_GetIssueComments(t *testing.T) { } } +func Test_UpdateIssueComment(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := UpdateIssueComment(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "update_issue_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, "commentId") + assert.Contains(t, tool.InputSchema.Properties, "body") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "commentId", "body"}) + + // Setup mock comment for success case + mockUpdatedComment := &github.IssueComment{ + ID: github.Ptr(int64(789)), + Body: github.Ptr("Updated issue comment text"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/1#issuecomment-789"), + UpdatedAt: &github.Timestamp{Time: time.Now()}, + User: &github.User{ + Login: github.Ptr("testuser"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedComment *github.IssueComment + expectedErrMsg string + }{ + { + name: "successful update", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesCommentsByOwnerByRepoByCommentId, + expectRequestBody(t, map[string]any{ + "body": "Updated issue comment text", + }).andThen( + mockResponse(t, http.StatusOK, mockUpdatedComment), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "testowner", + "repo": "testrepo", + "commentId": float64(789), + "body": "Updated issue comment text", + }, + expectError: false, + expectedComment: mockUpdatedComment, + }, + { + name: "missing required parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.PatchReposIssuesCommentsByOwnerByRepoByCommentId, + mockUpdatedComment, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "testowner", + "repo": "testrepo", + // Missing commentId and body + }, + expectError: true, + expectedErrMsg: "missing required parameter: commentId", + }, + { + name: "http error", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesCommentsByOwnerByRepoByCommentId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "testowner", + "repo": "testrepo", + "commentId": float64(789), + "body": "New comment text", + }, + expectError: true, + expectedErrMsg: "failed to update issue comment", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + _, handler := UpdateIssueComment(stubGetClientFn(client), translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + if tc.expectError { + if err != nil { + // For HTTP errors, the handler returns an error + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + } else { + // For validation errors, the handler returns a result with IsError=true + require.NoError(t, err) + textContent := getTextResult(t, result) + require.True(t, result.IsError) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + } + return + } + + require.NoError(t, err) + textContent := getTextResult(t, result) + + // Parse the result for success case + require.False(t, result.IsError) + + var returnedComment *github.IssueComment + err = json.Unmarshal([]byte(textContent.Text), &returnedComment) + require.NoError(t, err) + + // Verify comment details + assert.Equal(t, *tc.expectedComment.ID, *returnedComment.ID) + assert.Equal(t, *tc.expectedComment.Body, *returnedComment.Body) + assert.Equal(t, *tc.expectedComment.HTMLURL, *returnedComment.HTMLURL) + }) + } +} + func TestAssignCopilotToIssue(t *testing.T) { t.Parallel() @@ -1515,7 +1649,6 @@ func TestAssignCopilotToIssue(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - t.Parallel() // Setup client with mock client := githubv4.NewClient(tc.mockedClient) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 88110e35..e3ae3b99 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1690,8 +1690,15 @@ func Test_UpdatePullRequestComment(t *testing.T) { }{ { name: "successful update", - mockedClient: httpMock( - NewJSONResponder(200, mockUpdatedComment), + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposPullsCommentsByOwnerByRepoByCommentId, + expectRequestBody(t, map[string]any{ + "body": "Updated comment text here", + }).andThen( + mockResponse(t, http.StatusOK, mockUpdatedComment), + ), + ), ), requestArgs: map[string]interface{}{ "owner": "testowner", @@ -1704,8 +1711,11 @@ func Test_UpdatePullRequestComment(t *testing.T) { }, { name: "missing required parameters", - mockedClient: httpMock( - NewJSONResponder(200, mockUpdatedComment), + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.PatchReposPullsCommentsByOwnerByRepoByCommentId, + mockUpdatedComment, + ), ), requestArgs: map[string]interface{}{ "owner": "testowner", @@ -1713,12 +1723,18 @@ func Test_UpdatePullRequestComment(t *testing.T) { // Missing commentId and body }, expectError: true, - expectedErrMsg: "commentId is required", + expectedErrMsg: "missing required parameter: commentId", }, { name: "http error", - mockedClient: httpMock( - NewStringResponder(400, "Bad Request"), + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposPullsCommentsByOwnerByRepoByCommentId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message": "Bad Request"}`)) + }), + ), ), requestArgs: map[string]interface{}{ "owner": "testowner", @@ -1740,16 +1756,25 @@ func Test_UpdatePullRequestComment(t *testing.T) { // Call handler result, err := handler(context.Background(), request) - require.NoError(t, err) - - textContent := getTextResult(t, result) if tc.expectError { - require.True(t, result.IsError) - assert.Contains(t, textContent.Text, tc.expectedErrMsg) + if err != nil { + // For HTTP errors, the handler returns an error + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + } else { + // For validation errors, the handler returns a result with IsError=true + require.NoError(t, err) + textContent := getTextResult(t, result) + require.True(t, result.IsError) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + } return } + require.NoError(t, err) + textContent := getTextResult(t, result) + // Parse the result for success case require.False(t, result.IsError) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 50310649..ce1c0885 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -52,6 +52,7 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, toolsets.NewServerTool(AddIssueComment(getClient, t)), toolsets.NewServerTool(UpdateIssue(getClient, t)), toolsets.NewServerTool(AssignCopilotToIssue(getGQLClient, t)), + toolsets.NewServerTool(UpdateIssueComment(getClient, t)), ) users := toolsets.NewToolset("users", "GitHub User related tools"). AddReadTools( @@ -73,7 +74,6 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, toolsets.NewServerTool(CreatePullRequest(getClient, t)), toolsets.NewServerTool(UpdatePullRequest(getClient, t)), toolsets.NewServerTool(RequestCopilotReview(getClient, t)), - toolsets.NewServerTool(AddPullRequestReviewComment(getClient, t)), toolsets.NewServerTool(UpdatePullRequestComment(getClient, t)), // Reviews From 3670d147239ae02de3c3980dcd847e1a914273bb Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Tue, 29 Apr 2025 08:47:54 -0700 Subject: [PATCH 3/3] add annotations --- pkg/github/issues.go | 4 ++++ pkg/github/pullrequests.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index ae66c944..edab78a4 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -157,6 +157,10 @@ func AddIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc func UpdateIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("update_issue_comment", mcp.WithDescription(t("TOOL_UPDATE_ISSUE_COMMENT_DESCRIPTION", "Update a comment on an issue")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_UPDATE_ISSUE_COMMENT_USER_TITLE", "Update issue comment"), + ReadOnlyHint: toBoolPtr(false), + }), mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner"), diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index dc65bbd9..9684207d 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -963,6 +963,10 @@ func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translation func UpdatePullRequestComment(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("update_pull_request_comment", mcp.WithDescription(t("TOOL_UPDATE_PULL_REQUEST_COMMENT_DESCRIPTION", "Update a review comment on a pull request")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_UPDATE_PULL_REQUEST_COMMENT_USER_TITLE", "Update pull request comment"), + ReadOnlyHint: toBoolPtr(false), + }), mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner"),