feat: Add mark_pr_ready_for_review tool #423

efouts · 2025-05-22T06:25:56Z

This commit introduces a new tool, mark_pr_ready_for_review, which allows changing a GitHub pull request from a draft state to ready for review.

This tool utilizes the GitHub GraphQL API's markPullRequestReadyForReview mutation, as the REST API does not support this specific state transition for existing draft PRs.

The implementation includes the tool logic, its registration in the pull_requests toolset, and comprehensive tests.

Copilot

Pull Request Overview

Introduce a new tool to transition draft GitHub pull requests to ready-for-review state via GraphQL, including command registration, implementation, and tests.

Register the mark_pr_ready_for_review tool in the server toolset.
Implement the GraphQL query and mutation for marking a PR as ready for review.
Add comprehensive unit tests for success and failure scenarios.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
pkg/github/tools.go	Register new `mark_pr_ready_for_review` tool.
pkg/github/pullrequests.go	Implement GraphQL logic for marking draft PRs as ready for review.
pkg/github/pullrequests_test.go	Add unit tests covering various success/failure cases for the new tool.

Comments suppressed due to low confidence (2)

pkg/github/pullrequests_test.go:2295

Add a test case for failure to obtain the GraphQL client (e.g., have getGQLClient return an error) to verify the handler surfaces this as a tool error instead of returning a raw error.

func TestMarkPullRequestReadyForReview(t *testing.T) {

pkg/github/pullrequests.go:1638

Add a test to cover parameter decoding failures (e.g., missing or wrong-type owner/repo/pullNumber) so you can confirm those errors are returned as tool errors.

if err := mapstructure.Decode(request.Params.Arguments, &params); err != nil {

pkg/github/pullrequests.go

Copilot

Pull Request Overview

Adds a new CLI tool (mark_pr_ready_for_review) that transitions a draft GitHub pull request to ready-for-review via the GraphQL API.

Registers the new tool in the existing toolset.
Implements the GraphQL query/mutation logic in pullrequests.go.
Provides comprehensive unit tests for success and failure scenarios.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
pkg/github/tools.go	Registers `MarkPullRequestReadyForReview` in toolset
pkg/github/pullrequests.go	Implements the new GraphQL-based ready-for-review tool
pkg/github/pullrequests_test.go	Adds tests covering all new tool behaviors

Comments suppressed due to low confidence (1)

pkg/github/pullrequests.go:1664

The local variables owner, repo, and pullNumber are parsed above, but the code mistakenly references an undefined params struct here. Update the variables map to use the parsed locals, e.g.: "owner": githubv4.String(owner), "repo": githubv4.String(repo), and "prNum": githubv4.Int(pullNumber).

"owner": githubv4.String(params.Owner),

Copilot

Pull Request Overview

This PR introduces a new tool, mark_pr_ready_for_review, to change a draft pull request into a ready for review state by using the GitHub GraphQL API.

Registers the new tool in the toolset.
Implements the GraphQL mutation to mark PRs as ready.
Adds comprehensive tests to validate the tool's behavior.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
pkg/github/tools.go	Registers the new MarkPullRequestReadyForReview tool
pkg/github/pullrequests_test.go	Adds tests covering success and various failure scenarios for the new tool
pkg/github/pullrequests.go	Implements the GraphQL logic for marking a pull request ready for review

SamMorrowDrums · 2025-05-23T06:22:27Z

Thanks very much for the contribution @efouts I was thinking about this PR and wondering if it might make sense to convert it to set_pr_state and provide an enum for setting the PR to draft, as well as marking as ready tor review. WDYT?

efouts · 2025-05-23T12:21:45Z

Thanks very much for the contribution @efouts I was thinking about this PR and wondering if it might make sense to convert it to set_pr_state and provide an enum for setting the PR to draft, as well as marking as ready tor review. WDYT?

@SamMorrowDrums happy to do that or add another tool mirroring the convertpullrequesttodraft mutation. which is most in line with the project?

(edit) I went ahead and updated to be set_pr_state with the guess that is the preference.

Copilot

Pull Request Overview

This PR adds a new tool, mark_pr_ready_for_review, to allow changing a GitHub pull request's state from draft to ready-for-review (and vice versa), using the GitHub GraphQL API.

Added a new tool registration (SetPRStatus) in pkg/github/tools.go.
Implemented the tool logic in pkg/github/pullrequests.go with corresponding tests in pkg/github/pullrequests_test.go.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
pkg/github/tools.go	Registers the new SetPRStatus tool in the toolset.
pkg/github/pullrequests_test.go	Adds comprehensive tests for both state transition paths.
pkg/github/pullrequests.go	Implements the GraphQL logic for setting the pull request status.

efouts · 2025-05-27T19:39:45Z

@SamMorrowDrums can you help with next steps to get this new tool added?

efouts · 2025-06-03T12:30:41Z

@williammartin @SamMorrowDrums let me know what is needed to get this one merged.

This commit introduces a new tool that allows changing a GitHub pull request from draft state to ready for review. The implementation uses GitHub's GraphQL API with the 'markPullRequestReadyForReview' mutation, as the REST API doesn't support this state transition for existing draft PRs. Also fixes the GraphQL query by correcting 'Draft' to 'IsDraft' field name, ensuring proper detection of a PR's draft status. The changes include: - New tool implementation in the pull_requests toolset - Comprehensive tests - GraphQL query field correction

Based on code review feedback, made the following improvements: - Rename 'variables' to 'vars' for consistency with other GraphQL functions - Improve comment precision for GraphQL schema requirements These changes enhance code consistency and documentation clarity while maintaining full functionality. All tests, linting, and build checks pass.

- Replace mark_pr_ready_for_review with set_pr_status tool - Add bidirectional PR status changes (draft ↔ ready_for_review) - Use enum parameter for status: "draft", "ready_for_review" - Implement GraphQL mutations for both directions - Add comprehensive test suite with 8 test scenarios - Remove deprecated MarkPullRequestReadyForReview function Addresses user feedback to provide enum-based PR status management with support for setting PRs to both draft and ready-for-review states.

Copilot

Pull Request Overview

Adds a new GraphQL-based tool that toggles a pull request between draft and ready-for-review states.

Registers set_pr_status in the toolset
Implements the GraphQL query & mutations in pullrequests.go
Adds comprehensive tests covering success and mutation/query failures

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
pkg/github/tools.go	Registered new `set_pr_status` tool
pkg/github/pullrequests.go	Implemented `SetPRStatus` handler and GraphQL logic
pkg/github/pullrequests_test.go	Added `TestSetPRStatus` covering various success and error scenarios

Comments suppressed due to low confidence (1)

pkg/github/pullrequests_test.go:2295

The tests cover query and mutation errors but don’t simulate a failure from getGQLClient; adding a test case for client initialization error would improve coverage.

func TestSetPRStatus(t *testing.T) {

Copilot · 2025-06-03T13:00:52Z

pkg/github/pullrequests.go

+			var params struct {
+				Owner      string
+				Repo       string
+				PullNumber int32
+				Status     string
+			}
+			if err := mapstructure.Decode(request.Params.Arguments, &params); err != nil {
+				return mcp.NewToolResultError(err.Error()), nil
+			}
+
+			// Validate status parameter
+			if params.Status != "draft" && params.Status != "ready_for_review" {
+				return mcp.NewToolResultError("status must be either 'draft' or 'ready_for_review'"), nil
+			}
+


[nitpick] Consider using the existing parameter helpers (e.g., requiredParam, RequiredInt) instead of mapstructure.Decode for improved consistency in validation and clearer error messages.

Suggested change

var params struct {

Owner string

Repo string

PullNumber int32

Status string

}

if err := mapstructure.Decode(request.Params.Arguments, &params); err != nil {

return mcp.NewToolResultError(err.Error()), nil

}

// Validate status parameter

if params.Status != "draft" && params.Status != "ready_for_review" {

return mcp.NewToolResultError("status must be either 'draft' or 'ready_for_review'"), nil

}

owner, err := request.Params.GetString("owner")

if err != nil {

return mcp.NewToolResultError(fmt.Sprintf("invalid owner parameter: %v", err)), nil

}

repo, err := request.Params.GetString("repo")

if err != nil {

return mcp.NewToolResultError(fmt.Sprintf("invalid repo parameter: %v", err)), nil

}

pullNumber, err := request.Params.GetNumber("pullNumber")

if err != nil {

return mcp.NewToolResultError(fmt.Sprintf("invalid pullNumber parameter: %v", err)), nil

}

status, err := request.Params.GetString("status")

if err != nil {

return mcp.NewToolResultError(fmt.Sprintf("invalid status parameter: %v", err)), nil

}

Copilot · 2025-06-03T13:00:53Z

pkg/github/pullrequests.go

+			}
+
+			// Validate status parameter
+			if params.Status != "draft" && params.Status != "ready_for_review" {


[nitpick] You're validating against raw string literals; consider defining constants or an enum type for the allowed statuses to avoid typos and improve maintainability.

Copilot AI review requested due to automatic review settings May 22, 2025 06:25

efouts requested a review from a team as a code owner May 22, 2025 06:25

Copilot AI reviewed May 22, 2025

View reviewed changes

pkg/github/pullrequests.go Outdated Show resolved Hide resolved

pkg/github/pullrequests.go Outdated Show resolved Hide resolved

efouts requested a review from Copilot May 22, 2025 23:51

Copilot AI reviewed May 22, 2025

View reviewed changes

efouts force-pushed the feat/mark-pr-ready-for-review branch from 8731a4c to 43d6692 Compare May 23, 2025 00:41

efouts requested a review from Copilot May 23, 2025 00:42

Copilot AI reviewed May 23, 2025

View reviewed changes

efouts force-pushed the feat/mark-pr-ready-for-review branch from 43d6692 to 2b5ec67 Compare May 23, 2025 05:07

efouts force-pushed the feat/mark-pr-ready-for-review branch from 2b5ec67 to 650aac6 Compare May 23, 2025 16:38

efouts requested a review from Copilot May 23, 2025 16:41

Copilot AI reviewed May 23, 2025

View reviewed changes

efouts added 3 commits June 3, 2025 08:57

efouts force-pushed the feat/mark-pr-ready-for-review branch from d66769f to fcbc256 Compare June 3, 2025 12:57

efouts requested a review from Copilot June 3, 2025 12:58

Copilot AI reviewed Jun 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Add mark_pr_ready_for_review tool #423

feat: Add mark_pr_ready_for_review tool #423

Uh oh!

efouts commented May 22, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

SamMorrowDrums commented May 23, 2025

Uh oh!

efouts commented May 23, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

efouts commented May 27, 2025

Uh oh!

efouts commented Jun 3, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 3, 2025

Uh oh!

Copilot AI Jun 3, 2025

Uh oh!

Uh oh!

-			var params struct {
-				Owner      string
-				Repo       string
-				PullNumber int32
-				Status     string
-			}
-			if err := mapstructure.Decode(request.Params.Arguments, &params); err != nil {
-				return mcp.NewToolResultError(err.Error()), nil
-			}
-			// Validate status parameter
-			if params.Status != "draft" && params.Status != "ready_for_review" {
-				return mcp.NewToolResultError("status must be either 'draft' or 'ready_for_review'"), nil
-			}
+			owner, err := request.Params.GetString("owner")
+			if err != nil {
+				return mcp.NewToolResultError(fmt.Sprintf("invalid owner parameter: %v", err)), nil
+			}
+			repo, err := request.Params.GetString("repo")
+			if err != nil {
+				return mcp.NewToolResultError(fmt.Sprintf("invalid repo parameter: %v", err)), nil
+			}
+			pullNumber, err := request.Params.GetNumber("pullNumber")
+			if err != nil {
+				return mcp.NewToolResultError(fmt.Sprintf("invalid pullNumber parameter: %v", err)), nil
+			}
+			status, err := request.Params.GetString("status")
+			if err != nil {
+				return mcp.NewToolResultError(fmt.Sprintf("invalid status parameter: %v", err)), nil
+			}

feat: Add mark_pr_ready_for_review tool #423

Are you sure you want to change the base?

feat: Add mark_pr_ready_for_review tool #423

Uh oh!

Conversation

efouts commented May 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

SamMorrowDrums commented May 23, 2025

Uh oh!

efouts commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

efouts commented May 27, 2025

Uh oh!

efouts commented Jun 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

efouts commented May 23, 2025 •

edited

Loading