From cb334bd3b636ba679c6c2f0f864e9c4cd9e6cdad Mon Sep 17 00:00:00 2001 From: timrogers <116134+timrogers@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:08:47 +0000 Subject: [PATCH 1/5] Add hide_comment and unhide_comment granular issue tools Add two tools behind the issues_granular feature flag that hide (minimize) and unhide (unminimize) comments via the GraphQL minimizeComment/unminimizeComment mutations. They support issue and pull request conversation comments, pull request review comments, and pull request review bodies. Callers pass the numeric REST ID plus a comment_type; the tool resolves the GraphQL node ID via the matching REST endpoint before mutating. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 23 ++ pkg/github/__toolsnaps__/hide_comment.snap | 62 ++++ pkg/github/__toolsnaps__/unhide_comment.snap | 48 +++ pkg/github/comment_minimize.go | 207 ++++++++++++ pkg/github/comment_minimize_test.go | 334 +++++++++++++++++++ pkg/github/granular_tools_test.go | 4 + pkg/github/issues_granular.go | 67 ++++ pkg/github/tools.go | 2 + 8 files changed, 747 insertions(+) create mode 100644 pkg/github/__toolsnaps__/hide_comment.snap create mode 100644 pkg/github/__toolsnaps__/unhide_comment.snap create mode 100644 pkg/github/comment_minimize.go create mode 100644 pkg/github/comment_minimize_test.go diff --git a/docs/feature-flags.md b/docs/feature-flags.md index ec5282bd54..dcbfd0ca8d 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -184,6 +184,18 @@ as output formatting) won't appear here. - `repo`: Repository name (string, required) - `title`: Issue title (string, required) +- **hide_comment** - Hide Comment + - **OAuth Challenge Scopes**: `repo` + - `classifier`: The reason for hiding the comment (string, required) + - `comment_id`: The numeric ID of the comment, or of the review when comment_type is 'review' (integer, required) + - `comment_type`: The kind of comment: + - 'issue_comment' - a comment on an issue, or a conversation comment on a pull request. + - 'review_comment' - an inline comment on a pull request diff. + - 'review' - the body of a pull request review. Requires 'pull_number'. (string, required) + - `owner`: Repository owner (string, required) + - `pull_number`: Pull request number. Required when comment_type is 'review'. (number, optional) + - `repo`: Repository name (string, required) + - **remove_issue_comment_reaction** - Remove Reaction from Issue or Pull Request Comment - **OAuth Challenge Scopes**: `repo` - `comment_id`: The issue or pull request comment ID (number, required) @@ -221,6 +233,17 @@ as output formatting) won't appear here. - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) +- **unhide_comment** - Unhide Comment + - **OAuth Challenge Scopes**: `repo` + - `comment_id`: The numeric ID of the comment, or of the review when comment_type is 'review' (integer, required) + - `comment_type`: The kind of comment: + - 'issue_comment' - a comment on an issue, or a conversation comment on a pull request. + - 'review_comment' - an inline comment on a pull request diff. + - 'review' - the body of a pull request review. Requires 'pull_number'. (string, required) + - `owner`: Repository owner (string, required) + - `pull_number`: Pull request number. Required when comment_type is 'review'. (number, optional) + - `repo`: Repository name (string, required) + - **update_issue_assignees** - Update Issue Assignees - **OAuth Challenge Scopes**: `repo` - `assignees`: GitHub usernames to assign to this issue. ([], required) diff --git a/pkg/github/__toolsnaps__/hide_comment.snap b/pkg/github/__toolsnaps__/hide_comment.snap new file mode 100644 index 0000000000..26dcf58dec --- /dev/null +++ b/pkg/github/__toolsnaps__/hide_comment.snap @@ -0,0 +1,62 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Hide Comment" + }, + "description": "Hide (minimize) a comment on an issue or pull request. Supports issue and pull request conversation comments, pull request review comments, and pull request review bodies. Requires triage or write access to the repository, or authorship of the comment.", + "inputSchema": { + "properties": { + "classifier": { + "description": "The reason for hiding the comment", + "enum": [ + "SPAM", + "ABUSE", + "OFF_TOPIC", + "OUTDATED", + "DUPLICATE", + "RESOLVED", + "LOW_QUALITY" + ], + "type": "string" + }, + "comment_id": { + "description": "The numeric ID of the comment, or of the review when comment_type is 'review'", + "minimum": 1, + "type": "integer" + }, + "comment_type": { + "description": "The kind of comment:\n- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n- 'review_comment' - an inline comment on a pull request diff.\n- 'review' - the body of a pull request review. Requires 'pull_number'.", + "enum": [ + "issue_comment", + "review_comment", + "review" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "pull_number": { + "description": "Pull request number. Required when comment_type is 'review'.", + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_type", + "comment_id", + "classifier" + ], + "type": "object" + }, + "name": "hide_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/unhide_comment.snap b/pkg/github/__toolsnaps__/unhide_comment.snap new file mode 100644 index 0000000000..6cd1760745 --- /dev/null +++ b/pkg/github/__toolsnaps__/unhide_comment.snap @@ -0,0 +1,48 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Unhide Comment" + }, + "description": "Unhide (unminimize) a previously hidden comment on an issue or pull request. Supports issue and pull request conversation comments, pull request review comments, and pull request review bodies. Requires triage or write access to the repository, or authorship of the comment.", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The numeric ID of the comment, or of the review when comment_type is 'review'", + "minimum": 1, + "type": "integer" + }, + "comment_type": { + "description": "The kind of comment:\n- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n- 'review_comment' - an inline comment on a pull request diff.\n- 'review' - the body of a pull request review. Requires 'pull_number'.", + "enum": [ + "issue_comment", + "review_comment", + "review" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "pull_number": { + "description": "Pull request number. Required when comment_type is 'review'.", + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_type", + "comment_id" + ], + "type": "object" + }, + "name": "unhide_comment" +} \ No newline at end of file diff --git a/pkg/github/comment_minimize.go b/pkg/github/comment_minimize.go new file mode 100644 index 0000000000..30b0c90950 --- /dev/null +++ b/pkg/github/comment_minimize.go @@ -0,0 +1,207 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/go-github/v89/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" +) + +const ( + CommentTypeIssueComment = "issue_comment" + CommentTypeReviewComment = "review_comment" + CommentTypeReview = "review" +) + +// MinimizeCommentResult is the response returned by the minimize_comment tool. +type MinimizeCommentResult struct { + NodeID string `json:"node_id"` + IsMinimized bool `json:"is_minimized"` + MinimizedReason string `json:"minimized_reason,omitempty"` +} + +var commentClassifiers = []any{"SPAM", "ABUSE", "OFF_TOPIC", "OUTDATED", "DUPLICATE", "RESOLVED", "LOW_QUALITY"} + +const commentVisibilityDescriptionSuffix = "Supports issue and pull request conversation comments, pull request review comments, and pull request review bodies. " + + "Requires triage or write access to the repository, or authorship of the comment." + +// commentTargetProperties returns the schema properties that identify the comment to hide or unhide. +func commentTargetProperties() map[string]*jsonschema.Schema { + return map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "comment_type": { + Type: "string", + Description: "The kind of comment:\n" + + "- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n" + + "- 'review_comment' - an inline comment on a pull request diff.\n" + + "- 'review' - the body of a pull request review. Requires 'pull_number'.", + Enum: []any{CommentTypeIssueComment, CommentTypeReviewComment, CommentTypeReview}, + }, + "comment_id": { + Type: "integer", + Description: "The numeric ID of the comment, or of the review when comment_type is 'review'", + Minimum: jsonschema.Ptr(1.0), + }, + "pull_number": { + Type: "number", + Description: "Pull request number. Required when comment_type is 'review'.", + }, + } +} + +// setCommentVisibility resolves the comment identified by args and hides or unhides it. +func setCommentVisibility(ctx context.Context, deps ToolDependencies, args map[string]any, hide bool, classifier string) *mcp.CallToolResult { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()) + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()) + } + commentType, err := RequiredParam[string](args, "comment_type") + if err != nil { + return utils.NewToolResultError(err.Error()) + } + commentID, err := RequiredBigInt(args, "comment_id") + if err != nil { + return utils.NewToolResultError(err.Error()) + } + pullNumber, err := OptionalIntParam(args, "pull_number") + if err != nil { + return utils.NewToolResultError(err.Error()) + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err) + } + + nodeID, errResult := resolveCommentNodeID(ctx, client, owner, repo, commentType, commentID, pullNumber) + if errResult != nil { + return errResult + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err) + } + + var result MinimizeCommentResult + if hide { + result, errResult = minimizeComment(ctx, gqlClient, nodeID, strings.ToUpper(classifier)) + } else { + result, errResult = unminimizeComment(ctx, gqlClient, nodeID) + } + if errResult != nil { + return errResult + } + + r, err := json.Marshal(result) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err) + } + return utils.NewToolResultText(string(r)) +} + +// resolveCommentNodeID looks up the GraphQL node ID for a comment identified by its REST ID, +// since the minimize mutations only accept node IDs. +func resolveCommentNodeID(ctx context.Context, client *github.Client, owner, repo, commentType string, commentID int64, pullNumber int) (string, *mcp.CallToolResult) { + var ( + nodeID string + resp *github.Response + err error + ) + + switch commentType { + case CommentTypeIssueComment: + var comment *github.IssueComment + comment, resp, err = client.Issues.GetComment(ctx, owner, repo, commentID) + nodeID = comment.GetNodeID() + case CommentTypeReviewComment: + var comment *github.PullRequestComment + comment, resp, err = client.PullRequests.GetComment(ctx, owner, repo, commentID) + nodeID = comment.GetNodeID() + case CommentTypeReview: + if pullNumber <= 0 { + return "", utils.NewToolResultError("pull_number is required when comment_type is 'review'") + } + var review *github.PullRequestReview + review, resp, err = client.PullRequests.GetReview(ctx, owner, repo, pullNumber, commentID) + nodeID = review.GetNodeID() + default: + return "", utils.NewToolResultError(fmt.Sprintf("unknown comment_type: %s", commentType)) + } + + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return "", ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get comment", resp, err) + } + if nodeID == "" { + return "", utils.NewToolResultError("comment has no node ID") + } + return nodeID, nil +} + +func minimizeComment(ctx context.Context, client *githubv4.Client, nodeID, classifier string) (MinimizeCommentResult, *mcp.CallToolResult) { + var mutation struct { + MinimizeComment struct { + MinimizedComment struct { + IsMinimized githubv4.Boolean + MinimizedReason githubv4.String + } + } `graphql:"minimizeComment(input: $input)"` + } + + input := githubv4.MinimizeCommentInput{ + SubjectID: githubv4.ID(nodeID), + Classifier: githubv4.ReportedContentClassifiers(classifier), + } + if err := client.Mutate(ctx, &mutation, input, nil); err != nil { + return MinimizeCommentResult{}, ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to minimize comment", err) + } + + return MinimizeCommentResult{ + NodeID: nodeID, + IsMinimized: bool(mutation.MinimizeComment.MinimizedComment.IsMinimized), + MinimizedReason: string(mutation.MinimizeComment.MinimizedComment.MinimizedReason), + }, nil +} + +func unminimizeComment(ctx context.Context, client *githubv4.Client, nodeID string) (MinimizeCommentResult, *mcp.CallToolResult) { + var mutation struct { + UnminimizeComment struct { + UnminimizedComment struct { + IsMinimized githubv4.Boolean + } + } `graphql:"unminimizeComment(input: $input)"` + } + + input := githubv4.UnminimizeCommentInput{ + SubjectID: githubv4.ID(nodeID), + } + if err := client.Mutate(ctx, &mutation, input, nil); err != nil { + return MinimizeCommentResult{}, ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to unminimize comment", err) + } + + return MinimizeCommentResult{ + NodeID: nodeID, + IsMinimized: bool(mutation.UnminimizeComment.UnminimizedComment.IsMinimized), + }, nil +} diff --git a/pkg/github/comment_minimize_test.go b/pkg/github/comment_minimize_test.go new file mode 100644 index 0000000000..f4ba8f9bff --- /dev/null +++ b/pkg/github/comment_minimize_test.go @@ -0,0 +1,334 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + "github.com/github/github-mcp-server/internal/githubv4mock" + "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v89/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/shurcooL/githubv4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + getIssueCommentRoute = "GET /repos/{owner}/{repo}/issues/comments/{comment_id}" + getReviewCommentRoute = "GET /repos/{owner}/{repo}/pulls/comments/{comment_id}" + getReviewRoute = "GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}" +) + +func minimizeCommentMatcher(nodeID, classifier string) githubv4mock.Matcher { + return githubv4mock.NewMutationMatcher( + struct { + MinimizeComment struct { + MinimizedComment struct { + IsMinimized githubv4.Boolean + MinimizedReason githubv4.String + } + } `graphql:"minimizeComment(input: $input)"` + }{}, + githubv4.MinimizeCommentInput{ + SubjectID: githubv4.ID(nodeID), + Classifier: githubv4.ReportedContentClassifiers(classifier), + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "minimizeComment": map[string]any{ + "minimizedComment": map[string]any{ + "isMinimized": true, + "minimizedReason": classifier, + }, + }, + }), + ) +} + +func unminimizeCommentMatcher(nodeID string) githubv4mock.Matcher { + return githubv4mock.NewMutationMatcher( + struct { + UnminimizeComment struct { + UnminimizedComment struct { + IsMinimized githubv4.Boolean + } + } `graphql:"unminimizeComment(input: $input)"` + }{}, + githubv4.UnminimizeCommentInput{SubjectID: githubv4.ID(nodeID)}, + nil, + githubv4mock.DataResponse(map[string]any{ + "unminimizeComment": map[string]any{ + "unminimizedComment": map[string]any{"isMinimized": false}, + }, + }), + ) +} + +func Test_CommentVisibilityToolSchemas(t *testing.T) { + hide := GranularHideComment(translations.NullTranslationHelper).Tool + require.NoError(t, toolsnaps.Test(hide.Name, hide)) + assert.Equal(t, "hide_comment", hide.Name) + assert.False(t, hide.Annotations.ReadOnlyHint) + assert.ElementsMatch(t, hide.InputSchema.(*jsonschema.Schema).Required, + []string{"owner", "repo", "comment_type", "comment_id", "classifier"}) + + unhide := GranularUnhideComment(translations.NullTranslationHelper).Tool + require.NoError(t, toolsnaps.Test(unhide.Name, unhide)) + assert.Equal(t, "unhide_comment", unhide.Name) + assert.False(t, unhide.Annotations.ReadOnlyHint) + unhideSchema := unhide.InputSchema.(*jsonschema.Schema) + assert.ElementsMatch(t, unhideSchema.Required, []string{"owner", "repo", "comment_type", "comment_id"}) + assert.NotContains(t, unhideSchema.Properties, "classifier") +} + +func Test_HideAndUnhideComment(t *testing.T) { + tests := []struct { + name string + tool inventory.ServerTool + restHandlers map[string]http.HandlerFunc + gqlMatchers []githubv4mock.Matcher + requestArgs map[string]any + expectedResult MinimizeCommentResult + expectedErrMsg string + }{ + { + name: "hide issue comment", + tool: GranularHideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1)), NodeID: github.Ptr("IC_1")}), + }, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("IC_1", "SPAM")}, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "issue_comment", "comment_id": float64(1), "classifier": "spam", + }, + expectedResult: MinimizeCommentResult{NodeID: "IC_1", IsMinimized: true, MinimizedReason: "SPAM"}, + }, + { + name: "hide review comment", + tool: GranularHideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getReviewCommentRoute: mockResponse(t, http.StatusOK, &github.PullRequestComment{ID: github.Ptr(int64(2)), NodeID: github.Ptr("PRRC_2")}), + }, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRRC_2", "OUTDATED")}, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "review_comment", "comment_id": float64(2), "classifier": "OUTDATED", + }, + expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: true, MinimizedReason: "OUTDATED"}, + }, + { + name: "unhide review", + tool: GranularUnhideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getReviewRoute: mockResponse(t, http.StatusOK, &github.PullRequestReview{ID: github.Ptr(int64(3)), NodeID: github.Ptr("PRR_3")}), + }, + gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("PRR_3")}, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "review", "comment_id": float64(3), "pull_number": float64(42), + }, + expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: false}, + }, + { + name: "hide review", + tool: GranularHideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getReviewRoute: mockResponse(t, http.StatusOK, &github.PullRequestReview{ID: github.Ptr(int64(3)), NodeID: github.Ptr("PRR_3")}), + }, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRR_3", "RESOLVED")}, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "review", "comment_id": float64(3), "pull_number": float64(42), "classifier": "RESOLVED", + }, + expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: true, MinimizedReason: "RESOLVED"}, + }, + { + name: "unhide review comment", + tool: GranularUnhideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getReviewCommentRoute: mockResponse(t, http.StatusOK, &github.PullRequestComment{ID: github.Ptr(int64(2)), NodeID: github.Ptr("PRRC_2")}), + }, + gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("PRRC_2")}, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "review_comment", "comment_id": float64(2), + }, + expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: false}, + }, + { + name: "hide mutation fails", + tool: GranularHideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1)), NodeID: github.Ptr("IC_1")}), + }, + gqlMatchers: []githubv4mock.Matcher{ + githubv4mock.NewMutationMatcher( + struct { + MinimizeComment struct { + MinimizedComment struct { + IsMinimized githubv4.Boolean + MinimizedReason githubv4.String + } + } `graphql:"minimizeComment(input: $input)"` + }{}, + githubv4.MinimizeCommentInput{SubjectID: githubv4.ID("IC_1"), Classifier: githubv4.ReportedContentClassifiersSpam}, + nil, + githubv4mock.ErrorResponse("Resource not accessible by integration"), + ), + }, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "issue_comment", "comment_id": float64(1), "classifier": "SPAM", + }, + expectedErrMsg: "failed to minimize comment", + }, + { + name: "unhide mutation fails", + tool: GranularUnhideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1)), NodeID: github.Ptr("IC_1")}), + }, + gqlMatchers: []githubv4mock.Matcher{ + githubv4mock.NewMutationMatcher( + struct { + UnminimizeComment struct { + UnminimizedComment struct { + IsMinimized githubv4.Boolean + } + } `graphql:"unminimizeComment(input: $input)"` + }{}, + githubv4.UnminimizeCommentInput{SubjectID: githubv4.ID("IC_1")}, + nil, + githubv4mock.ErrorResponse("Resource not accessible by integration"), + ), + }, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "issue_comment", "comment_id": float64(1), + }, + expectedErrMsg: "failed to unminimize comment", + }, + { + name: "comment without node ID", + tool: GranularUnhideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1))}), + }, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "issue_comment", "comment_id": float64(1), + }, + expectedErrMsg: "comment has no node ID", + }, + { + name: "missing owner", + tool: GranularUnhideComment(translations.NullTranslationHelper), + requestArgs: map[string]any{ + "repo": "repo", "comment_type": "issue_comment", "comment_id": float64(1), + }, + expectedErrMsg: "owner", + }, + { + name: "missing repo", + tool: GranularUnhideComment(translations.NullTranslationHelper), + requestArgs: map[string]any{ + "owner": "owner", "comment_type": "issue_comment", "comment_id": float64(1), + }, + expectedErrMsg: "repo", + }, + { + name: "missing comment_type", + tool: GranularUnhideComment(translations.NullTranslationHelper), + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", "comment_id": float64(1), + }, + expectedErrMsg: "comment_type", + }, + { + name: "missing comment_id", + tool: GranularHideComment(translations.NullTranslationHelper), + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", "comment_type": "issue_comment", "classifier": "SPAM", + }, + expectedErrMsg: "comment_id", + }, + { + name: "invalid pull_number type", + tool: GranularUnhideComment(translations.NullTranslationHelper), + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", "comment_type": "review", + "comment_id": float64(3), "pull_number": "forty-two", + }, + expectedErrMsg: "pull_number", + }, + { + name: "hide without classifier", + tool: GranularHideComment(translations.NullTranslationHelper), + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "issue_comment", "comment_id": float64(1), + }, + expectedErrMsg: "classifier", + }, + { + name: "review without pull_number", + tool: GranularUnhideComment(translations.NullTranslationHelper), + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "review", "comment_id": float64(3), + }, + expectedErrMsg: "pull_number is required", + }, + { + name: "unknown comment type", + tool: GranularUnhideComment(translations.NullTranslationHelper), + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "commit_comment", "comment_id": float64(1), + }, + expectedErrMsg: "unknown comment_type", + }, + { + name: "comment not found", + tool: GranularUnhideComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{ + getIssueCommentRoute: mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), + }, + requestArgs: map[string]any{ + "owner": "owner", "repo": "repo", + "comment_type": "issue_comment", "comment_id": float64(1), + }, + expectedErrMsg: "failed to get comment", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(tc.restHandlers)), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient(tc.gqlMatchers...)), + } + handler := tc.tool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + if tc.expectedErrMsg != "" { + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, tc.expectedErrMsg) + return + } + + require.False(t, result.IsError, getTextResult(t, result).Text) + var got MinimizeCommentResult + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &got)) + assert.Equal(t, tc.expectedResult, got) + }) + } +} diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 425f954ef9..e120b136f2 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -56,6 +56,8 @@ func TestGranularToolSnaps(t *testing.T) { GranularRemoveIssueReaction, GranularAddIssueCommentReaction, GranularRemoveIssueCommentReaction, + GranularHideComment, + GranularUnhideComment, GranularUpdatePullRequestTitle, GranularUpdatePullRequestBody, GranularUpdatePullRequestState, @@ -105,6 +107,8 @@ func TestIssuesGranularToolset(t *testing.T) { "remove_issue_reaction", "add_issue_comment_reaction", "remove_issue_comment_reaction", + "hide_comment", + "unhide_comment", } for _, name := range expected { assert.Contains(t, toolNames, name) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index f22a8a1536..152bd4aabd 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1901,3 +1901,70 @@ func GranularRemoveIssueCommentReaction(t translations.TranslationHelperFunc) in st.FeatureRule = issuesGranularFeatureRule return st } + +// GranularHideComment hides (minimizes) a comment on an issue or pull request. +func GranularHideComment(t translations.TranslationHelperFunc) inventory.ServerTool { + properties := commentTargetProperties() + properties["classifier"] = &jsonschema.Schema{ + Type: "string", + Description: "The reason for hiding the comment", + Enum: commentClassifiers, + } + + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "hide_comment", + Description: t("TOOL_HIDE_COMMENT_DESCRIPTION", "Hide (minimize) a comment on an issue or pull request. "+commentVisibilityDescriptionSuffix), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_HIDE_COMMENT_USER_TITLE", "Hide Comment"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: properties, + Required: []string{"owner", "repo", "comment_type", "comment_id", "classifier"}, + }, + }, + scopes.RequireAll(scopes.Repo), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + classifier, err := RequiredParam[string](args, "classifier") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + return setCommentVisibility(ctx, deps, args, true, classifier), nil, nil + }, + ) + st.FeatureRule = issuesGranularFeatureRule + return st +} + +// GranularUnhideComment unhides (unminimizes) a previously hidden comment on an issue or pull request. +func GranularUnhideComment(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "unhide_comment", + Description: t("TOOL_UNHIDE_COMMENT_DESCRIPTION", "Unhide (unminimize) a previously hidden comment on an issue or pull request. "+commentVisibilityDescriptionSuffix), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_UNHIDE_COMMENT_USER_TITLE", "Unhide Comment"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: commentTargetProperties(), + Required: []string{"owner", "repo", "comment_type", "comment_id"}, + }, + }, + scopes.RequireAll(scopes.Repo), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + return setCommentVisibility(ctx, deps, args, false, ""), nil, nil + }, + ) + st.FeatureRule = issuesGranularFeatureRule + return st +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index b91664a67f..8d0d15d574 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -376,6 +376,8 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GranularRemoveIssueReaction(t), GranularAddIssueCommentReaction(t), GranularRemoveIssueCommentReaction(t), + GranularHideComment(t), + GranularUnhideComment(t), // Granular pull request tools (feature-flagged, replace consolidated update_pull_request/pull_request_review_write) GranularUpdatePullRequestTitle(t), From d6eebd1aaa481b1e735f738f8b2952d046913f6a Mon Sep 17 00:00:00 2001 From: timrogers <116134+timrogers@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:51:06 +0000 Subject: [PATCH 2/5] Rename comment_type enum values to pull_request_review(_comment) Use `pull_request_review` and `pull_request_review_comment` instead of `review` and `review_comment` so the comment_type values are explicit about which object they refer to. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 16 ++++++------- pkg/github/__toolsnaps__/hide_comment.snap | 10 ++++---- pkg/github/__toolsnaps__/unhide_comment.snap | 10 ++++---- pkg/github/comment_minimize.go | 24 ++++++++++---------- pkg/github/comment_minimize_test.go | 12 +++++----- 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index dcbfd0ca8d..7284bedf8d 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -187,13 +187,13 @@ as output formatting) won't appear here. - **hide_comment** - Hide Comment - **OAuth Challenge Scopes**: `repo` - `classifier`: The reason for hiding the comment (string, required) - - `comment_id`: The numeric ID of the comment, or of the review when comment_type is 'review' (integer, required) + - `comment_id`: The numeric ID of the comment, or of the review when comment_type is 'pull_request_review' (integer, required) - `comment_type`: The kind of comment: - 'issue_comment' - a comment on an issue, or a conversation comment on a pull request. - - 'review_comment' - an inline comment on a pull request diff. - - 'review' - the body of a pull request review. Requires 'pull_number'. (string, required) + - 'pull_request_review_comment' - an inline comment on a pull request diff. + - 'pull_request_review' - the body of a pull request review. Requires 'pull_number'. (string, required) - `owner`: Repository owner (string, required) - - `pull_number`: Pull request number. Required when comment_type is 'review'. (number, optional) + - `pull_number`: Pull request number. Required when comment_type is 'pull_request_review'. (number, optional) - `repo`: Repository name (string, required) - **remove_issue_comment_reaction** - Remove Reaction from Issue or Pull Request Comment @@ -235,13 +235,13 @@ as output formatting) won't appear here. - **unhide_comment** - Unhide Comment - **OAuth Challenge Scopes**: `repo` - - `comment_id`: The numeric ID of the comment, or of the review when comment_type is 'review' (integer, required) + - `comment_id`: The numeric ID of the comment, or of the review when comment_type is 'pull_request_review' (integer, required) - `comment_type`: The kind of comment: - 'issue_comment' - a comment on an issue, or a conversation comment on a pull request. - - 'review_comment' - an inline comment on a pull request diff. - - 'review' - the body of a pull request review. Requires 'pull_number'. (string, required) + - 'pull_request_review_comment' - an inline comment on a pull request diff. + - 'pull_request_review' - the body of a pull request review. Requires 'pull_number'. (string, required) - `owner`: Repository owner (string, required) - - `pull_number`: Pull request number. Required when comment_type is 'review'. (number, optional) + - `pull_number`: Pull request number. Required when comment_type is 'pull_request_review'. (number, optional) - `repo`: Repository name (string, required) - **update_issue_assignees** - Update Issue Assignees diff --git a/pkg/github/__toolsnaps__/hide_comment.snap b/pkg/github/__toolsnaps__/hide_comment.snap index 26dcf58dec..ccc7e4dc41 100644 --- a/pkg/github/__toolsnaps__/hide_comment.snap +++ b/pkg/github/__toolsnaps__/hide_comment.snap @@ -23,16 +23,16 @@ "type": "string" }, "comment_id": { - "description": "The numeric ID of the comment, or of the review when comment_type is 'review'", + "description": "The numeric ID of the comment, or of the review when comment_type is 'pull_request_review'", "minimum": 1, "type": "integer" }, "comment_type": { - "description": "The kind of comment:\n- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n- 'review_comment' - an inline comment on a pull request diff.\n- 'review' - the body of a pull request review. Requires 'pull_number'.", + "description": "The kind of comment:\n- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n- 'pull_request_review_comment' - an inline comment on a pull request diff.\n- 'pull_request_review' - the body of a pull request review. Requires 'pull_number'.", "enum": [ "issue_comment", - "review_comment", - "review" + "pull_request_review_comment", + "pull_request_review" ], "type": "string" }, @@ -41,7 +41,7 @@ "type": "string" }, "pull_number": { - "description": "Pull request number. Required when comment_type is 'review'.", + "description": "Pull request number. Required when comment_type is 'pull_request_review'.", "type": "number" }, "repo": { diff --git a/pkg/github/__toolsnaps__/unhide_comment.snap b/pkg/github/__toolsnaps__/unhide_comment.snap index 6cd1760745..b5b8ebb7b3 100644 --- a/pkg/github/__toolsnaps__/unhide_comment.snap +++ b/pkg/github/__toolsnaps__/unhide_comment.snap @@ -10,16 +10,16 @@ "inputSchema": { "properties": { "comment_id": { - "description": "The numeric ID of the comment, or of the review when comment_type is 'review'", + "description": "The numeric ID of the comment, or of the review when comment_type is 'pull_request_review'", "minimum": 1, "type": "integer" }, "comment_type": { - "description": "The kind of comment:\n- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n- 'review_comment' - an inline comment on a pull request diff.\n- 'review' - the body of a pull request review. Requires 'pull_number'.", + "description": "The kind of comment:\n- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n- 'pull_request_review_comment' - an inline comment on a pull request diff.\n- 'pull_request_review' - the body of a pull request review. Requires 'pull_number'.", "enum": [ "issue_comment", - "review_comment", - "review" + "pull_request_review_comment", + "pull_request_review" ], "type": "string" }, @@ -28,7 +28,7 @@ "type": "string" }, "pull_number": { - "description": "Pull request number. Required when comment_type is 'review'.", + "description": "Pull request number. Required when comment_type is 'pull_request_review'.", "type": "number" }, "repo": { diff --git a/pkg/github/comment_minimize.go b/pkg/github/comment_minimize.go index 30b0c90950..13e3aa4bb7 100644 --- a/pkg/github/comment_minimize.go +++ b/pkg/github/comment_minimize.go @@ -15,12 +15,12 @@ import ( ) const ( - CommentTypeIssueComment = "issue_comment" - CommentTypeReviewComment = "review_comment" - CommentTypeReview = "review" + CommentTypeIssueComment = "issue_comment" + CommentTypePullRequestReviewComment = "pull_request_review_comment" + CommentTypePullRequestReview = "pull_request_review" ) -// MinimizeCommentResult is the response returned by the minimize_comment tool. +// MinimizeCommentResult is the response returned by the hide_comment and unhide_comment tools. type MinimizeCommentResult struct { NodeID string `json:"node_id"` IsMinimized bool `json:"is_minimized"` @@ -47,18 +47,18 @@ func commentTargetProperties() map[string]*jsonschema.Schema { Type: "string", Description: "The kind of comment:\n" + "- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n" + - "- 'review_comment' - an inline comment on a pull request diff.\n" + - "- 'review' - the body of a pull request review. Requires 'pull_number'.", - Enum: []any{CommentTypeIssueComment, CommentTypeReviewComment, CommentTypeReview}, + "- 'pull_request_review_comment' - an inline comment on a pull request diff.\n" + + "- 'pull_request_review' - the body of a pull request review. Requires 'pull_number'.", + Enum: []any{CommentTypeIssueComment, CommentTypePullRequestReviewComment, CommentTypePullRequestReview}, }, "comment_id": { Type: "integer", - Description: "The numeric ID of the comment, or of the review when comment_type is 'review'", + Description: "The numeric ID of the comment, or of the review when comment_type is 'pull_request_review'", Minimum: jsonschema.Ptr(1.0), }, "pull_number": { Type: "number", - Description: "Pull request number. Required when comment_type is 'review'.", + Description: "Pull request number. Required when comment_type is 'pull_request_review'.", }, } } @@ -132,13 +132,13 @@ func resolveCommentNodeID(ctx context.Context, client *github.Client, owner, rep var comment *github.IssueComment comment, resp, err = client.Issues.GetComment(ctx, owner, repo, commentID) nodeID = comment.GetNodeID() - case CommentTypeReviewComment: + case CommentTypePullRequestReviewComment: var comment *github.PullRequestComment comment, resp, err = client.PullRequests.GetComment(ctx, owner, repo, commentID) nodeID = comment.GetNodeID() - case CommentTypeReview: + case CommentTypePullRequestReview: if pullNumber <= 0 { - return "", utils.NewToolResultError("pull_number is required when comment_type is 'review'") + return "", utils.NewToolResultError("pull_number is required when comment_type is 'pull_request_review'") } var review *github.PullRequestReview review, resp, err = client.PullRequests.GetReview(ctx, owner, repo, pullNumber, commentID) diff --git a/pkg/github/comment_minimize_test.go b/pkg/github/comment_minimize_test.go index f4ba8f9bff..4fe260a046 100644 --- a/pkg/github/comment_minimize_test.go +++ b/pkg/github/comment_minimize_test.go @@ -117,7 +117,7 @@ func Test_HideAndUnhideComment(t *testing.T) { gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRRC_2", "OUTDATED")}, requestArgs: map[string]any{ "owner": "owner", "repo": "repo", - "comment_type": "review_comment", "comment_id": float64(2), "classifier": "OUTDATED", + "comment_type": "pull_request_review_comment", "comment_id": float64(2), "classifier": "OUTDATED", }, expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: true, MinimizedReason: "OUTDATED"}, }, @@ -130,7 +130,7 @@ func Test_HideAndUnhideComment(t *testing.T) { gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("PRR_3")}, requestArgs: map[string]any{ "owner": "owner", "repo": "repo", - "comment_type": "review", "comment_id": float64(3), "pull_number": float64(42), + "comment_type": "pull_request_review", "comment_id": float64(3), "pull_number": float64(42), }, expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: false}, }, @@ -143,7 +143,7 @@ func Test_HideAndUnhideComment(t *testing.T) { gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRR_3", "RESOLVED")}, requestArgs: map[string]any{ "owner": "owner", "repo": "repo", - "comment_type": "review", "comment_id": float64(3), "pull_number": float64(42), "classifier": "RESOLVED", + "comment_type": "pull_request_review", "comment_id": float64(3), "pull_number": float64(42), "classifier": "RESOLVED", }, expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: true, MinimizedReason: "RESOLVED"}, }, @@ -156,7 +156,7 @@ func Test_HideAndUnhideComment(t *testing.T) { gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("PRRC_2")}, requestArgs: map[string]any{ "owner": "owner", "repo": "repo", - "comment_type": "review_comment", "comment_id": float64(2), + "comment_type": "pull_request_review_comment", "comment_id": float64(2), }, expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: false}, }, @@ -261,7 +261,7 @@ func Test_HideAndUnhideComment(t *testing.T) { name: "invalid pull_number type", tool: GranularUnhideComment(translations.NullTranslationHelper), requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", "comment_type": "review", + "owner": "owner", "repo": "repo", "comment_type": "pull_request_review", "comment_id": float64(3), "pull_number": "forty-two", }, expectedErrMsg: "pull_number", @@ -280,7 +280,7 @@ func Test_HideAndUnhideComment(t *testing.T) { tool: GranularUnhideComment(translations.NullTranslationHelper), requestArgs: map[string]any{ "owner": "owner", "repo": "repo", - "comment_type": "review", "comment_id": float64(3), + "comment_type": "pull_request_review", "comment_id": float64(3), }, expectedErrMsg: "pull_number is required", }, From 27c0dd5cf0692d0f9f5d593c26743060f488d72c Mon Sep 17 00:00:00 2001 From: timrogers <116134+timrogers@users.noreply.github.com> Date: Wed, 30 Sep 2026 01:21:55 +0000 Subject: [PATCH 3/5] Use realistic minimizedReason values in hide_comment test mocks GitHub returns minimizedReason in lowercase, hyphenated form (e.g. "off-topic") rather than echoing the classifier enum, as confirmed by an end-to-end run against the live API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/comment_minimize_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/github/comment_minimize_test.go b/pkg/github/comment_minimize_test.go index 4fe260a046..679d202fd9 100644 --- a/pkg/github/comment_minimize_test.go +++ b/pkg/github/comment_minimize_test.go @@ -23,7 +23,8 @@ const ( getReviewRoute = "GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}" ) -func minimizeCommentMatcher(nodeID, classifier string) githubv4mock.Matcher { +// minimizedReason is the lowercase, hyphenated form GitHub returns (e.g. "off-topic"), not the classifier enum. +func minimizeCommentMatcher(nodeID, classifier, minimizedReason string) githubv4mock.Matcher { return githubv4mock.NewMutationMatcher( struct { MinimizeComment struct { @@ -42,7 +43,7 @@ func minimizeCommentMatcher(nodeID, classifier string) githubv4mock.Matcher { "minimizeComment": map[string]any{ "minimizedComment": map[string]any{ "isMinimized": true, - "minimizedReason": classifier, + "minimizedReason": minimizedReason, }, }, }), @@ -101,12 +102,12 @@ func Test_HideAndUnhideComment(t *testing.T) { restHandlers: map[string]http.HandlerFunc{ getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1)), NodeID: github.Ptr("IC_1")}), }, - gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("IC_1", "SPAM")}, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("IC_1", "SPAM", "spam")}, requestArgs: map[string]any{ "owner": "owner", "repo": "repo", "comment_type": "issue_comment", "comment_id": float64(1), "classifier": "spam", }, - expectedResult: MinimizeCommentResult{NodeID: "IC_1", IsMinimized: true, MinimizedReason: "SPAM"}, + expectedResult: MinimizeCommentResult{NodeID: "IC_1", IsMinimized: true, MinimizedReason: "spam"}, }, { name: "hide review comment", @@ -114,12 +115,12 @@ func Test_HideAndUnhideComment(t *testing.T) { restHandlers: map[string]http.HandlerFunc{ getReviewCommentRoute: mockResponse(t, http.StatusOK, &github.PullRequestComment{ID: github.Ptr(int64(2)), NodeID: github.Ptr("PRRC_2")}), }, - gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRRC_2", "OUTDATED")}, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRRC_2", "OUTDATED", "outdated")}, requestArgs: map[string]any{ "owner": "owner", "repo": "repo", "comment_type": "pull_request_review_comment", "comment_id": float64(2), "classifier": "OUTDATED", }, - expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: true, MinimizedReason: "OUTDATED"}, + expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: true, MinimizedReason: "outdated"}, }, { name: "unhide review", @@ -140,12 +141,12 @@ func Test_HideAndUnhideComment(t *testing.T) { restHandlers: map[string]http.HandlerFunc{ getReviewRoute: mockResponse(t, http.StatusOK, &github.PullRequestReview{ID: github.Ptr(int64(3)), NodeID: github.Ptr("PRR_3")}), }, - gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRR_3", "RESOLVED")}, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRR_3", "RESOLVED", "resolved")}, requestArgs: map[string]any{ "owner": "owner", "repo": "repo", "comment_type": "pull_request_review", "comment_id": float64(3), "pull_number": float64(42), "classifier": "RESOLVED", }, - expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: true, MinimizedReason: "RESOLVED"}, + expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: true, MinimizedReason: "resolved"}, }, { name: "unhide review comment", From 07bf50e436f896f1a41d12d16c29e737ef3c2e20 Mon Sep 17 00:00:00 2001 From: timrogers <116134+timrogers@users.noreply.github.com> Date: Wed, 30 Sep 2026 04:27:57 +0000 Subject: [PATCH 4/5] Split hide/unhide comment tools per object type Replace hide_comment/unhide_comment, which took a comment_type argument, with a granular pair per object type: - hide_issue_comment / unhide_issue_comment (issues_granular): issue comments and pull request conversation comments - hide_pull_request_review_comment / unhide_pull_request_review_comment (pull_requests_granular): inline pull request review comments - hide_pull_request_review / unhide_pull_request_review (pull_requests_granular): pull request review bodies Every argument is now unconditionally required, so the schemas no longer need prose explaining which arguments apply to which type. The six tools are built from one shared helper, parameterised by a commentVisibilityTarget that describes each object type and how to resolve its GraphQL node ID. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 50 ++- pkg/github/__toolsnaps__/hide_comment.snap | 62 --- .../__toolsnaps__/hide_issue_comment.snap | 48 +++ .../hide_pull_request_review.snap | 54 +++ .../hide_pull_request_review_comment.snap | 48 +++ pkg/github/__toolsnaps__/unhide_comment.snap | 48 --- .../__toolsnaps__/unhide_issue_comment.snap | 34 ++ .../unhide_pull_request_review.snap | 40 ++ .../unhide_pull_request_review_comment.snap | 34 ++ pkg/github/comment_minimize.go | 244 +++++++---- pkg/github/comment_minimize_test.go | 386 +++++++++--------- pkg/github/granular_tools_test.go | 16 +- pkg/github/issues_granular.go | 69 +--- pkg/github/pullrequests_granular.go | 20 + pkg/github/tools.go | 8 +- 15 files changed, 707 insertions(+), 454 deletions(-) delete mode 100644 pkg/github/__toolsnaps__/hide_comment.snap create mode 100644 pkg/github/__toolsnaps__/hide_issue_comment.snap create mode 100644 pkg/github/__toolsnaps__/hide_pull_request_review.snap create mode 100644 pkg/github/__toolsnaps__/hide_pull_request_review_comment.snap delete mode 100644 pkg/github/__toolsnaps__/unhide_comment.snap create mode 100644 pkg/github/__toolsnaps__/unhide_issue_comment.snap create mode 100644 pkg/github/__toolsnaps__/unhide_pull_request_review.snap create mode 100644 pkg/github/__toolsnaps__/unhide_pull_request_review_comment.snap diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 7284bedf8d..272e7153a5 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -184,16 +184,11 @@ as output formatting) won't appear here. - `repo`: Repository name (string, required) - `title`: Issue title (string, required) -- **hide_comment** - Hide Comment +- **hide_issue_comment** - Hide Issue Comment - **OAuth Challenge Scopes**: `repo` - `classifier`: The reason for hiding the comment (string, required) - - `comment_id`: The numeric ID of the comment, or of the review when comment_type is 'pull_request_review' (integer, required) - - `comment_type`: The kind of comment: - - 'issue_comment' - a comment on an issue, or a conversation comment on a pull request. - - 'pull_request_review_comment' - an inline comment on a pull request diff. - - 'pull_request_review' - the body of a pull request review. Requires 'pull_number'. (string, required) - - `owner`: Repository owner (string, required) - - `pull_number`: Pull request number. Required when comment_type is 'pull_request_review'. (number, optional) + - `comment_id`: The numeric ID of the issue or pull request conversation comment (number, required) + - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **remove_issue_comment_reaction** - Remove Reaction from Issue or Pull Request Comment @@ -233,15 +228,10 @@ as output formatting) won't appear here. - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) -- **unhide_comment** - Unhide Comment +- **unhide_issue_comment** - Unhide Issue Comment - **OAuth Challenge Scopes**: `repo` - - `comment_id`: The numeric ID of the comment, or of the review when comment_type is 'pull_request_review' (integer, required) - - `comment_type`: The kind of comment: - - 'issue_comment' - a comment on an issue, or a conversation comment on a pull request. - - 'pull_request_review_comment' - an inline comment on a pull request diff. - - 'pull_request_review' - the body of a pull request review. Requires 'pull_number'. (string, required) - - `owner`: Repository owner (string, required) - - `pull_number`: Pull request number. Required when comment_type is 'pull_request_review'. (number, optional) + - `comment_id`: The numeric ID of the issue or pull request conversation comment (number, required) + - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - **update_issue_assignees** - Update Issue Assignees @@ -338,6 +328,21 @@ as output formatting) won't appear here. - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) +- **hide_pull_request_review** - Hide Pull Request Review + - **OAuth Challenge Scopes**: `repo` + - `classifier`: The reason for hiding the comment (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `review_id`: The numeric ID of the pull request review (number, required) + +- **hide_pull_request_review_comment** - Hide Pull Request Review Comment + - **OAuth Challenge Scopes**: `repo` + - `classifier`: The reason for hiding the comment (string, 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) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - **remove_pull_request_review_comment_reaction** - Remove Pull Request Review Comment Reaction - **OAuth Challenge Scopes**: `repo` - `comment_id`: The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...). (number, required) @@ -364,6 +369,19 @@ as output formatting) won't appear here. - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) +- **unhide_pull_request_review** - Unhide Pull Request Review + - **OAuth Challenge Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `review_id`: The numeric ID of the pull request review (number, required) + +- **unhide_pull_request_review_comment** - Unhide Pull Request Review Comment + - **OAuth Challenge Scopes**: `repo` + - `comment_id`: The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...). (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - **unresolve_review_thread** - Unresolve Review Thread - **OAuth Challenge Scopes**: `repo` - `threadID`: The node ID of the review thread to unresolve (e.g., PRRT_kwDOxxx) (string, required) diff --git a/pkg/github/__toolsnaps__/hide_comment.snap b/pkg/github/__toolsnaps__/hide_comment.snap deleted file mode 100644 index ccc7e4dc41..0000000000 --- a/pkg/github/__toolsnaps__/hide_comment.snap +++ /dev/null @@ -1,62 +0,0 @@ -{ - "annotations": { - "destructiveHint": false, - "idempotentHint": false, - "openWorldHint": true, - "readOnlyHint": false, - "title": "Hide Comment" - }, - "description": "Hide (minimize) a comment on an issue or pull request. Supports issue and pull request conversation comments, pull request review comments, and pull request review bodies. Requires triage or write access to the repository, or authorship of the comment.", - "inputSchema": { - "properties": { - "classifier": { - "description": "The reason for hiding the comment", - "enum": [ - "SPAM", - "ABUSE", - "OFF_TOPIC", - "OUTDATED", - "DUPLICATE", - "RESOLVED", - "LOW_QUALITY" - ], - "type": "string" - }, - "comment_id": { - "description": "The numeric ID of the comment, or of the review when comment_type is 'pull_request_review'", - "minimum": 1, - "type": "integer" - }, - "comment_type": { - "description": "The kind of comment:\n- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n- 'pull_request_review_comment' - an inline comment on a pull request diff.\n- 'pull_request_review' - the body of a pull request review. Requires 'pull_number'.", - "enum": [ - "issue_comment", - "pull_request_review_comment", - "pull_request_review" - ], - "type": "string" - }, - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pull_number": { - "description": "Pull request number. Required when comment_type is 'pull_request_review'.", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "comment_type", - "comment_id", - "classifier" - ], - "type": "object" - }, - "name": "hide_comment" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/hide_issue_comment.snap b/pkg/github/__toolsnaps__/hide_issue_comment.snap new file mode 100644 index 0000000000..bb66c3fc22 --- /dev/null +++ b/pkg/github/__toolsnaps__/hide_issue_comment.snap @@ -0,0 +1,48 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Hide Issue Comment" + }, + "description": "Hide (minimize) a comment on an issue, or a conversation comment on a pull request. Requires triage or write access to the repository, or being its author.", + "inputSchema": { + "properties": { + "classifier": { + "description": "The reason for hiding the comment", + "enum": [ + "SPAM", + "ABUSE", + "OFF_TOPIC", + "OUTDATED", + "DUPLICATE", + "RESOLVED", + "LOW_QUALITY" + ], + "type": "string" + }, + "comment_id": { + "description": "The numeric ID of the issue or pull request conversation comment", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "classifier" + ], + "type": "object" + }, + "name": "hide_issue_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/hide_pull_request_review.snap b/pkg/github/__toolsnaps__/hide_pull_request_review.snap new file mode 100644 index 0000000000..09ecfdf6cd --- /dev/null +++ b/pkg/github/__toolsnaps__/hide_pull_request_review.snap @@ -0,0 +1,54 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Hide Pull Request Review" + }, + "description": "Hide (minimize) the body of a submitted pull request review. Requires triage or write access to the repository, or being its author.", + "inputSchema": { + "properties": { + "classifier": { + "description": "The reason for hiding the comment", + "enum": [ + "SPAM", + "ABUSE", + "OFF_TOPIC", + "OUTDATED", + "DUPLICATE", + "RESOLVED", + "LOW_QUALITY" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "review_id": { + "description": "The numeric ID of the pull request review", + "minimum": 1, + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "review_id", + "classifier" + ], + "type": "object" + }, + "name": "hide_pull_request_review" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/hide_pull_request_review_comment.snap b/pkg/github/__toolsnaps__/hide_pull_request_review_comment.snap new file mode 100644 index 0000000000..46c39c9885 --- /dev/null +++ b/pkg/github/__toolsnaps__/hide_pull_request_review_comment.snap @@ -0,0 +1,48 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Hide Pull Request Review Comment" + }, + "description": "Hide (minimize) an inline review comment on a pull request diff. Requires triage or write access to the repository, or being its author.", + "inputSchema": { + "properties": { + "classifier": { + "description": "The reason for hiding the comment", + "enum": [ + "SPAM", + "ABUSE", + "OFF_TOPIC", + "OUTDATED", + "DUPLICATE", + "RESOLVED", + "LOW_QUALITY" + ], + "type": "string" + }, + "comment_id": { + "description": "The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...).", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "classifier" + ], + "type": "object" + }, + "name": "hide_pull_request_review_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/unhide_comment.snap b/pkg/github/__toolsnaps__/unhide_comment.snap deleted file mode 100644 index b5b8ebb7b3..0000000000 --- a/pkg/github/__toolsnaps__/unhide_comment.snap +++ /dev/null @@ -1,48 +0,0 @@ -{ - "annotations": { - "destructiveHint": false, - "idempotentHint": false, - "openWorldHint": true, - "readOnlyHint": false, - "title": "Unhide Comment" - }, - "description": "Unhide (unminimize) a previously hidden comment on an issue or pull request. Supports issue and pull request conversation comments, pull request review comments, and pull request review bodies. Requires triage or write access to the repository, or authorship of the comment.", - "inputSchema": { - "properties": { - "comment_id": { - "description": "The numeric ID of the comment, or of the review when comment_type is 'pull_request_review'", - "minimum": 1, - "type": "integer" - }, - "comment_type": { - "description": "The kind of comment:\n- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n- 'pull_request_review_comment' - an inline comment on a pull request diff.\n- 'pull_request_review' - the body of a pull request review. Requires 'pull_number'.", - "enum": [ - "issue_comment", - "pull_request_review_comment", - "pull_request_review" - ], - "type": "string" - }, - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pull_number": { - "description": "Pull request number. Required when comment_type is 'pull_request_review'.", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "comment_type", - "comment_id" - ], - "type": "object" - }, - "name": "unhide_comment" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/unhide_issue_comment.snap b/pkg/github/__toolsnaps__/unhide_issue_comment.snap new file mode 100644 index 0000000000..ea258d12f3 --- /dev/null +++ b/pkg/github/__toolsnaps__/unhide_issue_comment.snap @@ -0,0 +1,34 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Unhide Issue Comment" + }, + "description": "Unhide (unminimize) a previously hidden comment on an issue, or conversation comment on a pull request. Requires triage or write access to the repository, or being its author.", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The numeric ID of the issue or pull request conversation comment", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id" + ], + "type": "object" + }, + "name": "unhide_issue_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/unhide_pull_request_review.snap b/pkg/github/__toolsnaps__/unhide_pull_request_review.snap new file mode 100644 index 0000000000..2e627d519f --- /dev/null +++ b/pkg/github/__toolsnaps__/unhide_pull_request_review.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Unhide Pull Request Review" + }, + "description": "Unhide (unminimize) the previously hidden body of a submitted pull request review. Requires triage or write access to the repository, or being its author.", + "inputSchema": { + "properties": { + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "review_id": { + "description": "The numeric ID of the pull request review", + "minimum": 1, + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "review_id" + ], + "type": "object" + }, + "name": "unhide_pull_request_review" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/unhide_pull_request_review_comment.snap b/pkg/github/__toolsnaps__/unhide_pull_request_review_comment.snap new file mode 100644 index 0000000000..38ff864f15 --- /dev/null +++ b/pkg/github/__toolsnaps__/unhide_pull_request_review_comment.snap @@ -0,0 +1,34 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Unhide Pull Request Review Comment" + }, + "description": "Unhide (unminimize) a previously hidden inline review comment on a pull request diff. Requires triage or write access to the repository, or being its author.", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...).", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id" + ], + "type": "object" + }, + "name": "unhide_pull_request_review_comment" +} \ No newline at end of file diff --git a/pkg/github/comment_minimize.go b/pkg/github/comment_minimize.go index 13e3aa4bb7..5a48efbff8 100644 --- a/pkg/github/comment_minimize.go +++ b/pkg/github/comment_minimize.go @@ -3,10 +3,13 @@ package github import ( "context" "encoding/json" - "fmt" + "maps" "strings" ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" + "github.com/github/github-mcp-server/pkg/translations" "github.com/github/github-mcp-server/pkg/utils" "github.com/google/go-github/v89/github" "github.com/google/jsonschema-go/jsonschema" @@ -14,13 +17,7 @@ import ( "github.com/shurcooL/githubv4" ) -const ( - CommentTypeIssueComment = "issue_comment" - CommentTypePullRequestReviewComment = "pull_request_review_comment" - CommentTypePullRequestReview = "pull_request_review" -) - -// MinimizeCommentResult is the response returned by the hide_comment and unhide_comment tools. +// MinimizeCommentResult is the response returned by the hide and unhide comment tools. type MinimizeCommentResult struct { NodeID string `json:"node_id"` IsMinimized bool `json:"is_minimized"` @@ -29,42 +26,170 @@ type MinimizeCommentResult struct { var commentClassifiers = []any{"SPAM", "ABUSE", "OFF_TOPIC", "OUTDATED", "DUPLICATE", "RESOLVED", "LOW_QUALITY"} -const commentVisibilityDescriptionSuffix = "Supports issue and pull request conversation comments, pull request review comments, and pull request review bodies. " + - "Requires triage or write access to the repository, or authorship of the comment." +const commentVisibilityPermissionNote = " Requires triage or write access to the repository, or being its author." + +// commentVisibilityTarget describes one kind of hideable object: how the tools that +// hide and unhide it are named and described, and how to find its GraphQL node ID. +type commentVisibilityTarget struct { + toolset inventory.ToolsetMetadata + featureRule inventory.FeatureRule + // name is appended to "hide_" and "unhide_" to form the tool names. + name string + title string + hideDescription string + unhideDescription string + properties func() map[string]*jsonschema.Schema + required []string + resolveNodeID func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) +} + +var issueCommentVisibilityTarget = commentVisibilityTarget{ + toolset: ToolsetMetadataIssues, + featureRule: issuesGranularFeatureRule, + name: "issue_comment", + title: "Issue Comment", + hideDescription: "Hide (minimize) a comment on an issue, or a conversation comment on a pull request.", + unhideDescription: "Unhide (unminimize) a previously hidden comment on an issue, or conversation comment on a pull request.", + properties: func() map[string]*jsonschema.Schema { + return map[string]*jsonschema.Schema{ + "comment_id": { + Type: "number", + Description: "The numeric ID of the issue or pull request conversation comment", + Minimum: jsonschema.Ptr(1.0), + }, + } + }, + required: []string{"comment_id"}, + resolveNodeID: func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) { + commentID, err := RequiredBigInt(args, "comment_id") + if err != nil { + return "", utils.NewToolResultError(err.Error()) + } + comment, resp, err := client.Issues.GetComment(ctx, owner, repo, commentID) + return nodeIDFromResponse(ctx, "failed to get issue comment", comment.GetNodeID(), resp, err) + }, +} + +var pullRequestReviewCommentVisibilityTarget = commentVisibilityTarget{ + toolset: ToolsetMetadataPullRequests, + featureRule: pullRequestsGranularFeatureRule, + name: "pull_request_review_comment", + title: "Pull Request Review Comment", + hideDescription: "Hide (minimize) an inline review comment on a pull request diff.", + unhideDescription: "Unhide (unminimize) a previously hidden inline review comment on a pull request diff.", + properties: func() map[string]*jsonschema.Schema { + return map[string]*jsonschema.Schema{ + "comment_id": { + Type: "number", + Description: "The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...).", + Minimum: jsonschema.Ptr(1.0), + }, + } + }, + required: []string{"comment_id"}, + resolveNodeID: func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) { + commentID, err := RequiredBigInt(args, "comment_id") + if err != nil { + return "", utils.NewToolResultError(err.Error()) + } + comment, resp, err := client.PullRequests.GetComment(ctx, owner, repo, commentID) + return nodeIDFromResponse(ctx, "failed to get pull request review comment", comment.GetNodeID(), resp, err) + }, +} + +var pullRequestReviewVisibilityTarget = commentVisibilityTarget{ + toolset: ToolsetMetadataPullRequests, + featureRule: pullRequestsGranularFeatureRule, + name: "pull_request_review", + title: "Pull Request Review", + hideDescription: "Hide (minimize) the body of a submitted pull request review.", + unhideDescription: "Unhide (unminimize) the previously hidden body of a submitted pull request review.", + properties: func() map[string]*jsonschema.Schema { + return map[string]*jsonschema.Schema{ + "pullNumber": { + Type: "number", + Description: "The pull request number", + Minimum: jsonschema.Ptr(1.0), + }, + "review_id": { + Type: "number", + Description: "The numeric ID of the pull request review", + Minimum: jsonschema.Ptr(1.0), + }, + } + }, + required: []string{"pullNumber", "review_id"}, + resolveNodeID: func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) { + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return "", utils.NewToolResultError(err.Error()) + } + reviewID, err := RequiredBigInt(args, "review_id") + if err != nil { + return "", utils.NewToolResultError(err.Error()) + } + review, resp, err := client.PullRequests.GetReview(ctx, owner, repo, pullNumber, reviewID) + return nodeIDFromResponse(ctx, "failed to get pull request review", review.GetNodeID(), resp, err) + }, +} -// commentTargetProperties returns the schema properties that identify the comment to hide or unhide. -func commentTargetProperties() map[string]*jsonschema.Schema { - return map[string]*jsonschema.Schema{ +// commentVisibilityTool builds the hide_ tool when hide is true, and the unhide_ tool otherwise. +func commentVisibilityTool(t translations.TranslationHelperFunc, target commentVisibilityTarget, hide bool) inventory.ServerTool { + action, titleAction, description := "unhide", "Unhide", target.unhideDescription + if hide { + action, titleAction, description = "hide", "Hide", target.hideDescription + } + name := action + "_" + target.name + + properties := map[string]*jsonschema.Schema{ "owner": { Type: "string", - Description: "Repository owner", + Description: "Repository owner (username or organization)", }, "repo": { Type: "string", Description: "Repository name", }, - "comment_type": { - Type: "string", - Description: "The kind of comment:\n" + - "- 'issue_comment' - a comment on an issue, or a conversation comment on a pull request.\n" + - "- 'pull_request_review_comment' - an inline comment on a pull request diff.\n" + - "- 'pull_request_review' - the body of a pull request review. Requires 'pull_number'.", - Enum: []any{CommentTypeIssueComment, CommentTypePullRequestReviewComment, CommentTypePullRequestReview}, - }, - "comment_id": { - Type: "integer", - Description: "The numeric ID of the comment, or of the review when comment_type is 'pull_request_review'", - Minimum: jsonschema.Ptr(1.0), + } + maps.Copy(properties, target.properties()) + required := append([]string{"owner", "repo"}, target.required...) + if hide { + properties["classifier"] = &jsonschema.Schema{ + Type: "string", + Description: "The reason for hiding the comment", + Enum: commentClassifiers, + } + required = append(required, "classifier") + } + + st := NewTool( + target.toolset, + mcp.Tool{ + Name: name, + Description: t("TOOL_"+strings.ToUpper(name)+"_DESCRIPTION", description+commentVisibilityPermissionNote), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_"+strings.ToUpper(name)+"_USER_TITLE", titleAction+" "+target.title), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: properties, + Required: required, + }, }, - "pull_number": { - Type: "number", - Description: "Pull request number. Required when comment_type is 'pull_request_review'.", + scopes.RequireAll(scopes.Repo), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + return setCommentVisibility(ctx, deps, target, args, hide), nil, nil }, - } + ) + st.FeatureRule = target.featureRule + return st } -// setCommentVisibility resolves the comment identified by args and hides or unhides it. -func setCommentVisibility(ctx context.Context, deps ToolDependencies, args map[string]any, hide bool, classifier string) *mcp.CallToolResult { +// setCommentVisibility resolves the object identified by args and hides or unhides it. +func setCommentVisibility(ctx context.Context, deps ToolDependencies, target commentVisibilityTarget, args map[string]any, hide bool) *mcp.CallToolResult { owner, err := RequiredParam[string](args, "owner") if err != nil { return utils.NewToolResultError(err.Error()) @@ -73,17 +198,12 @@ func setCommentVisibility(ctx context.Context, deps ToolDependencies, args map[s if err != nil { return utils.NewToolResultError(err.Error()) } - commentType, err := RequiredParam[string](args, "comment_type") - if err != nil { - return utils.NewToolResultError(err.Error()) - } - commentID, err := RequiredBigInt(args, "comment_id") - if err != nil { - return utils.NewToolResultError(err.Error()) - } - pullNumber, err := OptionalIntParam(args, "pull_number") - if err != nil { - return utils.NewToolResultError(err.Error()) + var classifier string + if hide { + classifier, err = RequiredParam[string](args, "classifier") + if err != nil { + return utils.NewToolResultError(err.Error()) + } } client, err := deps.GetClient(ctx) @@ -91,7 +211,7 @@ func setCommentVisibility(ctx context.Context, deps ToolDependencies, args map[s return utils.NewToolResultErrorFromErr("failed to get GitHub client", err) } - nodeID, errResult := resolveCommentNodeID(ctx, client, owner, repo, commentType, commentID, pullNumber) + nodeID, errResult := target.resolveNodeID(ctx, client, owner, repo, args) if errResult != nil { return errResult } @@ -118,43 +238,17 @@ func setCommentVisibility(ctx context.Context, deps ToolDependencies, args map[s return utils.NewToolResultText(string(r)) } -// resolveCommentNodeID looks up the GraphQL node ID for a comment identified by its REST ID, -// since the minimize mutations only accept node IDs. -func resolveCommentNodeID(ctx context.Context, client *github.Client, owner, repo, commentType string, commentID int64, pullNumber int) (string, *mcp.CallToolResult) { - var ( - nodeID string - resp *github.Response - err error - ) - - switch commentType { - case CommentTypeIssueComment: - var comment *github.IssueComment - comment, resp, err = client.Issues.GetComment(ctx, owner, repo, commentID) - nodeID = comment.GetNodeID() - case CommentTypePullRequestReviewComment: - var comment *github.PullRequestComment - comment, resp, err = client.PullRequests.GetComment(ctx, owner, repo, commentID) - nodeID = comment.GetNodeID() - case CommentTypePullRequestReview: - if pullNumber <= 0 { - return "", utils.NewToolResultError("pull_number is required when comment_type is 'pull_request_review'") - } - var review *github.PullRequestReview - review, resp, err = client.PullRequests.GetReview(ctx, owner, repo, pullNumber, commentID) - nodeID = review.GetNodeID() - default: - return "", utils.NewToolResultError(fmt.Sprintf("unknown comment_type: %s", commentType)) - } - +// nodeIDFromResponse turns the result of a REST lookup into a GraphQL node ID, since the +// minimize mutations only accept node IDs. +func nodeIDFromResponse(ctx context.Context, errMessage, nodeID string, resp *github.Response, err error) (string, *mcp.CallToolResult) { if resp != nil && resp.Body != nil { defer func() { _ = resp.Body.Close() }() } if err != nil { - return "", ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get comment", resp, err) + return "", ghErrors.NewGitHubAPIErrorResponse(ctx, errMessage, resp, err) } if nodeID == "" { - return "", utils.NewToolResultError("comment has no node ID") + return "", utils.NewToolResultError(errMessage + ": response has no node ID") } return nodeID, nil } diff --git a/pkg/github/comment_minimize_test.go b/pkg/github/comment_minimize_test.go index 679d202fd9..01e639513c 100644 --- a/pkg/github/comment_minimize_test.go +++ b/pkg/github/comment_minimize_test.go @@ -69,24 +69,116 @@ func unminimizeCommentMatcher(nodeID string) githubv4mock.Matcher { ) } +func minimizeCommentErrorMatcher(nodeID, classifier string) githubv4mock.Matcher { + return githubv4mock.NewMutationMatcher( + struct { + MinimizeComment struct { + MinimizedComment struct { + IsMinimized githubv4.Boolean + MinimizedReason githubv4.String + } + } `graphql:"minimizeComment(input: $input)"` + }{}, + githubv4.MinimizeCommentInput{ + SubjectID: githubv4.ID(nodeID), + Classifier: githubv4.ReportedContentClassifiers(classifier), + }, + nil, + githubv4mock.ErrorResponse("Resource not accessible by integration"), + ) +} + +func unminimizeCommentErrorMatcher(nodeID string) githubv4mock.Matcher { + return githubv4mock.NewMutationMatcher( + struct { + UnminimizeComment struct { + UnminimizedComment struct { + IsMinimized githubv4.Boolean + } + } `graphql:"unminimizeComment(input: $input)"` + }{}, + githubv4.UnminimizeCommentInput{SubjectID: githubv4.ID(nodeID)}, + nil, + githubv4mock.ErrorResponse("Resource not accessible by integration"), + ) +} + func Test_CommentVisibilityToolSchemas(t *testing.T) { - hide := GranularHideComment(translations.NullTranslationHelper).Tool - require.NoError(t, toolsnaps.Test(hide.Name, hide)) - assert.Equal(t, "hide_comment", hide.Name) - assert.False(t, hide.Annotations.ReadOnlyHint) - assert.ElementsMatch(t, hide.InputSchema.(*jsonschema.Schema).Required, - []string{"owner", "repo", "comment_type", "comment_id", "classifier"}) + tests := []struct { + tool inventory.ServerTool + name string + toolset inventory.ToolsetID + featureFlag string + expectedRequire []string + }{ + { + tool: GranularHideIssueComment(translations.NullTranslationHelper), + name: "hide_issue_comment", + toolset: ToolsetMetadataIssues.ID, + featureFlag: FeatureFlagIssuesGranular, + expectedRequire: []string{"owner", "repo", "comment_id", "classifier"}, + }, + { + tool: GranularUnhideIssueComment(translations.NullTranslationHelper), + name: "unhide_issue_comment", + toolset: ToolsetMetadataIssues.ID, + featureFlag: FeatureFlagIssuesGranular, + expectedRequire: []string{"owner", "repo", "comment_id"}, + }, + { + tool: GranularHidePullRequestReviewComment(translations.NullTranslationHelper), + name: "hide_pull_request_review_comment", + toolset: ToolsetMetadataPullRequests.ID, + featureFlag: FeatureFlagPullRequestsGranular, + expectedRequire: []string{"owner", "repo", "comment_id", "classifier"}, + }, + { + tool: GranularUnhidePullRequestReviewComment(translations.NullTranslationHelper), + name: "unhide_pull_request_review_comment", + toolset: ToolsetMetadataPullRequests.ID, + featureFlag: FeatureFlagPullRequestsGranular, + expectedRequire: []string{"owner", "repo", "comment_id"}, + }, + { + tool: GranularHidePullRequestReview(translations.NullTranslationHelper), + name: "hide_pull_request_review", + toolset: ToolsetMetadataPullRequests.ID, + featureFlag: FeatureFlagPullRequestsGranular, + expectedRequire: []string{"owner", "repo", "pullNumber", "review_id", "classifier"}, + }, + { + tool: GranularUnhidePullRequestReview(translations.NullTranslationHelper), + name: "unhide_pull_request_review", + toolset: ToolsetMetadataPullRequests.ID, + featureFlag: FeatureFlagPullRequestsGranular, + expectedRequire: []string{"owner", "repo", "pullNumber", "review_id"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tool := tc.tool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) - unhide := GranularUnhideComment(translations.NullTranslationHelper).Tool - require.NoError(t, toolsnaps.Test(unhide.Name, unhide)) - assert.Equal(t, "unhide_comment", unhide.Name) - assert.False(t, unhide.Annotations.ReadOnlyHint) - unhideSchema := unhide.InputSchema.(*jsonschema.Schema) - assert.ElementsMatch(t, unhideSchema.Required, []string{"owner", "repo", "comment_type", "comment_id"}) - assert.NotContains(t, unhideSchema.Properties, "classifier") + assert.Equal(t, tc.name, tool.Name) + assert.NotEmpty(t, tool.Description) + assert.False(t, tool.Annotations.ReadOnlyHint) + assert.Equal(t, tc.toolset, tc.tool.Toolset.ID) + assert.Equal(t, []inventory.FeatureFlag{inventory.FeatureFlag(tc.featureFlag)}, tc.tool.FeatureRule.Features()) + + schema := tool.InputSchema.(*jsonschema.Schema) + assert.ElementsMatch(t, tc.expectedRequire, schema.Required) + assert.Len(t, schema.Properties, len(tc.expectedRequire), "every property should be required") + }) + } } -func Test_HideAndUnhideComment(t *testing.T) { +func Test_HideAndUnhideComments(t *testing.T) { + issueComment := mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1)), NodeID: github.Ptr("IC_1")}) + reviewComment := mockResponse(t, http.StatusOK, &github.PullRequestComment{ID: github.Ptr(int64(2)), NodeID: github.Ptr("PRRC_2")}) + review := mockResponse(t, http.StatusOK, &github.PullRequestReview{ID: github.Ptr(int64(3)), NodeID: github.Ptr("PRR_3")}) + notFound := mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`) + tests := []struct { name string tool inventory.ServerTool @@ -97,214 +189,140 @@ func Test_HideAndUnhideComment(t *testing.T) { expectedErrMsg string }{ { - name: "hide issue comment", - tool: GranularHideComment(translations.NullTranslationHelper), - restHandlers: map[string]http.HandlerFunc{ - getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1)), NodeID: github.Ptr("IC_1")}), - }, - gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("IC_1", "SPAM", "spam")}, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "issue_comment", "comment_id": float64(1), "classifier": "spam", - }, + name: "hide issue comment", + tool: GranularHideIssueComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getIssueCommentRoute: issueComment}, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("IC_1", "SPAM", "spam")}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1), "classifier": "spam"}, expectedResult: MinimizeCommentResult{NodeID: "IC_1", IsMinimized: true, MinimizedReason: "spam"}, }, { - name: "hide review comment", - tool: GranularHideComment(translations.NullTranslationHelper), - restHandlers: map[string]http.HandlerFunc{ - getReviewCommentRoute: mockResponse(t, http.StatusOK, &github.PullRequestComment{ID: github.Ptr(int64(2)), NodeID: github.Ptr("PRRC_2")}), - }, - gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRRC_2", "OUTDATED", "outdated")}, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "pull_request_review_comment", "comment_id": float64(2), "classifier": "OUTDATED", - }, + name: "unhide issue comment", + tool: GranularUnhideIssueComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getIssueCommentRoute: issueComment}, + gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("IC_1")}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1)}, + expectedResult: MinimizeCommentResult{NodeID: "IC_1", IsMinimized: false}, + }, + { + name: "hide pull request review comment", + tool: GranularHidePullRequestReviewComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getReviewCommentRoute: reviewComment}, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRRC_2", "OUTDATED", "outdated")}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(2), "classifier": "OUTDATED"}, expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: true, MinimizedReason: "outdated"}, }, { - name: "unhide review", - tool: GranularUnhideComment(translations.NullTranslationHelper), - restHandlers: map[string]http.HandlerFunc{ - getReviewRoute: mockResponse(t, http.StatusOK, &github.PullRequestReview{ID: github.Ptr(int64(3)), NodeID: github.Ptr("PRR_3")}), - }, - gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("PRR_3")}, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "pull_request_review", "comment_id": float64(3), "pull_number": float64(42), - }, - expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: false}, + name: "unhide pull request review comment", + tool: GranularUnhidePullRequestReviewComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getReviewCommentRoute: reviewComment}, + gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("PRRC_2")}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(2)}, + expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: false}, }, { - name: "hide review", - tool: GranularHideComment(translations.NullTranslationHelper), - restHandlers: map[string]http.HandlerFunc{ - getReviewRoute: mockResponse(t, http.StatusOK, &github.PullRequestReview{ID: github.Ptr(int64(3)), NodeID: github.Ptr("PRR_3")}), - }, - gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRR_3", "RESOLVED", "resolved")}, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "pull_request_review", "comment_id": float64(3), "pull_number": float64(42), "classifier": "RESOLVED", - }, + name: "hide pull request review", + tool: GranularHidePullRequestReview(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getReviewRoute: review}, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentMatcher("PRR_3", "RESOLVED", "resolved")}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(42), "review_id": float64(3), "classifier": "RESOLVED"}, expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: true, MinimizedReason: "resolved"}, }, { - name: "unhide review comment", - tool: GranularUnhideComment(translations.NullTranslationHelper), - restHandlers: map[string]http.HandlerFunc{ - getReviewCommentRoute: mockResponse(t, http.StatusOK, &github.PullRequestComment{ID: github.Ptr(int64(2)), NodeID: github.Ptr("PRRC_2")}), - }, - gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("PRRC_2")}, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "pull_request_review_comment", "comment_id": float64(2), - }, - expectedResult: MinimizeCommentResult{NodeID: "PRRC_2", IsMinimized: false}, + name: "unhide pull request review", + tool: GranularUnhidePullRequestReview(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getReviewRoute: review}, + gqlMatchers: []githubv4mock.Matcher{unminimizeCommentMatcher("PRR_3")}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(42), "review_id": float64(3)}, + expectedResult: MinimizeCommentResult{NodeID: "PRR_3", IsMinimized: false}, }, { - name: "hide mutation fails", - tool: GranularHideComment(translations.NullTranslationHelper), - restHandlers: map[string]http.HandlerFunc{ - getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1)), NodeID: github.Ptr("IC_1")}), - }, - gqlMatchers: []githubv4mock.Matcher{ - githubv4mock.NewMutationMatcher( - struct { - MinimizeComment struct { - MinimizedComment struct { - IsMinimized githubv4.Boolean - MinimizedReason githubv4.String - } - } `graphql:"minimizeComment(input: $input)"` - }{}, - githubv4.MinimizeCommentInput{SubjectID: githubv4.ID("IC_1"), Classifier: githubv4.ReportedContentClassifiersSpam}, - nil, - githubv4mock.ErrorResponse("Resource not accessible by integration"), - ), - }, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "issue_comment", "comment_id": float64(1), "classifier": "SPAM", - }, - expectedErrMsg: "failed to minimize comment", + name: "issue comment not found", + tool: GranularUnhideIssueComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getIssueCommentRoute: notFound}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1)}, + expectedErrMsg: "failed to get issue comment", }, { - name: "unhide mutation fails", - tool: GranularUnhideComment(translations.NullTranslationHelper), - restHandlers: map[string]http.HandlerFunc{ - getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1)), NodeID: github.Ptr("IC_1")}), - }, - gqlMatchers: []githubv4mock.Matcher{ - githubv4mock.NewMutationMatcher( - struct { - UnminimizeComment struct { - UnminimizedComment struct { - IsMinimized githubv4.Boolean - } - } `graphql:"unminimizeComment(input: $input)"` - }{}, - githubv4.UnminimizeCommentInput{SubjectID: githubv4.ID("IC_1")}, - nil, - githubv4mock.ErrorResponse("Resource not accessible by integration"), - ), - }, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "issue_comment", "comment_id": float64(1), - }, - expectedErrMsg: "failed to unminimize comment", + name: "pull request review comment not found", + tool: GranularUnhidePullRequestReviewComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getReviewCommentRoute: notFound}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(2)}, + expectedErrMsg: "failed to get pull request review comment", + }, + { + name: "pull request review not found", + tool: GranularUnhidePullRequestReview(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getReviewRoute: notFound}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(42), "review_id": float64(3)}, + expectedErrMsg: "failed to get pull request review", }, { - name: "comment without node ID", - tool: GranularUnhideComment(translations.NullTranslationHelper), + name: "response without node ID", + tool: GranularUnhideIssueComment(translations.NullTranslationHelper), restHandlers: map[string]http.HandlerFunc{ getIssueCommentRoute: mockResponse(t, http.StatusOK, &github.IssueComment{ID: github.Ptr(int64(1))}), }, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "issue_comment", "comment_id": float64(1), - }, - expectedErrMsg: "comment has no node ID", + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1)}, + expectedErrMsg: "response has no node ID", }, { - name: "missing owner", - tool: GranularUnhideComment(translations.NullTranslationHelper), - requestArgs: map[string]any{ - "repo": "repo", "comment_type": "issue_comment", "comment_id": float64(1), - }, - expectedErrMsg: "owner", + name: "hide mutation fails", + tool: GranularHideIssueComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getIssueCommentRoute: issueComment}, + gqlMatchers: []githubv4mock.Matcher{minimizeCommentErrorMatcher("IC_1", "SPAM")}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1), "classifier": "SPAM"}, + expectedErrMsg: "failed to minimize comment", }, { - name: "missing repo", - tool: GranularUnhideComment(translations.NullTranslationHelper), - requestArgs: map[string]any{ - "owner": "owner", "comment_type": "issue_comment", "comment_id": float64(1), - }, - expectedErrMsg: "repo", + name: "unhide mutation fails", + tool: GranularUnhideIssueComment(translations.NullTranslationHelper), + restHandlers: map[string]http.HandlerFunc{getIssueCommentRoute: issueComment}, + gqlMatchers: []githubv4mock.Matcher{unminimizeCommentErrorMatcher("IC_1")}, + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1)}, + expectedErrMsg: "failed to unminimize comment", }, { - name: "missing comment_type", - tool: GranularUnhideComment(translations.NullTranslationHelper), - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", "comment_id": float64(1), - }, - expectedErrMsg: "comment_type", + name: "missing owner", + tool: GranularUnhideIssueComment(translations.NullTranslationHelper), + requestArgs: map[string]any{"repo": "repo", "comment_id": float64(1)}, + expectedErrMsg: "owner", }, { - name: "missing comment_id", - tool: GranularHideComment(translations.NullTranslationHelper), - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", "comment_type": "issue_comment", "classifier": "SPAM", - }, - expectedErrMsg: "comment_id", + name: "missing repo", + tool: GranularUnhideIssueComment(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "comment_id": float64(1)}, + expectedErrMsg: "repo", }, { - name: "invalid pull_number type", - tool: GranularUnhideComment(translations.NullTranslationHelper), - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", "comment_type": "pull_request_review", - "comment_id": float64(3), "pull_number": "forty-two", - }, - expectedErrMsg: "pull_number", + name: "missing classifier", + tool: GranularHideIssueComment(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1)}, + expectedErrMsg: "classifier", }, { - name: "hide without classifier", - tool: GranularHideComment(translations.NullTranslationHelper), - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "issue_comment", "comment_id": float64(1), - }, - expectedErrMsg: "classifier", + name: "missing issue comment_id", + tool: GranularUnhideIssueComment(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo"}, + expectedErrMsg: "comment_id", }, { - name: "review without pull_number", - tool: GranularUnhideComment(translations.NullTranslationHelper), - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "pull_request_review", "comment_id": float64(3), - }, - expectedErrMsg: "pull_number is required", + name: "missing pull request review comment_id", + tool: GranularUnhidePullRequestReviewComment(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo"}, + expectedErrMsg: "comment_id", }, { - name: "unknown comment type", - tool: GranularUnhideComment(translations.NullTranslationHelper), - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "commit_comment", "comment_id": float64(1), - }, - expectedErrMsg: "unknown comment_type", + name: "missing pullNumber", + tool: GranularUnhidePullRequestReview(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "review_id": float64(3)}, + expectedErrMsg: "pullNumber", }, { - name: "comment not found", - tool: GranularUnhideComment(translations.NullTranslationHelper), - restHandlers: map[string]http.HandlerFunc{ - getIssueCommentRoute: mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), - }, - requestArgs: map[string]any{ - "owner": "owner", "repo": "repo", - "comment_type": "issue_comment", "comment_id": float64(1), - }, - expectedErrMsg: "failed to get comment", + name: "missing review_id", + tool: GranularUnhidePullRequestReview(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(42)}, + expectedErrMsg: "review_id", }, } diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index e120b136f2..0d36ab97a5 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -56,8 +56,8 @@ func TestGranularToolSnaps(t *testing.T) { GranularRemoveIssueReaction, GranularAddIssueCommentReaction, GranularRemoveIssueCommentReaction, - GranularHideComment, - GranularUnhideComment, + GranularHideIssueComment, + GranularUnhideIssueComment, GranularUpdatePullRequestTitle, GranularUpdatePullRequestBody, GranularUpdatePullRequestState, @@ -71,6 +71,10 @@ func TestGranularToolSnaps(t *testing.T) { GranularUnresolveReviewThread, GranularAddPullRequestReviewCommentReaction, GranularRemovePullRequestReviewCommentReaction, + GranularHidePullRequestReviewComment, + GranularUnhidePullRequestReviewComment, + GranularHidePullRequestReview, + GranularUnhidePullRequestReview, } for _, constructor := range toolConstructors { @@ -107,8 +111,8 @@ func TestIssuesGranularToolset(t *testing.T) { "remove_issue_reaction", "add_issue_comment_reaction", "remove_issue_comment_reaction", - "hide_comment", - "unhide_comment", + "hide_issue_comment", + "unhide_issue_comment", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -146,6 +150,10 @@ func TestPullRequestsGranularToolset(t *testing.T) { "unresolve_review_thread", "add_pull_request_review_comment_reaction", "remove_pull_request_review_comment_reaction", + "hide_pull_request_review_comment", + "unhide_pull_request_review_comment", + "hide_pull_request_review", + "unhide_pull_request_review", } for _, name := range expected { assert.Contains(t, toolNames, name) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 152bd4aabd..bed003bef3 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -1902,69 +1902,12 @@ func GranularRemoveIssueCommentReaction(t translations.TranslationHelperFunc) in return st } -// GranularHideComment hides (minimizes) a comment on an issue or pull request. -func GranularHideComment(t translations.TranslationHelperFunc) inventory.ServerTool { - properties := commentTargetProperties() - properties["classifier"] = &jsonschema.Schema{ - Type: "string", - Description: "The reason for hiding the comment", - Enum: commentClassifiers, - } - - st := NewTool( - ToolsetMetadataIssues, - mcp.Tool{ - Name: "hide_comment", - Description: t("TOOL_HIDE_COMMENT_DESCRIPTION", "Hide (minimize) a comment on an issue or pull request. "+commentVisibilityDescriptionSuffix), - Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_HIDE_COMMENT_USER_TITLE", "Hide Comment"), - ReadOnlyHint: false, - DestructiveHint: jsonschema.Ptr(false), - OpenWorldHint: jsonschema.Ptr(true), - }, - InputSchema: &jsonschema.Schema{ - Type: "object", - Properties: properties, - Required: []string{"owner", "repo", "comment_type", "comment_id", "classifier"}, - }, - }, - scopes.RequireAll(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - classifier, err := RequiredParam[string](args, "classifier") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - return setCommentVisibility(ctx, deps, args, true, classifier), nil, nil - }, - ) - st.FeatureRule = issuesGranularFeatureRule - return st +// GranularHideIssueComment hides (minimizes) an issue or pull request conversation comment. +func GranularHideIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { + return commentVisibilityTool(t, issueCommentVisibilityTarget, true) } -// GranularUnhideComment unhides (unminimizes) a previously hidden comment on an issue or pull request. -func GranularUnhideComment(t translations.TranslationHelperFunc) inventory.ServerTool { - st := NewTool( - ToolsetMetadataIssues, - mcp.Tool{ - Name: "unhide_comment", - Description: t("TOOL_UNHIDE_COMMENT_DESCRIPTION", "Unhide (unminimize) a previously hidden comment on an issue or pull request. "+commentVisibilityDescriptionSuffix), - Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_UNHIDE_COMMENT_USER_TITLE", "Unhide Comment"), - ReadOnlyHint: false, - DestructiveHint: jsonschema.Ptr(false), - OpenWorldHint: jsonschema.Ptr(true), - }, - InputSchema: &jsonschema.Schema{ - Type: "object", - Properties: commentTargetProperties(), - Required: []string{"owner", "repo", "comment_type", "comment_id"}, - }, - }, - scopes.RequireAll(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - return setCommentVisibility(ctx, deps, args, false, ""), nil, nil - }, - ) - st.FeatureRule = issuesGranularFeatureRule - return st +// GranularUnhideIssueComment unhides (unminimizes) an issue or pull request conversation comment. +func GranularUnhideIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { + return commentVisibilityTool(t, issueCommentVisibilityTarget, false) } diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index e670da9a34..92e7fb6f5d 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -973,3 +973,23 @@ func GranularRemovePullRequestReviewCommentReaction(t translations.TranslationHe st.FeatureRule = pullRequestsGranularFeatureRule return st } + +// GranularHidePullRequestReviewComment hides (minimizes) an inline pull request review comment. +func GranularHidePullRequestReviewComment(t translations.TranslationHelperFunc) inventory.ServerTool { + return commentVisibilityTool(t, pullRequestReviewCommentVisibilityTarget, true) +} + +// GranularUnhidePullRequestReviewComment unhides (unminimizes) an inline pull request review comment. +func GranularUnhidePullRequestReviewComment(t translations.TranslationHelperFunc) inventory.ServerTool { + return commentVisibilityTool(t, pullRequestReviewCommentVisibilityTarget, false) +} + +// GranularHidePullRequestReview hides (minimizes) the body of a pull request review. +func GranularHidePullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { + return commentVisibilityTool(t, pullRequestReviewVisibilityTarget, true) +} + +// GranularUnhidePullRequestReview unhides (unminimizes) the body of a pull request review. +func GranularUnhidePullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { + return commentVisibilityTool(t, pullRequestReviewVisibilityTarget, false) +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 8d0d15d574..eb1113754a 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -376,8 +376,8 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GranularRemoveIssueReaction(t), GranularAddIssueCommentReaction(t), GranularRemoveIssueCommentReaction(t), - GranularHideComment(t), - GranularUnhideComment(t), + GranularHideIssueComment(t), + GranularUnhideIssueComment(t), // Granular pull request tools (feature-flagged, replace consolidated update_pull_request/pull_request_review_write) GranularUpdatePullRequestTitle(t), @@ -394,6 +394,10 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GranularUnresolveReviewThread(t), GranularAddPullRequestReviewCommentReaction(t), GranularRemovePullRequestReviewCommentReaction(t), + GranularHidePullRequestReviewComment(t), + GranularUnhidePullRequestReviewComment(t), + GranularHidePullRequestReview(t), + GranularUnhidePullRequestReview(t), }) } From 04359ff4539679e72cbb1cfbd19dca42b4f0ca78 Mon Sep 17 00:00:00 2001 From: timrogers <116134+timrogers@users.noreply.github.com> Date: Wed, 30 Sep 2026 04:31:02 +0000 Subject: [PATCH 5/5] Validate classifier and IDs before calling the GitHub API JSON schema constraints (enum, minimum) aren't enforced when tool arguments are unmarshalled, so an invalid classifier or a negative ID previously reached GitHub and surfaced as a confusing 404 or GraphQL error. Reject them up front with a clear argument error instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/comment_minimize.go | 30 +++++++++++++++++++++++++---- pkg/github/comment_minimize_test.go | 30 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/pkg/github/comment_minimize.go b/pkg/github/comment_minimize.go index 5a48efbff8..0bdabae44f 100644 --- a/pkg/github/comment_minimize.go +++ b/pkg/github/comment_minimize.go @@ -3,7 +3,9 @@ package github import ( "context" "encoding/json" + "fmt" "maps" + "slices" "strings" ghErrors "github.com/github/github-mcp-server/pkg/errors" @@ -61,7 +63,7 @@ var issueCommentVisibilityTarget = commentVisibilityTarget{ }, required: []string{"comment_id"}, resolveNodeID: func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) { - commentID, err := RequiredBigInt(args, "comment_id") + commentID, err := requiredPositiveBigInt(args, "comment_id") if err != nil { return "", utils.NewToolResultError(err.Error()) } @@ -88,7 +90,7 @@ var pullRequestReviewCommentVisibilityTarget = commentVisibilityTarget{ }, required: []string{"comment_id"}, resolveNodeID: func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) { - commentID, err := RequiredBigInt(args, "comment_id") + commentID, err := requiredPositiveBigInt(args, "comment_id") if err != nil { return "", utils.NewToolResultError(err.Error()) } @@ -124,7 +126,10 @@ var pullRequestReviewVisibilityTarget = commentVisibilityTarget{ if err != nil { return "", utils.NewToolResultError(err.Error()) } - reviewID, err := RequiredBigInt(args, "review_id") + if pullNumber < 1 { + return "", utils.NewToolResultError("pullNumber must be greater than 0") + } + reviewID, err := requiredPositiveBigInt(args, "review_id") if err != nil { return "", utils.NewToolResultError(err.Error()) } @@ -204,6 +209,10 @@ func setCommentVisibility(ctx context.Context, deps ToolDependencies, target com if err != nil { return utils.NewToolResultError(err.Error()) } + classifier = strings.ToUpper(classifier) + if !slices.Contains(commentClassifiers, any(classifier)) { + return utils.NewToolResultError(fmt.Sprintf("invalid classifier %q: must be one of %v", classifier, commentClassifiers)) + } } client, err := deps.GetClient(ctx) @@ -223,7 +232,7 @@ func setCommentVisibility(ctx context.Context, deps ToolDependencies, target com var result MinimizeCommentResult if hide { - result, errResult = minimizeComment(ctx, gqlClient, nodeID, strings.ToUpper(classifier)) + result, errResult = minimizeComment(ctx, gqlClient, nodeID, classifier) } else { result, errResult = unminimizeComment(ctx, gqlClient, nodeID) } @@ -238,6 +247,19 @@ func setCommentVisibility(ctx context.Context, deps ToolDependencies, target com return utils.NewToolResultText(string(r)) } +// requiredPositiveBigInt reads a required ID argument. The schema's minimum is not enforced +// when arguments are unmarshalled, so negative values are rejected here before any API call. +func requiredPositiveBigInt(args map[string]any, p string) (int64, error) { + v, err := RequiredBigInt(args, p) + if err != nil { + return 0, err + } + if v < 1 { + return 0, fmt.Errorf("%s must be greater than 0", p) + } + return v, nil +} + // nodeIDFromResponse turns the result of a REST lookup into a GraphQL node ID, since the // minimize mutations only accept node IDs. func nodeIDFromResponse(ctx context.Context, errMessage, nodeID string, resp *github.Response, err error) (string, *mcp.CallToolResult) { diff --git a/pkg/github/comment_minimize_test.go b/pkg/github/comment_minimize_test.go index 01e639513c..75e97e0fbb 100644 --- a/pkg/github/comment_minimize_test.go +++ b/pkg/github/comment_minimize_test.go @@ -282,6 +282,36 @@ func Test_HideAndUnhideComments(t *testing.T) { requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1)}, expectedErrMsg: "failed to unminimize comment", }, + { + name: "invalid classifier is rejected before any API call", + tool: GranularHideIssueComment(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(1), "classifier": "BOGUS"}, + expectedErrMsg: `invalid classifier "BOGUS"`, + }, + { + name: "negative issue comment_id", + tool: GranularHideIssueComment(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(-1), "classifier": "SPAM"}, + expectedErrMsg: "comment_id must be greater than 0", + }, + { + name: "negative pull request review comment_id", + tool: GranularUnhidePullRequestReviewComment(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(-2)}, + expectedErrMsg: "comment_id must be greater than 0", + }, + { + name: "negative review_id", + tool: GranularUnhidePullRequestReview(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(42), "review_id": float64(-3)}, + expectedErrMsg: "review_id must be greater than 0", + }, + { + name: "negative pullNumber", + tool: GranularUnhidePullRequestReview(translations.NullTranslationHelper), + requestArgs: map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(-42), "review_id": float64(3)}, + expectedErrMsg: "pullNumber must be greater than 0", + }, { name: "missing owner", tool: GranularUnhideIssueComment(translations.NullTranslationHelper),