From fa69f9058ee03cb78859bfddb0c2b1a6c5a3e9b4 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Fri, 19 Jun 2026 16:54:25 +0100 Subject: [PATCH 01/11] Add reaction tools for issues and pull requests Implement three granular-only tools for adding emoji reactions: - add_issue_reaction: Add reaction to an issue - add_issue_comment_reaction: Add reaction to an issue comment - add_pull_request_review_comment_reaction: Add reaction to a PR review comment All tools are feature-flagged with FeatureFlagEnable set to enable them only when clients request granular toolsets. Tools use go-github's Reactions service and return minimal ID response on success (HTTP 201). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../add_issue_comment_reaction.snap | 47 +++++ .../__toolsnaps__/add_issue_reaction.snap | 47 +++++ ..._pull_request_review_comment_reaction.snap | 47 +++++ pkg/github/granular_tools_test.go | 183 ++++++++++++++++++ pkg/github/helper_test.go | 3 + pkg/github/issues_granular.go | 162 ++++++++++++++++ pkg/github/pullrequests_granular.go | 81 ++++++++ pkg/github/tools.go | 3 + 8 files changed, 573 insertions(+) create mode 100644 pkg/github/__toolsnaps__/add_issue_comment_reaction.snap create mode 100644 pkg/github/__toolsnaps__/add_issue_reaction.snap create mode 100644 pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap diff --git a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap new file mode 100644 index 0000000000..f74c8b154b --- /dev/null +++ b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap @@ -0,0 +1,47 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Add Issue Comment Reaction" + }, + "description": "Add an emoji reaction to a GitHub issue comment", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The issue comment ID", + "minimum": 1, + "type": "number" + }, + "content": { + "description": "The emoji reaction type", + "enum": [ + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "content" + ], + "type": "object" + }, + "name": "add_issue_comment_reaction" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/add_issue_reaction.snap b/pkg/github/__toolsnaps__/add_issue_reaction.snap new file mode 100644 index 0000000000..79b0779553 --- /dev/null +++ b/pkg/github/__toolsnaps__/add_issue_reaction.snap @@ -0,0 +1,47 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Add Issue Reaction" + }, + "description": "Add an emoji reaction to a GitHub issue", + "inputSchema": { + "properties": { + "content": { + "description": "The emoji reaction type", + "enum": [ + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": "string" + }, + "issue_number": { + "description": "The issue number", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "content" + ], + "type": "object" + }, + "name": "add_issue_reaction" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap b/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap new file mode 100644 index 0000000000..7278e6523e --- /dev/null +++ b/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap @@ -0,0 +1,47 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Add Pull Request Review Comment Reaction" + }, + "description": "Add an emoji reaction to a GitHub pull request review comment", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The pull request review comment ID", + "minimum": 1, + "type": "number" + }, + "content": { + "description": "The emoji reaction type", + "enum": [ + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "content" + ], + "type": "object" + }, + "name": "add_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 4a274ac318..ac4ab54d29 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -43,6 +43,8 @@ func TestGranularToolSnaps(t *testing.T) { GranularRemoveSubIssue, GranularReprioritizeSubIssue, GranularSetIssueFields, + AddIssueReaction, + AddIssueCommentReaction, GranularUpdatePullRequestTitle, GranularUpdatePullRequestBody, GranularUpdatePullRequestState, @@ -54,6 +56,7 @@ func TestGranularToolSnaps(t *testing.T) { GranularAddPullRequestReviewComment, GranularResolveReviewThread, GranularUnresolveReviewThread, + AddPullRequestReviewCommentReaction, } for _, constructor := range toolConstructors { @@ -86,6 +89,8 @@ func TestIssuesGranularToolset(t *testing.T) { "remove_sub_issue", "reprioritize_sub_issue", "set_issue_fields", + "add_issue_reaction", + "add_issue_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -121,6 +126,7 @@ func TestPullRequestsGranularToolset(t *testing.T) { "add_pull_request_review_comment", "resolve_review_thread", "unresolve_review_thread", + "add_pull_request_review_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -2025,3 +2031,180 @@ func TestGranularSetIssueFields(t *testing.T) { assert.Equal(t, "update_issue_suggestions", spy.captured.Get(headers.GraphQLFeaturesHeader)) }) } + +// --- Reaction granular tool handler tests --- + +func TestAddIssueReaction(t *testing.T) { + mockReaction := &gogithub.Reaction{ + ID: gogithub.Ptr(int64(12345)), + Content: gogithub.Ptr("+1"), + } + + tests := []struct { + name string + mockedClient *http.Client + args map[string]any + expectErr bool + }{ + { + name: "add reaction to issue successfully", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusCreated, mockReaction), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "content": "+1", + }, + expectErr: false, + }, + { + name: "missing owner returns error", + mockedClient: MockHTTPClientWithHandlers(nil), + args: map[string]any{ + "repo": "repo", + "issue_number": float64(42), + "content": "+1", + }, + expectErr: true, + }, + { + name: "missing content returns error", + mockedClient: MockHTTPClientWithHandlers(nil), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + serverTool := AddIssueReaction(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if tc.expectErr { + assert.True(t, result.IsError) + } else { + assert.False(t, result.IsError) + } + }) + } +} + +func TestAddIssueCommentReaction(t *testing.T) { + mockReaction := &gogithub.Reaction{ + ID: gogithub.Ptr(int64(67890)), + Content: gogithub.Ptr("heart"), + } + + tests := []struct { + name string + mockedClient *http.Client + args map[string]any + expectErr bool + }{ + { + name: "add reaction to issue comment successfully", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusCreated, mockReaction), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(999), + "content": "heart", + }, + expectErr: false, + }, + { + name: "missing comment_id returns error", + mockedClient: MockHTTPClientWithHandlers(nil), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "content": "heart", + }, + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + serverTool := AddIssueCommentReaction(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if tc.expectErr { + assert.True(t, result.IsError) + } else { + assert.False(t, result.IsError) + } + }) + } +} + +func TestAddPullRequestReviewCommentReaction(t *testing.T) { + mockReaction := &gogithub.Reaction{ + ID: gogithub.Ptr(int64(54321)), + Content: gogithub.Ptr("rocket"), + } + + tests := []struct { + name string + mockedClient *http.Client + args map[string]any + expectErr bool + }{ + { + name: "add reaction to PR review comment successfully", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposPullsCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusCreated, mockReaction), + }), + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(888), + "content": "rocket", + }, + expectErr: false, + }, + { + name: "missing repo returns error", + mockedClient: MockHTTPClientWithHandlers(nil), + args: map[string]any{ + "owner": "owner", + "comment_id": float64(888), + "content": "rocket", + }, + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + serverTool := AddPullRequestReviewCommentReaction(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if tc.expectErr { + assert.True(t, result.IsError) + } else { + assert.False(t, result.IsError) + } + }) + } +} diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 2ad1736794..1d50ef0dec 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -61,11 +61,13 @@ 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" + 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" 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" // Pull request endpoints GetReposPullsByOwnerByRepo = "GET /repos/{owner}/{repo}/pulls" @@ -79,6 +81,7 @@ const ( PutReposPullsUpdateBranchByOwnerByRepoByPullNumber = "PUT /repos/{owner}/{repo}/pulls/{pull_number}/update-branch" 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" // Notifications endpoints GetNotifications = "GET /notifications" diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 157d5595fd..296f302802 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1198,3 +1198,165 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv st.FeatureFlagEnable = FeatureFlagIssuesGranular return st } + +// AddIssueReaction adds a reaction to an issue. +func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "add_issue_reaction", + Description: t("TOOL_ADD_ISSUE_REACTION_DESCRIPTION", "Add an emoji reaction to a GitHub issue"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ADD_ISSUE_REACTION_USER_TITLE", "Add Issue Reaction"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + 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), + }, + "content": { + Type: "string", + Description: "The emoji reaction type", + Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, + }, + }, + Required: []string{"owner", "repo", "issue_number", "content"}, + }, + }, + []scopes.Scope{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 + } + content, err := RequiredParam[string](args, "content") + 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 + } + + reaction, resp, err := client.Reactions.CreateIssueReaction(ctx, owner, repo, issueNumber, content) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to issue", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st +} + +// AddIssueCommentReaction adds a reaction to an issue comment. +func AddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "add_issue_comment_reaction", + Description: t("TOOL_ADD_ISSUE_COMMENT_REACTION_DESCRIPTION", "Add an emoji reaction to a GitHub issue comment"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ADD_ISSUE_COMMENT_REACTION_USER_TITLE", "Add Issue Comment Reaction"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + 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 comment ID", + Minimum: jsonschema.Ptr(1.0), + }, + "content": { + Type: "string", + Description: "The emoji reaction type", + Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, + }, + }, + Required: []string{"owner", "repo", "comment_id", "content"}, + }, + }, + []scopes.Scope{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 + } + content, err := RequiredParam[string](args, "content") + 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 + } + + reaction, resp, err := client.Reactions.CreateIssueCommentReaction(ctx, owner, repo, commentID, content) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to issue comment", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st +} diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index 6bc2b99f36..c7c0bc2a85 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -757,3 +757,84 @@ func GranularUnresolveReviewThread(t translations.TranslationHelperFunc) invento st.FeatureFlagEnable = FeatureFlagPullRequestsGranular return st } + +// AddPullRequestReviewCommentReaction adds a reaction to a pull request review comment. +func AddPullRequestReviewCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "add_pull_request_review_comment_reaction", + Description: t("TOOL_ADD_PULL_REQUEST_REVIEW_COMMENT_REACTION_DESCRIPTION", "Add an emoji reaction to a GitHub pull request review comment"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ADD_PULL_REQUEST_REVIEW_COMMENT_REACTION_USER_TITLE", "Add Pull Request Review Comment Reaction"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + 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 pull request review comment ID", + Minimum: jsonschema.Ptr(1.0), + }, + "content": { + Type: "string", + Description: "The emoji reaction type", + Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, + }, + }, + Required: []string{"owner", "repo", "comment_id", "content"}, + }, + }, + []scopes.Scope{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 + } + content, err := RequiredParam[string](args, "content") + 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 + } + + reaction, resp, err := client.Reactions.CreatePullRequestCommentReaction(ctx, owner, repo, commentID, content) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to pull request review comment", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular + return st +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index b937f8bfd6..9cc37179aa 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -215,6 +215,8 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { ListIssueFields(t), IssueWrite(t), AddIssueComment(t), + AddIssueReaction(t), + AddIssueCommentReaction(t), SubIssueWrite(t), // User tools @@ -234,6 +236,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { PullRequestReviewWrite(t), AddCommentToPendingReview(t), AddReplyToPullRequestComment(t), + AddPullRequestReviewCommentReaction(t), // Copilot tools AssignCopilotToIssue(t), From a5511caef84655516b6957e4c025fb09a469f328 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Fri, 19 Jun 2026 17:00:19 +0100 Subject: [PATCH 02/11] Make reaction tools available in both granular and non-granular modes Remove feature flag gates from reaction tools so they're available to all clients regardless of granular toolset preference. Reaction tools are naturally atomic operations and work equally well in both modes. Updates test expectations to exclude reaction tools from granular-only test assertions, since they're now always available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 21 +++++++++++++++++++++ pkg/github/granular_tools_test.go | 3 --- pkg/github/issues_granular.go | 2 -- pkg/github/pullrequests_granular.go | 1 - 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 1b72ff22f8..0179f0ea0b 100644 --- a/README.md +++ b/README.md @@ -846,6 +846,20 @@ The following sets of tools are available: - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) +- **add_issue_comment_reaction** - Add Issue Comment Reaction + - **Required OAuth Scopes**: `repo` + - `comment_id`: The issue comment ID (number, required) + - `content`: The emoji reaction type (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **add_issue_reaction** - Add Issue Reaction + - **Required OAuth Scopes**: `repo` + - `content`: The emoji reaction type (string, required) + - `issue_number`: The issue number (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - **get_label** - Get a specific label from a repository - **Required OAuth Scopes**: `repo` - `name`: Label name. (string, required) @@ -1095,6 +1109,13 @@ The following sets of tools are available: - `startSide`: For multi-line comments, the starting side of the diff that the comment applies to. LEFT indicates the previous state, RIGHT indicates the new state (string, optional) - `subjectType`: The level at which the comment is targeted (string, required) +- **add_pull_request_review_comment_reaction** - Add Pull Request Review Comment Reaction + - **Required OAuth Scopes**: `repo` + - `comment_id`: The pull request review comment ID (number, required) + - `content`: The emoji reaction type (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - **add_reply_to_pull_request_comment** - Add reply to pull request comment - **Required OAuth Scopes**: `repo` - `body`: The text of the reply (string, required) diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index ac4ab54d29..0529ace86f 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -89,8 +89,6 @@ func TestIssuesGranularToolset(t *testing.T) { "remove_sub_issue", "reprioritize_sub_issue", "set_issue_fields", - "add_issue_reaction", - "add_issue_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -126,7 +124,6 @@ func TestPullRequestsGranularToolset(t *testing.T) { "add_pull_request_review_comment", "resolve_review_thread", "unresolve_review_thread", - "add_pull_request_review_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 296f302802..3f00fa727a 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1276,7 +1276,6 @@ func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultText(string(r)), nil, nil }, ) - st.FeatureFlagEnable = FeatureFlagIssuesGranular return st } @@ -1357,6 +1356,5 @@ func AddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.Ser return utils.NewToolResultText(string(r)), nil, nil }, ) - st.FeatureFlagEnable = FeatureFlagIssuesGranular return st } diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index c7c0bc2a85..c1aa02f627 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -835,6 +835,5 @@ func AddPullRequestReviewCommentReaction(t translations.TranslationHelperFunc) i return utils.NewToolResultText(string(r)), nil, nil }, ) - st.FeatureFlagEnable = FeatureFlagPullRequestsGranular return st } From 774b75bba98e8b491eb4a6b2f977276dd7439063 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Fri, 19 Jun 2026 17:05:19 +0100 Subject: [PATCH 03/11] Update toolsnaps with aligned reaction tool descriptions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/__toolsnaps__/add_issue_comment_reaction.snap | 2 +- pkg/github/__toolsnaps__/add_issue_reaction.snap | 2 +- .../__toolsnaps__/add_pull_request_review_comment_reaction.snap | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap index f74c8b154b..19d8dde3ee 100644 --- a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap +++ b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap @@ -4,7 +4,7 @@ "openWorldHint": true, "title": "Add Issue Comment Reaction" }, - "description": "Add an emoji reaction to a GitHub issue comment", + "description": "Add a reaction to an issue comment.", "inputSchema": { "properties": { "comment_id": { diff --git a/pkg/github/__toolsnaps__/add_issue_reaction.snap b/pkg/github/__toolsnaps__/add_issue_reaction.snap index 79b0779553..36af1e3855 100644 --- a/pkg/github/__toolsnaps__/add_issue_reaction.snap +++ b/pkg/github/__toolsnaps__/add_issue_reaction.snap @@ -4,7 +4,7 @@ "openWorldHint": true, "title": "Add Issue Reaction" }, - "description": "Add an emoji reaction to a GitHub issue", + "description": "Add a reaction to an issue.", "inputSchema": { "properties": { "content": { diff --git a/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap b/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap index 7278e6523e..1554cf8b07 100644 --- a/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap +++ b/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap @@ -4,7 +4,7 @@ "openWorldHint": true, "title": "Add Pull Request Review Comment Reaction" }, - "description": "Add an emoji reaction to a GitHub pull request review comment", + "description": "Add a reaction to a pull request review comment.", "inputSchema": { "properties": { "comment_id": { From 94c6f2c8c95b5e5a7065199a14aab7fb6ed92265 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Fri, 19 Jun 2026 17:07:26 +0100 Subject: [PATCH 04/11] Align reaction tool descriptions with codebase style Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/issues_granular.go | 4 ++-- pkg/github/pullrequests_granular.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 3f00fa727a..14829bfc96 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1205,7 +1205,7 @@ func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_reaction", - Description: t("TOOL_ADD_ISSUE_REACTION_DESCRIPTION", "Add an emoji reaction to a GitHub issue"), + Description: t("TOOL_ADD_ISSUE_REACTION_DESCRIPTION", "Add a reaction to an issue."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_ADD_ISSUE_REACTION_USER_TITLE", "Add Issue Reaction"), ReadOnlyHint: false, @@ -1285,7 +1285,7 @@ func AddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.Ser ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_comment_reaction", - Description: t("TOOL_ADD_ISSUE_COMMENT_REACTION_DESCRIPTION", "Add an emoji reaction to a GitHub issue comment"), + Description: t("TOOL_ADD_ISSUE_COMMENT_REACTION_DESCRIPTION", "Add a reaction to an issue comment."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_ADD_ISSUE_COMMENT_REACTION_USER_TITLE", "Add Issue Comment Reaction"), ReadOnlyHint: false, diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index c1aa02f627..63bd50dec7 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -764,7 +764,7 @@ func AddPullRequestReviewCommentReaction(t translations.TranslationHelperFunc) i ToolsetMetadataPullRequests, mcp.Tool{ Name: "add_pull_request_review_comment_reaction", - Description: t("TOOL_ADD_PULL_REQUEST_REVIEW_COMMENT_REACTION_DESCRIPTION", "Add an emoji reaction to a GitHub pull request review comment"), + Description: t("TOOL_ADD_PULL_REQUEST_REVIEW_COMMENT_REACTION_DESCRIPTION", "Add a reaction to a pull request review comment."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_ADD_PULL_REQUEST_REVIEW_COMMENT_REACTION_USER_TITLE", "Add Pull Request Review Comment Reaction"), ReadOnlyHint: false, From 30da9da36f5dfde159d4937523f2ca1456ac580a Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Fri, 19 Jun 2026 18:28:22 +0100 Subject: [PATCH 05/11] Improve reaction tool responses and wording Return reaction URLs in minimal responses and clarify issue tools apply to pull requests where applicable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 4 ++-- .../add_issue_comment_reaction.snap | 4 ++-- .../__toolsnaps__/add_issue_reaction.snap | 4 ++-- pkg/github/granular_tools_test.go | 16 ++++++++++++++++ pkg/github/issues_granular.go | 18 ++++++++++-------- pkg/github/pullrequests_granular.go | 3 ++- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 0179f0ea0b..e40bbdb9f1 100644 --- a/README.md +++ b/README.md @@ -846,14 +846,14 @@ The following sets of tools are available: - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) -- **add_issue_comment_reaction** - Add Issue Comment Reaction +- **add_issue_comment_reaction** - Add Reaction to Issue or Pull Request Comment - **Required OAuth Scopes**: `repo` - `comment_id`: The issue comment ID (number, required) - `content`: The emoji reaction type (string, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) -- **add_issue_reaction** - Add Issue Reaction +- **add_issue_reaction** - Add Reaction to Issue or Pull Request - **Required OAuth Scopes**: `repo` - `content`: The emoji reaction type (string, required) - `issue_number`: The issue number (number, required) diff --git a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap index 19d8dde3ee..908d4ac620 100644 --- a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap +++ b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap @@ -2,9 +2,9 @@ "annotations": { "destructiveHint": false, "openWorldHint": true, - "title": "Add Issue Comment Reaction" + "title": "Add Reaction to Issue or Pull Request Comment" }, - "description": "Add a reaction to an issue comment.", + "description": "Add a reaction to an issue or pull request comment.", "inputSchema": { "properties": { "comment_id": { diff --git a/pkg/github/__toolsnaps__/add_issue_reaction.snap b/pkg/github/__toolsnaps__/add_issue_reaction.snap index 36af1e3855..db1148dee8 100644 --- a/pkg/github/__toolsnaps__/add_issue_reaction.snap +++ b/pkg/github/__toolsnaps__/add_issue_reaction.snap @@ -2,9 +2,9 @@ "annotations": { "destructiveHint": false, "openWorldHint": true, - "title": "Add Issue Reaction" + "title": "Add Reaction to Issue or Pull Request" }, - "description": "Add a reaction to an issue.", + "description": "Add a reaction to an issue or pull request.", "inputSchema": { "properties": { "content": { diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 0529ace86f..8abc3e379b 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -2,6 +2,7 @@ package github import ( "context" + "encoding/json" "net/http" "strings" "testing" @@ -2091,6 +2092,11 @@ func TestAddIssueReaction(t *testing.T) { assert.True(t, result.IsError) } else { assert.False(t, result.IsError) + textContent := getTextResult(t, result) + var response MinimalResponse + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response)) + assert.Equal(t, "12345", response.ID) + assert.Equal(t, "https://api.github.com/repos/owner/repo/issues/42/reactions/12345", response.URL) } }) } @@ -2146,6 +2152,11 @@ func TestAddIssueCommentReaction(t *testing.T) { assert.True(t, result.IsError) } else { assert.False(t, result.IsError) + textContent := getTextResult(t, result) + var response MinimalResponse + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response)) + assert.Equal(t, "67890", response.ID) + assert.Equal(t, "https://api.github.com/repos/owner/repo/issues/comments/999/reactions/67890", response.URL) } }) } @@ -2201,6 +2212,11 @@ func TestAddPullRequestReviewCommentReaction(t *testing.T) { assert.True(t, result.IsError) } else { assert.False(t, result.IsError) + textContent := getTextResult(t, result) + var response MinimalResponse + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response)) + assert.Equal(t, "54321", response.ID) + assert.Equal(t, "https://api.github.com/repos/owner/repo/pulls/comments/888/reactions/54321", response.URL) } }) } diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 14829bfc96..29206feba3 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1199,15 +1199,15 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv return st } -// AddIssueReaction adds a reaction to an issue. +// AddIssueReaction adds a reaction to an issue or pull request. func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_reaction", - Description: t("TOOL_ADD_ISSUE_REACTION_DESCRIPTION", "Add a reaction to an issue."), + Description: t("TOOL_ADD_ISSUE_REACTION_DESCRIPTION", "Add a reaction to an issue or pull request."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_ADD_ISSUE_REACTION_USER_TITLE", "Add Issue Reaction"), + Title: t("TOOL_ADD_ISSUE_REACTION_USER_TITLE", "Add Reaction to Issue or Pull Request"), ReadOnlyHint: false, DestructiveHint: jsonschema.Ptr(false), OpenWorldHint: jsonschema.Ptr(true), @@ -1268,7 +1268,8 @@ func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool defer func() { _ = resp.Body.Close() }() r, err := json.Marshal(MinimalResponse{ - ID: fmt.Sprintf("%d", reaction.GetID()), + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/issues/%d/reactions/%d", client.BaseURL(), owner, repo, issueNumber, reaction.GetID()), }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil @@ -1279,15 +1280,15 @@ func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool return st } -// AddIssueCommentReaction adds a reaction to an issue comment. +// AddIssueCommentReaction adds a reaction to an issue or pull request comment. func AddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_comment_reaction", - Description: t("TOOL_ADD_ISSUE_COMMENT_REACTION_DESCRIPTION", "Add a reaction to an issue comment."), + Description: t("TOOL_ADD_ISSUE_COMMENT_REACTION_DESCRIPTION", "Add a reaction to an issue or pull request comment."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_ADD_ISSUE_COMMENT_REACTION_USER_TITLE", "Add Issue Comment Reaction"), + Title: t("TOOL_ADD_ISSUE_COMMENT_REACTION_USER_TITLE", "Add Reaction to Issue or Pull Request Comment"), ReadOnlyHint: false, DestructiveHint: jsonschema.Ptr(false), OpenWorldHint: jsonschema.Ptr(true), @@ -1348,7 +1349,8 @@ func AddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.Ser defer func() { _ = resp.Body.Close() }() r, err := json.Marshal(MinimalResponse{ - ID: fmt.Sprintf("%d", reaction.GetID()), + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/issues/comments/%d/reactions/%d", client.BaseURL(), owner, repo, commentID, reaction.GetID()), }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index 63bd50dec7..e73758d399 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -827,7 +827,8 @@ func AddPullRequestReviewCommentReaction(t translations.TranslationHelperFunc) i defer func() { _ = resp.Body.Close() }() r, err := json.Marshal(MinimalResponse{ - ID: fmt.Sprintf("%d", reaction.GetID()), + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/pulls/comments/%d/reactions/%d", client.BaseURL(), owner, repo, commentID, reaction.GetID()), }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil From 009bb7b5c0a062c06aa6a19efd270d2f45bb4a87 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 23 Jun 2026 14:29:35 +0200 Subject: [PATCH 06/11] Expose reactions through default comment tools Keep the standalone reaction tools behind granular feature flags to avoid expanding the default tool count. Add optional reaction support to the existing issue comment and pull request comment reply tools, requiring at least one of body or reaction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 33 +---- docs/feature-flags.md | 21 +++ .../__toolsnaps__/add_issue_comment.snap | 23 ++- .../add_reply_to_pull_request_comment.snap | 26 +++- pkg/github/granular_tools_test.go | 3 + pkg/github/issues.go | 90 +++++++++--- pkg/github/issues_granular.go | 2 + pkg/github/issues_test.go | 133 +++++++++++++++++- pkg/github/pullrequests.go | 86 ++++++++--- pkg/github/pullrequests_granular.go | 1 + pkg/github/pullrequests_test.go | 51 ++++++- 11 files changed, 384 insertions(+), 85 deletions(-) diff --git a/README.md b/README.md index e40bbdb9f1..e35a202aea 100644 --- a/README.md +++ b/README.md @@ -841,23 +841,10 @@ The following sets of tools are available: - **add_issue_comment** - Add comment to issue or pull request - **Required OAuth Scopes**: `repo` - - `body`: Comment content (string, required) - - `issue_number`: Issue number to comment on (number, required) + - `body`: Comment content. Required unless reaction is provided. (string, optional) + - `issue_number`: Issue number to comment on or react to (number, required) - `owner`: Repository owner (string, required) - - `repo`: Repository name (string, required) - -- **add_issue_comment_reaction** - Add Reaction to Issue or Pull Request Comment - - **Required OAuth Scopes**: `repo` - - `comment_id`: The issue comment ID (number, required) - - `content`: The emoji reaction type (string, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **add_issue_reaction** - Add Reaction to Issue or Pull Request - - **Required OAuth Scopes**: `repo` - - `content`: The emoji reaction type (string, required) - - `issue_number`: The issue number (number, required) - - `owner`: Repository owner (username or organization) (string, required) + - `reaction`: Emoji reaction to add. Required unless body is provided. (string, optional) - `repo`: Repository name (string, required) - **get_label** - Get a specific label from a repository @@ -1109,19 +1096,13 @@ The following sets of tools are available: - `startSide`: For multi-line comments, the starting side of the diff that the comment applies to. LEFT indicates the previous state, RIGHT indicates the new state (string, optional) - `subjectType`: The level at which the comment is targeted (string, required) -- **add_pull_request_review_comment_reaction** - Add Pull Request Review Comment Reaction - - **Required OAuth Scopes**: `repo` - - `comment_id`: The pull request review comment ID (number, required) - - `content`: The emoji reaction type (string, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - **add_reply_to_pull_request_comment** - Add reply to pull request comment - **Required OAuth Scopes**: `repo` - - `body`: The text of the reply (string, required) - - `commentId`: The ID of the comment to reply to (number, required) + - `body`: The text of the reply. Required unless reaction is provided. (string, optional) + - `commentId`: The ID of the comment to reply or react to (number, required) - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) + - `pullNumber`: Pull request number. Required when body is provided. (number, optional) + - `reaction`: Emoji reaction to add. Required unless body is provided. (string, optional) - `repo`: Repository name (string, required) - **create_pull_request** - Open new pull request diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 6ebc2dec95..0f0790f8f7 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -98,6 +98,20 @@ runtime behavior (such as output formatting) won't appear here. ### `issues_granular` +- **add_issue_comment_reaction** - Add Reaction to Issue or Pull Request Comment + - **Required OAuth Scopes**: `repo` + - `comment_id`: The issue comment ID (number, required) + - `content`: The emoji reaction type (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **add_issue_reaction** - Add Reaction to Issue or Pull Request + - **Required OAuth Scopes**: `repo` + - `content`: The emoji reaction type (string, required) + - `issue_number`: The issue number (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - **add_sub_issue** - Add Sub-Issue - **Required OAuth Scopes**: `repo` - `issue_number`: The parent issue number (number, required) @@ -204,6 +218,13 @@ runtime behavior (such as output formatting) won't appear here. - `startSide`: The start side of a multi-line comment (optional) (string, optional) - `subjectType`: The subject type of the comment (string, required) +- **add_pull_request_review_comment_reaction** - Add Pull Request Review Comment Reaction + - **Required OAuth Scopes**: `repo` + - `comment_id`: The pull request review comment ID (number, required) + - `content`: The emoji reaction type (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - **create_pull_request_review** - Create Pull Request Review - **Required OAuth Scopes**: `repo` - `body`: The review body text (optional) (string, optional) diff --git a/pkg/github/__toolsnaps__/add_issue_comment.snap b/pkg/github/__toolsnaps__/add_issue_comment.snap index 5479a16a60..3805ec8f77 100644 --- a/pkg/github/__toolsnaps__/add_issue_comment.snap +++ b/pkg/github/__toolsnaps__/add_issue_comment.snap @@ -2,21 +2,35 @@ "annotations": { "title": "Add comment to issue or pull request" }, - "description": "Add a comment to a specific issue in a GitHub repository. Use this tool to add comments to pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add review comments.", + "description": "Add a comment and/or reaction to a specific issue in a GitHub repository. Use this tool with pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add or react to review comments. At least one of body or reaction is required.", "inputSchema": { "properties": { "body": { - "description": "Comment content", + "description": "Comment content. Required unless reaction is provided.", "type": "string" }, "issue_number": { - "description": "Issue number to comment on", + "description": "Issue number to comment on or react to", "type": "number" }, "owner": { "description": "Repository owner", "type": "string" }, + "reaction": { + "description": "Emoji reaction to add. Required unless body is provided.", + "enum": [ + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": "string" + }, "repo": { "description": "Repository name", "type": "string" @@ -25,8 +39,7 @@ "required": [ "owner", "repo", - "issue_number", - "body" + "issue_number" ], "type": "object" }, diff --git a/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap b/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap index e2187478e8..495f5e27d3 100644 --- a/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap +++ b/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap @@ -2,15 +2,15 @@ "annotations": { "title": "Add reply to pull request comment" }, - "description": "Add a reply to an existing pull request comment. This creates a new comment that is linked as a reply to the specified comment.", + "description": "Add a reply and/or reaction to an existing pull request comment. This can create a new comment linked as a reply to the specified comment, add an emoji reaction to the specified comment, or do both. At least one of body or reaction is required.", "inputSchema": { "properties": { "body": { - "description": "The text of the reply", + "description": "The text of the reply. Required unless reaction is provided.", "type": "string" }, "commentId": { - "description": "The ID of the comment to reply to", + "description": "The ID of the comment to reply or react to", "type": "number" }, "owner": { @@ -18,9 +18,23 @@ "type": "string" }, "pullNumber": { - "description": "Pull request number", + "description": "Pull request number. Required when body is provided.", "type": "number" }, + "reaction": { + "description": "Emoji reaction to add. Required unless body is provided.", + "enum": [ + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": "string" + }, "repo": { "description": "Repository name", "type": "string" @@ -29,9 +43,7 @@ "required": [ "owner", "repo", - "pullNumber", - "commentId", - "body" + "commentId" ], "type": "object" }, diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 8abc3e379b..9442b81f0d 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -90,6 +90,8 @@ func TestIssuesGranularToolset(t *testing.T) { "remove_sub_issue", "reprioritize_sub_issue", "set_issue_fields", + "add_issue_reaction", + "add_issue_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -125,6 +127,7 @@ func TestPullRequestsGranularToolset(t *testing.T) { "add_pull_request_review_comment", "resolve_review_thread", "unresolve_review_thread", + "add_pull_request_review_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index fa685ba671..1986ab9d6a 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1051,13 +1051,13 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool { }) } -// AddIssueComment creates a tool to add a comment to an issue. +// AddIssueComment creates a tool to add a comment or reaction to an issue. func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { return NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_comment", - Description: t("TOOL_ADD_ISSUE_COMMENT_DESCRIPTION", "Add a comment to a specific issue in a GitHub repository. Use this tool to add comments to pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add review comments."), + Description: t("TOOL_ADD_ISSUE_COMMENT_DESCRIPTION", "Add a comment and/or reaction to a specific issue in a GitHub repository. Use this tool with pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add or react to review comments. At least one of body or reaction is required."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_ADD_ISSUE_COMMENT_USER_TITLE", "Add comment to issue or pull request"), ReadOnlyHint: false, @@ -1075,14 +1075,19 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }, "issue_number": { Type: "number", - Description: "Issue number to comment on", + Description: "Issue number to comment on or react to", }, "body": { Type: "string", - Description: "Comment content", + Description: "Comment content. Required unless reaction is provided.", + }, + "reaction": { + Type: "string", + Description: "Emoji reaction to add. Required unless body is provided.", + Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, }, }, - Required: []string{"owner", "repo", "issue_number", "body"}, + Required: []string{"owner", "repo", "issue_number"}, }, }, []scopes.Scope{scopes.Repo}, @@ -1099,39 +1104,82 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return utils.NewToolResultError(err.Error()), 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 } - - comment := &github.IssueComment{ - Body: github.Ptr(body), + reactionContent, hasReaction, err := OptionalParamOK[string](args, "reaction") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if !hasBody && !hasReaction { + return utils.NewToolResultError("at least one of body or reaction is required"), nil, nil + } + if hasBody && body == "" { + return utils.NewToolResultError("body cannot be empty when provided"), nil, nil + } + if hasReaction && reactionContent == "" { + return utils.NewToolResultError("reaction cannot be empty when provided"), nil, nil } client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to create comment", err), nil, nil + + var commentResponse *MinimalResponse + if hasBody { + comment := &github.IssueComment{ + Body: github.Ptr(body), + } + createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create comment", err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + bodyBytes, err := io.ReadAll(resp.Body) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create comment", resp, bodyBytes), nil, nil + } + + commentResponse = &MinimalResponse{ + ID: fmt.Sprintf("%d", createdComment.GetID()), + URL: createdComment.GetHTMLURL(), + } } - defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusCreated { - body, err := io.ReadAll(resp.Body) + var reactionResponse *MinimalResponse + if hasReaction { + reaction, resp, err := client.Reactions.CreateIssueReaction(ctx, owner, repo, issueNumber, reactionContent) if err != nil { - return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to issue", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + reactionResponse = &MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/issues/%d/reactions/%d", client.BaseURL(), owner, repo, issueNumber, reaction.GetID()), } - return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create comment", resp, body), nil, nil } - minimalResponse := MinimalResponse{ - ID: fmt.Sprintf("%d", createdComment.GetID()), - URL: createdComment.GetHTMLURL(), + var result any + switch { + case hasBody && hasReaction: + result = map[string]MinimalResponse{ + "comment": *commentResponse, + "reaction": *reactionResponse, + } + case hasReaction: + result = reactionResponse + default: + result = commentResponse } - r, err := json.Marshal(minimalResponse) + r, err := json.Marshal(result) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 29206feba3..c30637945a 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1277,6 +1277,7 @@ func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultText(string(r)), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular return st } @@ -1358,5 +1359,6 @@ func AddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.Ser return utils.NewToolResultText(string(r)), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular return st } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 27fad92527..c71f6b2368 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -572,7 +572,8 @@ func Test_AddIssueComment(t *testing.T) { assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "repo") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "issue_number") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "body") - assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"owner", "repo", "issue_number", "body"}) + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "reaction") + assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"owner", "repo", "issue_number"}) // Setup mock comment for success case mockComment := &github.IssueComment{ @@ -618,10 +619,10 @@ func Test_AddIssueComment(t *testing.T) { "owner": "owner", "repo": "repo", "issue_number": float64(42), - "body": "", + "body": "This is a test comment", }, expectError: false, - expectedErrMsg: "missing required parameter: body", + expectedErrMsg: "failed to create comment", }, } @@ -4165,6 +4166,132 @@ func Test_GetSubIssues(t *testing.T) { assert.Equal(t, *tc.expectedSubIssues[i].Body, *subIssue.Body) } } + + } + }) + } +} + +func TestAddIssueComment(t *testing.T) { + t.Parallel() + + serverTool := AddIssueComment(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "add_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, "issue_number") + assert.Contains(t, schema.Properties, "body") + assert.Contains(t, schema.Properties, "reaction") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "issue_number"}) + + mockComment := &github.IssueComment{ + ID: github.Ptr(int64(456)), + Body: github.Ptr("This is a comment"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42#issuecomment-456"), + } + mockReaction := &github.Reaction{ + ID: github.Ptr(int64(789)), + Content: github.Ptr("heart"), + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectToolError bool + expectedToolErrMsg string + }{ + { + name: "successful comment on issue", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesCommentsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusCreated, mockComment), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "body": "This is a comment", + }, + }, + { + name: "successful reaction to issue", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusCreated, mockReaction), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "reaction": "heart", + }, + }, + { + name: "successful comment and reaction to issue", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesCommentsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusCreated, mockComment), + PostReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusCreated, mockReaction), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "body": "This is a comment", + "reaction": "heart", + }, + }, + { + name: "missing body and reaction", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectToolError: true, + expectedToolErrMsg: "at least one of body or reaction is required", + }, + { + name: "missing required parameter issue_number", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "reaction": "heart", + }, + expectToolError: true, + expectedToolErrMsg: "missing required parameter: issue_number", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + 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) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedToolErrMsg) + return + } + + require.False(t, result.IsError) + textContent := getTextResult(t, result) + if _, ok := tc.requestArgs["body"]; ok { + assert.Contains(t, textContent.Text, "456") + } + if _, ok := tc.requestArgs["reaction"]; ok { + assert.Contains(t, textContent.Text, "789") } }) } diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index ef3e9c0839..91c7e909f9 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1196,7 +1196,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo return st } -// AddReplyToPullRequestComment creates a tool to add a reply to an existing pull request comment. +// AddReplyToPullRequestComment creates a tool to add a reply or reaction to an existing pull request comment. func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ Type: "object", @@ -1211,25 +1211,30 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor }, "pullNumber": { Type: "number", - Description: "Pull request number", + Description: "Pull request number. Required when body is provided.", }, "commentId": { Type: "number", - Description: "The ID of the comment to reply to", + Description: "The ID of the comment to reply or react to", }, "body": { Type: "string", - Description: "The text of the reply", + Description: "The text of the reply. Required unless reaction is provided.", + }, + "reaction": { + Type: "string", + Description: "Emoji reaction to add. Required unless body is provided.", + Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, }, }, - Required: []string{"owner", "repo", "pullNumber", "commentId", "body"}, + Required: []string{"owner", "repo", "commentId"}, } return NewTool( ToolsetMetadataPullRequests, mcp.Tool{ Name: "add_reply_to_pull_request_comment", - Description: t("TOOL_ADD_REPLY_TO_PULL_REQUEST_COMMENT_DESCRIPTION", "Add a reply to an existing pull request comment. This creates a new comment that is linked as a reply to the specified comment."), + Description: t("TOOL_ADD_REPLY_TO_PULL_REQUEST_COMMENT_DESCRIPTION", "Add a reply and/or reaction to an existing pull request comment. This can create a new comment linked as a reply to the specified comment, add an emoji reaction to the specified comment, or do both. At least one of body or reaction is required."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_ADD_REPLY_TO_PULL_REQUEST_COMMENT_USER_TITLE", "Add reply to pull request comment"), ReadOnlyHint: false, @@ -1246,39 +1251,84 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - pullNumber, err := RequiredInt(args, "pullNumber") + commentID, err := RequiredBigInt(args, "commentId") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - commentID, err := RequiredInt(args, "commentId") + body, hasBody, err := OptionalParamOK[string](args, "body") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - body, err := RequiredParam[string](args, "body") + reactionContent, hasReaction, err := OptionalParamOK[string](args, "reaction") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if !hasBody && !hasReaction { + return utils.NewToolResultError("at least one of body or reaction is required"), nil, nil + } + if hasBody && body == "" { + return utils.NewToolResultError("body cannot be empty when provided"), nil, nil + } + if hasReaction && reactionContent == "" { + return utils.NewToolResultError("reaction cannot be empty when provided"), nil, nil + } client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - comment, resp, err := client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, int64(commentID)) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reply to pull request comment", resp, err), nil, nil + var comment *github.PullRequestComment + if hasBody { + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + var resp *github.Response + comment, resp, err = client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, commentID) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reply to pull request comment", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + bodyBytes, err := io.ReadAll(resp.Body) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add reply to pull request comment", resp, bodyBytes), nil, nil + } } - defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusCreated { - bodyBytes, err := io.ReadAll(resp.Body) + var reactionResponse *MinimalResponse + if hasReaction { + reaction, resp, err := client.Reactions.CreatePullRequestCommentReaction(ctx, owner, repo, commentID, reactionContent) if err != nil { - return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to pull request review comment", resp, err), nil, nil } - return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add reply to pull request comment", resp, bodyBytes), nil, nil + defer func() { _ = resp.Body.Close() }() + + reactionResponse = &MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/pulls/comments/%d/reactions/%d", client.BaseURL(), owner, repo, commentID, reaction.GetID()), + } + } + + var result any + switch { + case hasBody && hasReaction: + result = map[string]any{ + "comment": comment, + "reaction": reactionResponse, + } + case hasReaction: + result = reactionResponse + default: + result = comment } - r, err := json.Marshal(comment) + r, err := json.Marshal(result) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index e73758d399..aa759746bd 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -836,5 +836,6 @@ func AddPullRequestReviewCommentReaction(t translations.TranslationHelperFunc) i return utils.NewToolResultText(string(r)), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular return st } diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 0f372519e5..2089967419 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -3991,7 +3991,8 @@ func TestAddReplyToPullRequestComment(t *testing.T) { assert.Contains(t, schema.Properties, "pullNumber") assert.Contains(t, schema.Properties, "commentId") assert.Contains(t, schema.Properties, "body") - assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "pullNumber", "commentId", "body"}) + assert.Contains(t, schema.Properties, "reaction") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "commentId"}) // Setup mock reply comment for success case mockReplyComment := &github.PullRequestComment{ @@ -4005,6 +4006,10 @@ func TestAddReplyToPullRequestComment(t *testing.T) { CreatedAt: &github.Timestamp{Time: time.Now()}, UpdatedAt: &github.Timestamp{Time: time.Now()}, } + mockReaction := &github.Reaction{ + ID: github.Ptr(int64(789)), + Content: github.Ptr("rocket"), + } tests := []struct { name string @@ -4053,7 +4058,7 @@ func TestAddReplyToPullRequestComment(t *testing.T) { expectedToolErrMsg: "missing required parameter: repo", }, { - name: "missing required parameter pullNumber", + name: "missing required parameter pullNumber when replying", requestArgs: map[string]any{ "owner": "owner", "repo": "repo", @@ -4075,7 +4080,7 @@ func TestAddReplyToPullRequestComment(t *testing.T) { expectedToolErrMsg: "missing required parameter: commentId", }, { - name: "missing required parameter body", + name: "missing body and reaction", requestArgs: map[string]any{ "owner": "owner", "repo": "repo", @@ -4083,7 +4088,38 @@ func TestAddReplyToPullRequestComment(t *testing.T) { "commentId": float64(123), }, expectToolError: true, - expectedToolErrMsg: "missing required parameter: body", + expectedToolErrMsg: "at least one of body or reaction is required", + }, + { + name: "successful reaction to pull request comment", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "commentId": float64(123), + "reaction": "rocket", + }, + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposPullsCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusCreated, mockReaction), + }), + }, + { + name: "successful reply and reaction to pull request comment", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "commentId": float64(123), + "body": "This is a reply to the comment", + "reaction": "rocket", + }, + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposPullsCommentsByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusCreated) + responseData, _ := json.Marshal(mockReplyComment) + _, _ = w.Write(responseData) + }, + PostReposPullsCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusCreated, mockReaction), + }), }, { name: "API error when adding reply", @@ -4134,7 +4170,12 @@ func TestAddReplyToPullRequestComment(t *testing.T) { // Parse the result and verify it's not an error require.False(t, result.IsError) textContent := getTextResult(t, result) - assert.Contains(t, textContent.Text, "This is a reply to the comment") + if _, ok := tc.requestArgs["body"]; ok { + assert.Contains(t, textContent.Text, "This is a reply to the comment") + } + if _, ok := tc.requestArgs["reaction"]; ok { + assert.Contains(t, textContent.Text, "789") + } }) } } From e6276cf84738b8bd5d3abb66a5f563e393365c10 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 23 Jun 2026 15:09:42 +0200 Subject: [PATCH 07/11] Clarify PR review comment IDs for reactions Document that PR review comment reaction inputs require the numeric review comment ID, not the GraphQL review thread node ID returned by review thread APIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- docs/feature-flags.md | 2 +- .../__toolsnaps__/add_pull_request_review_comment_reaction.snap | 2 +- pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap | 2 +- pkg/github/pullrequests.go | 2 +- pkg/github/pullrequests_granular.go | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index e35a202aea..14185d39df 100644 --- a/README.md +++ b/README.md @@ -1099,7 +1099,7 @@ The following sets of tools are available: - **add_reply_to_pull_request_comment** - Add reply to pull request comment - **Required OAuth Scopes**: `repo` - `body`: The text of the reply. Required unless reaction is provided. (string, optional) - - `commentId`: The ID of the comment to reply or react to (number, required) + - `commentId`: The numeric ID of the pull request review comment to reply or react to. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...). (number, required) - `owner`: Repository owner (string, required) - `pullNumber`: Pull request number. Required when body is provided. (number, optional) - `reaction`: Emoji reaction to add. Required unless body is provided. (string, optional) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 0f0790f8f7..718aead717 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -220,7 +220,7 @@ runtime behavior (such as output formatting) won't appear here. - **add_pull_request_review_comment_reaction** - Add Pull Request Review Comment Reaction - **Required OAuth Scopes**: `repo` - - `comment_id`: The pull request review comment ID (number, required) + - `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) - `content`: The emoji reaction type (string, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) diff --git a/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap b/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap index 1554cf8b07..e5fed3b013 100644 --- a/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap +++ b/pkg/github/__toolsnaps__/add_pull_request_review_comment_reaction.snap @@ -8,7 +8,7 @@ "inputSchema": { "properties": { "comment_id": { - "description": "The pull request review 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" }, diff --git a/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap b/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap index 495f5e27d3..1a4d35a3f1 100644 --- a/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap +++ b/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap @@ -10,7 +10,7 @@ "type": "string" }, "commentId": { - "description": "The ID of the comment to reply or react to", + "description": "The numeric ID of the pull request review comment to reply or react to. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...).", "type": "number" }, "owner": { diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 91c7e909f9..18024e2471 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1215,7 +1215,7 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor }, "commentId": { Type: "number", - Description: "The ID of the comment to reply or react to", + Description: "The numeric ID of the pull request review comment to reply or react to. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...).", }, "body": { Type: "string", diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index aa759746bd..98fb8333cf 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -784,7 +784,7 @@ func AddPullRequestReviewCommentReaction(t translations.TranslationHelperFunc) i }, "comment_id": { Type: "number", - Description: "The pull request review 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: jsonschema.Ptr(1.0), }, "content": { From 7c53cde023ee313b4196d31ef18ee1087c828e82 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 23 Jun 2026 15:31:03 +0200 Subject: [PATCH 08/11] Support issue comment reactions in add_issue_comment Add optional comment_id support so the default add_issue_comment tool can react to a specific issue or pull request comment without exposing a separate default reaction tool. Keep body creation tied to issue_number and require reaction targets to provide either issue_number or comment_id. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 3 +- .../__toolsnaps__/add_issue_comment.snap | 12 ++-- pkg/github/issues.go | 66 +++++++++++++++---- pkg/github/issues_test.go | 32 +++++++-- 4 files changed, 90 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 14185d39df..38d091d755 100644 --- a/README.md +++ b/README.md @@ -842,7 +842,8 @@ The following sets of tools are available: - **add_issue_comment** - Add comment to issue or pull request - **Required OAuth Scopes**: `repo` - `body`: Comment content. Required unless reaction is provided. (string, optional) - - `issue_number`: Issue number to comment on or react to (number, required) + - `comment_id`: The numeric ID of the issue or pull request comment to react to. Use this for reactions to comments; omit it to react to the issue or pull request itself. (number, optional) + - `issue_number`: Issue or pull request number to comment on or react to. Required when body is provided, or when reaction targets an issue or pull request. (number, optional) - `owner`: Repository owner (string, required) - `reaction`: Emoji reaction to add. Required unless body is provided. (string, optional) - `repo`: Repository name (string, required) diff --git a/pkg/github/__toolsnaps__/add_issue_comment.snap b/pkg/github/__toolsnaps__/add_issue_comment.snap index 3805ec8f77..8099244e52 100644 --- a/pkg/github/__toolsnaps__/add_issue_comment.snap +++ b/pkg/github/__toolsnaps__/add_issue_comment.snap @@ -2,15 +2,20 @@ "annotations": { "title": "Add comment to issue or pull request" }, - "description": "Add a comment and/or reaction to a specific issue in a GitHub repository. Use this tool with pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add or react to review comments. At least one of body or reaction is required.", + "description": "Add a comment and/or reaction to a specific issue or issue comment in a GitHub repository. Use this tool with pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add or react to review comments. At least one of body or reaction is required.", "inputSchema": { "properties": { "body": { "description": "Comment content. Required unless reaction is provided.", "type": "string" }, + "comment_id": { + "description": "The numeric ID of the issue or pull request comment to react to. Use this for reactions to comments; omit it to react to the issue or pull request itself.", + "minimum": 1, + "type": "number" + }, "issue_number": { - "description": "Issue number to comment on or react to", + "description": "Issue or pull request number to comment on or react to. Required when body is provided, or when reaction targets an issue or pull request.", "type": "number" }, "owner": { @@ -38,8 +43,7 @@ }, "required": [ "owner", - "repo", - "issue_number" + "repo" ], "type": "object" }, diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 1986ab9d6a..3fcb16db94 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1057,7 +1057,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_comment", - Description: t("TOOL_ADD_ISSUE_COMMENT_DESCRIPTION", "Add a comment and/or reaction to a specific issue in a GitHub repository. Use this tool with pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add or react to review comments. At least one of body or reaction is required."), + Description: t("TOOL_ADD_ISSUE_COMMENT_DESCRIPTION", "Add a comment and/or reaction to a specific issue or issue comment in a GitHub repository. Use this tool with pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add or react to review comments. At least one of body or reaction is required."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_ADD_ISSUE_COMMENT_USER_TITLE", "Add comment to issue or pull request"), ReadOnlyHint: false, @@ -1075,7 +1075,12 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }, "issue_number": { Type: "number", - Description: "Issue number to comment on or react to", + Description: "Issue or pull request number to comment on or react to. Required when body is provided, or when reaction targets an issue or pull request.", + }, + "comment_id": { + Type: "number", + Description: "The numeric ID of the issue or pull request comment to react to. Use this for reactions to comments; omit it to react to the issue or pull request itself.", + Minimum: jsonschema.Ptr(1.0), }, "body": { Type: "string", @@ -1087,7 +1092,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, }, }, - Required: []string{"owner", "repo", "issue_number"}, + Required: []string{"owner", "repo"}, }, }, []scopes.Scope{scopes.Repo}, @@ -1100,9 +1105,23 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool 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 + var issueNumber int + hasIssueNumber := false + if _, ok := args["issue_number"]; ok { + issueNumber, err = RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + hasIssueNumber = true + } + var commentID int64 + hasCommentID := false + if _, ok := args["comment_id"]; ok { + commentID, err = RequiredBigInt(args, "comment_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + hasCommentID = true } body, hasBody, err := OptionalParamOK[string](args, "body") if err != nil { @@ -1115,6 +1134,12 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool if !hasBody && !hasReaction { return utils.NewToolResultError("at least one of body or reaction is required"), nil, nil } + if hasBody && !hasIssueNumber { + return utils.NewToolResultError("issue_number is required when body is provided"), nil, nil + } + if hasReaction && !hasIssueNumber && !hasCommentID { + return utils.NewToolResultError("issue_number or comment_id is required when reaction is provided"), nil, nil + } if hasBody && body == "" { return utils.NewToolResultError("body cannot be empty when provided"), nil, nil } @@ -1154,15 +1179,28 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool var reactionResponse *MinimalResponse if hasReaction { - reaction, resp, err := client.Reactions.CreateIssueReaction(ctx, owner, repo, issueNumber, reactionContent) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to issue", resp, err), nil, nil - } - defer func() { _ = resp.Body.Close() }() + if hasCommentID { + reaction, resp, err := client.Reactions.CreateIssueCommentReaction(ctx, owner, repo, commentID, reactionContent) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to issue comment", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + reactionResponse = &MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/issues/comments/%d/reactions/%d", client.BaseURL(), owner, repo, commentID, reaction.GetID()), + } + } else { + reaction, resp, err := client.Reactions.CreateIssueReaction(ctx, owner, repo, issueNumber, reactionContent) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to issue", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() - reactionResponse = &MinimalResponse{ - ID: fmt.Sprintf("%d", reaction.GetID()), - URL: fmt.Sprintf("%srepos/%s/%s/issues/%d/reactions/%d", client.BaseURL(), owner, repo, issueNumber, reaction.GetID()), + reactionResponse = &MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/issues/%d/reactions/%d", client.BaseURL(), owner, repo, issueNumber, reaction.GetID()), + } } } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index c71f6b2368..664d3cae97 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -571,9 +571,10 @@ func Test_AddIssueComment(t *testing.T) { assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "owner") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "repo") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "issue_number") + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "comment_id") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "body") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "reaction") - assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"owner", "repo", "issue_number"}) + assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"owner", "repo"}) // Setup mock comment for success case mockComment := &github.IssueComment{ @@ -4185,9 +4186,10 @@ func TestAddIssueComment(t *testing.T) { assert.Contains(t, schema.Properties, "owner") assert.Contains(t, schema.Properties, "repo") assert.Contains(t, schema.Properties, "issue_number") + assert.Contains(t, schema.Properties, "comment_id") assert.Contains(t, schema.Properties, "body") assert.Contains(t, schema.Properties, "reaction") - assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "issue_number"}) + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"}) mockComment := &github.IssueComment{ ID: github.Ptr(int64(456)), @@ -4230,6 +4232,18 @@ func TestAddIssueComment(t *testing.T) { "reaction": "heart", }, }, + { + name: "successful reaction to issue comment", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusCreated, mockReaction), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "comment_id": float64(999), + "reaction": "heart", + }, + }, { name: "successful comment and reaction to issue", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ @@ -4255,14 +4269,24 @@ func TestAddIssueComment(t *testing.T) { expectedToolErrMsg: "at least one of body or reaction is required", }, { - name: "missing required parameter issue_number", + name: "missing target for reaction", requestArgs: map[string]any{ "owner": "owner", "repo": "repo", "reaction": "heart", }, expectToolError: true, - expectedToolErrMsg: "missing required parameter: issue_number", + expectedToolErrMsg: "issue_number or comment_id is required when reaction is provided", + }, + { + name: "missing issue_number for body", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "body": "This is a comment", + }, + expectToolError: true, + expectedToolErrMsg: "issue_number is required when body is provided", }, } From 2c9410b5e8733c7da55232490215721a6da156f8 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 23 Jun 2026 21:32:28 +0200 Subject: [PATCH 09/11] Harden combined comment reaction tools Apply reactions before creating comments or replies so retrying a failed combined call cannot duplicate the non-idempotent comment operation. Also reject issue comment IDs without a reaction target to avoid silently ignoring the field. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/issues.go | 53 +++++++++++++++++---------------- pkg/github/issues_test.go | 43 ++++++++++++++++++++++++++ pkg/github/pullrequests.go | 38 ++++++++++++----------- pkg/github/pullrequests_test.go | 44 +++++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 43 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 9b66c4247b..940f82b585 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1190,6 +1190,9 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool if hasReaction && !hasIssueNumber && !hasCommentID { return utils.NewToolResultError("issue_number or comment_id is required when reaction is provided"), nil, nil } + if hasCommentID && !hasReaction { + return utils.NewToolResultError("comment_id can only be provided when reaction is provided"), nil, nil + } if hasBody && body == "" { return utils.NewToolResultError("body cannot be empty when provided"), nil, nil } @@ -1202,31 +1205,6 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - var commentResponse *MinimalResponse - if hasBody { - comment := &github.IssueComment{ - Body: github.Ptr(body), - } - createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to create comment", err), nil, nil - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusCreated { - bodyBytes, err := io.ReadAll(resp.Body) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil - } - return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create comment", resp, bodyBytes), nil, nil - } - - commentResponse = &MinimalResponse{ - ID: fmt.Sprintf("%d", createdComment.GetID()), - URL: createdComment.GetHTMLURL(), - } - } - var reactionResponse *MinimalResponse if hasReaction { if hasCommentID { @@ -1254,6 +1232,31 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool } } + var commentResponse *MinimalResponse + if hasBody { + comment := &github.IssueComment{ + Body: github.Ptr(body), + } + createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create comment", err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + bodyBytes, err := io.ReadAll(resp.Body) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create comment", resp, bodyBytes), nil, nil + } + + commentResponse = &MinimalResponse{ + ID: fmt.Sprintf("%d", createdComment.GetID()), + URL: createdComment.GetHTMLURL(), + } + } + var result any switch { case hasBody && hasReaction: diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 14f348d5aa..e18755887d 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "strings" + "sync/atomic" "testing" "time" @@ -4322,6 +4323,7 @@ func TestAddIssueComment(t *testing.T) { ID: github.Ptr(int64(789)), Content: github.Ptr("heart"), } + commentCreatedAfterReactionFailure := &atomic.Bool{} tests := []struct { name string @@ -4329,6 +4331,7 @@ func TestAddIssueComment(t *testing.T) { requestArgs map[string]any expectToolError bool expectedToolErrMsg string + unexpectedCall *atomic.Bool }{ { name: "successful comment on issue", @@ -4410,6 +4413,43 @@ func TestAddIssueComment(t *testing.T) { expectToolError: true, expectedToolErrMsg: "issue_number is required when body is provided", }, + { + name: "comment_id without reaction", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "comment_id": float64(999), + "body": "This is a comment", + }, + expectToolError: true, + expectedToolErrMsg: "comment_id can only be provided when reaction is provided", + }, + { + name: "does not create comment when reaction fails", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesReactionsByOwnerByRepoByIssueNumber: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"message": "server error"}`)) + }, + PostReposIssuesCommentsByOwnerByRepoByIssueNumber: func(w http.ResponseWriter, _ *http.Request) { + commentCreatedAfterReactionFailure.Store(true) + w.WriteHeader(http.StatusCreated) + responseData, _ := json.Marshal(mockComment) + _, _ = w.Write(responseData) + }, + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "body": "This is a comment", + "reaction": "heart", + }, + expectToolError: true, + expectedToolErrMsg: "failed to add reaction to issue", + unexpectedCall: commentCreatedAfterReactionFailure, + }, } for _, tc := range tests { @@ -4428,6 +4468,9 @@ func TestAddIssueComment(t *testing.T) { require.True(t, result.IsError) errorContent := getErrorResult(t, result) assert.Contains(t, errorContent.Text, tc.expectedToolErrMsg) + if tc.unexpectedCall != nil { + assert.False(t, tc.unexpectedCall.Load()) + } return } diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 18024e2471..19de84e751 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1272,19 +1272,35 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor if hasReaction && reactionContent == "" { return utils.NewToolResultError("reaction cannot be empty when provided"), nil, nil } + var pullNumber int + if hasBody { + pullNumber, err = RequiredInt(args, "pullNumber") + 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 } - var comment *github.PullRequestComment - if hasBody { - pullNumber, err := RequiredInt(args, "pullNumber") + var reactionResponse *MinimalResponse + if hasReaction { + reaction, resp, err := client.Reactions.CreatePullRequestCommentReaction(ctx, owner, repo, commentID, reactionContent) if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to pull request review comment", resp, err), nil, nil } + defer func() { _ = resp.Body.Close() }() + reactionResponse = &MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/pulls/comments/%d/reactions/%d", client.BaseURL(), owner, repo, commentID, reaction.GetID()), + } + } + + var comment *github.PullRequestComment + if hasBody { var resp *github.Response comment, resp, err = client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, commentID) if err != nil { @@ -1301,20 +1317,6 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor } } - var reactionResponse *MinimalResponse - if hasReaction { - reaction, resp, err := client.Reactions.CreatePullRequestCommentReaction(ctx, owner, repo, commentID, reactionContent) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to pull request review comment", resp, err), nil, nil - } - defer func() { _ = resp.Body.Close() }() - - reactionResponse = &MinimalResponse{ - ID: fmt.Sprintf("%d", reaction.GetID()), - URL: fmt.Sprintf("%srepos/%s/%s/pulls/comments/%d/reactions/%d", client.BaseURL(), owner, repo, commentID, reaction.GetID()), - } - } - var result any switch { case hasBody && hasReaction: diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 2089967419..a96c2c0ac2 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "net/http" + "sync/atomic" "testing" "time" @@ -4010,6 +4011,7 @@ func TestAddReplyToPullRequestComment(t *testing.T) { ID: github.Ptr(int64(789)), Content: github.Ptr("rocket"), } + replyCreatedAfterReactionFailure := &atomic.Bool{} tests := []struct { name string @@ -4017,6 +4019,7 @@ func TestAddReplyToPullRequestComment(t *testing.T) { requestArgs map[string]any expectToolError bool expectedToolErrMsg string + unexpectedCall *atomic.Bool }{ { name: "successful reply to pull request comment", @@ -4068,6 +4071,18 @@ func TestAddReplyToPullRequestComment(t *testing.T) { expectToolError: true, expectedToolErrMsg: "missing required parameter: pullNumber", }, + { + name: "missing required parameter pullNumber when replying with reaction", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "commentId": float64(123), + "body": "This is a reply to the comment", + "reaction": "rocket", + }, + expectToolError: true, + expectedToolErrMsg: "missing required parameter: pullNumber", + }, { name: "missing required parameter commentId", requestArgs: map[string]any{ @@ -4139,6 +4154,32 @@ func TestAddReplyToPullRequestComment(t *testing.T) { expectToolError: true, expectedToolErrMsg: "failed to add reply to pull request comment", }, + { + name: "does not create reply when reaction fails", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposPullsCommentsReactionsByOwnerByRepoByCommentID: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"message": "server error"}`)) + }, + PostReposPullsCommentsByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) { + replyCreatedAfterReactionFailure.Store(true) + w.WriteHeader(http.StatusCreated) + responseData, _ := json.Marshal(mockReplyComment) + _, _ = w.Write(responseData) + }, + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "commentId": float64(123), + "body": "This is a reply to the comment", + "reaction": "rocket", + }, + expectToolError: true, + expectedToolErrMsg: "failed to add reaction to pull request review comment", + unexpectedCall: replyCreatedAfterReactionFailure, + }, } for _, tc := range tests { @@ -4164,6 +4205,9 @@ func TestAddReplyToPullRequestComment(t *testing.T) { require.True(t, result.IsError) errorContent := getErrorResult(t, result) assert.Contains(t, errorContent.Text, tc.expectedToolErrMsg) + if tc.unexpectedCall != nil { + assert.False(t, tc.unexpectedCall.Load()) + } return } From d218ebe2c07ed9d0b5b7d3cf52b002c6633816d0 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 23 Jun 2026 21:38:35 +0200 Subject: [PATCH 10/11] Group reaction tools with granular registrations Keep standalone reaction tool registrations next to the granular issue and pull request tools they belong with, so the default compound tool sections stay focused. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/tools.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 9cc37179aa..7fd3a07b48 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -215,8 +215,6 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { ListIssueFields(t), IssueWrite(t), AddIssueComment(t), - AddIssueReaction(t), - AddIssueCommentReaction(t), SubIssueWrite(t), // User tools @@ -236,7 +234,6 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { PullRequestReviewWrite(t), AddCommentToPendingReview(t), AddReplyToPullRequestComment(t), - AddPullRequestReviewCommentReaction(t), // Copilot tools AssignCopilotToIssue(t), @@ -317,6 +314,8 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GranularRemoveSubIssue(t), GranularReprioritizeSubIssue(t), GranularSetIssueFields(t), + AddIssueReaction(t), + AddIssueCommentReaction(t), // Granular pull request tools (feature-flagged, replace consolidated update_pull_request/pull_request_review_write) GranularUpdatePullRequestTitle(t), @@ -330,6 +329,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GranularAddPullRequestReviewComment(t), GranularResolveReviewThread(t), GranularUnresolveReviewThread(t), + AddPullRequestReviewCommentReaction(t), }) } From 7fff6f23d70edb744e62095048cccdb1efa59dd9 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 23 Jun 2026 21:44:36 +0200 Subject: [PATCH 11/11] Align granular reaction tool constructors Rename the standalone reaction tool constructors to match the granular tool naming convention and clarify issue comment reaction IDs for pull request comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 2 +- .../add_issue_comment_reaction.snap | 2 +- pkg/github/granular_tools_test.go | 18 +++++++++--------- pkg/github/issues_granular.go | 10 +++++----- pkg/github/pullrequests_granular.go | 4 ++-- pkg/github/tools.go | 6 +++--- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 718aead717..52dca26fc7 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -100,7 +100,7 @@ runtime behavior (such as output formatting) won't appear here. - **add_issue_comment_reaction** - Add Reaction to Issue or Pull Request Comment - **Required OAuth Scopes**: `repo` - - `comment_id`: The issue comment ID (number, required) + - `comment_id`: The issue or pull request comment ID (number, required) - `content`: The emoji reaction type (string, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) diff --git a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap index 908d4ac620..5e2953a46c 100644 --- a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap +++ b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap @@ -8,7 +8,7 @@ "inputSchema": { "properties": { "comment_id": { - "description": "The issue comment ID", + "description": "The issue or pull request comment ID", "minimum": 1, "type": "number" }, diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 9442b81f0d..e302435ce5 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -44,8 +44,8 @@ func TestGranularToolSnaps(t *testing.T) { GranularRemoveSubIssue, GranularReprioritizeSubIssue, GranularSetIssueFields, - AddIssueReaction, - AddIssueCommentReaction, + GranularAddIssueReaction, + GranularAddIssueCommentReaction, GranularUpdatePullRequestTitle, GranularUpdatePullRequestBody, GranularUpdatePullRequestState, @@ -57,7 +57,7 @@ func TestGranularToolSnaps(t *testing.T) { GranularAddPullRequestReviewComment, GranularResolveReviewThread, GranularUnresolveReviewThread, - AddPullRequestReviewCommentReaction, + GranularAddPullRequestReviewCommentReaction, } for _, constructor := range toolConstructors { @@ -2035,7 +2035,7 @@ func TestGranularSetIssueFields(t *testing.T) { // --- Reaction granular tool handler tests --- -func TestAddIssueReaction(t *testing.T) { +func TestGranularAddIssueReaction(t *testing.T) { mockReaction := &gogithub.Reaction{ ID: gogithub.Ptr(int64(12345)), Content: gogithub.Ptr("+1"), @@ -2086,7 +2086,7 @@ func TestAddIssueReaction(t *testing.T) { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, tc.mockedClient) deps := BaseDeps{Client: client} - serverTool := AddIssueReaction(translations.NullTranslationHelper) + serverTool := GranularAddIssueReaction(translations.NullTranslationHelper) handler := serverTool.Handler(deps) request := createMCPRequest(tc.args) result, err := handler(ContextWithDeps(context.Background(), deps), &request) @@ -2105,7 +2105,7 @@ func TestAddIssueReaction(t *testing.T) { } } -func TestAddIssueCommentReaction(t *testing.T) { +func TestGranularAddIssueCommentReaction(t *testing.T) { mockReaction := &gogithub.Reaction{ ID: gogithub.Ptr(int64(67890)), Content: gogithub.Ptr("heart"), @@ -2146,7 +2146,7 @@ func TestAddIssueCommentReaction(t *testing.T) { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, tc.mockedClient) deps := BaseDeps{Client: client} - serverTool := AddIssueCommentReaction(translations.NullTranslationHelper) + serverTool := GranularAddIssueCommentReaction(translations.NullTranslationHelper) handler := serverTool.Handler(deps) request := createMCPRequest(tc.args) result, err := handler(ContextWithDeps(context.Background(), deps), &request) @@ -2165,7 +2165,7 @@ func TestAddIssueCommentReaction(t *testing.T) { } } -func TestAddPullRequestReviewCommentReaction(t *testing.T) { +func TestGranularAddPullRequestReviewCommentReaction(t *testing.T) { mockReaction := &gogithub.Reaction{ ID: gogithub.Ptr(int64(54321)), Content: gogithub.Ptr("rocket"), @@ -2206,7 +2206,7 @@ func TestAddPullRequestReviewCommentReaction(t *testing.T) { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, tc.mockedClient) deps := BaseDeps{Client: client} - serverTool := AddPullRequestReviewCommentReaction(translations.NullTranslationHelper) + serverTool := GranularAddPullRequestReviewCommentReaction(translations.NullTranslationHelper) handler := serverTool.Handler(deps) request := createMCPRequest(tc.args) result, err := handler(ContextWithDeps(context.Background(), deps), &request) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index c30637945a..e965ce9458 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1199,8 +1199,8 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv return st } -// AddIssueReaction adds a reaction to an issue or pull request. -func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool { +// GranularAddIssueReaction adds a reaction to an issue or pull request. +func GranularAddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ @@ -1281,8 +1281,8 @@ func AddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool return st } -// AddIssueCommentReaction adds a reaction to an issue or pull request comment. -func AddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { +// GranularAddIssueCommentReaction adds a reaction to an issue or pull request comment. +func GranularAddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ @@ -1307,7 +1307,7 @@ func AddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.Ser }, "comment_id": { Type: "number", - Description: "The issue comment ID", + Description: "The issue or pull request comment ID", Minimum: jsonschema.Ptr(1.0), }, "content": { diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index 98fb8333cf..d83d648533 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -758,8 +758,8 @@ func GranularUnresolveReviewThread(t translations.TranslationHelperFunc) invento return st } -// AddPullRequestReviewCommentReaction adds a reaction to a pull request review comment. -func AddPullRequestReviewCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { +// GranularAddPullRequestReviewCommentReaction adds a reaction to a pull request review comment. +func GranularAddPullRequestReviewCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataPullRequests, mcp.Tool{ diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 7fd3a07b48..c2da38e082 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -314,8 +314,8 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GranularRemoveSubIssue(t), GranularReprioritizeSubIssue(t), GranularSetIssueFields(t), - AddIssueReaction(t), - AddIssueCommentReaction(t), + GranularAddIssueReaction(t), + GranularAddIssueCommentReaction(t), // Granular pull request tools (feature-flagged, replace consolidated update_pull_request/pull_request_review_write) GranularUpdatePullRequestTitle(t), @@ -329,7 +329,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GranularAddPullRequestReviewComment(t), GranularResolveReviewThread(t), GranularUnresolveReviewThread(t), - AddPullRequestReviewCommentReaction(t), + GranularAddPullRequestReviewCommentReaction(t), }) }