diff --git a/README.md b/README.md index 13b0586e4..2cf7e5de4 100644 --- a/README.md +++ b/README.md @@ -822,20 +822,6 @@ The following sets of tools are available (all are on by default): - `startSide`: For multi-line comments, the starting side of the diff that the comment applies to. LEFT indicates the previous state, RIGHT indicates the new state (string, optional) - `subjectType`: The level at which the comment is targeted (string, required) -- **create_and_submit_pull_request_review** - Create and submit a pull request review without comments - - `body`: Review comment text (string, required) - - `commitID`: SHA of commit to review (string, optional) - - `event`: Review action to perform (string, required) - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - -- **create_pending_pull_request_review** - Create pending pull request review - - `commitID`: SHA of commit to review (string, optional) - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - - **create_pull_request** - Open new pull request - `base`: Branch to merge into (string, required) - `body`: PR description (string, optional) @@ -846,11 +832,6 @@ The following sets of tools are available (all are on by default): - `repo`: Repository name (string, required) - `title`: PR title (string, required) -- **delete_pending_pull_request_review** - Delete the requester's latest pending pull request review - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - - **list_pull_requests** - List pull requests - `base`: Filter by base branch (string, optional) - `direction`: Sort direction (string, optional) @@ -886,6 +867,15 @@ Possible options: - `pullNumber`: Pull request number (number, required) - `repo`: Repository name (string, required) +- **pull_request_review_write** - Write operations (create, submit, delete) on pull request reviews. + - `body`: Review comment text (string, optional) + - `commitID`: SHA of commit to review (string, optional) + - `event`: Review action to perform. (string, optional) + - `method`: The write operation to perform on pull request review. (string, required) + - `owner`: Repository owner (string, required) + - `pullNumber`: Pull request number (number, required) + - `repo`: Repository name (string, required) + - **request_copilot_review** - Request Copilot review - `owner`: Repository owner (string, required) - `pullNumber`: Pull request number (number, required) @@ -900,13 +890,6 @@ Possible options: - `repo`: Optional repository name. If provided with owner, only pull requests for this repository are listed. (string, optional) - `sort`: Sort field by number of matches of categories, defaults to best match (string, optional) -- **submit_pending_pull_request_review** - Submit the requester's latest pending pull request review - - `body`: The text of the review comment (string, optional) - - `event`: The event to perform (string, required) - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - - **update_pull_request** - Edit pull request - `base`: New base branch name (string, optional) - `body`: New description (string, optional) diff --git a/pkg/github/__toolsnaps__/create_and_submit_pull_request_review.snap b/pkg/github/__toolsnaps__/create_and_submit_pull_request_review.snap deleted file mode 100644 index 85874cfc7..000000000 --- a/pkg/github/__toolsnaps__/create_and_submit_pull_request_review.snap +++ /dev/null @@ -1,49 +0,0 @@ -{ - "annotations": { - "title": "Create and submit a pull request review without comments", - "readOnlyHint": false - }, - "description": "Create and submit a review for a pull request without review comments.", - "inputSchema": { - "properties": { - "body": { - "description": "Review comment text", - "type": "string" - }, - "commitID": { - "description": "SHA of commit to review", - "type": "string" - }, - "event": { - "description": "Review action to perform", - "enum": [ - "APPROVE", - "REQUEST_CHANGES", - "COMMENT" - ], - "type": "string" - }, - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber", - "body", - "event" - ], - "type": "object" - }, - "name": "create_and_submit_pull_request_review" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/create_pending_pull_request_review.snap b/pkg/github/__toolsnaps__/create_pending_pull_request_review.snap deleted file mode 100644 index 3eea5e6af..000000000 --- a/pkg/github/__toolsnaps__/create_pending_pull_request_review.snap +++ /dev/null @@ -1,34 +0,0 @@ -{ - "annotations": { - "title": "Create pending pull request review", - "readOnlyHint": false - }, - "description": "Create a pending review for a pull request. Call this first before attempting to add comments to a pending review, and ultimately submitting it. A pending pull request review means a pull request review, it is pending because you create it first and submit it later, and the PR author will not see it until it is submitted.", - "inputSchema": { - "properties": { - "commitID": { - "description": "SHA of commit to review", - "type": "string" - }, - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber" - ], - "type": "object" - }, - "name": "create_pending_pull_request_review" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/delete_pending_pull_request_review.snap b/pkg/github/__toolsnaps__/delete_pending_pull_request_review.snap deleted file mode 100644 index 9aff7356c..000000000 --- a/pkg/github/__toolsnaps__/delete_pending_pull_request_review.snap +++ /dev/null @@ -1,30 +0,0 @@ -{ - "annotations": { - "title": "Delete the requester's latest pending pull request review", - "readOnlyHint": false - }, - "description": "Delete the requester's latest pending pull request review. Use this after the user decides not to submit a pending review, if you don't know if they already created one then check first.", - "inputSchema": { - "properties": { - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber" - ], - "type": "object" - }, - "name": "delete_pending_pull_request_review" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/pull_request_read.snap b/pkg/github/__toolsnaps__/pull_request_read.snap index 2079dd2c9..fa9de698c 100644 --- a/pkg/github/__toolsnaps__/pull_request_read.snap +++ b/pkg/github/__toolsnaps__/pull_request_read.snap @@ -8,6 +8,14 @@ "properties": { "method": { "description": "Action to specify what pull request data needs to be retrieved from GitHub. \nPossible options: \n 1. get - Get details of a specific pull request.\n 2. get_diff - Get the diff of a pull request.\n 3. get_status - Get status of a head commit in a pull request. This reflects status of builds and checks.\n 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.\n 5. get_review_comments - Get the review comments on a pull request. Use with pagination parameters to control the number of results returned.\n 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method.\n", + "enum": [ + "get", + "get_diff", + "get_status", + "get_files", + "get_review_comments", + "get_reviews" + ], "type": "string" }, "owner": { diff --git a/pkg/github/__toolsnaps__/pull_request_review_write.snap b/pkg/github/__toolsnaps__/pull_request_review_write.snap new file mode 100644 index 000000000..e1702787c --- /dev/null +++ b/pkg/github/__toolsnaps__/pull_request_review_write.snap @@ -0,0 +1,57 @@ +{ + "annotations": { + "title": "Write operations (create, submit, delete) on pull request reviews.", + "readOnlyHint": false + }, + "description": "Create and/or submit, delete review of a pull request.\n\nAvailable methods:\n- create: Create a new review of a pull request. If \"event\" parameter is provided, the review is submitted. If \"event\" is omitted, a pending review is created.\n- submit_pending: Submit an existing pending review of a pull request. This requires that a pending review exists for the current user on the specified pull request. The \"body\" and \"event\" parameters are used when submitting the review.\n- delete_pending: Delete an existing pending review of a pull request. This requires that a pending review exists for the current user on the specified pull request.\n", + "inputSchema": { + "properties": { + "body": { + "description": "Review comment text", + "type": "string" + }, + "commitID": { + "description": "SHA of commit to review", + "type": "string" + }, + "event": { + "description": "Review action to perform.", + "enum": [ + "APPROVE", + "REQUEST_CHANGES", + "COMMENT" + ], + "type": "string" + }, + "method": { + "description": "The write operation to perform on pull request review.", + "enum": [ + "create", + "submit_pending", + "delete_pending" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "pullNumber": { + "description": "Pull request number", + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "pullNumber" + ], + "type": "object" + }, + "name": "pull_request_review_write" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/submit_pending_pull_request_review.snap b/pkg/github/__toolsnaps__/submit_pending_pull_request_review.snap deleted file mode 100644 index f3541922b..000000000 --- a/pkg/github/__toolsnaps__/submit_pending_pull_request_review.snap +++ /dev/null @@ -1,44 +0,0 @@ -{ - "annotations": { - "title": "Submit the requester's latest pending pull request review", - "readOnlyHint": false - }, - "description": "Submit the requester's latest pending pull request review, normally this is a final step after creating a pending review, adding comments first, unless you know that the user already did the first two steps, you should check before calling this.", - "inputSchema": { - "properties": { - "body": { - "description": "The text of the review comment", - "type": "string" - }, - "event": { - "description": "The event to perform", - "enum": [ - "APPROVE", - "REQUEST_CHANGES", - "COMMENT" - ], - "type": "string" - }, - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber", - "event" - ], - "type": "object" - }, - "name": "submit_pending_pull_request_review" -} \ No newline at end of file diff --git a/pkg/github/instructions.go b/pkg/github/instructions.go index 3d58011aa..e783c6c08 100644 --- a/pkg/github/instructions.go +++ b/pkg/github/instructions.go @@ -51,11 +51,17 @@ Tool usage guidance: func getToolsetInstructions(toolset string) string { switch toolset { case "pull_requests": - return "## Pull Requests\n\nPR review workflow: Always use 'create_pending_pull_request_review' → 'add_comment_to_pending_review' → 'submit_pending_pull_request_review' for complex reviews with line-specific comments." + return `## Pull Requests + +PR review workflow: Always use 'pull_request_review_write' with method 'create' to create a pending review, then 'add_comment_to_pending_review' to add comments, and finally 'pull_request_review_write' with method 'submit_pending' to submit the review for complex reviews with line-specific comments.` case "issues": - return "## Issues\n\nCheck 'list_issue_types' first for organizations to use proper issue types. Use 'search_issues' before creating new issues to avoid duplicates. Always set 'state_reason' when closing issues." + return `## Issues + +Check 'list_issue_types' first for organizations to use proper issue types. Use 'search_issues' before creating new issues to avoid duplicates. Always set 'state_reason' when closing issues.` case "discussions": - return "## Discussions\n\nUse 'list_discussion_categories' to understand available categories before creating discussions. Filter by category for better organization." + return `## Discussions + +Use 'list_discussion_categories' to understand available categories before creating discussions. Filter by category for better organization.` default: return "" } diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 867b4ee1f..829cd56a1 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -36,6 +36,8 @@ Possible options: 5. get_review_comments - Get the review comments on a pull request. Use with pagination parameters to control the number of results returned. 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. `), + + mcp.Enum("get", "get_diff", "get_status", "get_files", "get_review_comments", "get_reviews"), ), mcp.WithString("owner", mcp.Required(), @@ -1028,16 +1030,37 @@ func UpdatePullRequestBranch(getClient GetClientFn, t translations.TranslationHe } } -func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("create_and_submit_pull_request_review", - mcp.WithDescription(t("TOOL_CREATE_AND_SUBMIT_PULL_REQUEST_REVIEW_DESCRIPTION", "Create and submit a review for a pull request without review comments.")), +type PullRequestReviewWriteParams struct { + Method string + Owner string + Repo string + PullNumber int32 + Body string + Event string + CommitID *string +} + +func PullRequestReviewWrite(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { + return mcp.NewTool("pull_request_review_write", + mcp.WithDescription(t("TOOL_PULL_REQUEST_REVIEW_WRITE_DESCRIPTION", `Create and/or submit, delete review of a pull request. + +Available methods: +- create: Create a new review of a pull request. If "event" parameter is provided, the review is submitted. If "event" is omitted, a pending review is created. +- submit_pending: Submit an existing pending review of a pull request. This requires that a pending review exists for the current user on the specified pull request. The "body" and "event" parameters are used when submitting the review. +- delete_pending: Delete an existing pending review of a pull request. This requires that a pending review exists for the current user on the specified pull request. +`)), mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_CREATE_AND_SUBMIT_PULL_REQUEST_REVIEW_USER_TITLE", "Create and submit a pull request review without comments"), + Title: t("TOOL_PULL_REQUEST_REVIEW_WRITE_USER_TITLE", "Write operations (create, submit, delete) on pull request reviews."), ReadOnlyHint: ToBoolPtr(false), }), // Either we need the PR GQL Id directly, or we need owner, repo and PR number to look it up. // Since our other Pull Request tools are working with the REST Client, will handle the lookup // internally for now. + mcp.WithString("method", + mcp.Required(), + mcp.Description("The write operation to perform on pull request review."), + mcp.Enum("create", "submit_pending", "delete_pending"), + ), mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner"), @@ -1051,12 +1074,10 @@ func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translation mcp.Description("Pull request number"), ), mcp.WithString("body", - mcp.Required(), mcp.Description("Review comment text"), ), mcp.WithString("event", - mcp.Required(), - mcp.Description("Review action to perform"), + mcp.Description("Review action to perform."), mcp.Enum("APPROVE", "REQUEST_CHANGES", "COMMENT"), ), mcp.WithString("commitID", @@ -1064,14 +1085,7 @@ func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translation ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - var params struct { - Owner string - Repo string - PullNumber int32 - Body string - Event string - CommitID *string - } + var params PullRequestReviewWriteParams if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -1082,144 +1096,240 @@ func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translation return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil } - var getPullRequestQuery struct { - Repository struct { - PullRequest struct { - ID githubv4.ID - } `graphql:"pullRequest(number: $prNum)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - if err := client.Query(ctx, &getPullRequestQuery, map[string]any{ - "owner": githubv4.String(params.Owner), - "repo": githubv4.String(params.Repo), - "prNum": githubv4.Int(params.PullNumber), - }); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get pull request", - err, - ), nil + switch params.Method { + case "create": + return CreatePullRequestReview(ctx, client, params) + case "submit_pending": + return SubmitPendingPullRequestReview(ctx, client, params) + case "delete_pending": + return DeletePendingPullRequestReview(ctx, client, params) + default: + return mcp.NewToolResultError(fmt.Sprintf("unknown method: %s", params.Method)), nil } + } +} - // Now we have the GQL ID, we can create a review - var addPullRequestReviewMutation struct { - AddPullRequestReview struct { - PullRequestReview struct { - ID githubv4.ID // We don't need this, but a selector is required or GQL complains. - } - } `graphql:"addPullRequestReview(input: $input)"` - } +func CreatePullRequestReview(ctx context.Context, client *githubv4.Client, params PullRequestReviewWriteParams) (*mcp.CallToolResult, error) { + var getPullRequestQuery struct { + Repository struct { + PullRequest struct { + ID githubv4.ID + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } - if err := client.Mutate( - ctx, - &addPullRequestReviewMutation, - githubv4.AddPullRequestReviewInput{ - PullRequestID: getPullRequestQuery.Repository.PullRequest.ID, - Body: githubv4.NewString(githubv4.String(params.Body)), - Event: newGQLStringlike[githubv4.PullRequestReviewEvent](params.Event), - CommitOID: newGQLStringlikePtr[githubv4.GitObjectID](params.CommitID), - }, - nil, - ); err != nil { - return mcp.NewToolResultError(err.Error()), nil + if err := client.Query(ctx, &getPullRequestQuery, map[string]any{ + "owner": githubv4.String(params.Owner), + "repo": githubv4.String(params.Repo), + "prNum": githubv4.Int(params.PullNumber), + }); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, + "failed to get pull request", + err, + ), nil + } + + // Now we have the GQL ID, we can create a review + var addPullRequestReviewMutation struct { + AddPullRequestReview struct { + PullRequestReview struct { + ID githubv4.ID // We don't need this, but a selector is required or GQL complains. } + } `graphql:"addPullRequestReview(input: $input)"` + } - // Return nothing interesting, just indicate success for the time being. - // In future, we may want to return the review ID, but for the moment, we're not leaking - // API implementation details to the LLM. - return mcp.NewToolResultText("pull request review submitted successfully"), nil - } + addPullRequestReviewInput := githubv4.AddPullRequestReviewInput{ + PullRequestID: getPullRequestQuery.Repository.PullRequest.ID, + CommitOID: newGQLStringlikePtr[githubv4.GitObjectID](params.CommitID), + } + + // Event and Body are provided if we submit a review + if params.Event != "" { + addPullRequestReviewInput.Event = newGQLStringlike[githubv4.PullRequestReviewEvent](params.Event) + addPullRequestReviewInput.Body = githubv4.NewString(githubv4.String(params.Body)) + } + + if err := client.Mutate( + ctx, + &addPullRequestReviewMutation, + addPullRequestReviewInput, + nil, + ); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Return nothing interesting, just indicate success for the time being. + // In future, we may want to return the review ID, but for the moment, we're not leaking + // API implementation details to the LLM. + if params.Event == "" { + return mcp.NewToolResultText("pending pull request created"), nil + } + return mcp.NewToolResultText("pull request review submitted successfully"), nil } -// CreatePendingPullRequestReview creates a tool to create a pending review on a pull request. -func CreatePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("create_pending_pull_request_review", - mcp.WithDescription(t("TOOL_CREATE_PENDING_PULL_REQUEST_REVIEW_DESCRIPTION", "Create a pending review for a pull request. Call this first before attempting to add comments to a pending review, and ultimately submitting it. A pending pull request review means a pull request review, it is pending because you create it first and submit it later, and the PR author will not see it until it is submitted.")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_CREATE_PENDING_PULL_REQUEST_REVIEW_USER_TITLE", "Create pending pull request review"), - ReadOnlyHint: ToBoolPtr(false), - }), - // Either we need the PR GQL Id directly, or we need owner, repo and PR number to look it up. - // Since our other Pull Request tools are working with the REST Client, will handle the lookup - // internally for now. - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithNumber("pullNumber", - mcp.Required(), - mcp.Description("Pull request number"), - ), - mcp.WithString("commitID", - mcp.Description("SHA of commit to review"), - ), - // Event is omitted here because we always want to create a pending review. - // Threads are omitted for the moment, and we'll see if the LLM can use the appropriate tool. - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - var params struct { - Owner string - Repo string - PullNumber int32 - CommitID *string - } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } +func SubmitPendingPullRequestReview(ctx context.Context, client *githubv4.Client, params PullRequestReviewWriteParams) (*mcp.CallToolResult, error) { + // First we'll get the current user + var getViewerQuery struct { + Viewer struct { + Login githubv4.String + } + } - // Given our owner, repo and PR number, lookup the GQL ID of the PR. - client, err := getGQLClient(ctx) - if err != nil { - return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil - } + if err := client.Query(ctx, &getViewerQuery, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, + "failed to get current user", + err, + ), nil + } - var getPullRequestQuery struct { - Repository struct { - PullRequest struct { - ID githubv4.ID - } `graphql:"pullRequest(number: $prNum)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - if err := client.Query(ctx, &getPullRequestQuery, map[string]any{ - "owner": githubv4.String(params.Owner), - "repo": githubv4.String(params.Repo), - "prNum": githubv4.Int(params.PullNumber), - }); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get pull request", - err, - ), nil + var getLatestReviewForViewerQuery struct { + Repository struct { + PullRequest struct { + Reviews struct { + Nodes []struct { + ID githubv4.ID + State githubv4.PullRequestReviewState + URL githubv4.URI + } + } `graphql:"reviews(first: 1, author: $author)"` + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + vars := map[string]any{ + "author": githubv4.String(getViewerQuery.Viewer.Login), + "owner": githubv4.String(params.Owner), + "name": githubv4.String(params.Repo), + "prNum": githubv4.Int(params.PullNumber), + } + + if err := client.Query(context.Background(), &getLatestReviewForViewerQuery, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, + "failed to get latest review for current user", + err, + ), nil + } + + // Validate there is one review and the state is pending + if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { + return mcp.NewToolResultError("No pending review found for the viewer"), nil + } + + review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] + if review.State != githubv4.PullRequestReviewStatePending { + errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) + return mcp.NewToolResultError(errText), nil + } + + // Prepare the mutation + var submitPullRequestReviewMutation struct { + SubmitPullRequestReview struct { + PullRequestReview struct { + ID githubv4.ID // We don't need this, but a selector is required or GQL complains. } + } `graphql:"submitPullRequestReview(input: $input)"` + } - // Now we have the GQL ID, we can create a pending review - var addPullRequestReviewMutation struct { - AddPullRequestReview struct { - PullRequestReview struct { - ID githubv4.ID // We don't need this, but a selector is required or GQL complains. + if err := client.Mutate( + ctx, + &submitPullRequestReviewMutation, + githubv4.SubmitPullRequestReviewInput{ + PullRequestReviewID: &review.ID, + Event: githubv4.PullRequestReviewEvent(params.Event), + Body: newGQLStringlikePtr[githubv4.String](¶ms.Body), + }, + nil, + ); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, + "failed to submit pull request review", + err, + ), nil + } + + // Return nothing interesting, just indicate success for the time being. + // In future, we may want to return the review ID, but for the moment, we're not leaking + // API implementation details to the LLM. + return mcp.NewToolResultText("pending pull request review successfully submitted"), nil +} + +func DeletePendingPullRequestReview(ctx context.Context, client *githubv4.Client, params PullRequestReviewWriteParams) (*mcp.CallToolResult, error) { + // First we'll get the current user + var getViewerQuery struct { + Viewer struct { + Login githubv4.String + } + } + + if err := client.Query(ctx, &getViewerQuery, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, + "failed to get current user", + err, + ), nil + } + + var getLatestReviewForViewerQuery struct { + Repository struct { + PullRequest struct { + Reviews struct { + Nodes []struct { + ID githubv4.ID + State githubv4.PullRequestReviewState + URL githubv4.URI } - } `graphql:"addPullRequestReview(input: $input)"` - } + } `graphql:"reviews(first: 1, author: $author)"` + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } - if err := client.Mutate( - ctx, - &addPullRequestReviewMutation, - githubv4.AddPullRequestReviewInput{ - PullRequestID: getPullRequestQuery.Repository.PullRequest.ID, - CommitOID: newGQLStringlikePtr[githubv4.GitObjectID](params.CommitID), - }, - nil, - ); err != nil { - return mcp.NewToolResultError(err.Error()), nil + vars := map[string]any{ + "author": githubv4.String(getViewerQuery.Viewer.Login), + "owner": githubv4.String(params.Owner), + "name": githubv4.String(params.Repo), + "prNum": githubv4.Int(params.PullNumber), + } + + if err := client.Query(context.Background(), &getLatestReviewForViewerQuery, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, + "failed to get latest review for current user", + err, + ), nil + } + + // Validate there is one review and the state is pending + if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { + return mcp.NewToolResultError("No pending review found for the viewer"), nil + } + + review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] + if review.State != githubv4.PullRequestReviewStatePending { + errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) + return mcp.NewToolResultError(errText), nil + } + + // Prepare the mutation + var deletePullRequestReviewMutation struct { + DeletePullRequestReview struct { + PullRequestReview struct { + ID githubv4.ID // We don't need this, but a selector is required or GQL complains. } + } `graphql:"deletePullRequestReview(input: $input)"` + } - // Return nothing interesting, just indicate success for the time being. - // In future, we may want to return the review ID, but for the moment, we're not leaking - // API implementation details to the LLM. - return mcp.NewToolResultText("pending pull request created"), nil - } + if err := client.Mutate( + ctx, + &deletePullRequestReviewMutation, + githubv4.DeletePullRequestReviewInput{ + PullRequestReviewID: &review.ID, + }, + nil, + ); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Return nothing interesting, just indicate success for the time being. + // In future, we may want to return the review ID, but for the moment, we're not leaking + // API implementation details to the LLM. + return mcp.NewToolResultText("pending pull request review successfully deleted"), nil } // AddCommentToPendingReview creates a tool to add a comment to a pull request review. @@ -1388,260 +1498,6 @@ func AddCommentToPendingReview(getGQLClient GetGQLClientFn, t translations.Trans } } -// SubmitPendingPullRequestReview creates a tool to submit a pull request review. -func SubmitPendingPullRequestReview(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("submit_pending_pull_request_review", - mcp.WithDescription(t("TOOL_SUBMIT_PENDING_PULL_REQUEST_REVIEW_DESCRIPTION", "Submit the requester's latest pending pull request review, normally this is a final step after creating a pending review, adding comments first, unless you know that the user already did the first two steps, you should check before calling this.")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_SUBMIT_PENDING_PULL_REQUEST_REVIEW_USER_TITLE", "Submit the requester's latest pending pull request review"), - ReadOnlyHint: ToBoolPtr(false), - }), - // Ideally, for performance sake this would just accept the pullRequestReviewID. However, we would need to - // add a new tool to get that ID for clients that aren't in the same context as the original pending review - // creation. So for now, we'll just accept the owner, repo and pull number and assume this is submitting - // the latest review from a user, since only one can be active at a time. - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithNumber("pullNumber", - mcp.Required(), - mcp.Description("Pull request number"), - ), - mcp.WithString("event", - mcp.Required(), - mcp.Description("The event to perform"), - mcp.Enum("APPROVE", "REQUEST_CHANGES", "COMMENT"), - ), - mcp.WithString("body", - mcp.Description("The text of the review comment"), - ), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - var params struct { - Owner string - Repo string - PullNumber int32 - Event string - Body *string - } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - client, err := getGQLClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub GQL client: %w", err) - } - - // First we'll get the current user - var getViewerQuery struct { - Viewer struct { - Login githubv4.String - } - } - - if err := client.Query(ctx, &getViewerQuery, nil); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get current user", - err, - ), nil - } - - var getLatestReviewForViewerQuery struct { - Repository struct { - PullRequest struct { - Reviews struct { - Nodes []struct { - ID githubv4.ID - State githubv4.PullRequestReviewState - URL githubv4.URI - } - } `graphql:"reviews(first: 1, author: $author)"` - } `graphql:"pullRequest(number: $prNum)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "author": githubv4.String(getViewerQuery.Viewer.Login), - "owner": githubv4.String(params.Owner), - "name": githubv4.String(params.Repo), - "prNum": githubv4.Int(params.PullNumber), - } - - if err := client.Query(context.Background(), &getLatestReviewForViewerQuery, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get latest review for current user", - err, - ), nil - } - - // Validate there is one review and the state is pending - if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return mcp.NewToolResultError("No pending review found for the viewer"), nil - } - - review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] - if review.State != githubv4.PullRequestReviewStatePending { - errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) - return mcp.NewToolResultError(errText), nil - } - - // Prepare the mutation - var submitPullRequestReviewMutation struct { - SubmitPullRequestReview struct { - PullRequestReview struct { - ID githubv4.ID // We don't need this, but a selector is required or GQL complains. - } - } `graphql:"submitPullRequestReview(input: $input)"` - } - - if err := client.Mutate( - ctx, - &submitPullRequestReviewMutation, - githubv4.SubmitPullRequestReviewInput{ - PullRequestReviewID: &review.ID, - Event: githubv4.PullRequestReviewEvent(params.Event), - Body: newGQLStringlikePtr[githubv4.String](params.Body), - }, - nil, - ); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to submit pull request review", - err, - ), nil - } - - // Return nothing interesting, just indicate success for the time being. - // In future, we may want to return the review ID, but for the moment, we're not leaking - // API implementation details to the LLM. - return mcp.NewToolResultText("pending pull request review successfully submitted"), nil - } -} - -func DeletePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("delete_pending_pull_request_review", - mcp.WithDescription(t("TOOL_DELETE_PENDING_PULL_REQUEST_REVIEW_DESCRIPTION", "Delete the requester's latest pending pull request review. Use this after the user decides not to submit a pending review, if you don't know if they already created one then check first.")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_DELETE_PENDING_PULL_REQUEST_REVIEW_USER_TITLE", "Delete the requester's latest pending pull request review"), - ReadOnlyHint: ToBoolPtr(false), - }), - // Ideally, for performance sake this would just accept the pullRequestReviewID. However, we would need to - // add a new tool to get that ID for clients that aren't in the same context as the original pending review - // creation. So for now, we'll just accept the owner, repo and pull number and assume this is deleting - // the latest pending review from a user, since only one can be active at a time. - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithNumber("pullNumber", - mcp.Required(), - mcp.Description("Pull request number"), - ), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - var params struct { - Owner string - Repo string - PullNumber int32 - } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - client, err := getGQLClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub GQL client: %w", err) - } - - // First we'll get the current user - var getViewerQuery struct { - Viewer struct { - Login githubv4.String - } - } - - if err := client.Query(ctx, &getViewerQuery, nil); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get current user", - err, - ), nil - } - - var getLatestReviewForViewerQuery struct { - Repository struct { - PullRequest struct { - Reviews struct { - Nodes []struct { - ID githubv4.ID - State githubv4.PullRequestReviewState - URL githubv4.URI - } - } `graphql:"reviews(first: 1, author: $author)"` - } `graphql:"pullRequest(number: $prNum)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "author": githubv4.String(getViewerQuery.Viewer.Login), - "owner": githubv4.String(params.Owner), - "name": githubv4.String(params.Repo), - "prNum": githubv4.Int(params.PullNumber), - } - - if err := client.Query(context.Background(), &getLatestReviewForViewerQuery, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, - "failed to get latest review for current user", - err, - ), nil - } - - // Validate there is one review and the state is pending - if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return mcp.NewToolResultError("No pending review found for the viewer"), nil - } - - review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] - if review.State != githubv4.PullRequestReviewStatePending { - errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) - return mcp.NewToolResultError(errText), nil - } - - // Prepare the mutation - var deletePullRequestReviewMutation struct { - DeletePullRequestReview struct { - PullRequestReview struct { - ID githubv4.ID // We don't need this, but a selector is required or GQL complains. - } - } `graphql:"deletePullRequestReview(input: $input)"` - } - - if err := client.Mutate( - ctx, - &deletePullRequestReviewMutation, - githubv4.DeletePullRequestReviewInput{ - PullRequestReviewID: &review.ID, - }, - nil, - ); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - // Return nothing interesting, just indicate success for the time being. - // In future, we may want to return the review ID, but for the moment, we're not leaking - // API implementation details to the LLM. - return mcp.NewToolResultText("pending pull request review successfully deleted"), nil - } -} - // RequestCopilotReview creates a tool to request a Copilot review for a pull request. // Note that this tool will not work on GHES where this feature is unsupported. In future, we should not expose this // tool if the configured host does not support it. diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 417a9082a..b74b78e13 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1983,18 +1983,19 @@ func TestCreateAndSubmitPullRequestReview(t *testing.T) { // Verify tool definition once mockClient := githubv4.NewClient(nil) - tool, _ := CreateAndSubmitPullRequestReview(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestReviewWrite(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "create_and_submit_pull_request_review", tool.Name) + assert.Equal(t, "pull_request_review_write", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") assert.Contains(t, tool.InputSchema.Properties, "body") assert.Contains(t, tool.InputSchema.Properties, "event") assert.Contains(t, tool.InputSchema.Properties, "commitID") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber", "body", "event"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) tests := []struct { name string @@ -2048,6 +2049,7 @@ func TestCreateAndSubmitPullRequestReview(t *testing.T) { ), ), requestArgs: map[string]any{ + "method": "create", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2077,6 +2079,7 @@ func TestCreateAndSubmitPullRequestReview(t *testing.T) { ), ), requestArgs: map[string]any{ + "method": "create", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2132,6 +2135,7 @@ func TestCreateAndSubmitPullRequestReview(t *testing.T) { ), ), requestArgs: map[string]any{ + "method": "create", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2150,7 +2154,7 @@ func TestCreateAndSubmitPullRequestReview(t *testing.T) { // Setup client with mock client := githubv4.NewClient(tc.mockedClient) - _, handler := CreateAndSubmitPullRequestReview(stubGetGQLClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestReviewWrite(stubGetGQLClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -2291,16 +2295,17 @@ func TestCreatePendingPullRequestReview(t *testing.T) { // Verify tool definition once mockClient := githubv4.NewClient(nil) - tool, _ := CreatePendingPullRequestReview(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestReviewWrite(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "create_pending_pull_request_review", tool.Name) + assert.Equal(t, "pull_request_review_write", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") assert.Contains(t, tool.InputSchema.Properties, "commitID") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) tests := []struct { name string @@ -2352,6 +2357,7 @@ func TestCreatePendingPullRequestReview(t *testing.T) { ), ), requestArgs: map[string]any{ + "method": "create", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2379,6 +2385,7 @@ func TestCreatePendingPullRequestReview(t *testing.T) { ), ), requestArgs: map[string]any{ + "method": "create", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2430,6 +2437,7 @@ func TestCreatePendingPullRequestReview(t *testing.T) { ), ), requestArgs: map[string]any{ + "method": "create", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2446,7 +2454,7 @@ func TestCreatePendingPullRequestReview(t *testing.T) { // Setup client with mock client := githubv4.NewClient(tc.mockedClient) - _, handler := CreatePendingPullRequestReview(stubGetGQLClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestReviewWrite(stubGetGQLClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -2464,7 +2472,7 @@ func TestCreatePendingPullRequestReview(t *testing.T) { } // Parse the result and get the text content if no error - require.Equal(t, textContent.Text, "pending pull request created") + require.Equal(t, "pending pull request created", textContent.Text) }) } } @@ -2587,17 +2595,18 @@ func TestSubmitPendingPullRequestReview(t *testing.T) { // Verify tool definition once mockClient := githubv4.NewClient(nil) - tool, _ := SubmitPendingPullRequestReview(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestReviewWrite(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "submit_pending_pull_request_review", tool.Name) + assert.Equal(t, "pull_request_review_write", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") assert.Contains(t, tool.InputSchema.Properties, "event") assert.Contains(t, tool.InputSchema.Properties, "body") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber", "event"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) tests := []struct { name string @@ -2609,6 +2618,7 @@ func TestSubmitPendingPullRequestReview(t *testing.T) { { name: "successful review submission", requestArgs: map[string]any{ + "method": "submit_pending", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2657,7 +2667,7 @@ func TestSubmitPendingPullRequestReview(t *testing.T) { // Setup client with mock client := githubv4.NewClient(tc.mockedClient) - _, handler := SubmitPendingPullRequestReview(stubGetGQLClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestReviewWrite(stubGetGQLClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -2685,15 +2695,16 @@ func TestDeletePendingPullRequestReview(t *testing.T) { // Verify tool definition once mockClient := githubv4.NewClient(nil) - tool, _ := DeletePendingPullRequestReview(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestReviewWrite(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "delete_pending_pull_request_review", tool.Name) + assert.Equal(t, "pull_request_review_write", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) tests := []struct { name string @@ -2705,6 +2716,7 @@ func TestDeletePendingPullRequestReview(t *testing.T) { { name: "successful review deletion", requestArgs: map[string]any{ + "method": "delete_pending", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2749,7 +2761,7 @@ func TestDeletePendingPullRequestReview(t *testing.T) { // Setup client with mock client := githubv4.NewClient(tc.mockedClient) - _, handler := DeletePendingPullRequestReview(stubGetGQLClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestReviewWrite(stubGetGQLClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 495135ae4..a982060de 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -214,11 +214,8 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(RequestCopilotReview(getClient, t)), // Reviews - toolsets.NewServerTool(CreateAndSubmitPullRequestReview(getGQLClient, t)), - toolsets.NewServerTool(CreatePendingPullRequestReview(getGQLClient, t)), + toolsets.NewServerTool(PullRequestReviewWrite(getGQLClient, t)), toolsets.NewServerTool(AddCommentToPendingReview(getGQLClient, t)), - toolsets.NewServerTool(SubmitPendingPullRequestReview(getGQLClient, t)), - toolsets.NewServerTool(DeletePendingPullRequestReview(getGQLClient, t)), ) codeSecurity := toolsets.NewToolset(ToolsetMetadataCodeSecurity.ID, ToolsetMetadataCodeSecurity.Description). AddReadTools(