From fa69f9058ee03cb78859bfddb0c2b1a6c5a3e9b4 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Fri, 19 Jun 2026 16:54:25 +0100 Subject: [PATCH 01/15] 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 000000000..f74c8b154 --- /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 000000000..79b077955 --- /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 000000000..7278e6523 --- /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 4a274ac31..ac4ab54d2 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 2ad173679..1d50ef0de 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 157d5595f..296f30280 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 6bc2b99f3..c7c0bc2a8 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 b937f8bfd..9cc37179a 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/15] 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 1b72ff22f..0179f0ea0 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 ac4ab54d2..0529ace86 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 296f30280..3f00fa727 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 c7c0bc2a8..c1aa02f62 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/15] 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 f74c8b154..19d8dde3e 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 79b077955..36af1e385 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 7278e6523..1554cf8b0 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/15] 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 3f00fa727..14829bfc9 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 c1aa02f62..63bd50dec 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/15] 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 0179f0ea0..e40bbdb9f 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 19d8dde3e..908d4ac62 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 36af1e385..db1148dee 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 0529ace86..8abc3e379 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 14829bfc9..29206feba 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 63bd50dec..e73758d39 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/15] 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 e40bbdb9f..e35a202ae 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 6ebc2dec9..0f0790f8f 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 5479a16a6..3805ec8f7 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 e2187478e..495f5e27d 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 8abc3e379..9442b81f0 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 fa685ba67..1986ab9d6 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 29206feba..c30637945 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 27fad9252..c71f6b236 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 ef3e9c083..91c7e909f 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 e73758d39..aa759746b 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 0f372519e..208996741 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/15] 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 e35a202ae..14185d39d 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 0f0790f8f..718aead71 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 1554cf8b0..e5fed3b01 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 495f5e27d..1a4d35a3f 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 91c7e909f..18024e247 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 aa759746b..98fb8333c 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/15] 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 14185d39d..38d091d75 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 3805ec8f7..8099244e5 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 1986ab9d6..3fcb16db9 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 c71f6b236..664d3cae9 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/15] 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 9b66c4247..940f82b58 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 14f348d5a..e18755887 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 18024e247..19de84e75 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 208996741..a96c2c0ac 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/15] 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 9cc37179a..7fd3a07b4 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/15] 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 718aead71..52dca26fc 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 908d4ac62..5e2953a46 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 9442b81f0..e302435ce 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 c30637945..e965ce945 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 98fb8333c..d83d64853 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 7fd3a07b4..c2da38e08 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), }) } From 9855c065dcdd3915db6d25cafabca1ee6101bd6d Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 24 Jun 2026 20:49:01 +0200 Subject: [PATCH 12/15] Require issue number for comment reactions Make issue_number required on the consolidated add_issue_comment tool even when reacting to a specific issue comment, keeping the default tool input shape explicit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- .../__toolsnaps__/add_issue_comment.snap | 5 +++-- pkg/github/issues.go | 21 +++++-------------- pkg/github/issues_test.go | 19 +++++++++-------- 4 files changed, 19 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index f168f3576..f54db6644 100644 --- a/README.md +++ b/README.md @@ -843,7 +843,7 @@ The following sets of tools are available: - **Required OAuth Scopes**: `repo` - `body`: Comment content. Required unless reaction is provided. (string, optional) - `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) + - `issue_number`: Issue or pull request number to comment on or react to. (number, required) - `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 8099244e5..01ca0a0db 100644 --- a/pkg/github/__toolsnaps__/add_issue_comment.snap +++ b/pkg/github/__toolsnaps__/add_issue_comment.snap @@ -15,7 +15,7 @@ "type": "number" }, "issue_number": { - "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.", + "description": "Issue or pull request number to comment on or react to.", "type": "number" }, "owner": { @@ -43,7 +43,8 @@ }, "required": [ "owner", - "repo" + "repo", + "issue_number" ], "type": "object" }, diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 940f82b58..13bbffb51 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1125,7 +1125,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }, "issue_number": { Type: "number", - 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.", + Description: "Issue or pull request number to comment on or react to.", }, "comment_id": { Type: "number", @@ -1142,7 +1142,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, }, }, - Required: []string{"owner", "repo"}, + Required: []string{"owner", "repo", "issue_number"}, }, }, []scopes.Scope{scopes.Repo}, @@ -1155,14 +1155,9 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool 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 + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } var commentID int64 hasCommentID := false @@ -1184,12 +1179,6 @@ 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 hasCommentID && !hasReaction { return utils.NewToolResultError("comment_id can only be provided when reaction is provided"), nil, nil } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index e18755887..d4311410a 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -575,7 +575,7 @@ func Test_AddIssueComment(t *testing.T) { 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"}) + assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"owner", "repo", "issue_number"}) // Setup mock comment for success case mockComment := &github.IssueComment{ @@ -4312,7 +4312,7 @@ func TestAddIssueComment(t *testing.T) { 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"}) + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "issue_number"}) mockComment := &github.IssueComment{ ID: github.Ptr(int64(456)), @@ -4363,10 +4363,11 @@ func TestAddIssueComment(t *testing.T) { PostReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusCreated, mockReaction), }), requestArgs: map[string]any{ - "owner": "owner", - "repo": "repo", - "comment_id": float64(999), - "reaction": "heart", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "comment_id": float64(999), + "reaction": "heart", }, }, { @@ -4394,14 +4395,14 @@ func TestAddIssueComment(t *testing.T) { expectedToolErrMsg: "at least one of body or reaction is required", }, { - name: "missing target for reaction", + name: "missing issue_number for reaction", requestArgs: map[string]any{ "owner": "owner", "repo": "repo", "reaction": "heart", }, expectToolError: true, - expectedToolErrMsg: "issue_number or comment_id is required when reaction is provided", + expectedToolErrMsg: "missing required parameter: issue_number", }, { name: "missing issue_number for body", @@ -4411,7 +4412,7 @@ func TestAddIssueComment(t *testing.T) { "body": "This is a comment", }, expectToolError: true, - expectedToolErrMsg: "issue_number is required when body is provided", + expectedToolErrMsg: "missing required parameter: issue_number", }, { name: "comment_id without reaction", From b0d11b05355ab4e8464d263f6245991b962732f9 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 26 Jun 2026 08:26:14 +0200 Subject: [PATCH 13/15] Address issue comment feedback Return structured GitHub API errors for issue comment creation failures and assert MCP error results in tests. Also reject comment_id with body on the consolidated comment tool to avoid ambiguous comment-plus-comment-reaction requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- .../__toolsnaps__/add_issue_comment.snap | 2 +- pkg/github/issues.go | 11 ++++--- pkg/github/issues_test.go | 30 +++++++++++-------- 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index f54db6644..84c2f6c3f 100644 --- a/README.md +++ b/README.md @@ -842,7 +842,7 @@ 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) - - `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) + - `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. Cannot be combined with body. (number, optional) - `issue_number`: Issue or pull request number to comment on or react to. (number, required) - `owner`: Repository owner (string, required) - `reaction`: Emoji reaction to add. Required unless body is provided. (string, optional) diff --git a/pkg/github/__toolsnaps__/add_issue_comment.snap b/pkg/github/__toolsnaps__/add_issue_comment.snap index 01ca0a0db..356edac2f 100644 --- a/pkg/github/__toolsnaps__/add_issue_comment.snap +++ b/pkg/github/__toolsnaps__/add_issue_comment.snap @@ -10,7 +10,7 @@ "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.", + "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. Cannot be combined with body.", "minimum": 1, "type": "number" }, diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 13bbffb51..e1f811c13 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1129,7 +1129,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }, "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.", + 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. Cannot be combined with body.", Minimum: jsonschema.Ptr(1.0), }, "body": { @@ -1176,12 +1176,15 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool 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 hasCommentID && hasBody { + return utils.NewToolResultError("comment_id cannot be combined with body"), nil, nil } if hasCommentID && !hasReaction { return utils.NewToolResultError("comment_id can only be provided when reaction is provided"), 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 } @@ -1228,7 +1231,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool } createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment) if err != nil { - return utils.NewToolResultErrorFromErr("failed to create comment", err), nil, nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create comment", resp, err), nil, nil } defer func() { _ = resp.Body.Close() }() diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index d4311410a..5ebd8dbde 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -623,7 +623,7 @@ func Test_AddIssueComment(t *testing.T) { "issue_number": float64(42), "body": "This is a test comment", }, - expectError: false, + expectError: true, expectedErrMsg: "failed to create comment", }, } @@ -643,17 +643,11 @@ func Test_AddIssueComment(t *testing.T) { // Call handler result, err := handler(ContextWithDeps(context.Background(), deps), &request) - // Verify results if tc.expectError { - require.Error(t, err) - assert.Contains(t, err.Error(), tc.expectedErrMsg) - return - } - - if tc.expectedErrMsg != "" { - require.NotNil(t, result) - textContent := getTextResult(t, result) - assert.Contains(t, textContent.Text, tc.expectedErrMsg) + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) return } @@ -4421,11 +4415,23 @@ func TestAddIssueComment(t *testing.T) { "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: "comment_id with body", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "comment_id": float64(999), + "body": "This is a comment", + "reaction": "heart", + }, + expectToolError: true, + expectedToolErrMsg: "comment_id cannot be combined with body", + }, { name: "does not create comment when reaction fails", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ From e522c77fd44d624f35a40385c3c5b7d997a69a5d Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 26 Jun 2026 08:34:31 +0200 Subject: [PATCH 14/15] Validate issue comment reaction target Use the required issue_number to verify issue comment reaction targets before creating the reaction, and remove overlapping add_issue_comment test coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/helper_test.go | 1 + pkg/github/issues.go | 23 ++++++ pkg/github/issues_test.go | 147 +++++++++++--------------------------- 3 files changed, 65 insertions(+), 106 deletions(-) diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 1d50ef0de..cc1a676eb 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -58,6 +58,7 @@ const ( // Issues endpoints GetReposIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}" + GetReposIssuesCommentByOwnerByRepoByCommentID = "GET /repos/{owner}/{repo}/issues/comments/{comment_id}" GetReposIssuesCommentsByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/comments" PostReposIssuesByOwnerByRepo = "POST /repos/{owner}/{repo}/issues" PostReposIssuesCommentsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/comments" diff --git a/pkg/github/issues.go b/pkg/github/issues.go index bdade8ccc..653fe8189 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1200,6 +1200,20 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool var reactionResponse *MinimalResponse if hasReaction { if hasCommentID { + comment, resp, err := client.Issues.GetComment(ctx, owner, repo, commentID) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get issue comment", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + commentIssueNumber, err := issueNumberFromIssueURL(comment.GetIssueURL()) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to determine issue number for comment", err), nil, nil + } + if commentIssueNumber != issueNumber { + return utils.NewToolResultError(fmt.Sprintf("comment_id does not belong to issue_number %d", issueNumber)), nil, nil + } + 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 @@ -1271,6 +1285,15 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }) } +func issueNumberFromIssueURL(issueURL string) (int, error) { + issueNumberString := issueURL[strings.LastIndex(issueURL, "/")+1:] + issueNumber, err := strconv.Atoi(issueNumberString) + if err != nil { + return 0, fmt.Errorf("invalid issue URL %q: %w", issueURL, err) + } + return issueNumber, nil +} + // SubIssueWrite creates a tool to add a sub-issue to a parent issue. func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 4f001a0b3..34eef373f 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -560,112 +560,6 @@ func Test_GetIssue_FieldValues_Enriched(t *testing.T) { assert.Equal(t, "2.5", returnedIssue.FieldValues[1].Value) } -func Test_AddIssueComment(t *testing.T) { - // Verify tool definition once - 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) - - 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"}) - - // Setup mock comment for success case - mockComment := &github.IssueComment{ - ID: github.Ptr(int64(123)), - Body: github.Ptr("This is a test comment"), - User: &github.User{ - Login: github.Ptr("testuser"), - }, - HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42#issuecomment-123"), - } - - tests := []struct { - name string - mockedClient *http.Client - requestArgs map[string]any - expectError bool - expectedComment *github.IssueComment - expectedErrMsg string - }{ - { - name: "successful comment creation", - 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 test comment", - }, - expectError: false, - expectedComment: mockComment, - }, - { - name: "comment creation fails", - mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - PostReposIssuesCommentsByOwnerByRepoByIssueNumber: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusUnprocessableEntity) - _, _ = w.Write([]byte(`{"message": "Invalid request"}`)) - }), - }), - requestArgs: map[string]any{ - "owner": "owner", - "repo": "repo", - "issue_number": float64(42), - "body": "This is a test comment", - }, - expectError: true, - expectedErrMsg: "failed to create comment", - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - // Setup client with mock - client := mustNewGHClient(t, tc.mockedClient) - deps := BaseDeps{ - Client: client, - } - handler := serverTool.Handler(deps) - - // Create call request - request := createMCPRequest(tc.requestArgs) - - // Call handler - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - - if tc.expectError { - require.NoError(t, err) - require.True(t, result.IsError) - errorContent := getErrorResult(t, result) - assert.Contains(t, errorContent.Text, tc.expectedErrMsg) - return - } - - require.NoError(t, err) - - // Parse the result and get the text content if no error - textContent := getTextResult(t, result) - - // Unmarshal and verify the result contains minimal response - var minimalResponse MinimalResponse - err = json.Unmarshal([]byte(textContent.Text), &minimalResponse) - require.NoError(t, err) - assert.Equal(t, fmt.Sprintf("%d", tc.expectedComment.GetID()), minimalResponse.ID) - assert.Equal(t, tc.expectedComment.GetHTMLURL(), minimalResponse.URL) - }) - } -} - func Test_SearchIssues(t *testing.T) { // Verify tool definition once serverTool := SearchIssues(translations.NullTranslationHelper) @@ -4316,6 +4210,10 @@ func TestAddIssueComment(t *testing.T) { ID: github.Ptr(int64(789)), Content: github.Ptr("heart"), } + mockIssueComment := &github.IssueComment{ + ID: github.Ptr(int64(999)), + IssueURL: github.Ptr("https://api.github.com/repos/owner/repo/issues/42"), + } commentCreatedAfterReactionFailure := &atomic.Bool{} tests := []struct { @@ -4353,6 +4251,7 @@ func TestAddIssueComment(t *testing.T) { { name: "successful reaction to issue comment", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: mockResponse(t, http.StatusOK, mockIssueComment), PostReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusCreated, mockReaction), }), requestArgs: map[string]any{ @@ -4363,6 +4262,42 @@ func TestAddIssueComment(t *testing.T) { "reaction": "heart", }, }, + { + name: "issue comment reaction requires matching issue_number", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: mockResponse(t, http.StatusOK, &github.IssueComment{ + ID: github.Ptr(int64(999)), + IssueURL: github.Ptr("https://api.github.com/repos/owner/repo/issues/43"), + }), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "comment_id": float64(999), + "reaction": "heart", + }, + expectToolError: true, + expectedToolErrMsg: "comment_id does not belong to issue_number 42", + }, + { + name: "issue comment lookup fails", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }, + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "comment_id": float64(999), + "reaction": "heart", + }, + expectToolError: true, + expectedToolErrMsg: "failed to get issue comment", + }, { name: "successful comment and reaction to issue", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ From ea14878787d2ec5773645d74fd91ea749665c818 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 26 Jun 2026 08:41:46 +0200 Subject: [PATCH 15/15] Validate positive comment IDs Reject non-positive issue and pull request comment IDs in handlers and add the missing schema minimum for pull request review comment IDs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../add_reply_to_pull_request_comment.snap | 1 + pkg/github/issues.go | 3 +++ pkg/github/issues_test.go | 12 ++++++++++++ pkg/github/pullrequests.go | 4 ++++ pkg/github/pullrequests_test.go | 11 +++++++++++ 5 files changed, 31 insertions(+) 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 1a4d35a3f..a22720dcb 100644 --- a/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap +++ b/pkg/github/__toolsnaps__/add_reply_to_pull_request_comment.snap @@ -11,6 +11,7 @@ }, "commentId": { "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_...).", + "minimum": 1, "type": "number" }, "owner": { diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 653fe8189..29dec74f3 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1166,6 +1166,9 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if commentID < 1 { + return utils.NewToolResultError("comment_id must be greater than 0"), nil, nil + } hasCommentID = true } body, hasBody, err := OptionalParamOK[string](args, "body") diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 34eef373f..e7a810bac 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -4353,6 +4353,18 @@ func TestAddIssueComment(t *testing.T) { expectToolError: true, expectedToolErrMsg: "comment_id can only be provided when reaction is provided", }, + { + name: "negative comment_id", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "comment_id": float64(-1), + "reaction": "heart", + }, + expectToolError: true, + expectedToolErrMsg: "comment_id must be greater than 0", + }, { name: "comment_id with body", requestArgs: map[string]any{ diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 6f3defd8a..a40fff14d 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1216,6 +1216,7 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor "commentId": { Type: "number", 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_...).", + Minimum: jsonschema.Ptr(1.0), }, "body": { Type: "string", @@ -1255,6 +1256,9 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if commentID < 1 { + return utils.NewToolResultError("commentId must be greater than 0"), nil, nil + } body, hasBody, err := OptionalParamOK[string](args, "body") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index a96c2c0ac..a643c1467 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -4094,6 +4094,17 @@ func TestAddReplyToPullRequestComment(t *testing.T) { expectToolError: true, expectedToolErrMsg: "missing required parameter: commentId", }, + { + name: "negative commentId", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "commentId": float64(-123), + "reaction": "rocket", + }, + expectToolError: true, + expectedToolErrMsg: "commentId must be greater than 0", + }, { name: "missing body and reaction", requestArgs: map[string]any{