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>
SamMorrowDrums · Jun 27, 2026 · Jun 19, 2026 · Jun 19, 2026 · Jun 19, 2026 · Jun 19, 2026
commit 2c9410b5e8733c7da55232490215721a6da156f8
@@ -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:

@@ -8,6 +8,7 @@ import (
 	"io"
 	"net/http"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -4322,13 +4323,15 @@ func TestAddIssueComment(t *testing.T) {
 		ID:      github.Ptr(int64(789)),
 		Content: github.Ptr("heart"),
 	}
+	commentCreatedAfterReactionFailure := &atomic.Bool{}
 
 	tests := []struct {
 		name               string
 		mockedClient       *http.Client
 		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
 			}
 

@@ -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:

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"net/http"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -4010,13 +4011,15 @@ func TestAddReplyToPullRequestComment(t *testing.T) {
 		ID:      github.Ptr(int64(789)),
 		Content: github.Ptr("rocket"),
 	}
+	replyCreatedAfterReactionFailure := &atomic.Bool{}
 
 	tests := []struct {
 		name               string
 		mockedClient       *http.Client
 		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
 			}