From b971c959c565501e978a7e8f5cdd3f97459f75c1 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Tue, 15 Sep 2026 09:16:44 -0700 Subject: [PATCH 1/2] feat: add reaction removal tools Add granular tools for removing reactions from issues, issue comments, and pull request review comments, with destructive annotations, tests, snapshots, and generated documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9acf9c83-49aa-41af-a42d-ad75de34d132 --- docs/feature-flags.md | 21 +++ .../remove_issue_comment_reaction.snap | 40 ++++ .../__toolsnaps__/remove_issue_reaction.snap | 40 ++++ ..._pull_request_review_comment_reaction.snap | 40 ++++ pkg/github/granular_tools_test.go | 178 ++++++++++++++++++ pkg/github/helper_test.go | 3 + pkg/github/issues_granular.go | 150 +++++++++++++++ pkg/github/pullrequests_granular.go | 75 ++++++++ pkg/github/tools.go | 3 + 9 files changed, 550 insertions(+) create mode 100644 pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap create mode 100644 pkg/github/__toolsnaps__/remove_issue_reaction.snap create mode 100644 pkg/github/__toolsnaps__/remove_pull_request_review_comment_reaction.snap diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 33f1f9ea82..ec5282bd54 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -184,6 +184,20 @@ as output formatting) won't appear here. - `repo`: Repository name (string, required) - `title`: Issue title (string, required) +- **remove_issue_comment_reaction** - Remove Reaction from Issue or Pull Request Comment + - **OAuth Challenge Scopes**: `repo` + - `comment_id`: The issue or pull request comment ID (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `reaction_id`: The reaction ID to remove (number, required) + - `repo`: Repository name (string, required) + +- **remove_issue_reaction** - Remove Reaction from Issue or Pull Request + - **OAuth Challenge Scopes**: `repo` + - `issue_number`: The issue number (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `reaction_id`: The reaction ID to remove (number, required) + - `repo`: Repository name (string, required) + - **remove_sub_issue** - Remove Sub-Issue - **OAuth Challenge Scopes**: `repo` - `issue_number`: The parent issue number (number, required) @@ -301,6 +315,13 @@ as output formatting) won't appear here. - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) +- **remove_pull_request_review_comment_reaction** - Remove Pull Request Review Comment Reaction + - **OAuth Challenge Scopes**: `repo` + - `comment_id`: The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...). (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `reaction_id`: The reaction ID to remove (number, required) + - `repo`: Repository name (string, required) + - **request_pull_request_reviewers** - Request Pull Request Reviewers - **OAuth Challenge Scopes**: `repo` - `owner`: Repository owner (username or organization) (string, required) diff --git a/pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap b/pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap new file mode 100644 index 0000000000..de53a1dae8 --- /dev/null +++ b/pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Remove Reaction from Issue or Pull Request Comment" + }, + "description": "Remove a reaction from an issue or pull request comment.", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The issue or pull request comment ID", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "reaction_id": { + "description": "The reaction ID to remove", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "reaction_id" + ], + "type": "object" + }, + "name": "remove_issue_comment_reaction" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/remove_issue_reaction.snap b/pkg/github/__toolsnaps__/remove_issue_reaction.snap new file mode 100644 index 0000000000..4222e308fe --- /dev/null +++ b/pkg/github/__toolsnaps__/remove_issue_reaction.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Remove Reaction from Issue or Pull Request" + }, + "description": "Remove a reaction from an issue or pull request.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The issue number", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "reaction_id": { + "description": "The reaction ID to remove", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "reaction_id" + ], + "type": "object" + }, + "name": "remove_issue_reaction" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/remove_pull_request_review_comment_reaction.snap b/pkg/github/__toolsnaps__/remove_pull_request_review_comment_reaction.snap new file mode 100644 index 0000000000..57514e6484 --- /dev/null +++ b/pkg/github/__toolsnaps__/remove_pull_request_review_comment_reaction.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Remove Pull Request Review Comment Reaction" + }, + "description": "Remove a reaction from a pull request review comment.", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...).", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "reaction_id": { + "description": "The reaction ID to remove", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "reaction_id" + ], + "type": "object" + }, + "name": "remove_pull_request_review_comment_reaction" +} \ No newline at end of file diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 456129a0fe..e6f2e50df3 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -53,7 +53,9 @@ func TestGranularToolSnaps(t *testing.T) { GranularReprioritizeSubIssue, GranularSetIssueFields, GranularAddIssueReaction, + GranularRemoveIssueReaction, GranularAddIssueCommentReaction, + GranularRemoveIssueCommentReaction, GranularUpdatePullRequestTitle, GranularUpdatePullRequestBody, GranularUpdatePullRequestState, @@ -66,6 +68,7 @@ func TestGranularToolSnaps(t *testing.T) { GranularResolveReviewThread, GranularUnresolveReviewThread, GranularAddPullRequestReviewCommentReaction, + GranularRemovePullRequestReviewCommentReaction, } for _, constructor := range toolConstructors { @@ -99,7 +102,9 @@ func TestIssuesGranularToolset(t *testing.T) { "reprioritize_sub_issue", "set_issue_fields", "add_issue_reaction", + "remove_issue_reaction", "add_issue_comment_reaction", + "remove_issue_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -136,6 +141,7 @@ func TestPullRequestsGranularToolset(t *testing.T) { "resolve_review_thread", "unresolve_review_thread", "add_pull_request_review_comment_reaction", + "remove_pull_request_review_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -2472,6 +2478,72 @@ func TestGranularAddIssueReaction(t *testing.T) { } } +func TestGranularRemoveIssueReaction(t *testing.T) { + tests := []struct { + name string + mockedClient *http.Client + args map[string]any + expectedErrMsg string + }{ + { + name: "remove reaction from issue successfully", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusNoContent, nil), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "reaction_id": float64(12345), + }, + }, + { + name: "missing reaction_id returns error", + mockedClient: MockHTTPClientWithHandlers(nil), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectedErrMsg: "missing required parameter: reaction_id", + }, + { + name: "API error", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusNotFound, `{"message":"Not Found"}`), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "reaction_id": float64(12345), + }, + expectedErrMsg: "failed to remove reaction from issue", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + serverTool := GranularRemoveIssueReaction(translations.NullTranslationHelper) + require.NotNil(t, serverTool.Tool.Annotations.DestructiveHint) + assert.True(t, *serverTool.Tool.Annotations.DestructiveHint) + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if tc.expectedErrMsg != "" { + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, tc.expectedErrMsg) + return + } + require.False(t, result.IsError) + assert.Equal(t, "reaction successfully removed from issue", getTextResult(t, result).Text) + }) + } +} + func TestGranularAddIssueCommentReaction(t *testing.T) { mockReaction := &gogithub.Reaction{ ID: gogithub.Ptr(int64(67890)), @@ -2532,6 +2604,59 @@ func TestGranularAddIssueCommentReaction(t *testing.T) { } } +func TestGranularRemoveIssueCommentReaction(t *testing.T) { + tests := []struct { + name string + mockedClient *http.Client + args map[string]any + expectedErrMsg string + }{ + { + name: "remove reaction from issue comment successfully", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusNoContent, nil), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(999), + "reaction_id": float64(67890), + }, + }, + { + name: "missing comment_id returns error", + mockedClient: MockHTTPClientWithHandlers(nil), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "reaction_id": float64(67890), + }, + expectedErrMsg: "missing required parameter: comment_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + serverTool := GranularRemoveIssueCommentReaction(translations.NullTranslationHelper) + require.NotNil(t, serverTool.Tool.Annotations.DestructiveHint) + assert.True(t, *serverTool.Tool.Annotations.DestructiveHint) + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if tc.expectedErrMsg != "" { + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, tc.expectedErrMsg) + return + } + require.False(t, result.IsError) + assert.Equal(t, "reaction successfully removed from issue comment", getTextResult(t, result).Text) + }) + } +} + func TestGranularAddPullRequestReviewCommentReaction(t *testing.T) { mockReaction := &gogithub.Reaction{ ID: gogithub.Ptr(int64(54321)), @@ -2591,3 +2716,56 @@ func TestGranularAddPullRequestReviewCommentReaction(t *testing.T) { }) } } + +func TestGranularRemovePullRequestReviewCommentReaction(t *testing.T) { + tests := []struct { + name string + mockedClient *http.Client + args map[string]any + expectedErrMsg string + }{ + { + name: "remove reaction from PR review comment successfully", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposPullsCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusNoContent, nil), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(888), + "reaction_id": float64(54321), + }, + }, + { + name: "missing repo returns error", + mockedClient: MockHTTPClientWithHandlers(nil), + args: map[string]any{ + "owner": "owner", + "comment_id": float64(888), + "reaction_id": float64(54321), + }, + expectedErrMsg: "missing required parameter: repo", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + serverTool := GranularRemovePullRequestReviewCommentReaction(translations.NullTranslationHelper) + require.NotNil(t, serverTool.Tool.Annotations.DestructiveHint) + assert.True(t, *serverTool.Tool.Annotations.DestructiveHint) + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if tc.expectedErrMsg != "" { + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, tc.expectedErrMsg) + return + } + require.False(t, result.IsError) + assert.Equal(t, "reaction successfully removed from pull request review comment", getTextResult(t, result).Text) + }) + } +} diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 5fc541d45d..b5aeda34bd 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -65,12 +65,14 @@ const ( PostReposIssuesByOwnerByRepo = "POST /repos/{owner}/{repo}/issues" PostReposIssuesCommentsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/comments" PostReposIssuesReactionsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/reactions" + DeleteReposIssuesReactionsByOwnerByRepoByIssueNumber = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/reactions/{reaction_id}" PatchReposIssuesByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}" GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/sub_issue" PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}/sub_issues/priority" PostReposIssuesCommentsReactionsByOwnerByRepoByCommentID = "POST /repos/{owner}/{repo}/issues/comments/{comment_id}/reactions" + DeleteReposIssuesCommentsReactionsByOwnerByRepoByCommentID = "DELETE /repos/{owner}/{repo}/issues/comments/{comment_id}/reactions/{reaction_id}" DeleteReposIssuesIssueFieldValueByOwnerByRepoByIssueNumber = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/issue-field-values/{issue_field_id}" // Pull request endpoints @@ -86,6 +88,7 @@ const ( PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber = "POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers" PostReposPullsCommentsByOwnerByRepoByPullNumber = "POST /repos/{owner}/{repo}/pulls/{pull_number}/comments" PostReposPullsCommentsReactionsByOwnerByRepoByCommentID = "POST /repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions" + DeleteReposPullsCommentsReactionsByOwnerByRepoByCommentID = "DELETE /repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions/{reaction_id}" // Notifications endpoints GetNotifications = "GET /notifications" diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index c6544495c6..456efad744 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1666,6 +1666,81 @@ func GranularAddIssueReaction(t translations.TranslationHelperFunc) inventory.Se return st } +// GranularRemoveIssueReaction removes a reaction from an issue or pull request. +func GranularRemoveIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "remove_issue_reaction", + Description: t("TOOL_REMOVE_ISSUE_REACTION_DESCRIPTION", "Remove a reaction from an issue or pull request."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REMOVE_ISSUE_REACTION_USER_TITLE", "Remove Reaction from Issue or Pull Request"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(true), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "The issue number", + Minimum: jsonschema.Ptr(1.0), + }, + "reaction_id": { + Type: "number", + Description: "The reaction ID to remove", + Minimum: jsonschema.Ptr(1.0), + }, + }, + Required: []string{"owner", "repo", "issue_number", "reaction_id"}, + }, + }, + scopes.RequireAll(scopes.Repo), + 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 + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + reactionID, err := RequiredBigInt(args, "reaction_id") + 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 + } + + resp, err := client.Reactions.DeleteIssueReaction(ctx, owner, repo, issueNumber, reactionID) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove reaction from issue", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + return utils.NewToolResultText("reaction successfully removed from issue"), nil, nil + }, + ) + st.FeatureRule = issuesGranularFeatureRule + return st +} + // GranularAddIssueCommentReaction adds a reaction to an issue or pull request comment. func GranularAddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( @@ -1747,3 +1822,78 @@ func GranularAddIssueCommentReaction(t translations.TranslationHelperFunc) inven st.FeatureRule = issuesGranularFeatureRule return st } + +// GranularRemoveIssueCommentReaction removes a reaction from an issue or pull request comment. +func GranularRemoveIssueCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "remove_issue_comment_reaction", + Description: t("TOOL_REMOVE_ISSUE_COMMENT_REACTION_DESCRIPTION", "Remove a reaction from an issue or pull request comment."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REMOVE_ISSUE_COMMENT_REACTION_USER_TITLE", "Remove Reaction from Issue or Pull Request Comment"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(true), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "comment_id": { + Type: "number", + Description: "The issue or pull request comment ID", + Minimum: jsonschema.Ptr(1.0), + }, + "reaction_id": { + Type: "number", + Description: "The reaction ID to remove", + Minimum: jsonschema.Ptr(1.0), + }, + }, + Required: []string{"owner", "repo", "comment_id", "reaction_id"}, + }, + }, + scopes.RequireAll(scopes.Repo), + 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 + } + reactionID, err := RequiredBigInt(args, "reaction_id") + 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 + } + + resp, err := client.Reactions.DeleteIssueCommentReaction(ctx, owner, repo, commentID, reactionID) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove reaction from issue comment", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + return utils.NewToolResultText("reaction successfully removed from issue comment"), nil, nil + }, + ) + st.FeatureRule = issuesGranularFeatureRule + return st +} diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index a32723b3c7..66dc354313 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -896,3 +896,78 @@ func GranularAddPullRequestReviewCommentReaction(t translations.TranslationHelpe st.FeatureRule = pullRequestsGranularFeatureRule return st } + +// GranularRemovePullRequestReviewCommentReaction removes a reaction from a pull request review comment. +func GranularRemovePullRequestReviewCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "remove_pull_request_review_comment_reaction", + Description: t("TOOL_REMOVE_PULL_REQUEST_REVIEW_COMMENT_REACTION_DESCRIPTION", "Remove a reaction from a pull request review comment."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REMOVE_PULL_REQUEST_REVIEW_COMMENT_REACTION_USER_TITLE", "Remove Pull Request Review Comment Reaction"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(true), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "comment_id": { + Type: "number", + Description: "The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...).", + Minimum: jsonschema.Ptr(1.0), + }, + "reaction_id": { + Type: "number", + Description: "The reaction ID to remove", + Minimum: jsonschema.Ptr(1.0), + }, + }, + Required: []string{"owner", "repo", "comment_id", "reaction_id"}, + }, + }, + scopes.RequireAll(scopes.Repo), + 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 + } + reactionID, err := RequiredBigInt(args, "reaction_id") + 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 + } + + resp, err := client.Reactions.DeletePullRequestCommentReaction(ctx, owner, repo, commentID, reactionID) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove reaction from pull request review comment", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + return utils.NewToolResultText("reaction successfully removed from pull request review comment"), nil, nil + }, + ) + st.FeatureRule = pullRequestsGranularFeatureRule + return st +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 6764edfc26..03c6d002ee 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -372,7 +372,9 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GranularReprioritizeSubIssue(t), GranularSetIssueFields(t), GranularAddIssueReaction(t), + GranularRemoveIssueReaction(t), GranularAddIssueCommentReaction(t), + GranularRemoveIssueCommentReaction(t), // Granular pull request tools (feature-flagged, replace consolidated update_pull_request/pull_request_review_write) GranularUpdatePullRequestTitle(t), @@ -388,6 +390,7 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GranularResolveReviewThreadWithResolutionReason(t, opts...), GranularUnresolveReviewThread(t), GranularAddPullRequestReviewCommentReaction(t), + GranularRemovePullRequestReviewCommentReaction(t), }) } From 5135adf04b8c7985cf81eb24df18854b4bb5116f Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Tue, 15 Sep 2026 09:26:35 -0700 Subject: [PATCH 2/2] fix: close reaction removal responses on errors Register nil-safe response body cleanup before handling go-github errors for all reaction removal tools, and cover every API-error path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9acf9c83-49aa-41af-a42d-ad75de34d132 --- pkg/github/granular_tools_test.go | 26 ++++++++++++++++++++++++++ pkg/github/issues_granular.go | 8 ++++++-- pkg/github/pullrequests_granular.go | 4 +++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index e6f2e50df3..425f954ef9 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -2633,6 +2633,19 @@ func TestGranularRemoveIssueCommentReaction(t *testing.T) { }, expectedErrMsg: "missing required parameter: comment_id", }, + { + name: "API error", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusNotFound, `{"message":"Not Found"}`), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(999), + "reaction_id": float64(67890), + }, + expectedErrMsg: "failed to remove reaction from issue comment", + }, } for _, tc := range tests { @@ -2746,6 +2759,19 @@ func TestGranularRemovePullRequestReviewCommentReaction(t *testing.T) { }, expectedErrMsg: "missing required parameter: repo", }, + { + name: "API error", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposPullsCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusNotFound, `{"message":"Not Found"}`), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(888), + "reaction_id": float64(54321), + }, + expectedErrMsg: "failed to remove reaction from pull request review comment", + }, } for _, tc := range tests { diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 456efad744..f22a8a1536 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1729,10 +1729,12 @@ func GranularRemoveIssueReaction(t translations.TranslationHelperFunc) inventory } resp, err := client.Reactions.DeleteIssueReaction(ctx, owner, repo, issueNumber, reactionID) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove reaction from issue", resp, err), nil, nil } - defer func() { _ = resp.Body.Close() }() return utils.NewToolResultText("reaction successfully removed from issue"), nil, nil }, @@ -1886,10 +1888,12 @@ func GranularRemoveIssueCommentReaction(t translations.TranslationHelperFunc) in } resp, err := client.Reactions.DeleteIssueCommentReaction(ctx, owner, repo, commentID, reactionID) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove reaction from issue comment", resp, err), nil, nil } - defer func() { _ = resp.Body.Close() }() return utils.NewToolResultText("reaction successfully removed from issue comment"), nil, nil }, diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index 66dc354313..e670da9a34 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -960,10 +960,12 @@ func GranularRemovePullRequestReviewCommentReaction(t translations.TranslationHe } resp, err := client.Reactions.DeletePullRequestCommentReaction(ctx, owner, repo, commentID, reactionID) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove reaction from pull request review comment", resp, err), nil, nil } - defer func() { _ = resp.Body.Close() }() return utils.NewToolResultText("reaction successfully removed from pull request review comment"), nil, nil },