diff --git a/.agents/skills/create-pr/SKILL.md b/.agents/skills/create-pr/SKILL.md index 1f18900d0f..3ab1ccf5d5 100644 --- a/.agents/skills/create-pr/SKILL.md +++ b/.agents/skills/create-pr/SKILL.md @@ -93,8 +93,8 @@ gh pr create --repo penpot/penpot --base "" --title "" \ repository default, which is wrong for a branch cut from `staging`. `--project "Main"` is required by `mem:workflow/creating-prs`. -If an issue is present, run the explicit assignment and verification command -from `mem:workflow/creating-prs` before reporting success: +If an issue is present, run the explicit assignment command from +`mem:workflow/creating-prs` before reporting success: ```bash python3 scripts/gh.py link-issue <ISSUE_NUMBER> <PR_NUMBER> @@ -117,8 +117,7 @@ gh pr view <NUMBER> --repo penpot/penpot --json title,body ``` If the updated body contains `Closes #NNNN`, run the explicit assignment -command from `mem:workflow/creating-prs` and require its verification to -succeed: +command from `mem:workflow/creating-prs`: ```bash python3 scripts/gh.py link-issue <ISSUE_NUMBER> <PR_NUMBER> diff --git a/.serena/memories/scripts/gh.md b/.serena/memories/scripts/gh.md index ef061edd60..89ca288bb7 100644 --- a/.serena/memories/scripts/gh.md +++ b/.serena/memories/scripts/gh.md @@ -9,7 +9,7 @@ repository via GraphQL and REST APIs through the authenticated `gh` CLI. - Finding issues with no milestone. - Fetching PR details by number or by milestone. - Comparing milestone issues against CHANGES.md to find missing entries. -- Explicitly linking a GitHub issue to a pull request and verifying both sides. +- Explicitly linking a GitHub issue to a pull request. - Listing or inspecting GitHub Security Advisories (GHSA). ## Prerequisites @@ -76,14 +76,14 @@ python3 scripts/gh.py prs --milestone "2.16.0" --state all ### `link-issue` -Explicitly assign a GitHub issue to a pull request and verify the relationship from both sides: +Explicitly assign a GitHub issue to a pull request: ```bash python3 scripts/gh.py link-issue <ISSUE_NUMBER> <PR_NUMBER> # Short alias: python3 scripts/gh.py link <ISSUE_NUMBER> <PR_NUMBER> ``` -The command resolves both node IDs, calls `addCloseIssueReferences`, and checks the issue's manually linked PRs and the PR's closing issue references. It is safe to rerun, works for merged PRs, and does not close an issue retroactively. JSON goes to stdout; progress and errors go to stderr; a missing link exits non-zero. +The command resolves both node IDs and calls `addCloseIssueReferences`. It trusts the successful mutation instead of re-querying, because GitHub does not reliably report mutation-created links through `closedByPullRequestsReferences(userLinkedOnly: true)`. It is safe to rerun, works for merged PRs, and does not close an issue retroactively. JSON goes to stdout; progress and errors go to stderr; a missing issue/PR or a failed mutation exits non-zero. ### `advisories` diff --git a/.serena/memories/workflow/creating-issues.md b/.serena/memories/workflow/creating-issues.md index 92f950a1b3..1032606ef4 100644 --- a/.serena/memories/workflow/creating-issues.md +++ b/.serena/memories/workflow/creating-issues.md @@ -277,7 +277,7 @@ Add `Closes #<ISSUE_NUMBER>` to the PR body for readable context, then run the e python3 scripts/gh.py link-issue <ISSUE_NUMBER> <PR_NUMBER> ``` -The command creates the GitHub Development link and verifies it from both the issue and PR. It is safe to rerun and does not close an issue retroactively when the PR is already merged. Do not rely on the body keyword as the assignment operation. +The command creates the GitHub Development link by calling `addCloseIssueReferences` and trusts the successful mutation (GitHub does not reliably report mutation-created links back through the API). It is safe to rerun and does not close an issue retroactively when the PR is already merged. Do not rely on the body keyword as the assignment operation. ### Clean up diff --git a/.serena/memories/workflow/creating-prs.md b/.serena/memories/workflow/creating-prs.md index 5d5b132a6b..f9b41275ef 100644 --- a/.serena/memories/workflow/creating-prs.md +++ b/.serena/memories/workflow/creating-prs.md @@ -80,7 +80,7 @@ The "Note:" line is required at the top. Adjust if this is a manual (non-AI) PR. ## Explicit Issue Assignment - For each GitHub issue that a PR resolves, run `python3 scripts/gh.py link-issue <ISSUE_NUMBER> <PR_NUMBER>` after creating or editing the PR. Do not rely on `Closes #NNNN` in the body; it is only human-readable context. -- The command calls `addCloseIssueReferences`, verifies the relationship from both the issue and PR, and exits non-zero if either side is missing. It is safe to rerun and also works for an already merged PR; it does not close an issue retroactively. +- The command calls `addCloseIssueReferences` and trusts the successful mutation: GitHub does not reliably report mutation-created links back through `closedByPullRequestsReferences(userLinkedOnly: true)`, so the command exits non-zero only when a link target is missing or the mutation fails. It is safe to rerun and also works for an already merged PR; it does not close an issue retroactively. - Skip this process for `Relates to #NNNN` and Taiga references, which do not represent a closing relationship. ## Before Opening diff --git a/scripts/gh.py b/scripts/gh.py index df4a4682e6..9e2454e556 100755 --- a/scripts/gh.py +++ b/scripts/gh.py @@ -100,34 +100,13 @@ mutation($issueId: ID!, $pullRequestIds: [ID!]!) { } """ -GQL_VERIFY_ISSUE_LINK_QUERY = """\ -query($owner: String!, $repo: String!, $issueNumber: Int!, $prNumber: Int!) { - repository(owner: $owner, name: $repo) { - issue(number: $issueNumber) { - number - state - closedByPullRequestsReferences( - includeClosedPrs: true - userLinkedOnly: true - first: 100 - ) { - nodes { number state url } - } - } - pullRequest(number: $prNumber) { - number - state - closingIssuesReferences(first: 100) { - nodes { number state url } - } - } - } -} -""" - - def link_issue_to_pr(issue_number: int, pr_number: int) -> dict: - """Add and verify an explicit GitHub issue-to-PR link.""" + """Add an explicit GitHub issue-to-PR link. + + We trust the successful ``addCloseIssueReferences`` mutation instead of + re-querying: GitHub does not reliably report mutation-created links + through ``closedByPullRequestsReferences(userLinkedOnly: true)``. + """ if issue_number <= 0 or pr_number <= 0: raise ValueError("issue and pull request numbers must be positive") @@ -158,40 +137,10 @@ def link_issue_to_pr(issue_number: int, pr_number: int) -> dict: if linked_issue.get("number") != issue_number: raise RuntimeError(f"GitHub did not link issue #{issue_number}") - verification_data = run_gh_graphql(GQL_VERIFY_ISSUE_LINK_QUERY, variables) - repository = verification_data.get("repository") or {} - issue = repository.get("issue") or {} - pull_request = repository.get("pullRequest") or {} - if not issue or not pull_request: - raise RuntimeError("GitHub did not return both link targets during verification") - - issue_links = [ - node - for node in issue["closedByPullRequestsReferences"]["nodes"] - if node.get("number") == pr_number - ] - pr_links = [ - node - for node in pull_request["closingIssuesReferences"]["nodes"] - if node.get("number") == issue_number - ] - if not issue_links or not pr_links: - raise RuntimeError( - f"issue #{issue_number} and pull request #{pr_number} are not linked" - ) - return { "linked": True, - "issue": { - "number": issue["number"], - "state": issue["state"], - "linked_pull_requests": issue_links, - }, - "pull_request": { - "number": pull_request["number"], - "state": pull_request["state"], - "linked_issues": pr_links, - }, + "issue": {"number": issue_number}, + "pull_request": {"number": pr_number}, } @@ -208,7 +157,7 @@ def cmd_link_issue(args: argparse.Namespace) -> None: sys.exit(1) print( - f"Verified issue #{args.issue_number} -> pull request #{args.pr_number}", + f"Linked issue #{args.issue_number} -> pull request #{args.pr_number}", file=sys.stderr, ) print(json.dumps(result, indent=2)) @@ -905,7 +854,7 @@ def main() -> None: p_link = sub.add_parser( "link-issue", aliases=["link"], - help="Explicitly link an issue to a pull request and verify both sides", + help="Explicitly link an issue to a pull request", ) p_link.add_argument("issue_number", type=int, help="Issue number") p_link.add_argument("pr_number", type=int, help="Pull request number") diff --git a/scripts/test_gh.py b/scripts/test_gh.py index 83e80ea5fd..5fb6840a22 100644 --- a/scripts/test_gh.py +++ b/scripts/test_gh.py @@ -49,82 +49,41 @@ class LinkIssueTests(unittest.TestCase): "issue": {"id": "issue-id", "number": 11235} } } - self.verification_response = { - "repository": { - "issue": { - "number": 11235, - "state": "OPEN", - "closedByPullRequestsReferences": { - "nodes": [ - { - "number": 11243, - "state": "MERGED", - "url": "https://github.com/penpot/penpot/pull/11243", - } - ] - }, - }, - "pullRequest": { - "number": 11243, - "state": "MERGED", - "closingIssuesReferences": { - "nodes": [ - { - "number": 11235, - "state": "OPEN", - "url": "https://github.com/penpot/penpot/issues/11235", - } - ] - }, - }, - } - } @patch.object(gh, "run_gh_graphql") - def test_link_issue_to_pr_adds_and_verifies_reference(self, run_graphql): + def test_link_issue_to_pr_adds_reference(self, run_graphql): run_graphql.side_effect = [ self.target_response, self.mutation_response, - self.verification_response, ] result = gh.link_issue_to_pr(11235, 11243) self.assertTrue(result["linked"]) - self.assertEqual( - result["issue"]["linked_pull_requests"][0]["number"], - 11243, - ) - self.assertEqual( - result["pull_request"]["linked_issues"][0]["number"], - 11235, - ) - self.assertEqual(run_graphql.call_count, 3) + self.assertEqual(result["issue"]["number"], 11235) + self.assertEqual(result["pull_request"]["number"], 11243) + self.assertEqual(run_graphql.call_count, 2) self.assertEqual( run_graphql.call_args_list[1].args[1], {"issueId": "issue-id", "pullRequestIds": ["pr-id"]}, ) @patch.object(gh, "run_gh_graphql") - def test_link_issue_to_pr_fails_when_verification_is_missing(self, run_graphql): - self.verification_response["repository"]["issue"][ - "closedByPullRequestsReferences" - ]["nodes"] = [] + def test_link_issue_to_pr_fails_when_mutation_did_not_link(self, run_graphql): run_graphql.side_effect = [ self.target_response, - self.mutation_response, - self.verification_response, + {"addCloseIssueReferences": {"issue": {"id": "issue-id", "number": 999}}}, ] - with self.assertRaisesRegex(RuntimeError, "are not linked"): + with self.assertRaisesRegex(RuntimeError, "did not link issue #11235"): gh.link_issue_to_pr(11235, 11243) @patch.object(gh, "link_issue_to_pr") - def test_cmd_link_issue_outputs_verified_result(self, link_issue): + def test_cmd_link_issue_outputs_result(self, link_issue): expected = { "linked": True, - "issue": {"number": 11235, "state": "OPEN"}, - "pull_request": {"number": 11243, "state": "MERGED"}, + "issue": {"number": 11235}, + "pull_request": {"number": 11243}, } link_issue.return_value = expected args = types.SimpleNamespace(issue_number=11235, pr_number=11243) @@ -135,11 +94,11 @@ class LinkIssueTests(unittest.TestCase): gh.cmd_link_issue(args) self.assertEqual(json.loads(stdout.getvalue()), expected) - self.assertIn("Verified issue #11235", stderr.getvalue()) + self.assertIn("Linked issue #11235", stderr.getvalue()) link_issue.assert_called_once_with(11235, 11243) @patch.object(gh, "link_issue_to_pr", side_effect=RuntimeError("link missing")) - def test_cmd_link_issue_fails_when_verification_is_missing(self, _link_issue): + def test_cmd_link_issue_exits_on_error(self, _link_issue): args = types.SimpleNamespace(issue_number=11235, pr_number=11243) stderr = io.StringIO()