Add a new tool to find closing PRs for issues by lumaxis · Pull Request #816 · github/github-mcp-server

lumaxis · 2025-08-04T14:30:34Z

This PR adds a new tool that uses closedByPullRequestsReferences on Issue to specifically find PRs in a repo that closed the specified issues. We'd like to use this to find code changes that solved similar issues to a given new issue.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

This PR adds a new tool called find_closing_pull_requests that leverages GitHub's GraphQL API to find pull requests that closed specific issues using the closedByPullRequestsReferences field. This functionality helps identify code changes that resolved similar issues.

Key changes:

Added FindClosingPullRequests tool with advanced GraphQL parameters including pagination, filtering, and ordering options
Implemented helper functions OptionalIntArrayParam and RequiredIntArrayParam for handling integer array parameters
Added comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/github/tools.go	Added the new tool to the default toolset group
pkg/github/server.go	Added helper functions for handling integer array parameters
pkg/github/issues.go	Implemented the core FindClosingPullRequests tool with GraphQL queries
pkg/github/server_test.go	Added unit tests for the new integer array parameter functions
pkg/github/find_closing_prs_integration_test.go	Added integration tests for the new tool
e2e/e2e_test.go	Added comprehensive end-to-end tests covering various scenarios
README.md	Updated documentation with the new tool's parameters

Comments suppressed due to low confidence (2)

e2e/e2e_test.go:1737

The JSON field name closingPullRequests doesn't match the actual field name closing_pull_requests used in the response structure. This should be closing_pull_requests to match the API response format.

					} `json:"closingPullRequests"`

e2e/e2e_test.go:1738

The JSON field name totalCount doesn't match the actual field name total_count used in the response structure. This should be total_count to match the API response format.

					TotalCount int    `json:"totalCount"`

Copilot · 2025-08-05T10:26:00Z

pkg/github/issues.go

+	variables := map[string]any{
+		"owner":  githubv4.String(owner),
+		"repo":   githubv4.String(repo),
+		"number": githubv4.Int(issueNumber), // #nosec G115 - issueNumber validated to be positive


The comment references validation that doesn't exist. The function only checks if issueNumber < 0 but doesn't validate it to be positive (greater than 0). Consider adding proper validation or updating the comment to be accurate.

Copilot · 2025-08-05T10:26:01Z

pkg/github/issues.go

+	}
+
+	// Validate issue number (basic bounds check)
+	if issueNumber < 0 {


Issue number validation should check for both negative values and zero, as GitHub issue numbers start from 1. Consider changing the condition to issueNumber <= 0 for more accurate validation.

Suggested change

if issueNumber < 0 {

if issueNumber <= 0 {

Copilot · 2025-08-05T10:26:01Z

pkg/github/issues.go

+		variables["last"] = githubv4.Int(params.Last) // #nosec G115 - params.Last validated to be positive
+		variables["first"] = (*githubv4.Int)(nil)
+	} else {
+		variables["first"] = githubv4.Int(params.First) // #nosec G115 - params.First validated to be positive


The comments claim validation that doesn't match the actual validation logic. The validation only checks if values are non-negative, not positive. Update the comments to reflect the actual validation or improve the validation to match the comments.

Suggested change

variables["first"] = githubv4.Int(params.First) // #nosec G115 - params.First validated to be positive

"number": githubv4.Int(issueNumber), // #nosec G115 - issueNumber validated to be non-negative

}

if params.Last != 0 {

variables["last"] = githubv4.Int(params.Last) // #nosec G115 - params.Last validated to be non-negative

variables["first"] = (*githubv4.Int)(nil)

} else {

variables["first"] = githubv4.Int(params.First) // #nosec G115 - params.First validated to be non-negative

Copilot · 2025-08-05T10:26:02Z

pkg/github/issues.go

+		variables["last"] = githubv4.Int(params.Last) // #nosec G115 - params.Last validated to be positive
+		variables["first"] = (*githubv4.Int)(nil)
+	} else {
+		variables["first"] = githubv4.Int(params.First) // #nosec G115 - params.First validated to be positive


The comments claim validation that doesn't match the actual validation logic. The validation only checks if values are non-negative, not positive. Update the comments to reflect the actual validation or improve the validation to match the comments.

Suggested change

variables["first"] = githubv4.Int(params.First) // #nosec G115 - params.First validated to be positive

"number": githubv4.Int(issueNumber), // #nosec G115 - issueNumber validated to be non-negative

}

if params.Last != 0 {

variables["last"] = githubv4.Int(params.Last) // #nosec G115 - params.Last validated to be non-negative

variables["first"] = (*githubv4.Int)(nil)

} else {

variables["first"] = githubv4.Int(params.First) // #nosec G115 - params.First validated to be non-negative

LuluBeatson

Thank you @lumaxis for adding this new tool!

I'd like to suggest some small changes:

Issue number should be positive (as mentioned in your comments). So you should use issueNumber <=0 as the invalid range. This should resolve the 4 copilot review comments about issueNumber validation
We have existing GQL pagination parameter parsing helpers. You can find an example usage in ListDiscussions in ./pkg/github/discussions.go. This should resolve the 4 copilot review comments about nosec G115.

A few other items to better fit into existing patterns:

testing, mocking, toolsnaps: details in comment
Error tool results: For errors from the GitHub REST/GraphQL clients we use NewGitHubAPIErrorResponse/NewGitHubGraphQLErrorResponse. More info can be found in ./docs/error-handling.md

And finally, a more subjective comment: I think the tool should be named get_issue_pull_requests and take a single issue number instead of an array of them. This mirrors get_issue_comments, makes it simpler/easier for models to use.

e2e/e2e_test.go

LuluBeatson · 2025-08-08T09:09:49Z

pkg/github/find_closing_prs_integration_test.go

+)
+
+// TestFindClosingPullRequestsIntegration tests the FindClosingPullRequests tool with real GitHub API calls
+func TestFindClosingPullRequestsIntegration(t *testing.T) {


Integration tests are better placed in the ./e2e/ directory.

Alternatively/additionally, you should add a unit test to ./pkg/github/issues_test.go, naming it Test_FindClosingPullRequests and mock the github clients to prevent breaking tests should any live data change. Test_FindClosingPullRequestsshould also include a toolsnaps.Test (e.g Test_CreateIssue) and you can generate/update toolsnaps when running tests by using:

UPDATE_TOOLSNAPS=true go test ./...

More info: https://github.com/github/github-mcp-server/blob/main/docs/testing.md

LuluBeatson · 2025-08-08T09:20:33Z

pkg/github/issues.go

+			mcp.WithString("after",
+				mcp.Description("Cursor for forward pagination (use with first/limit)"),
+			),
+			mcp.WithString("before",
+				mcp.Description("Cursor for backward pagination (use with last)"),
+			),
+			mcp.WithNumber("last",
+				mcp.Description(fmt.Sprintf(
+					"Number of results from end for backward pagination (max: %d)",
+					MaxGraphQLPageSize,
+				)),
+			),


We have existing GQL pagination parameter parsing helpers: WithCursorPagination, OptionalCursorPaginationParams in ./pkg/github/server.go. You can find an example usage in ListDiscussions in ./pkg/github/discussions.go. Instead of limit/after/before/last, we keep it simple by just using perPage/after.

This will also resolve many of the Copilot review comments about pagination param validation logic.

lumaxis added 7 commits August 4, 2025 09:20

Initial version of find_closing_pull_requests

d7c32ff

Add support for structured issue queries

f21928a

Support only finding closing PRs within one repo

bea8311

Fix formatting

bce7b38

Add support for API options

66ce1a6

Update docs

13de1b9

Fix lint issue

8977899

lumaxis requested a review from Copilot August 4, 2025 14:54

This comment was marked as outdated.

Sign in to view

lumaxis added 2 commits August 4, 2025 16:11

Address review comments

02f356d

Adjust for actual GitHub API limits

34b53db

lumaxis requested a review from Copilot August 4, 2025 16:26

This comment was marked as outdated.

Sign in to view

Update limit assertions and comments

2aa02b2

lumaxis marked this pull request as ready for review August 5, 2025 09:09

lumaxis requested a review from a team as a code owner August 5, 2025 09:09

Copilot AI review requested due to automatic review settings August 5, 2025 09:09

This comment was marked as outdated.

Sign in to view

Update pkg/github/server.go

230ca60

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings August 5, 2025 09:27

This comment was marked as outdated.

Sign in to view

lumaxis added 2 commits August 5, 2025 10:24

Use constant for limit

596ce58

Add new RequiredIntArrayParam helper

94af699

Copilot AI review requested due to automatic review settings August 5, 2025 10:24

lumaxis force-pushed the lumaxis/add-tool-issue-closing-pr branch from ec67616 to 94af699 Compare August 5, 2025 10:24

Copilot AI reviewed Aug 5, 2025

View reviewed changes

LuluBeatson suggested changes Aug 8, 2025

View reviewed changes

Use helper functions from tools

d13ca17

-		variables["first"] = githubv4.Int(params.First) // #nosec G115 - params.First validated to be positive
+		"number": githubv4.Int(issueNumber), // #nosec G115 - issueNumber validated to be non-negative
+	}
+	if params.Last != 0 {
+		variables["last"] = githubv4.Int(params.Last) // #nosec G115 - params.Last validated to be non-negative
+		variables["first"] = (*githubv4.Int)(nil)
+	} else {
+		variables["first"] = githubv4.Int(params.First) // #nosec G115 - params.First validated to be non-negative

Comments

Conversation

lumaxis commented Aug 4, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

LuluBeatson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LuluBeatson Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuluBeatson Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LuluBeatson left a comment •

edited

Loading

LuluBeatson Aug 8, 2025 •

edited

Loading

LuluBeatson Aug 8, 2025 •

edited

Loading