diff --git a/docs/feature-flags.md b/docs/feature-flags.md index ec5282bd54..095d619c7c 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -84,6 +84,15 @@ user-controllable flag enabled individually. Complex multi-flag rules may require separate documentation. Flags that only affect runtime behavior (such as output formatting) won't appear here. +### `issues_granular` and `pull_requests_granular` together + +By default, `add_issue_comment` and `update_issue_comment` work on both issues +and pull requests. When **both** granular flags are enabled, they only accept +issues and issue comments. Pull request conversation comments are then handled +by `add_pull_request_comment` and `update_pull_request_comment` from +`pull_requests_granular`. With only one of the flags enabled, both tools keep +their default behavior, so pull request commenting is always available. + ### `remote_mcp_ui_apps` @@ -152,14 +161,14 @@ as output formatting) won't appear here. ### `issues_granular` -- **add_issue_comment_reaction** - Add Reaction to Issue or Pull Request Comment +- **add_issue_comment_reaction** - Add Reaction to Issue Comment - **OAuth Challenge Scopes**: `repo` - - `comment_id`: The issue or pull request comment ID (number, required) + - `comment_id`: The issue comment ID (number, required) - `content`: The emoji reaction type (string, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) -- **add_issue_reaction** - Add Reaction to Issue or Pull Request +- **add_issue_reaction** - Add Reaction to Issue - **OAuth Challenge Scopes**: `repo` - `content`: The emoji reaction type (string, required) - `issue_number`: The issue number (number, required) @@ -184,14 +193,14 @@ as output formatting) won't appear here. - `repo`: Repository name (string, required) - `title`: Issue title (string, required) -- **remove_issue_comment_reaction** - Remove Reaction from Issue or Pull Request Comment +- **remove_issue_comment_reaction** - Remove Reaction from Issue Comment - **OAuth Challenge Scopes**: `repo` - - `comment_id`: The issue or pull request comment ID (number, required) + - `comment_id`: The issue comment ID (number, required) - `owner`: Repository owner (username or organization) (string, required) - `reaction_id`: The reaction ID to remove (number, required) - `repo`: Repository name (string, required) -- **remove_issue_reaction** - Remove Reaction from Issue or Pull Request +- **remove_issue_reaction** - Remove Reaction from Issue - **OAuth Challenge Scopes**: `repo` - `issue_number`: The issue number (number, required) - `owner`: Repository owner (username or organization) (string, required) @@ -280,6 +289,27 @@ as output formatting) won't appear here. ### `pull_requests_granular` +- **add_pull_request_comment** - Add Pull Request Comment + - **OAuth Challenge Scopes**: `repo` + - `body`: Comment content (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **add_pull_request_comment_reaction** - Add Reaction to Pull Request Comment + - **OAuth Challenge Scopes**: `repo` + - `comment_id`: The pull request conversation comment ID (number, required) + - `content`: The emoji reaction type (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **add_pull_request_reaction** - Add Reaction to Pull Request + - **OAuth Challenge Scopes**: `repo` + - `content`: The emoji reaction type (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - **add_pull_request_review_comment** - Add Pull Request Review Comment - **OAuth Challenge Scopes**: `repo` - `body`: The comment body (string, required) @@ -315,6 +345,20 @@ as output formatting) won't appear here. - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) +- **remove_pull_request_comment_reaction** - Remove Reaction from Pull Request Comment + - **OAuth Challenge Scopes**: `repo` + - `comment_id`: The pull request conversation comment ID (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `reaction_id`: The reaction ID to remove (number, required) + - `repo`: Repository name (string, required) + +- **remove_pull_request_reaction** - Remove Reaction from Pull Request + - **OAuth Challenge Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `reaction_id`: The reaction ID to remove (number, required) + - `repo`: Repository name (string, required) + - **remove_pull_request_review_comment_reaction** - Remove Pull Request Review Comment Reaction - **OAuth Challenge Scopes**: `repo` - `comment_id`: The numeric pull request review comment ID. Use the number from a #discussion_r... anchor, not the GraphQL thread node ID (PRRT_...). (number, required) @@ -345,6 +389,13 @@ as output formatting) won't appear here. - **OAuth Challenge Scopes**: `repo` - `threadID`: The node ID of the review thread to unresolve (e.g., PRRT_kwDOxxx) (string, required) +- **update_pull_request_assignees** - Update Pull Request Assignees + - **OAuth Challenge Scopes**: `repo` + - `assignees`: GitHub usernames to assign to this pull request (string[], required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - **update_pull_request_body** - Update Pull Request Body - **OAuth Challenge Scopes**: `repo` - `body`: The new body content for the pull request (string, required) @@ -352,6 +403,13 @@ as output formatting) won't appear here. - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) +- **update_pull_request_comment** - Update Pull Request Comment + - **OAuth Challenge Scopes**: `repo` + - `body`: New comment content (string, required) + - `comment_id`: The numeric ID of the pull request conversation comment to update. Do not use a pull request review comment ID. (integer, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - **update_pull_request_draft_state** - Update Pull Request Draft State - **OAuth Challenge Scopes**: `repo` - `draft`: Set to true to convert to draft, false to mark as ready for review (boolean, required) @@ -359,6 +417,20 @@ as output formatting) won't appear here. - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) +- **update_pull_request_labels** - Update Pull Request Labels + - **OAuth Challenge Scopes**: `repo` + - `labels`: Labels to apply to this pull request (string[], required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **update_pull_request_milestone** - Update Pull Request Milestone + - **OAuth Challenge Scopes**: `repo` + - `milestone`: The milestone number to set on the pull request (integer, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - **update_pull_request_state** - Update Pull Request State - **OAuth Challenge Scopes**: `repo` - `owner`: Repository owner (username or organization) (string, required) diff --git a/pkg/github/__toolsnaps__/add_issue_comment_ff_issues_granular.snap b/pkg/github/__toolsnaps__/add_issue_comment_ff_issues_granular.snap new file mode 100644 index 0000000000..e42f018f05 --- /dev/null +++ b/pkg/github/__toolsnaps__/add_issue_comment_ff_issues_granular.snap @@ -0,0 +1,55 @@ +{ + "annotations": { + "idempotentHint": false, + "readOnlyHint": false, + "title": "Add comment to issue" + }, + "description": "Add a comment and/or reaction to a specific issue or issue comment in a GitHub repository. Only works on issues; for pull requests use add_pull_request_comment. At least one of body or reaction is required.", + "inputSchema": { + "properties": { + "body": { + "description": "Comment content. Required unless reaction is provided.", + "minLength": 1, + "type": "string" + }, + "comment_id": { + "description": "The numeric ID of the issue comment to react to. Use this for reactions to comments; omit it to react to the issue itself. Cannot be combined with body.", + "minimum": 1, + "type": "integer" + }, + "issue_number": { + "description": "Issue number to comment on or react to.", + "type": "number" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "reaction": { + "description": "Emoji reaction to add. Required unless body is provided.", + "enum": [ + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number" + ], + "type": "object" + }, + "name": "add_issue_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap index fea8434a62..9120b3f36d 100644 --- a/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap +++ b/pkg/github/__toolsnaps__/add_issue_comment_reaction.snap @@ -4,13 +4,13 @@ "idempotentHint": false, "openWorldHint": true, "readOnlyHint": false, - "title": "Add Reaction to Issue or Pull Request Comment" + "title": "Add Reaction to Issue Comment" }, - "description": "Add a reaction to an issue or pull request comment.", + "description": "Add a reaction to an issue comment. Only works on issue comments; for pull request conversation comments use add_pull_request_comment_reaction.", "inputSchema": { "properties": { "comment_id": { - "description": "The issue or pull request comment ID", + "description": "The issue comment ID", "minimum": 1, "type": "number" }, diff --git a/pkg/github/__toolsnaps__/add_issue_reaction.snap b/pkg/github/__toolsnaps__/add_issue_reaction.snap index 49175d1412..65b37502b9 100644 --- a/pkg/github/__toolsnaps__/add_issue_reaction.snap +++ b/pkg/github/__toolsnaps__/add_issue_reaction.snap @@ -4,9 +4,9 @@ "idempotentHint": false, "openWorldHint": true, "readOnlyHint": false, - "title": "Add Reaction to Issue or Pull Request" + "title": "Add Reaction to Issue" }, - "description": "Add a reaction to an issue or pull request.", + "description": "Add a reaction to an issue. Only works on issues; for pull requests use add_pull_request_reaction.", "inputSchema": { "properties": { "content": { diff --git a/pkg/github/__toolsnaps__/add_pull_request_comment.snap b/pkg/github/__toolsnaps__/add_pull_request_comment.snap new file mode 100644 index 0000000000..41f568e9cf --- /dev/null +++ b/pkg/github/__toolsnaps__/add_pull_request_comment.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Add Pull Request Comment" + }, + "description": "Add a conversation comment to a pull request. This does not create a review comment on a specific line; use add_pull_request_review_comment for that.", + "inputSchema": { + "properties": { + "body": { + "description": "Comment content", + "minLength": 1, + "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" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "body" + ], + "type": "object" + }, + "name": "add_pull_request_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/add_pull_request_comment_reaction.snap b/pkg/github/__toolsnaps__/add_pull_request_comment_reaction.snap new file mode 100644 index 0000000000..13432f682f --- /dev/null +++ b/pkg/github/__toolsnaps__/add_pull_request_comment_reaction.snap @@ -0,0 +1,49 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Add Reaction to Pull Request Comment" + }, + "description": "Add a reaction to a pull request conversation comment. For review comments on specific lines, use add_pull_request_review_comment_reaction.", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The pull request conversation comment ID", + "minimum": 1, + "type": "number" + }, + "content": { + "description": "The emoji reaction type", + "enum": [ + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "content" + ], + "type": "object" + }, + "name": "add_pull_request_comment_reaction" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/add_pull_request_reaction.snap b/pkg/github/__toolsnaps__/add_pull_request_reaction.snap new file mode 100644 index 0000000000..48c099fe9e --- /dev/null +++ b/pkg/github/__toolsnaps__/add_pull_request_reaction.snap @@ -0,0 +1,49 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Add Reaction to Pull Request" + }, + "description": "Add a reaction to a pull request.", + "inputSchema": { + "properties": { + "content": { + "description": "The emoji reaction type", + "enum": [ + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "content" + ], + "type": "object" + }, + "name": "add_pull_request_reaction" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap b/pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap index de53a1dae8..2c54bea352 100644 --- a/pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap +++ b/pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap @@ -4,13 +4,13 @@ "idempotentHint": false, "openWorldHint": true, "readOnlyHint": false, - "title": "Remove Reaction from Issue or Pull Request Comment" + "title": "Remove Reaction from Issue Comment" }, - "description": "Remove a reaction from an issue or pull request comment.", + "description": "Remove a reaction from an issue comment. Only works on issue comments; for pull request conversation comments use remove_pull_request_comment_reaction.", "inputSchema": { "properties": { "comment_id": { - "description": "The issue or pull request comment ID", + "description": "The issue comment ID", "minimum": 1, "type": "number" }, diff --git a/pkg/github/__toolsnaps__/remove_issue_reaction.snap b/pkg/github/__toolsnaps__/remove_issue_reaction.snap index 4222e308fe..ddc0e00a67 100644 --- a/pkg/github/__toolsnaps__/remove_issue_reaction.snap +++ b/pkg/github/__toolsnaps__/remove_issue_reaction.snap @@ -4,9 +4,9 @@ "idempotentHint": false, "openWorldHint": true, "readOnlyHint": false, - "title": "Remove Reaction from Issue or Pull Request" + "title": "Remove Reaction from Issue" }, - "description": "Remove a reaction from an issue or pull request.", + "description": "Remove a reaction from an issue. Only works on issues; for pull requests use remove_pull_request_reaction.", "inputSchema": { "properties": { "issue_number": { diff --git a/pkg/github/__toolsnaps__/remove_pull_request_comment_reaction.snap b/pkg/github/__toolsnaps__/remove_pull_request_comment_reaction.snap new file mode 100644 index 0000000000..4078832219 --- /dev/null +++ b/pkg/github/__toolsnaps__/remove_pull_request_comment_reaction.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Remove Reaction from Pull Request Comment" + }, + "description": "Remove a reaction from a pull request conversation comment. For review comments on specific lines, use remove_pull_request_review_comment_reaction.", + "inputSchema": { + "properties": { + "comment_id": { + "description": "The pull request conversation comment ID", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "reaction_id": { + "description": "The reaction ID to remove", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "reaction_id" + ], + "type": "object" + }, + "name": "remove_pull_request_comment_reaction" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/remove_pull_request_reaction.snap b/pkg/github/__toolsnaps__/remove_pull_request_reaction.snap new file mode 100644 index 0000000000..1bcaf11c00 --- /dev/null +++ b/pkg/github/__toolsnaps__/remove_pull_request_reaction.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Remove Reaction from Pull Request" + }, + "description": "Remove a reaction from a pull request.", + "inputSchema": { + "properties": { + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "reaction_id": { + "description": "The reaction ID to remove", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "reaction_id" + ], + "type": "object" + }, + "name": "remove_pull_request_reaction" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_assignees.snap b/pkg/github/__toolsnaps__/update_issue_assignees.snap index 51a407c164..7bc787ac30 100644 --- a/pkg/github/__toolsnaps__/update_issue_assignees.snap +++ b/pkg/github/__toolsnaps__/update_issue_assignees.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue Assignees" }, - "description": "Update the assignees of an existing issue. This replaces the current assignees with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", + "description": "Update the assignees of an existing issue. Only works on issues; for pull requests use update_pull_request_assignees. This replaces the current assignees with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "assignees": { diff --git a/pkg/github/__toolsnaps__/update_issue_body.snap b/pkg/github/__toolsnaps__/update_issue_body.snap index e17043331d..804f2aee89 100644 --- a/pkg/github/__toolsnaps__/update_issue_body.snap +++ b/pkg/github/__toolsnaps__/update_issue_body.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue Body" }, - "description": "Update the body content of an existing issue.", + "description": "Update the body content of an existing issue. Only works on issues; for pull requests use update_pull_request_body.", "inputSchema": { "properties": { "body": { diff --git a/pkg/github/__toolsnaps__/update_issue_comment_ff_issues_granular.snap b/pkg/github/__toolsnaps__/update_issue_comment_ff_issues_granular.snap new file mode 100644 index 0000000000..1db5674398 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_comment_ff_issues_granular.snap @@ -0,0 +1,38 @@ +{ + "annotations": { + "idempotentHint": false, + "readOnlyHint": false, + "title": "Update issue comment" + }, + "description": "Update the body of an existing issue comment. Only works on issue comments; for pull request conversation comments use update_pull_request_comment.", + "inputSchema": { + "properties": { + "body": { + "description": "New comment content", + "minLength": 1, + "type": "string" + }, + "comment_id": { + "description": "The numeric ID of the issue comment to update.", + "minimum": 1, + "type": "integer" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "body" + ], + "type": "object" + }, + "name": "update_issue_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_labels.snap b/pkg/github/__toolsnaps__/update_issue_labels.snap index 2ad3877586..4250259f94 100644 --- a/pkg/github/__toolsnaps__/update_issue_labels.snap +++ b/pkg/github/__toolsnaps__/update_issue_labels.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue Labels" }, - "description": "Update the labels of an existing issue. This replaces the current labels with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", + "description": "Update the labels of an existing issue. Only works on issues; for pull requests use update_pull_request_labels. This replaces the current labels with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "issue_number": { diff --git a/pkg/github/__toolsnaps__/update_issue_milestone.snap b/pkg/github/__toolsnaps__/update_issue_milestone.snap index cae51246b4..76fbd1af01 100644 --- a/pkg/github/__toolsnaps__/update_issue_milestone.snap +++ b/pkg/github/__toolsnaps__/update_issue_milestone.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue Milestone" }, - "description": "Update the milestone of an existing issue.", + "description": "Update the milestone of an existing issue. Only works on issues; for pull requests use update_pull_request_milestone.", "inputSchema": { "properties": { "issue_number": { diff --git a/pkg/github/__toolsnaps__/update_issue_state.snap b/pkg/github/__toolsnaps__/update_issue_state.snap index 2a26e8be64..ca855c30d7 100644 --- a/pkg/github/__toolsnaps__/update_issue_state.snap +++ b/pkg/github/__toolsnaps__/update_issue_state.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue State" }, - "description": "Update the state of an existing issue (open or closed), with an optional state reason. When closing, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the decision. Use is_suggestion to propose the change without applying it directly.", + "description": "Update the state of an existing issue (open or closed), with an optional state reason. Only works on issues; for pull requests use update_pull_request_state. When closing, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the decision. Use is_suggestion to propose the change without applying it directly.", "inputSchema": { "properties": { "confidence": { diff --git a/pkg/github/__toolsnaps__/update_issue_title.snap b/pkg/github/__toolsnaps__/update_issue_title.snap index 48c74d98ef..5493d788ca 100644 --- a/pkg/github/__toolsnaps__/update_issue_title.snap +++ b/pkg/github/__toolsnaps__/update_issue_title.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue Title" }, - "description": "Update the title of an existing issue.", + "description": "Update the title of an existing issue. Only works on issues; for pull requests use update_pull_request_title.", "inputSchema": { "properties": { "issue_number": { diff --git a/pkg/github/__toolsnaps__/update_issue_type.snap b/pkg/github/__toolsnaps__/update_issue_type.snap index fbe8c90bb1..4ab801b9db 100644 --- a/pkg/github/__toolsnaps__/update_issue_type.snap +++ b/pkg/github/__toolsnaps__/update_issue_type.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue Type" }, - "description": "Set or remove the type of an existing issue. Pass null to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", + "description": "Set or remove the type of an existing issue. Only works on issues. Pass null to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "confidence": { diff --git a/pkg/github/__toolsnaps__/update_pull_request_assignees.snap b/pkg/github/__toolsnaps__/update_pull_request_assignees.snap new file mode 100644 index 0000000000..03e939a545 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_pull_request_assignees.snap @@ -0,0 +1,42 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Update Pull Request Assignees" + }, + "description": "Update the assignees of an existing pull request. This replaces the current assignees with the provided list.", + "inputSchema": { + "properties": { + "assignees": { + "description": "GitHub usernames to assign to this pull request", + "items": { + "type": "string" + }, + "type": "array" + }, + "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" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "assignees" + ], + "type": "object" + }, + "name": "update_pull_request_assignees" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_pull_request_comment.snap b/pkg/github/__toolsnaps__/update_pull_request_comment.snap new file mode 100644 index 0000000000..f3e5645153 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_pull_request_comment.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Update Pull Request Comment" + }, + "description": "Update the body of an existing pull request conversation comment. This tool cannot update pull request review comments.", + "inputSchema": { + "properties": { + "body": { + "description": "New comment content", + "minLength": 1, + "type": "string" + }, + "comment_id": { + "description": "The numeric ID of the pull request conversation comment to update. Do not use a pull request review comment ID.", + "minimum": 1, + "type": "integer" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "comment_id", + "body" + ], + "type": "object" + }, + "name": "update_pull_request_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_pull_request_labels.snap b/pkg/github/__toolsnaps__/update_pull_request_labels.snap new file mode 100644 index 0000000000..4553253d11 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_pull_request_labels.snap @@ -0,0 +1,42 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Update Pull Request Labels" + }, + "description": "Update the labels of an existing pull request. This replaces the current labels with the provided list.", + "inputSchema": { + "properties": { + "labels": { + "description": "Labels to apply to this pull request", + "items": { + "type": "string" + }, + "type": "array" + }, + "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" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "labels" + ], + "type": "object" + }, + "name": "update_pull_request_labels" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_pull_request_milestone.snap b/pkg/github/__toolsnaps__/update_pull_request_milestone.snap new file mode 100644 index 0000000000..8c263ea2cb --- /dev/null +++ b/pkg/github/__toolsnaps__/update_pull_request_milestone.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Update Pull Request Milestone" + }, + "description": "Update the milestone of an existing pull request.", + "inputSchema": { + "properties": { + "milestone": { + "description": "The milestone number to set on the pull request", + "minimum": 1, + "type": "integer" + }, + "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" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "milestone" + ], + "type": "object" + }, + "name": "update_pull_request_milestone" +} \ No newline at end of file diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 94f335ebe5..bdf083a300 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -100,8 +100,25 @@ var ( issuesConsolidatedFeatureRule = featureDisabledRule(FeatureFlagIssuesGranular) pullRequestsGranularFeatureRule = featureEnabledRule(FeatureFlagPullRequestsGranular) pullRequestsConsolidatedRule = featureDisabledRule(FeatureFlagPullRequestsGranular) + + // Conversation comment tools are shared by issues and pull requests. They are + // only split into issue-only and pull-request-only tools when both granular + // flags are enabled, so enabling one flag never removes pull request commenting. + commentsGranularFeatureRule = bothGranularFlagsRule(true) + commentsConsolidatedFeatureRule = bothGranularFlagsRule(false) ) +func bothGranularFlagsRule(wantBoth bool) inventory.FeatureRule { + issues := inventory.FeatureFlag(FeatureFlagIssuesGranular) + pullRequests := inventory.FeatureFlag(FeatureFlagPullRequestsGranular) + return inventory.NewFeatureRule( + []inventory.FeatureFlag{issues, pullRequests}, + func(featureAsBool inventory.FeatureResolver) bool { + return (featureAsBool(issues) && featureAsBool(pullRequests)) == wantBoth + }, + ) +} + // ResolveFeatureFlags computes the effective set of enabled feature flags by: // 1. Taking the user-supplied flags (from --features or HTTP request // configuration) and diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index cc3fbf0837..af2f15def6 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -3,6 +3,7 @@ package github import ( "context" "encoding/json" + "strings" "testing" "github.com/github/github-mcp-server/pkg/translations" @@ -298,3 +299,47 @@ func TestThreadResolutionReasonToolVariants(t *testing.T) { }) } } + +func TestCommentToolVariants(t *testing.T) { + issues := inventory.FeatureFlag(FeatureFlagIssuesGranular) + pullRequests := inventory.FeatureFlag(FeatureFlagPullRequestsGranular) + + tests := []struct { + name string + flags []inventory.FeatureFlag + issueOnly bool + hasPRCommentTools bool + hasIssueReactTools bool + }{ + {name: "no granular flags"}, + {name: "issues granular only", flags: []inventory.FeatureFlag{issues}, hasIssueReactTools: true}, + {name: "pull requests granular only", flags: []inventory.FeatureFlag{pullRequests}, hasPRCommentTools: true}, + {name: "both granular flags", flags: []inventory.FeatureFlag{issues, pullRequests}, issueOnly: true, hasPRCommentTools: true, hasIssueReactTools: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inv, err := NewInventory(translations.NullTranslationHelper). + WithToolsets([]string{"all"}). + WithFeatureChecker(featureCheckerFor(tt.flags...)). + Build() + require.NoError(t, err) + + tools := map[string][]inventory.ServerTool{} + for _, tool := range inv.AvailableTools(context.Background()) { + tools[tool.Tool.Name] = append(tools[tool.Tool.Name], tool) + } + + for _, name := range []string{"add_issue_comment", "update_issue_comment"} { + require.Len(t, tools[name], 1, name) + assert.Equal(t, tt.issueOnly, strings.Contains(tools[name][0].Tool.Description, "Only works on issue"), name) + } + for _, name := range []string{"add_pull_request_comment", "update_pull_request_comment", "add_pull_request_comment_reaction", "remove_pull_request_comment_reaction"} { + assert.Equal(t, tt.hasPRCommentTools, len(tools[name]) == 1, name) + } + for _, name := range []string{"add_issue_comment_reaction", "remove_issue_comment_reaction"} { + assert.Equal(t, tt.hasIssueReactTools, len(tools[name]) == 1, name) + } + }) + } +} diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 425f954ef9..34432c7923 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -3,6 +3,7 @@ package github import ( "context" "encoding/json" + "fmt" "maps" "net/http" "strings" @@ -29,8 +30,9 @@ func granularToolsForToolset(toolsetID inventory.ToolsetID, featureFlag string) for _, feature := range features { usesFeature = usesFeature || feature == flag } - if tool.Toolset.ID == toolsetID && usesFeature && - tool.FeatureRule.Enabled(func(feature inventory.FeatureFlag) bool { return feature == flag }) { + enabledWithFlag := tool.FeatureRule.Enabled(func(feature inventory.FeatureFlag) bool { return feature == flag }) + enabledByDefault := tool.FeatureRule.Enabled(func(inventory.FeatureFlag) bool { return false }) + if tool.Toolset.ID == toolsetID && usesFeature && enabledWithFlag && !enabledByDefault { result = append(result, tool) } } @@ -69,6 +71,15 @@ func TestGranularToolSnaps(t *testing.T) { GranularUnresolveReviewThread, GranularAddPullRequestReviewCommentReaction, GranularRemovePullRequestReviewCommentReaction, + GranularUpdatePullRequestAssignees, + GranularUpdatePullRequestLabels, + GranularUpdatePullRequestMilestone, + GranularAddPullRequestReaction, + GranularRemovePullRequestReaction, + GranularAddPullRequestComment, + GranularUpdatePullRequestComment, + GranularAddPullRequestCommentReaction, + GranularRemovePullRequestCommentReaction, } for _, constructor := range toolConstructors { @@ -77,6 +88,16 @@ func TestGranularToolSnaps(t *testing.T) { require.NoError(t, toolsnaps.Test(serverTool.Tool.Name, serverTool.Tool)) }) } + + for _, constructor := range []func(translations.TranslationHelperFunc) inventory.ServerTool{ + GranularAddIssueComment, + GranularUpdateIssueComment, + } { + serverTool := constructor(translations.NullTranslationHelper) + t.Run(serverTool.Tool.Name+"_ff_"+FeatureFlagIssuesGranular, func(t *testing.T) { + require.NoError(t, toolsnaps.Test(serverTool.Tool.Name+"_ff_"+FeatureFlagIssuesGranular, serverTool.Tool)) + }) + } } func TestIssuesGranularToolset(t *testing.T) { @@ -142,6 +163,15 @@ func TestPullRequestsGranularToolset(t *testing.T) { "unresolve_review_thread", "add_pull_request_review_comment_reaction", "remove_pull_request_review_comment_reaction", + "update_pull_request_assignees", + "update_pull_request_labels", + "update_pull_request_milestone", + "add_pull_request_reaction", + "remove_pull_request_reaction", + "add_pull_request_comment", + "update_pull_request_comment", + "add_pull_request_comment_reaction", + "remove_pull_request_comment_reaction", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -220,6 +250,7 @@ func TestGranularCreateIssue(t *testing.T) { func TestGranularUpdateIssueTitle(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{ Number: gogithub.Ptr(42), Title: gogithub.Ptr("New Title"), @@ -242,6 +273,7 @@ func TestGranularUpdateIssueTitle(t *testing.T) { func TestGranularUpdateIssueBody(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ "body": "Updated body", }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{ @@ -266,6 +298,7 @@ func TestGranularUpdateIssueBody(t *testing.T) { func TestGranularUpdateIssueAssignees(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ "assignees": []any{"user1", "user2"}, }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), @@ -361,6 +394,7 @@ func TestGranularUpdateIssueAssigneesObjectForm(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) @@ -504,6 +538,7 @@ func TestGranularUpdateIssueLabels(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) @@ -582,6 +617,7 @@ func TestGranularUpdateIssueLabelsSuggest(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) @@ -731,6 +767,7 @@ func TestGranularUpdateIssueLabelsConfidence(t *testing.T) { } client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) @@ -748,6 +785,7 @@ func TestGranularUpdateIssueLabelsConfidence(t *testing.T) { func TestGranularUpdateIssueMilestone(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ "milestone": float64(5), }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), @@ -818,6 +856,7 @@ func TestGranularUpdateIssueType(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) @@ -917,6 +956,7 @@ func TestGranularUpdateIssueTypeSuggest(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) @@ -1055,6 +1095,7 @@ func TestGranularUpdateIssueTypeConfidence(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) @@ -1153,6 +1194,7 @@ func TestGranularUpdateIssueState(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{ Number: gogithub.Ptr(1), @@ -1236,6 +1278,7 @@ func TestGranularUpdateIssueStateSuggest(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) @@ -2423,6 +2466,7 @@ func TestGranularAddIssueReaction(t *testing.T) { { name: "add reaction to issue successfully", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PostReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusCreated, mockReaction), }), args: map[string]any{ @@ -2488,6 +2532,7 @@ func TestGranularRemoveIssueReaction(t *testing.T) { { name: "remove reaction from issue successfully", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), DeleteReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusNoContent, nil), }), args: map[string]any{ @@ -2510,6 +2555,7 @@ func TestGranularRemoveIssueReaction(t *testing.T) { { name: "API error", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), DeleteReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusNotFound, `{"message":"Not Found"}`), }), args: map[string]any{ @@ -2559,6 +2605,8 @@ func TestGranularAddIssueCommentReaction(t *testing.T) { { name: "add reaction to issue comment successfully", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: mockResponse(t, http.StatusOK, &gogithub.IssueComment{IssueURL: gogithub.Ptr("https://api.github.com/repos/owner/repo/issues/1")}), + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), PostReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusCreated, mockReaction), }), args: map[string]any{ @@ -2614,6 +2662,8 @@ func TestGranularRemoveIssueCommentReaction(t *testing.T) { { name: "remove reaction from issue comment successfully", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: mockResponse(t, http.StatusOK, &gogithub.IssueComment{IssueURL: gogithub.Ptr("https://api.github.com/repos/owner/repo/issues/1")}), + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), DeleteReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusNoContent, nil), }), args: map[string]any{ @@ -2636,6 +2686,8 @@ func TestGranularRemoveIssueCommentReaction(t *testing.T) { { name: "API error", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: mockResponse(t, http.StatusOK, &gogithub.IssueComment{IssueURL: gogithub.Ptr("https://api.github.com/repos/owner/repo/issues/1")}), + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), DeleteReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusNotFound, `{"message":"Not Found"}`), }), args: map[string]any{ @@ -2795,3 +2847,396 @@ func TestGranularRemovePullRequestReviewCommentReaction(t *testing.T) { }) } } + +// --- Issue vs pull request scoping tests --- + +func mockPullRequestIssue() *gogithub.Issue { + return &gogithub.Issue{ + Number: gogithub.Ptr(7), + PullRequestLinks: &gogithub.PullRequestLinks{URL: gogithub.Ptr("https://api.github.com/repos/owner/repo/pulls/7")}, + } +} + +func failIfCalled(t *testing.T) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusInternalServerError) + } +} + +func TestIssueToolsRejectPullRequests(t *testing.T) { + baseArgs := map[string]any{"owner": "owner", "repo": "repo", "issue_number": float64(7)} + tests := []struct { + constructor func(translations.TranslationHelperFunc) inventory.ServerTool + extraArgs map[string]any + }{ + {GranularUpdateIssueTitle, map[string]any{"title": "New title"}}, + {GranularUpdateIssueBody, map[string]any{"body": "New body"}}, + {GranularUpdateIssueAssignees, map[string]any{"assignees": []any{"octocat"}}}, + {GranularUpdateIssueLabels, map[string]any{"labels": []any{"bug"}}}, + {GranularUpdateIssueMilestone, map[string]any{"milestone": float64(1)}}, + {GranularUpdateIssueType, map[string]any{"issue_type": "Bug"}}, + {GranularUpdateIssueState, map[string]any{"state": "closed"}}, + {GranularAddIssueReaction, map[string]any{"content": "+1"}}, + {GranularRemoveIssueReaction, map[string]any{"reaction_id": float64(1)}}, + } + + for _, tc := range tests { + serverTool := tc.constructor(translations.NullTranslationHelper) + t.Run(serverTool.Tool.Name, func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockPullRequestIssue()), + PatchReposIssuesByOwnerByRepoByIssueNumber: failIfCalled(t), + PostReposIssuesReactionsByOwnerByRepoByIssueNumber: failIfCalled(t), + DeleteReposIssuesReactionsByOwnerByRepoByIssueNumber: failIfCalled(t), + })) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + args := maps.Clone(baseArgs) + maps.Copy(args, tc.extraArgs) + request := createMCPRequest(args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "#7 in owner/repo is a pull request, not an issue") + }) + } +} + +func TestIssueToolsReportLookupFailure(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusNotFound, `{"message":"Not Found"}`), + PatchReposIssuesByOwnerByRepoByIssueNumber: failIfCalled(t), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueTitle(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo", "issue_number": float64(7), "title": "x"}) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "failed to get #7") +} + +func TestGranularPullRequestIssueFieldTools(t *testing.T) { + tests := []struct { + constructor func(translations.TranslationHelperFunc) inventory.ServerTool + args map[string]any + expectedReq map[string]any + }{ + { + constructor: GranularUpdatePullRequestAssignees, + args: map[string]any{"assignees": []any{"octocat", "hubot"}}, + expectedReq: map[string]any{"assignees": []any{"octocat", "hubot"}}, + }, + { + constructor: GranularUpdatePullRequestAssignees, + args: map[string]any{"assignees": []any{}}, + expectedReq: map[string]any{"assignees": []any{}}, + }, + { + constructor: GranularUpdatePullRequestLabels, + args: map[string]any{"labels": []any{"bug", "enhancement"}}, + expectedReq: map[string]any{"labels": []any{"bug", "enhancement"}}, + }, + { + constructor: GranularUpdatePullRequestMilestone, + args: map[string]any{"milestone": float64(3)}, + expectedReq: map[string]any{"milestone": float64(3)}, + }, + } + + for _, tc := range tests { + serverTool := tc.constructor(translations.NullTranslationHelper) + t.Run(serverTool.Tool.Name, func(t *testing.T) { + t.Run("updates pull request", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockPullRequestIssue()), + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). + andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{ + ID: gogithub.Ptr(int64(700)), + HTMLURL: gogithub.Ptr("https://github.com/owner/repo/pull/7"), + })), + })) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + args := map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(7)} + maps.Copy(args, tc.args) + request := createMCPRequest(args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Contains(t, getTextResult(t, result).Text, "https://github.com/owner/repo/pull/7") + }) + + t.Run("rejects issues", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(7)}), + PatchReposIssuesByOwnerByRepoByIssueNumber: failIfCalled(t), + })) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + args := map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(7)} + maps.Copy(args, tc.args) + request := createMCPRequest(args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "#7 in owner/repo is an issue, not a pull request") + }) + }) + } +} + +func TestGranularPullRequestReactions(t *testing.T) { + tests := []struct { + name string + constructor func(translations.TranslationHelperFunc) inventory.ServerTool + args map[string]any + handlers map[string]http.HandlerFunc + expectText string + }{ + { + name: "add reaction", + constructor: GranularAddPullRequestReaction, + args: map[string]any{"content": "rocket"}, + handlers: map[string]http.HandlerFunc{ + PostReposIssuesReactionsByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{"content": "rocket"}). + andThen(mockResponse(t, http.StatusCreated, &gogithub.Reaction{ID: gogithub.Ptr(int64(99))})), + }, + expectText: "repos/owner/repo/issues/7/reactions/99", + }, + { + name: "remove reaction", + constructor: GranularRemovePullRequestReaction, + args: map[string]any{"reaction_id": float64(99)}, + handlers: map[string]http.HandlerFunc{ + DeleteReposIssuesReactionsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusNoContent, nil), + }, + expectText: "reaction successfully removed from pull request", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + serverTool := tc.constructor(translations.NullTranslationHelper) + args := map[string]any{"owner": "owner", "repo": "repo", "pullNumber": float64(7)} + maps.Copy(args, tc.args) + + t.Run("succeeds on pull request", func(t *testing.T) { + handlers := maps.Clone(tc.handlers) + handlers[GetReposIssuesByOwnerByRepoByIssueNumber] = mockResponse(t, http.StatusOK, mockPullRequestIssue()) + client := mustNewGHClient(t, MockHTTPClientWithHandlers(handlers)) + deps := BaseDeps{Client: client} + + request := createMCPRequest(args) + handler := serverTool.Handler(deps) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Contains(t, getTextResult(t, result).Text, tc.expectText) + }) + + t.Run("rejects issues", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(7)}), + PostReposIssuesReactionsByOwnerByRepoByIssueNumber: failIfCalled(t), + DeleteReposIssuesReactionsByOwnerByRepoByIssueNumber: failIfCalled(t), + })) + deps := BaseDeps{Client: client} + + request := createMCPRequest(args) + handler := serverTool.Handler(deps) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "#7 in owner/repo is an issue, not a pull request") + }) + }) + } +} + +func mockIssueComment(parentNumber int) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(&gogithub.IssueComment{ + ID: gogithub.Ptr(int64(55)), + IssueURL: gogithub.Ptr(fmt.Sprintf("https://api.github.com/repos/owner/repo/issues/%d", parentNumber)), + }) + } +} + +func TestIssueCommentToolsRejectPullRequestComments(t *testing.T) { + tests := []struct { + constructor func(translations.TranslationHelperFunc) inventory.ServerTool + args map[string]any + }{ + {GranularAddIssueComment, map[string]any{"issue_number": float64(7), "body": "hi"}}, + {GranularAddIssueComment, map[string]any{"issue_number": float64(7), "reaction": "+1"}}, + {GranularUpdateIssueComment, map[string]any{"comment_id": float64(55), "body": "hi"}}, + {GranularAddIssueCommentReaction, map[string]any{"comment_id": float64(55), "content": "+1"}}, + {GranularRemoveIssueCommentReaction, map[string]any{"comment_id": float64(55), "reaction_id": float64(1)}}, + } + + for _, tc := range tests { + serverTool := tc.constructor(translations.NullTranslationHelper) + t.Run(serverTool.Tool.Name, func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: mockIssueComment(7), + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockPullRequestIssue()), + PostReposIssuesCommentsByOwnerByRepoByIssueNumber: failIfCalled(t), + PatchReposIssuesCommentByOwnerByRepoByCommentID: failIfCalled(t), + PostReposIssuesReactionsByOwnerByRepoByIssueNumber: failIfCalled(t), + PostReposIssuesCommentsReactionsByOwnerByRepoByCommentID: failIfCalled(t), + DeleteReposIssuesCommentsReactionsByOwnerByRepoByCommentID: failIfCalled(t), + })) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + args := map[string]any{"owner": "owner", "repo": "repo"} + maps.Copy(args, tc.args) + request := createMCPRequest(args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "#7 in owner/repo is a pull request, not an issue") + }) + } +} + +func TestGranularIssueCommentToolsOnIssues(t *testing.T) { + t.Run("add_issue_comment", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(7)}), + PostReposIssuesCommentsByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{"body": "hi"}). + andThen(mockResponse(t, http.StatusCreated, &gogithub.IssueComment{ID: gogithub.Ptr(int64(55))})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularAddIssueComment(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo", "issue_number": float64(7), "body": "hi"}) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + }) + + t.Run("update_issue_comment", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: mockIssueComment(7), + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(7)}), + PatchReposIssuesCommentByOwnerByRepoByCommentID: expectRequestBody(t, map[string]any{"body": "edited"}). + andThen(mockResponse(t, http.StatusOK, &gogithub.IssueComment{ID: gogithub.Ptr(int64(55))})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueComment(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo", "comment_id": float64(55), "body": "edited"}) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + }) +} + +func TestGranularPullRequestCommentTools(t *testing.T) { + tests := []struct { + name string + constructor func(translations.TranslationHelperFunc) inventory.ServerTool + args map[string]any + handlers map[string]http.HandlerFunc + expectText string + }{ + { + name: "add comment", + constructor: GranularAddPullRequestComment, + args: map[string]any{"pullNumber": float64(7), "body": "hi"}, + handlers: map[string]http.HandlerFunc{ + PostReposIssuesCommentsByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{"body": "hi"}). + andThen(mockResponse(t, http.StatusCreated, &gogithub.IssueComment{ + ID: gogithub.Ptr(int64(55)), + HTMLURL: gogithub.Ptr("https://github.com/owner/repo/pull/7#issuecomment-55"), + })), + }, + expectText: "pull/7#issuecomment-55", + }, + { + name: "update comment", + constructor: GranularUpdatePullRequestComment, + args: map[string]any{"comment_id": float64(55), "body": "edited"}, + handlers: map[string]http.HandlerFunc{ + PatchReposIssuesCommentByOwnerByRepoByCommentID: expectRequestBody(t, map[string]any{"body": "edited"}). + andThen(mockResponse(t, http.StatusOK, &gogithub.IssueComment{ + ID: gogithub.Ptr(int64(55)), + HTMLURL: gogithub.Ptr("https://github.com/owner/repo/pull/7#issuecomment-55"), + })), + }, + expectText: "pull/7#issuecomment-55", + }, + { + name: "add comment reaction", + constructor: GranularAddPullRequestCommentReaction, + args: map[string]any{"comment_id": float64(55), "content": "heart"}, + handlers: map[string]http.HandlerFunc{ + PostReposIssuesCommentsReactionsByOwnerByRepoByCommentID: expectRequestBody(t, map[string]any{"content": "heart"}). + andThen(mockResponse(t, http.StatusCreated, &gogithub.Reaction{ID: gogithub.Ptr(int64(99))})), + }, + expectText: "issues/comments/55/reactions/99", + }, + { + name: "remove comment reaction", + constructor: GranularRemovePullRequestCommentReaction, + args: map[string]any{"comment_id": float64(55), "reaction_id": float64(99)}, + handlers: map[string]http.HandlerFunc{ + DeleteReposIssuesCommentsReactionsByOwnerByRepoByCommentID: mockResponse(t, http.StatusNoContent, nil), + }, + expectText: "reaction successfully removed from pull request comment", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + serverTool := tc.constructor(translations.NullTranslationHelper) + args := map[string]any{"owner": "owner", "repo": "repo"} + maps.Copy(args, tc.args) + + t.Run("succeeds on pull request", func(t *testing.T) { + handlers := maps.Clone(tc.handlers) + handlers[GetReposIssuesCommentByOwnerByRepoByCommentID] = mockIssueComment(7) + handlers[GetReposIssuesByOwnerByRepoByIssueNumber] = mockResponse(t, http.StatusOK, mockPullRequestIssue()) + client := mustNewGHClient(t, MockHTTPClientWithHandlers(handlers)) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + request := createMCPRequest(args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Contains(t, getTextResult(t, result).Text, tc.expectText) + }) + + t.Run("rejects issues", func(t *testing.T) { + handlers := map[string]http.HandlerFunc{ + GetReposIssuesCommentByOwnerByRepoByCommentID: mockIssueComment(7), + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(7)}), + } + for endpoint := range tc.handlers { + handlers[endpoint] = failIfCalled(t) + } + client := mustNewGHClient(t, MockHTTPClientWithHandlers(handlers)) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + request := createMCPRequest(args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "#7 in owner/repo is an issue, not a pull request") + }) + }) + } +} diff --git a/pkg/github/issues.go b/pkg/github/issues.go index f1de84eb27..cd153b84b1 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1361,15 +1361,39 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool { return st } -// AddIssueComment creates a tool to add a comment or reaction to an issue. +// AddIssueComment creates a tool to add a comment or reaction to an issue or pull request. func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + return addIssueComment(t, false) +} + +// GranularAddIssueComment is the issue-only variant of add_issue_comment. It is +// served instead of AddIssueComment when both granular feature flags are enabled, +// with add_pull_request_comment covering pull requests. +func GranularAddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { + return addIssueComment(t, true) +} + +func addIssueComment(t translations.TranslationHelperFunc, issueOnly bool) inventory.ServerTool { + description := "Add a comment and/or reaction to a specific issue or issue comment in a GitHub repository. Use this tool with pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add or react to review comments. At least one of body or reaction is required." + title := "Add comment to issue or pull request" + issueNumberDescription := "Issue or pull request number to comment on or react to." + commentIDDescription := "The numeric ID of the issue or pull request comment to react to. Use this for reactions to comments; omit it to react to the issue or pull request itself. Cannot be combined with body." + featureRule := commentsConsolidatedFeatureRule + if issueOnly { + description = "Add a comment and/or reaction to a specific issue or issue comment in a GitHub repository. Only works on issues; for pull requests use add_pull_request_comment. At least one of body or reaction is required." + title = "Add comment to issue" + issueNumberDescription = "Issue number to comment on or react to." + commentIDDescription = "The numeric ID of the issue comment to react to. Use this for reactions to comments; omit it to react to the issue itself. Cannot be combined with body." + featureRule = commentsGranularFeatureRule + } + + st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_comment", - Description: t("TOOL_ADD_ISSUE_COMMENT_DESCRIPTION", "Add a comment and/or reaction to a specific issue or issue comment in a GitHub repository. Use this tool with pull requests as well (in this case pass pull request number as issue_number), but only if user is not asking specifically to add or react to review comments. At least one of body or reaction is required."), + Description: t("TOOL_ADD_ISSUE_COMMENT_DESCRIPTION", description), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_ADD_ISSUE_COMMENT_USER_TITLE", "Add comment to issue or pull request"), + Title: t("TOOL_ADD_ISSUE_COMMENT_USER_TITLE", title), ReadOnlyHint: false, }, InputSchema: &jsonschema.Schema{ @@ -1385,11 +1409,11 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }, "issue_number": { Type: "number", - Description: "Issue or pull request number to comment on or react to.", + Description: issueNumberDescription, }, "comment_id": { Type: "integer", - Description: "The numeric ID of the issue or pull request comment to react to. Use this for reactions to comments; omit it to react to the issue or pull request itself. Cannot be combined with body.", + Description: commentIDDescription, Minimum: jsonschema.Ptr(1.0), }, "body": { @@ -1464,6 +1488,14 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if issueOnly { + // A comment_id is validated against issue_number below, so checking + // issue_number also covers reactions to comments. + if result := EnsureIssue(ctx, client, owner, repo, issueNumber, "add_issue_comment"); result != nil { + return result, nil, nil + } + } + var reactionResponse *MinimalResponse if hasReaction { if hasCommentID { @@ -1550,15 +1582,37 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultText(string(r)), nil, nil }) + st.FeatureRule = featureRule + return st } // UpdateIssueComment creates a tool to update an issue or pull request conversation comment. func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + return updateIssueComment(t, false) +} + +// GranularUpdateIssueComment is the issue-only variant of update_issue_comment. It +// is served instead of UpdateIssueComment when both granular feature flags are +// enabled, with update_pull_request_comment covering pull requests. +func GranularUpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool { + return updateIssueComment(t, true) +} + +func updateIssueComment(t translations.TranslationHelperFunc, issueOnly bool) inventory.ServerTool { + description := "Update the body of an existing issue or pull request conversation comment. This tool cannot update pull request review comments." + commentIDDescription := "The numeric ID of the issue or pull request conversation comment to update. Do not use a pull request review comment ID." + featureRule := commentsConsolidatedFeatureRule + if issueOnly { + description = "Update the body of an existing issue comment. Only works on issue comments; for pull request conversation comments use update_pull_request_comment." + commentIDDescription = "The numeric ID of the issue comment to update." + featureRule = commentsGranularFeatureRule + } + + st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_comment", - Description: t("TOOL_UPDATE_ISSUE_COMMENT_DESCRIPTION", "Update the body of an existing issue or pull request conversation comment. This tool cannot update pull request review comments."), + Description: t("TOOL_UPDATE_ISSUE_COMMENT_DESCRIPTION", description), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_COMMENT_USER_TITLE", "Update issue comment"), ReadOnlyHint: false, @@ -1576,7 +1630,7 @@ func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTo }, "comment_id": { Type: "integer", - Description: "The numeric ID of the issue or pull request conversation comment to update. Do not use a pull request review comment ID.", + Description: commentIDDescription, Minimum: jsonschema.Ptr(1.0), }, "body": { @@ -1621,6 +1675,12 @@ func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTo return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if issueOnly { + if result := EnsureIssueComment(ctx, client, owner, repo, commentID, "update_issue_comment"); result != nil { + return result, nil, nil + } + } + updatedComment, resp, err := client.Issues.EditComment(ctx, owner, repo, commentID, &github.IssueComment{ Body: github.Ptr(body), }) @@ -1641,6 +1701,8 @@ func UpdateIssueComment(t translations.TranslationHelperFunc) inventory.ServerTo return utils.NewToolResultText(string(r)), nil, nil }) + st.FeatureRule = featureRule + return st } func isValidIssueReaction(reaction string) bool { diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index f22a8a1536..f88fad39b0 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -23,6 +23,78 @@ func normalizeConfidence(confidence string) string { return strings.ToUpper(strings.TrimSpace(confidence)) } +// EnsureIssue returns an error result if the given number refers to a pull request +// rather than an issue. GitHub's issues API accepts pull request numbers, so tools +// that should only act on issues must check this explicitly. A nil result means +// the number refers to an issue. +func EnsureIssue(ctx context.Context, client *github.Client, owner, repo string, issueNumber int, companionTool ...string) *mcp.CallToolResult { + return ensureIssueKind(ctx, client, owner, repo, issueNumber, false, firstOrDefault(companionTool, "the pull request tools")) +} + +// EnsurePullRequest returns an error result if the given number refers to an issue +// rather than a pull request. A nil result means the number refers to a pull request. +func EnsurePullRequest(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, companionTool ...string) *mcp.CallToolResult { + return ensureIssueKind(ctx, client, owner, repo, pullNumber, true, firstOrDefault(companionTool, "the issue tools")) +} + +// EnsureIssueComment returns an error result if the given conversation comment +// belongs to a pull request rather than an issue. A nil result means the comment +// belongs to an issue. +func EnsureIssueComment(ctx context.Context, client *github.Client, owner, repo string, commentID int64, companionTool ...string) *mcp.CallToolResult { + return ensureCommentKind(ctx, client, owner, repo, commentID, false, firstOrDefault(companionTool, "the pull request comment tools")) +} + +// EnsurePullRequestComment returns an error result if the given conversation +// comment belongs to an issue rather than a pull request. A nil result means the +// comment belongs to a pull request. +func EnsurePullRequestComment(ctx context.Context, client *github.Client, owner, repo string, commentID int64, companionTool ...string) *mcp.CallToolResult { + return ensureCommentKind(ctx, client, owner, repo, commentID, true, firstOrDefault(companionTool, "the issue comment tools")) +} + +func firstOrDefault(values []string, fallback string) string { + if len(values) > 0 { + return values[0] + } + return fallback +} + +func ensureCommentKind(ctx context.Context, client *github.Client, owner, repo string, commentID int64, wantPullRequest bool, companionTool string) *mcp.CallToolResult { + comment, resp, err := client.Issues.GetComment(ctx, owner, repo, commentID) + if err != nil { + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + return ghErrors.NewGitHubAPIErrorResponse(ctx, fmt.Sprintf("failed to get comment %d", commentID), resp, err) + } + _ = resp.Body.Close() + + number, err := issueNumberFromIssueURL(comment.GetIssueURL()) + if err != nil { + return utils.NewToolResultErrorFromErr(fmt.Sprintf("failed to determine the issue or pull request for comment %d", commentID), err) + } + return ensureIssueKind(ctx, client, owner, repo, number, wantPullRequest, companionTool) +} + +func ensureIssueKind(ctx context.Context, client *github.Client, owner, repo string, number int, wantPullRequest bool, companionTool string) *mcp.CallToolResult { + issue, resp, err := client.Issues.Get(ctx, owner, repo, number) + if err != nil { + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + return ghErrors.NewGitHubAPIErrorResponse(ctx, fmt.Sprintf("failed to get #%d", number), resp, err) + } + _ = resp.Body.Close() + + isPullRequest := issue.IsPullRequest() + switch { + case isPullRequest && !wantPullRequest: + return utils.NewToolResultError(fmt.Sprintf("#%d in %s/%s is a pull request, not an issue. Use %s to modify it.", number, owner, repo, companionTool)) + case !isPullRequest && wantPullRequest: + return utils.NewToolResultError(fmt.Sprintf("#%d in %s/%s is an issue, not a pull request. Use %s to modify it.", number, owner, repo, companionTool)) + } + return nil +} + // issueUpdateTool is a helper to create single-field issue update tools. func issueUpdateTool( t translations.TranslationHelperFunc, @@ -92,6 +164,10 @@ func issueUpdateTool( return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssue(ctx, client, owner, repo, issueNumber, strings.Replace(name, "update_issue_", "update_pull_request_", 1)); result != nil { + return result, nil, nil + } + issue, resp, err := client.Issues.Update(ctx, owner, repo, issueNumber, issueReq) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue", resp, err), nil, nil @@ -242,7 +318,7 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT func GranularUpdateIssueTitle(t translations.TranslationHelperFunc) inventory.ServerTool { return issueUpdateTool(t, "update_issue_title", - "Update the title of an existing issue.", + "Update the title of an existing issue. Only works on issues; for pull requests use update_pull_request_title.", "Update Issue Title", map[string]*jsonschema.Schema{ "title": {Type: "string", Description: "The new title for the issue"}, @@ -262,7 +338,7 @@ func GranularUpdateIssueTitle(t translations.TranslationHelperFunc) inventory.Se func GranularUpdateIssueBody(t translations.TranslationHelperFunc) inventory.ServerTool { return issueUpdateTool(t, "update_issue_body", - "Update the body content of an existing issue.", + "Update the body content of an existing issue. Only works on issues; for pull requests use update_pull_request_body.", "Update Issue Body", map[string]*jsonschema.Schema{ "body": {Type: "string", Description: "The new body content for the issue"}, @@ -284,7 +360,7 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_assignees", - Description: t("TOOL_UPDATE_ISSUE_ASSIGNEES_DESCRIPTION", "Update the assignees of an existing issue. This replaces the current assignees with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), + Description: t("TOOL_UPDATE_ISSUE_ASSIGNEES_DESCRIPTION", "Update the assignees of an existing issue. Only works on issues; for pull requests use update_pull_request_assignees. This replaces the current assignees with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_ASSIGNEES_USER_TITLE", "Update Issue Assignees"), ReadOnlyHint: false, @@ -425,6 +501,10 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssue(ctx, client, owner, repo, issueNumber, "update_issue_assignees"); result != nil { + return result, nil, nil + } + var body any if useObjectForm { body = &assigneesUpdateRequest{Assignees: payload} @@ -502,7 +582,7 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_labels", - Description: t("TOOL_UPDATE_ISSUE_LABELS_DESCRIPTION", "Update the labels of an existing issue. This replaces the current labels with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), + Description: t("TOOL_UPDATE_ISSUE_LABELS_DESCRIPTION", "Update the labels of an existing issue. Only works on issues; for pull requests use update_pull_request_labels. This replaces the current labels with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_LABELS_USER_TITLE", "Update Issue Labels"), ReadOnlyHint: false, @@ -643,6 +723,10 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssue(ctx, client, owner, repo, issueNumber, "update_issue_labels"); result != nil { + return result, nil, nil + } + var body any if useObjectForm { body = &labelsUpdateRequest{Labels: payload} @@ -686,7 +770,7 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventory.ServerTool { return issueUpdateTool(t, "update_issue_milestone", - "Update the milestone of an existing issue.", + "Update the milestone of an existing issue. Only works on issues; for pull requests use update_pull_request_milestone.", "Update Issue Milestone", map[string]*jsonschema.Schema{ "milestone": { @@ -727,7 +811,7 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_type", - Description: t("TOOL_UPDATE_ISSUE_TYPE_DESCRIPTION", "Set or remove the type of an existing issue. Pass null to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), + Description: t("TOOL_UPDATE_ISSUE_TYPE_DESCRIPTION", "Set or remove the type of an existing issue. Only works on issues. Pass null to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_TYPE_USER_TITLE", "Update Issue Type"), ReadOnlyHint: false, @@ -826,6 +910,10 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssue(ctx, client, owner, repo, issueNumber, "update_issue_type"); result != nil { + return result, nil, nil + } + var body any switch { case issueType == nil: @@ -893,7 +981,7 @@ func GranularUpdateIssueState(t translations.TranslationHelperFunc) inventory.Se ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_state", - Description: t("TOOL_UPDATE_ISSUE_STATE_DESCRIPTION", "Update the state of an existing issue (open or closed), with an optional state reason. When closing, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the decision. Use is_suggestion to propose the change without applying it directly."), + Description: t("TOOL_UPDATE_ISSUE_STATE_DESCRIPTION", "Update the state of an existing issue (open or closed), with an optional state reason. Only works on issues; for pull requests use update_pull_request_state. When closing, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the decision. Use is_suggestion to propose the change without applying it directly."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_STATE_USER_TITLE", "Update Issue State"), ReadOnlyHint: false, @@ -1012,6 +1100,10 @@ func GranularUpdateIssueState(t translations.TranslationHelperFunc) inventory.Se return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssue(ctx, client, owner, repo, issueNumber, "update_issue_state"); result != nil { + return result, nil, nil + } + var body any if rationale != "" || isSuggestion || confidence != "" || duplicateOf != 0 { req := &stateUpdateRequest{ @@ -1584,15 +1676,15 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv return st } -// GranularAddIssueReaction adds a reaction to an issue or pull request. +// GranularAddIssueReaction adds a reaction to an issue. func GranularAddIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_reaction", - Description: t("TOOL_ADD_ISSUE_REACTION_DESCRIPTION", "Add a reaction to an issue or pull request."), + Description: t("TOOL_ADD_ISSUE_REACTION_DESCRIPTION", "Add a reaction to an issue. Only works on issues; for pull requests use add_pull_request_reaction."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_ADD_ISSUE_REACTION_USER_TITLE", "Add Reaction to Issue or Pull Request"), + Title: t("TOOL_ADD_ISSUE_REACTION_USER_TITLE", "Add Reaction to Issue"), ReadOnlyHint: false, DestructiveHint: jsonschema.Ptr(false), OpenWorldHint: jsonschema.Ptr(true), @@ -1646,6 +1738,10 @@ func GranularAddIssueReaction(t translations.TranslationHelperFunc) inventory.Se return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssue(ctx, client, owner, repo, issueNumber, "add_issue_reaction"); result != nil { + return result, nil, nil + } + reaction, resp, err := client.Reactions.CreateIssueReaction(ctx, owner, repo, issueNumber, content) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to issue", resp, err), nil, nil @@ -1666,15 +1762,15 @@ func GranularAddIssueReaction(t translations.TranslationHelperFunc) inventory.Se return st } -// GranularRemoveIssueReaction removes a reaction from an issue or pull request. +// GranularRemoveIssueReaction removes a reaction from an issue. func GranularRemoveIssueReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "remove_issue_reaction", - Description: t("TOOL_REMOVE_ISSUE_REACTION_DESCRIPTION", "Remove a reaction from an issue or pull request."), + Description: t("TOOL_REMOVE_ISSUE_REACTION_DESCRIPTION", "Remove a reaction from an issue. Only works on issues; for pull requests use remove_pull_request_reaction."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_REMOVE_ISSUE_REACTION_USER_TITLE", "Remove Reaction from Issue or Pull Request"), + Title: t("TOOL_REMOVE_ISSUE_REACTION_USER_TITLE", "Remove Reaction from Issue"), ReadOnlyHint: false, DestructiveHint: jsonschema.Ptr(true), OpenWorldHint: jsonschema.Ptr(true), @@ -1728,6 +1824,10 @@ func GranularRemoveIssueReaction(t translations.TranslationHelperFunc) inventory return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssue(ctx, client, owner, repo, issueNumber, "remove_issue_reaction"); result != nil { + return result, nil, nil + } + resp, err := client.Reactions.DeleteIssueReaction(ctx, owner, repo, issueNumber, reactionID) if resp != nil && resp.Body != nil { defer func() { _ = resp.Body.Close() }() @@ -1743,15 +1843,15 @@ func GranularRemoveIssueReaction(t translations.TranslationHelperFunc) inventory return st } -// GranularAddIssueCommentReaction adds a reaction to an issue or pull request comment. +// GranularAddIssueCommentReaction adds a reaction to an issue comment. func GranularAddIssueCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "add_issue_comment_reaction", - Description: t("TOOL_ADD_ISSUE_COMMENT_REACTION_DESCRIPTION", "Add a reaction to an issue or pull request comment."), + Description: t("TOOL_ADD_ISSUE_COMMENT_REACTION_DESCRIPTION", "Add a reaction to an issue comment. Only works on issue comments; for pull request conversation comments use add_pull_request_comment_reaction."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_ADD_ISSUE_COMMENT_REACTION_USER_TITLE", "Add Reaction to Issue or Pull Request Comment"), + Title: t("TOOL_ADD_ISSUE_COMMENT_REACTION_USER_TITLE", "Add Reaction to Issue Comment"), ReadOnlyHint: false, DestructiveHint: jsonschema.Ptr(false), OpenWorldHint: jsonschema.Ptr(true), @@ -1769,7 +1869,7 @@ func GranularAddIssueCommentReaction(t translations.TranslationHelperFunc) inven }, "comment_id": { Type: "number", - Description: "The issue or pull request comment ID", + Description: "The issue comment ID", Minimum: jsonschema.Ptr(1.0), }, "content": { @@ -1805,6 +1905,10 @@ func GranularAddIssueCommentReaction(t translations.TranslationHelperFunc) inven return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssueComment(ctx, client, owner, repo, commentID, "add_issue_comment_reaction"); result != nil { + return result, nil, nil + } + reaction, resp, err := client.Reactions.CreateIssueCommentReaction(ctx, owner, repo, commentID, content) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to issue comment", resp, err), nil, nil @@ -1825,15 +1929,15 @@ func GranularAddIssueCommentReaction(t translations.TranslationHelperFunc) inven return st } -// GranularRemoveIssueCommentReaction removes a reaction from an issue or pull request comment. +// GranularRemoveIssueCommentReaction removes a reaction from an issue comment. func GranularRemoveIssueCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "remove_issue_comment_reaction", - Description: t("TOOL_REMOVE_ISSUE_COMMENT_REACTION_DESCRIPTION", "Remove a reaction from an issue or pull request comment."), + Description: t("TOOL_REMOVE_ISSUE_COMMENT_REACTION_DESCRIPTION", "Remove a reaction from an issue comment. Only works on issue comments; for pull request conversation comments use remove_pull_request_comment_reaction."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_REMOVE_ISSUE_COMMENT_REACTION_USER_TITLE", "Remove Reaction from Issue or Pull Request Comment"), + Title: t("TOOL_REMOVE_ISSUE_COMMENT_REACTION_USER_TITLE", "Remove Reaction from Issue Comment"), ReadOnlyHint: false, DestructiveHint: jsonschema.Ptr(true), OpenWorldHint: jsonschema.Ptr(true), @@ -1851,7 +1955,7 @@ func GranularRemoveIssueCommentReaction(t translations.TranslationHelperFunc) in }, "comment_id": { Type: "number", - Description: "The issue or pull request comment ID", + Description: "The issue comment ID", Minimum: jsonschema.Ptr(1.0), }, "reaction_id": { @@ -1887,6 +1991,10 @@ func GranularRemoveIssueCommentReaction(t translations.TranslationHelperFunc) in return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if result := EnsureIssueComment(ctx, client, owner, repo, commentID, "remove_issue_comment_reaction"); result != nil { + return result, nil, nil + } + resp, err := client.Reactions.DeleteIssueCommentReaction(ctx, owner, repo, commentID, reactionID) if resp != nil && resp.Body != nil { defer func() { _ = resp.Body.Close() }() diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index e670da9a34..c0efbcc117 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -973,3 +973,695 @@ func GranularRemovePullRequestReviewCommentReaction(t translations.TranslationHe st.FeatureRule = pullRequestsGranularFeatureRule return st } + +// prIssueFieldUpdateTool is a helper to create single-field pull request update +// tools for fields that GitHub stores on a pull request's underlying issue +// (labels, assignees, milestone). It verifies the number refers to a pull request +// before updating it through the issues API. +func prIssueFieldUpdateTool( + t translations.TranslationHelperFunc, + name, description, title string, + extraProps map[string]*jsonschema.Schema, + extraRequired []string, + buildRequest func(args map[string]any) (gogithub.UpdateIssueRequest, error), +) inventory.ServerTool { + props := map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "pullNumber": { + Type: "number", + Description: "The pull request number", + Minimum: jsonschema.Ptr(1.0), + }, + } + maps.Copy(props, extraProps) + + required := append([]string{"owner", "repo", "pullNumber"}, extraRequired...) + + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: name, + Description: t("TOOL_"+strings.ToUpper(name)+"_DESCRIPTION", description), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_"+strings.ToUpper(name)+"_USER_TITLE", title), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: props, + Required: required, + }, + }, + scopes.RequireAll(scopes.Repo), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + issueReq, err := buildRequest(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + if result := EnsurePullRequest(ctx, client, owner, repo, pullNumber, name); result != nil { + return result, nil, nil + } + + issue, resp, err := client.Issues.Update(ctx, owner, repo, pullNumber, issueReq) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update pull request", resp, err), nil, nil + } + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", issue.GetID()), + URL: issue.GetHTMLURL(), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureRule = pullRequestsGranularFeatureRule + return st +} + +// GranularUpdatePullRequestAssignees creates a tool to update a PR's assignees. +func GranularUpdatePullRequestAssignees(t translations.TranslationHelperFunc) inventory.ServerTool { + return prIssueFieldUpdateTool(t, + "update_pull_request_assignees", + "Update the assignees of an existing pull request. This replaces the current assignees with the provided list.", + "Update Pull Request Assignees", + map[string]*jsonschema.Schema{ + "assignees": { + Type: "array", + Description: "GitHub usernames to assign to this pull request", + Items: &jsonschema.Schema{Type: "string"}, + }, + }, + []string{"assignees"}, + func(args map[string]any) (gogithub.UpdateIssueRequest, error) { + if value, ok := args["assignees"]; !ok || value == nil { + return gogithub.UpdateIssueRequest{}, fmt.Errorf("parameter assignees is required") + } + assignees, err := OptionalStringArrayParam(args, "assignees") + if err != nil { + return gogithub.UpdateIssueRequest{}, err + } + return gogithub.UpdateIssueRequest{Assignees: assignees}, nil + }, + ) +} + +// GranularUpdatePullRequestLabels creates a tool to update a PR's labels. +func GranularUpdatePullRequestLabels(t translations.TranslationHelperFunc) inventory.ServerTool { + return prIssueFieldUpdateTool(t, + "update_pull_request_labels", + "Update the labels of an existing pull request. This replaces the current labels with the provided list.", + "Update Pull Request Labels", + map[string]*jsonschema.Schema{ + "labels": { + Type: "array", + Description: "Labels to apply to this pull request", + Items: &jsonschema.Schema{Type: "string"}, + }, + }, + []string{"labels"}, + func(args map[string]any) (gogithub.UpdateIssueRequest, error) { + if value, ok := args["labels"]; !ok || value == nil { + return gogithub.UpdateIssueRequest{}, fmt.Errorf("parameter labels is required") + } + labels, err := OptionalStringArrayParam(args, "labels") + if err != nil { + return gogithub.UpdateIssueRequest{}, err + } + return gogithub.UpdateIssueRequest{Labels: labels}, nil + }, + ) +} + +// GranularUpdatePullRequestMilestone creates a tool to update a PR's milestone. +func GranularUpdatePullRequestMilestone(t translations.TranslationHelperFunc) inventory.ServerTool { + return prIssueFieldUpdateTool(t, + "update_pull_request_milestone", + "Update the milestone of an existing pull request.", + "Update Pull Request Milestone", + map[string]*jsonschema.Schema{ + "milestone": { + Type: "integer", + Description: "The milestone number to set on the pull request", + Minimum: jsonschema.Ptr(1.0), + }, + }, + []string{"milestone"}, + func(args map[string]any) (gogithub.UpdateIssueRequest, error) { + milestone, err := RequiredInt(args, "milestone") + if err != nil { + return gogithub.UpdateIssueRequest{}, err + } + return gogithub.UpdateIssueRequest{Milestone: &milestone}, nil + }, + ) +} + +// GranularAddPullRequestReaction adds a reaction to a pull request. +func GranularAddPullRequestReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "add_pull_request_reaction", + Description: t("TOOL_ADD_PULL_REQUEST_REACTION_DESCRIPTION", "Add a reaction to a pull request."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ADD_PULL_REQUEST_REACTION_USER_TITLE", "Add Reaction to Pull Request"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "pullNumber": { + Type: "number", + Description: "The pull request number", + Minimum: jsonschema.Ptr(1.0), + }, + "content": { + Type: "string", + Description: "The emoji reaction type", + Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, + }, + }, + Required: []string{"owner", "repo", "pullNumber", "content"}, + }, + }, + scopes.RequireAll(scopes.Repo), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + content, err := RequiredParam[string](args, "content") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + if result := EnsurePullRequest(ctx, client, owner, repo, pullNumber, "add_pull_request_reaction"); result != nil { + return result, nil, nil + } + + // Pull request reactions use the issues reactions endpoint. + reaction, resp, err := client.Reactions.CreateIssueReaction(ctx, owner, repo, pullNumber, content) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to pull request", resp, err), nil, nil + } + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/issues/%d/reactions/%d", client.BaseURL(), owner, repo, pullNumber, reaction.GetID()), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureRule = pullRequestsGranularFeatureRule + return st +} + +// GranularRemovePullRequestReaction removes a reaction from a pull request. +func GranularRemovePullRequestReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "remove_pull_request_reaction", + Description: t("TOOL_REMOVE_PULL_REQUEST_REACTION_DESCRIPTION", "Remove a reaction from a pull request."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REMOVE_PULL_REQUEST_REACTION_USER_TITLE", "Remove Reaction from Pull Request"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(true), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "pullNumber": { + Type: "number", + Description: "The pull request number", + Minimum: jsonschema.Ptr(1.0), + }, + "reaction_id": { + Type: "number", + Description: "The reaction ID to remove", + Minimum: jsonschema.Ptr(1.0), + }, + }, + Required: []string{"owner", "repo", "pullNumber", "reaction_id"}, + }, + }, + scopes.RequireAll(scopes.Repo), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + reactionID, err := RequiredBigInt(args, "reaction_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + if result := EnsurePullRequest(ctx, client, owner, repo, pullNumber, "remove_pull_request_reaction"); result != nil { + return result, nil, nil + } + + resp, err := client.Reactions.DeleteIssueReaction(ctx, owner, repo, pullNumber, reactionID) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove reaction from pull request", resp, err), nil, nil + } + + return utils.NewToolResultText("reaction successfully removed from pull request"), nil, nil + }, + ) + st.FeatureRule = pullRequestsGranularFeatureRule + return st +} + +// GranularAddPullRequestComment creates a tool to add a conversation comment to a pull request. +func GranularAddPullRequestComment(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "add_pull_request_comment", + Description: t("TOOL_ADD_PULL_REQUEST_COMMENT_DESCRIPTION", "Add a conversation comment to a pull request. This does not create a review comment on a specific line; use add_pull_request_review_comment for that."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ADD_PULL_REQUEST_COMMENT_USER_TITLE", "Add Pull Request Comment"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "pullNumber": { + Type: "number", + Description: "The pull request number", + Minimum: jsonschema.Ptr(1.0), + }, + "body": { + Type: "string", + Description: "Comment content", + MinLength: jsonschema.Ptr(1), + }, + }, + Required: []string{"owner", "repo", "pullNumber", "body"}, + }, + }, + publicRepositoryWriteScopeAccess(), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + body, err := RequiredParam[string](args, "body") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + if result := EnsurePullRequest(ctx, client, owner, repo, pullNumber, "add_pull_request_comment"); result != nil { + return result, nil, nil + } + + // Pull request conversation comments use the issues comments endpoint. + comment, resp, err := client.Issues.CreateComment(ctx, owner, repo, pullNumber, &gogithub.IssueComment{Body: gogithub.Ptr(body)}) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create pull request comment", resp, err), nil, nil + } + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", comment.GetID()), + URL: comment.GetHTMLURL(), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureRule = pullRequestsGranularFeatureRule + return st +} + +// GranularUpdatePullRequestComment creates a tool to update a pull request conversation comment. +func GranularUpdatePullRequestComment(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "update_pull_request_comment", + Description: t("TOOL_UPDATE_PULL_REQUEST_COMMENT_DESCRIPTION", "Update the body of an existing pull request conversation comment. This tool cannot update pull request review comments."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_UPDATE_PULL_REQUEST_COMMENT_USER_TITLE", "Update Pull Request Comment"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "comment_id": { + Type: "integer", + Description: "The numeric ID of the pull request conversation comment to update. Do not use a pull request review comment ID.", + Minimum: jsonschema.Ptr(1.0), + }, + "body": { + Type: "string", + Description: "New comment content", + MinLength: jsonschema.Ptr(1), + }, + }, + Required: []string{"owner", "repo", "comment_id", "body"}, + }, + }, + publicRepositoryWriteScopeAccess(), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + commentID, err := RequiredBigInt(args, "comment_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + body, err := RequiredParam[string](args, "body") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + if result := EnsurePullRequestComment(ctx, client, owner, repo, commentID, "update_pull_request_comment"); result != nil { + return result, nil, nil + } + + comment, resp, err := client.Issues.EditComment(ctx, owner, repo, commentID, &gogithub.IssueComment{Body: gogithub.Ptr(body)}) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update pull request comment", resp, err), nil, nil + } + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", comment.GetID()), + URL: comment.GetHTMLURL(), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureRule = pullRequestsGranularFeatureRule + return st +} + +// GranularAddPullRequestCommentReaction adds a reaction to a pull request conversation comment. +func GranularAddPullRequestCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "add_pull_request_comment_reaction", + Description: t("TOOL_ADD_PULL_REQUEST_COMMENT_REACTION_DESCRIPTION", "Add a reaction to a pull request conversation comment. For review comments on specific lines, use add_pull_request_review_comment_reaction."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ADD_PULL_REQUEST_COMMENT_REACTION_USER_TITLE", "Add Reaction to Pull Request Comment"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "comment_id": { + Type: "number", + Description: "The pull request conversation comment ID", + Minimum: jsonschema.Ptr(1.0), + }, + "content": { + Type: "string", + Description: "The emoji reaction type", + Enum: []any{"+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"}, + }, + }, + Required: []string{"owner", "repo", "comment_id", "content"}, + }, + }, + scopes.RequireAll(scopes.Repo), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + commentID, err := RequiredBigInt(args, "comment_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + content, err := RequiredParam[string](args, "content") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + if result := EnsurePullRequestComment(ctx, client, owner, repo, commentID, "add_pull_request_comment_reaction"); result != nil { + return result, nil, nil + } + + reaction, resp, err := client.Reactions.CreateIssueCommentReaction(ctx, owner, repo, commentID, content) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reaction to pull request comment", resp, err), nil, nil + } + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", reaction.GetID()), + URL: fmt.Sprintf("%srepos/%s/%s/issues/comments/%d/reactions/%d", client.BaseURL(), owner, repo, commentID, reaction.GetID()), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureRule = pullRequestsGranularFeatureRule + return st +} + +// GranularRemovePullRequestCommentReaction removes a reaction from a pull request conversation comment. +func GranularRemovePullRequestCommentReaction(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "remove_pull_request_comment_reaction", + Description: t("TOOL_REMOVE_PULL_REQUEST_COMMENT_REACTION_DESCRIPTION", "Remove a reaction from a pull request conversation comment. For review comments on specific lines, use remove_pull_request_review_comment_reaction."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REMOVE_PULL_REQUEST_COMMENT_REACTION_USER_TITLE", "Remove Reaction from Pull Request Comment"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(true), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "comment_id": { + Type: "number", + Description: "The pull request conversation comment ID", + Minimum: jsonschema.Ptr(1.0), + }, + "reaction_id": { + Type: "number", + Description: "The reaction ID to remove", + Minimum: jsonschema.Ptr(1.0), + }, + }, + Required: []string{"owner", "repo", "comment_id", "reaction_id"}, + }, + }, + scopes.RequireAll(scopes.Repo), + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + commentID, err := RequiredBigInt(args, "comment_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + reactionID, err := RequiredBigInt(args, "reaction_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + if result := EnsurePullRequestComment(ctx, client, owner, repo, commentID, "remove_pull_request_comment_reaction"); result != nil { + return result, nil, nil + } + + resp, err := client.Reactions.DeleteIssueCommentReaction(ctx, owner, repo, commentID, reactionID) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove reaction from pull request comment", resp, err), nil, nil + } + + return utils.NewToolResultText("reaction successfully removed from pull request comment"), nil, nil + }, + ) + st.FeatureRule = pullRequestsGranularFeatureRule + return st +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index b91664a67f..001d8f0698 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -260,6 +260,8 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent IssueWrite(t), AddIssueComment(t), UpdateIssueComment(t), + GranularAddIssueComment(t), + GranularUpdateIssueComment(t), SubIssueWrite(t), IssueDependencyRead(t), IssueDependencyWrite(t), @@ -382,6 +384,9 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GranularUpdatePullRequestBody(t), GranularUpdatePullRequestState(t), GranularUpdatePullRequestDraftState(t), + GranularUpdatePullRequestAssignees(t), + GranularUpdatePullRequestLabels(t), + GranularUpdatePullRequestMilestone(t), GranularRequestPullRequestReviewers(t), GranularCreatePullRequestReview(t), GranularSubmitPendingPullRequestReview(t), @@ -392,6 +397,12 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GranularUnresolveReviewThread(t), GranularAddPullRequestReviewCommentReaction(t), GranularRemovePullRequestReviewCommentReaction(t), + GranularAddPullRequestReaction(t), + GranularRemovePullRequestReaction(t), + GranularAddPullRequestComment(t), + GranularUpdatePullRequestComment(t), + GranularAddPullRequestCommentReaction(t), + GranularRemovePullRequestCommentReaction(t), }) } diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index c2ef660104..515a0a471f 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -436,7 +436,7 @@ func TestHandleProtectedResource(t *testing.T) { host: "api.example.com", method: http.MethodGet, expectedStatusCode: http.StatusOK, -expectedScopes: []string{ + expectedScopes: []string{ "repo", "read:org", "read:user",