From 258e754869bcef5a2111a1db58a9b47523afa6d7 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Tue, 15 Sep 2026 09:11:09 -0700 Subject: [PATCH 1/3] feat: add update issue comment tool Add an issues tool for replacing the body of an existing issue or pull request comment, with schema, scope, behavioral, snapshot, and generated documentation coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9acf9c83-49aa-41af-a42d-ad75de34d132 --- README.md | 7 + .../__toolsnaps__/update_issue_comment.snap | 38 ++++ pkg/github/helper_test.go | 1 + pkg/github/issues.go | 83 ++++++++ pkg/github/issues_test.go | 178 ++++++++++++++++++ pkg/github/public_repo_scopes_test.go | 2 + pkg/github/tools.go | 1 + 7 files changed, 310 insertions(+) create mode 100644 pkg/github/__toolsnaps__/update_issue_comment.snap diff --git a/README.md b/README.md index 5b90f64a58..cb7c8539ce 100644 --- a/README.md +++ b/README.md @@ -1057,6 +1057,13 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - `sub_issue_id`: The ID of the sub-issue to add. ID is not the same as issue number (number, required) +- **update_issue_comment** - Update issue comment + - **OAuth Challenge Scopes**: `repo` + - `body`: New comment content (string, required) + - `comment_id`: The numeric ID of the issue or pull request comment to update (integer, required) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) +
diff --git a/pkg/github/__toolsnaps__/update_issue_comment.snap b/pkg/github/__toolsnaps__/update_issue_comment.snap new file mode 100644 index 0000000000..33dcb56bc5 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_comment.snap @@ -0,0 +1,38 @@ +{ + "annotations": { + "idempotentHint": false, + "readOnlyHint": false, + "title": "Update issue comment" + }, + "description": "Update the body of an existing issue or pull request comment.", + "inputSchema": { + "properties": { + "body": { + "description": "New comment content", + "minLength": 1, + "type": "string" + }, + "comment_id": { + "description": "The numeric ID of the issue or pull request comment to update", + "minimum": 1, + "type": "integer" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "body" + ], + "type": "object" + }, + "name": "update_issue_comment" +} \ No newline at end of file diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 5fc541d45d..f982519f01 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -64,6 +64,7 @@ const ( GetReposIssuesCommentsByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/comments" PostReposIssuesByOwnerByRepo = "POST /repos/{owner}/{repo}/issues" PostReposIssuesCommentsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/comments" + PatchReposIssuesCommentByOwnerByRepoByCommentID = "PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}" PostReposIssuesReactionsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/reactions" PatchReposIssuesByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}" GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" diff --git a/pkg/github/issues.go b/pkg/github/issues.go index cc8bc599a1..914eccba86 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1552,6 +1552,89 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }) } +// UpdateIssueComment creates a tool to update an issue or pull request comment. +func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "update_issue_comment", + Description: t("TOOL_UPDATE_ISSUE_COMMENT_DESCRIPTION", "Update the body of an existing issue or pull request comment."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_UPDATE_ISSUE_COMMENT_USER_TITLE", "Update issue comment"), + ReadOnlyHint: false, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "comment_id": { + Type: "integer", + Description: "The numeric ID of the issue or pull request comment to update", + Minimum: jsonschema.Ptr(1.0), + }, + "body": { + Type: "string", + Description: "New comment content", + MinLength: jsonschema.Ptr(1), + }, + }, + Required: []string{"owner", "repo", "comment_id", "body"}, + }, + }, + publicRepositoryWriteScopeAccess(), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + commentID, err := RequiredBigInt(args, "comment_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if commentID < 1 { + return utils.NewToolResultError("comment_id must be greater than 0"), nil, nil + } + body, err := RequiredParam[string](args, "body") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + updatedComment, resp, err := client.Issues.EditComment(ctx, owner, repo, commentID, &github.IssueComment{ + Body: github.Ptr(body), + }) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue comment", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", updatedComment.GetID()), + URL: updatedComment.GetHTMLURL(), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + + return utils.NewToolResultText(string(r)), nil, nil + }) +} + func isValidIssueReaction(reaction string) bool { switch reaction { case "+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes": diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 83bedc5b54..c2666158b9 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -6639,6 +6639,184 @@ func TestAddIssueCommentHandler(t *testing.T) { } } +func TestUpdateIssueCommentSchema(t *testing.T) { + t.Parallel() + + tool := UpdateIssueComment(translations.NullTranslationHelper).Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "update_issue_comment", tool.Name) + assert.NotEmpty(t, tool.Description) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "comment_id") + assert.Contains(t, schema.Properties, "body") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "comment_id", "body"}) + + resolved, err := schema.Resolve(nil) + require.NoError(t, err) + + baseArgs := map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": 456, + "body": "Updated comment", + } + tests := []struct { + name string + args map[string]any + isValid bool + }{ + { + name: "valid arguments", + args: map[string]any{}, + isValid: true, + }, + { + name: "missing required body", + args: map[string]any{"body": nil}, + isValid: false, + }, + { + name: "empty body", + args: map[string]any{"body": ""}, + isValid: false, + }, + { + name: "zero comment ID", + args: map[string]any{"comment_id": 0}, + isValid: false, + }, + { + name: "fractional comment ID", + args: map[string]any{"comment_id": 1.5}, + isValid: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + args := maps.Clone(baseArgs) + maps.Copy(args, tc.args) + err := resolved.Validate(args) + if tc.isValid { + require.NoError(t, err) + return + } + require.Error(t, err) + }) + } +} + +func TestUpdateIssueCommentHandler(t *testing.T) { + t.Parallel() + + updatedComment := &github.IssueComment{ + ID: github.Ptr(int64(456)), + Body: github.Ptr("Updated comment"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42#issuecomment-456"), + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectToolError bool + expectedToolErrMsg string + }{ + { + name: "successful update", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesCommentByOwnerByRepoByCommentID: expectRequestBody(t, map[string]any{ + "body": "Updated comment", + }).andThen(mockResponse(t, http.StatusOK, updatedComment)), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(456), + "body": "Updated comment", + }, + }, + { + name: "missing body", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(456), + }, + expectToolError: true, + expectedToolErrMsg: "missing required parameter: body", + }, + { + name: "negative comment ID", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(-1), + "body": "Updated comment", + }, + expectToolError: true, + expectedToolErrMsg: "comment_id must be greater than 0", + }, + { + name: "fractional comment ID", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(1.5), + "body": "Updated comment", + }, + expectToolError: true, + expectedToolErrMsg: "parameter comment_id is not a valid number", + }, + { + name: "API error", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesCommentByOwnerByRepoByCommentID: mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(456), + "body": "Updated comment", + }, + expectToolError: true, + expectedToolErrMsg: "failed to update issue comment", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + serverTool := UpdateIssueComment(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + if tc.expectToolError { + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, tc.expectedToolErrMsg) + return + } + + require.False(t, result.IsError) + var response MinimalResponse + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, "456", response.ID) + assert.Equal(t, "https://github.com/owner/repo/issues/42#issuecomment-456", response.URL) + }) + } +} + func Test_RemoveSubIssue(t *testing.T) { // Verify tool definition once serverTool := SubIssueWrite(translations.NullTranslationHelper) diff --git a/pkg/github/public_repo_scopes_test.go b/pkg/github/public_repo_scopes_test.go index 754ca93d78..305410a433 100644 --- a/pkg/github/public_repo_scopes_test.go +++ b/pkg/github/public_repo_scopes_test.go @@ -22,6 +22,7 @@ func TestPublicRepoContributionToolScopeAccess(t *testing.T) { {name: "create_pull_request", tool: CreatePullRequest(translations.NullTranslationHelper)}, {name: "issue_write", tool: IssueWrite(translations.NullTranslationHelper)}, {name: "add_issue_comment", tool: AddIssueComment(translations.NullTranslationHelper)}, + {name: "update_issue_comment", tool: UpdateIssueComment(translations.NullTranslationHelper)}, } for _, tt := range tools { @@ -50,6 +51,7 @@ func TestPublicRepoContributionToolsVisibleToPATs(t *testing.T) { CreatePullRequest(translations.NullTranslationHelper), IssueWrite(translations.NullTranslationHelper), AddIssueComment(translations.NullTranslationHelper), + UpdateIssueComment(translations.NullTranslationHelper), } tests := []struct { diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 6764edfc26..6649743727 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -259,6 +259,7 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent ListIssueFields(t), IssueWrite(t), AddIssueComment(t), + UpdateIssueComment(t), SubIssueWrite(t), IssueDependencyRead(t), IssueDependencyWrite(t), From 4689e9cfc93a21cd5e0c5775121fb9e49e2b8bcd Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Tue, 15 Sep 2026 09:28:53 -0700 Subject: [PATCH 2/3] fix: close update comment responses on errors Register nil-safe response body cleanup before handling go-github errors from issue comment updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9acf9c83-49aa-41af-a42d-ad75de34d132 --- pkg/github/issues.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 914eccba86..3d6767e771 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1618,10 +1618,12 @@ func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTo updatedComment, resp, err := client.Issues.EditComment(ctx, owner, repo, commentID, &github.IssueComment{ Body: github.Ptr(body), }) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue comment", resp, err), nil, nil } - defer func() { _ = resp.Body.Close() }() r, err := json.Marshal(MinimalResponse{ ID: fmt.Sprintf("%d", updatedComment.GetID()), From e15181e3943a3c5aeab8a79938e7cd69be4c181c Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Tue, 15 Sep 2026 09:46:16 -0700 Subject: [PATCH 3/3] fix: clarify issue comment update contract Reject explicitly empty comment bodies at runtime and distinguish issue and pull request conversation comments from pull request review comments in the tool schema and generated docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9acf9c83-49aa-41af-a42d-ad75de34d132 --- README.md | 2 +- pkg/github/__toolsnaps__/update_issue_comment.snap | 4 ++-- pkg/github/issues.go | 14 ++++++++++---- pkg/github/issues_test.go | 11 +++++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index cb7c8539ce..d013303d81 100644 --- a/README.md +++ b/README.md @@ -1060,7 +1060,7 @@ The following sets of tools are available: - **update_issue_comment** - Update issue comment - **OAuth Challenge Scopes**: `repo` - `body`: New comment content (string, required) - - `comment_id`: The numeric ID of the issue or pull request comment to update (integer, required) + - `comment_id`: The numeric ID of the issue or pull request conversation comment to update. Do not use a pull request review comment ID. (integer, required) - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) diff --git a/pkg/github/__toolsnaps__/update_issue_comment.snap b/pkg/github/__toolsnaps__/update_issue_comment.snap index 33dcb56bc5..c8a1db29d2 100644 --- a/pkg/github/__toolsnaps__/update_issue_comment.snap +++ b/pkg/github/__toolsnaps__/update_issue_comment.snap @@ -4,7 +4,7 @@ "readOnlyHint": false, "title": "Update issue comment" }, - "description": "Update the body of an existing issue or pull request comment.", + "description": "Update the body of an existing issue or pull request conversation comment. This tool cannot update pull request review comments.", "inputSchema": { "properties": { "body": { @@ -13,7 +13,7 @@ "type": "string" }, "comment_id": { - "description": "The numeric ID of the issue or pull request comment to update", + "description": "The numeric ID of the issue or pull request conversation comment to update. Do not use a pull request review comment ID.", "minimum": 1, "type": "integer" }, diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 3d6767e771..f1de84eb27 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1552,13 +1552,13 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }) } -// UpdateIssueComment creates a tool to update an issue or pull request comment. +// UpdateIssueComment creates a tool to update an issue or pull request conversation comment. func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { return NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_comment", - Description: t("TOOL_UPDATE_ISSUE_COMMENT_DESCRIPTION", "Update the body of an existing issue or pull request comment."), + Description: t("TOOL_UPDATE_ISSUE_COMMENT_DESCRIPTION", "Update the body of an existing issue or pull request conversation comment. This tool cannot update pull request review comments."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_COMMENT_USER_TITLE", "Update issue comment"), ReadOnlyHint: false, @@ -1576,7 +1576,7 @@ func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTo }, "comment_id": { Type: "integer", - Description: "The numeric ID of the issue or pull request comment to update", + Description: "The numeric ID of the issue or pull request conversation comment to update. Do not use a pull request review comment ID.", Minimum: jsonschema.Ptr(1.0), }, "body": { @@ -1605,10 +1605,16 @@ func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTo if commentID < 1 { return utils.NewToolResultError("comment_id must be greater than 0"), nil, nil } - body, err := RequiredParam[string](args, "body") + body, hasBody, err := OptionalParamOK[string](args, "body") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if !hasBody { + return utils.NewToolResultError("missing required parameter: body"), nil, nil + } + if body == "" { + return utils.NewToolResultError("body cannot be empty when provided"), nil, nil + } client, err := deps.GetClient(ctx) if err != nil { diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index c2666158b9..7138148c21 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -6751,6 +6751,17 @@ func TestUpdateIssueCommentHandler(t *testing.T) { expectToolError: true, expectedToolErrMsg: "missing required parameter: body", }, + { + name: "empty body", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(456), + "body": "", + }, + expectToolError: true, + expectedToolErrMsg: "body cannot be empty when provided", + }, { name: "negative comment ID", requestArgs: map[string]any{