Repository navigation
🐛 fix(github): skip close-on-merge when the claimed target is a pull request - #14745
Conversation
…quest Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Read the tree at c61d45d, not just the diff. The new IsPullRequest() guard sits in the shared closeIssueForMergedPRClaimWithIssue after the Issues.Get fallback (src/pkg/github/close_on_merge.go:541), so it covers the ledger-claim path (:226, which passes issue == nil) as well as the backfill path that already filtered at :268. Because CloseOnMergeSkipped settles (:146), the sweep marks the claim settled rather than re-fetching it every pass. The test drives the claim path with a pull_request-shaped issue and asserts zero write requests, which is the right assertion for this bug. go build ./pkg/github/ passes.
Looks correct to me and matches #14744 without extra scope. Only the Sign-off-survives-the-squash runs show CANCELLED (re-run/infra, not this diff).
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read against the v5 tree at src/pkg/github/close_on_merge.go; this looks correct to me.
- The claim path (
closeIssueForMergedPRClaim, called from the sweep atclose_on_merge.go:226) fetches the target viaIssues.Getand went straight to theissue_not_openstate check. Only the backfill loop had theissue.IsPullRequest()filter (close_on_merge.go:268), so a cross-repo claim naming a PR number could close that PR. The new guard sits after theissue_not_foundbranch and before the state check, which is the right spot: a not-found target still reportsissue_not_found, and a PR target now short-circuits before any write. CloseOnMergeSkippedis already in the terminal-action list atclose_on_merge.go:146, so the new reason needs no other plumbing.- The regression test serves a
pull_requestobject on/repos/o/r/issues/14730and fails any non-GET, so it would fail without the guard.
PR body matches the diff. Nothing further from me.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Closes #14744
Why
A cross-repo claim from crux#25 (its body mentioned #14730) closed open PR hive#14730 with a false "Fixed by #25" record. GitHub's Issues API also returns pull requests, and the claim path (and the shared close function) had no
IsPullRequest()guard; only the backfill loop did.What
closeIssueForMergedPRClaimWithIssuenow skips withCloseOnMergeSkipped/target_is_pull_request(logged at INFO, as a verdict) before the state check. Existing reasons and exported names are unchanged. Adds a regression test and changelog fragment.— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 requested_by=@clubanderson