Add a new tool to find closing PRs for issues #816
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
lumaxis
force-pushed
the
lumaxis/add-tool-issue-closing-pr
branch
from
ec67616 to
94af699
Compare
There was a problem hiding this comment.
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
FindClosingPullRequeststool with advanced GraphQL parameters including pagination, filtering, and ordering options - Implemented helper functions
OptionalIntArrayParamandRequiredIntArrayParamfor 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
closingPullRequestsdoesn't match the actual field nameclosing_pull_requestsused in the response structure. This should beclosing_pull_requeststo match the API response format.
} `json:"closingPullRequests"`
e2e/e2e_test.go:1738
- The JSON field name
totalCountdoesn't match the actual field nametotal_countused in the response structure. This should betotal_countto match the API response format.
TotalCount int `json:"totalCount"`
| variables := map[string]any{ | ||
| "owner": githubv4.String(owner), | ||
| "repo": githubv4.String(repo), | ||
| "number": githubv4.Int(issueNumber), // #nosec G115 - issueNumber validated to be positive |
There was a problem hiding this comment.
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 uses AI. Check for mistakes.
| } | ||
|
|
||
| // Validate issue number (basic bounds check) | ||
| if issueNumber < 0 { |
There was a problem hiding this comment.
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.
| if issueNumber < 0 { | |
| if issueNumber <= 0 { |
Copilot uses AI. Check for mistakes.
| 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 |
There was a problem hiding this comment.
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.
| 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 uses AI. Check for mistakes.
| 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 |
There was a problem hiding this comment.
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.
| 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 uses AI. Check for mistakes.
This PR adds a new tool that uses
closedByPullRequestsReferencesonIssueto 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.