mirror of
https://github.com/penpot/penpot.git
synced 2026-09-24 21:06:14 +00:00
🔧 Remove the link-issue verification step from gh.py
GitHub does not report mutation-created issue-to-PR links through closedByPullRequestsReferences(userLinkedOnly: true), so the verification in `gh.py link-issue` failed even when addCloseIssueReferences succeeded and the link existed. Drop the re-query and trust the successful mutation: the command now fails only when a link target is missing or the mutation does not return the issue. Update the tests, the gh helper memory, the PR/issue workflow memories, and the create-pr skill so they no longer promise verification. AI-assisted-by: deepseek-v4.1-flash
This commit is contained in:
parent
cbb9e5d971
commit
25eff238ae
@ -93,8 +93,8 @@ gh pr create --repo penpot/penpot --base "<BASE>" --title "<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>
|
||||
|
||||
@ -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`
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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")
|
||||
|
||||
@ -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()
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user