Skip to content

Commit 991d6ee

Browse files
[feature] Flag invalid pull requests that do not reference validated issues #709
- Add pull_request_target edited/ready_for_review triggers - Implement regex issue references extraction helper - Add GraphQL project v2 board assignment check - Validate that PRs from external contributors link to open, labeled, and validated organization issues - Apply 'invalid' label and post warning comment if invalid - Automatically close invalid PRs after 24 hours in the stale PR workflow - Update documentation and add unit tests Closes #709
1 parent 9f3639a commit 991d6ee

10 files changed

Lines changed: 842 additions & 46 deletions

File tree

.github/actions/bot-autoassign/base.py

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,26 @@
22

33
from github import Github
44

5+
MAINTAINER_ROLES = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
6+
DEFAULT_EXCLUDE_PR_AUTHORS = "dependabot[bot]"
7+
REQUIRED_CONTRIBUTOR_PROJECTS = (
8+
"OpenWISP Contributor's Board",
9+
"OpenWISP Priorities for next releases",
10+
)
11+
INVALID_ISSUE_LABELS = frozenset({"invalid", "wontfix"})
12+
513

614
class GitHubBot:
715
def __init__(self):
816
self.github_token = os.environ.get("GITHUB_TOKEN")
917
self.repository_name = os.environ.get("REPOSITORY")
1018
self.event_name = os.environ.get("GITHUB_EVENT_NAME")
1119
self.event_payload = None
20+
bot_username = os.environ.get("BOT_USERNAME", "openwisp-companion")
21+
self.bot_username = bot_username
22+
self.bot_login = (
23+
bot_username if bot_username.endswith("[bot]") else f"{bot_username}[bot]"
24+
)
1225

1326
if self.github_token and self.repository_name:
1427
try:
@@ -25,3 +38,244 @@ def __init__(self):
2538

2639
def load_event_payload(self, event_payload):
2740
self.event_payload = event_payload
41+
42+
@staticmethod
43+
def _normalize_project_title(title: str) -> str:
44+
"""Normalize project titles for stable comparisons."""
45+
return " ".join(title.replace("\u2019", "'").split()).strip()
46+
47+
@classmethod
48+
def _project_title_key(cls, title: str) -> str:
49+
"""Loose match key: casefold + ignore apostrophes."""
50+
normalized = cls._normalize_project_title(title).casefold()
51+
return normalized.replace("'", "")
52+
53+
def get_issue_projects(self, owner, repo_name, issue_number):
54+
query = """
55+
query($owner: String!, $repo: String!, $issueNumber: Int!) {
56+
repository(owner: $owner, name: $repo) {
57+
issue(number: $issueNumber) {
58+
projectsV2(first: 20) {
59+
nodes {
60+
title
61+
}
62+
}
63+
}
64+
}
65+
}
66+
"""
67+
variables = {"owner": owner, "repo": repo_name, "issueNumber": issue_number}
68+
headers, result = self.github.requester.graphql_query(query, variables)
69+
print(f"DEBUG: Raw GraphQL response: {result}")
70+
repo_data = result.get("data", {}).get("repository")
71+
if not repo_data:
72+
raise ValueError(
73+
f"GraphQL could not access repository {owner}/{repo_name}; "
74+
"possible GitHub API or permission error"
75+
)
76+
issue_node = repo_data.get("issue")
77+
if issue_node is None:
78+
raise ValueError(
79+
f"GraphQL could not access issue {owner}/{repo_name}#{issue_number}; "
80+
"possible GitHub API or permission error"
81+
)
82+
projects_v2 = issue_node.get("projectsV2")
83+
if projects_v2 is None:
84+
raise ValueError(
85+
f"GraphQL could not read project assignments for "
86+
f"{owner}/{repo_name}#{issue_number}; "
87+
"possible GitHub API or permission error"
88+
)
89+
nodes = projects_v2.get("nodes") or []
90+
projects = []
91+
for node in nodes:
92+
if not node:
93+
continue
94+
title = node.get("title")
95+
if title:
96+
projects.append(self._normalize_project_title(title))
97+
return projects
98+
99+
def validate_pr_issues(self, pr):
100+
"""Validate if a pull request is from an exempt user or references a validated issue."""
101+
if not self.github or not self.repository_name:
102+
print("GitHub client or repository name not initialized")
103+
return False
104+
105+
pr_author = (
106+
pr.user.login
107+
if pr.user and isinstance(getattr(pr.user, "login", None), str)
108+
else ""
109+
)
110+
111+
exclude_authors_env = os.environ.get(
112+
"EXCLUDE_PR_AUTHORS", DEFAULT_EXCLUDE_PR_AUTHORS
113+
)
114+
excluded_authors = [
115+
auth.strip() for auth in exclude_authors_env.split(",") if auth.strip()
116+
]
117+
if pr_author in excluded_authors:
118+
print(f"Author {pr_author} is in the exclude list. Proceeding.")
119+
return True
120+
121+
author_association = str(getattr(pr, "author_association", "") or "")
122+
if author_association in MAINTAINER_ROLES:
123+
print(
124+
f"Author {pr_author} is exempt due to association: "
125+
f"{author_association}. Proceeding."
126+
)
127+
return True
128+
129+
from utils import extract_all_linked_issues
130+
131+
pr_body = pr.body if isinstance(pr.body, str) else ""
132+
linked_issues = extract_all_linked_issues(pr_body, self.repository_name)
133+
if not linked_issues:
134+
print("No linked issues found in PR body for external contributor.")
135+
return False
136+
137+
current_org = self.repository_name.split("/")[0].lower()
138+
required_projects = {
139+
self._project_title_key(p) for p in REQUIRED_CONTRIBUTOR_PROJECTS
140+
}
141+
142+
for owner, repo_name, issue_number in linked_issues:
143+
if owner.lower() != current_org:
144+
print(
145+
f"Issue {owner}/{repo_name}#{issue_number} does not belong "
146+
f"to organization {current_org}, skipping validation."
147+
)
148+
continue
149+
150+
try:
151+
from github import GithubException
152+
153+
target_repo = self.github.get_repo(f"{owner}/{repo_name}")
154+
issue = target_repo.get_issue(issue_number)
155+
except Exception as e:
156+
if isinstance(e, GithubException) and e.status == 404:
157+
print(
158+
f"Issue {owner}/{repo_name}#{issue_number} not found, skipping validation."
159+
)
160+
continue
161+
print(f"Error fetching issue {owner}/{repo_name}#{issue_number}: {e}")
162+
raise
163+
164+
if issue.pull_request:
165+
print(
166+
f"Reference {owner}/{repo_name}#{issue_number} is a pull request, skipping validation."
167+
)
168+
continue
169+
170+
if issue.state != "open":
171+
print(
172+
f"Issue {owner}/{repo_name}#{issue_number} is not open, skipping validation."
173+
)
174+
continue
175+
176+
issue_labels = [label.name.lower() for label in issue.labels]
177+
valid_labels = [
178+
lbl for lbl in issue_labels if lbl not in INVALID_ISSUE_LABELS
179+
]
180+
if not valid_labels:
181+
print(
182+
f"Issue {owner}/{repo_name}#{issue_number} has no valid labels, skipping validation."
183+
)
184+
continue
185+
if any(lbl in INVALID_ISSUE_LABELS for lbl in issue_labels):
186+
print(
187+
f"Issue {owner}/{repo_name}#{issue_number} contains "
188+
"invalid/wontfix label, skipping validation."
189+
)
190+
continue
191+
192+
try:
193+
projects = self.get_issue_projects(owner, repo_name, issue_number)
194+
except Exception as e:
195+
print(
196+
f"Error fetching projects for issue {owner}/{repo_name}#{issue_number}: {e}"
197+
)
198+
raise
199+
200+
print(
201+
f"DEBUG: Raw projects for {owner}/{repo_name}#{issue_number}: {projects}"
202+
)
203+
project_keys = [self._project_title_key(p) for p in projects]
204+
print(f"DEBUG: Normalized project keys: {project_keys}")
205+
print(f"DEBUG: Required project keys: {required_projects}")
206+
207+
has_valid_project = any(key in required_projects for key in project_keys)
208+
if has_valid_project:
209+
print(
210+
f"Issue {owner}/{repo_name}#{issue_number} is validated. PR is valid."
211+
)
212+
return True
213+
else:
214+
print(
215+
f"Issue {owner}/{repo_name}#{issue_number} is not assigned "
216+
f"to any required project (found: {projects or 'none'}), "
217+
"skipping validation."
218+
)
219+
220+
return False
221+
222+
def get_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
223+
"""Get the comment of this bot with the given marker if it exists.
224+
225+
If ``after_date`` is provided, only considers comments posted after that date.
226+
"""
227+
try:
228+
if issue_comments is None:
229+
issue_comments = list(pr.get_issue_comments())
230+
marker = f"<!-- bot:{comment_type} -->"
231+
for comment in issue_comments:
232+
if (
233+
comment.user
234+
and comment.user.login == self.bot_login
235+
and marker in comment.body
236+
):
237+
if after_date and comment.created_at <= after_date:
238+
continue
239+
return comment
240+
return None
241+
except Exception as e:
242+
print(f"Error getting bot comment for PR #{pr.number}: {e}")
243+
return None
244+
245+
def has_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
246+
"""Check if PR already has a specific type of bot comment.
247+
248+
Uses HTML markers. If ``after_date`` is provided,
249+
only considers comments posted after that date.
250+
"""
251+
return bool(self.get_bot_comment(pr, comment_type, after_date, issue_comments))
252+
253+
def get_invalid_unvalidated_issue_comment(self, pr_author):
254+
"""Returns the comment body warning that the PR is invalid/unvalidated."""
255+
greeting = f"Hi @{pr_author},\n\n" if pr_author else "Hi,\n\n"
256+
return (
257+
"<!-- bot:invalid_unvalidated_issue -->\n\n"
258+
f"{greeting}"
259+
"Thank you for your interest in contributing to OpenWISP.\n\n"
260+
"This pull request has been flagged because external contributors "
261+
"must target an issue validated by maintainers before requesting "
262+
"review.\n\n"
263+
"Please link this pull request to a validated issue by adding "
264+
"`Fixes #ISSUE_NUMBER`, `Closes #ISSUE_NUMBER`, or "
265+
"`Related to #ISSUE_NUMBER` to the pull request description. "
266+
"The issue may be in this repository or another OpenWISP "
267+
"repository.\n\n"
268+
"If there is no validated issue yet, please open one first and wait "
269+
"for maintainer validation before continuing with this pull "
270+
"request.\n\n"
271+
"An issue is considered validated when it is open, has an appropriate "
272+
"label other than `invalid` or `wontfix`, and is assigned to one of "
273+
"the OpenWISP contributor project boards mentioned in the "
274+
"contributing guidelines.\n\n"
275+
"Please see the OpenWISP policy on unsolicited and AI-assisted "
276+
"contributions:\n"
277+
"https://openwisp.io/docs/dev/general/code-of-conduct.html\n\n"
278+
"If this is not resolved within 24 hours, this pull request "
279+
"will be closed automatically. "
280+
"Thank you for your understanding."
281+
)

.github/actions/bot-autoassign/issue_assignment_bot.py

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import os
21
import re
32

43
from base import GitHubBot
@@ -14,9 +13,6 @@
1413

1514

1615
class IssueAssignmentBot(GitHubBot):
17-
def __init__(self):
18-
super().__init__()
19-
self.bot_username = os.environ.get("BOT_USERNAME", "openwisp-companion")
2016

2117
def is_bot_assign_command(self, comment_body):
2218
if not comment_body:
@@ -440,16 +436,41 @@ def handle_pull_request(self):
440436
if not all([pr_number, pr_author]):
441437
print("Missing required PR data")
442438
return False
443-
if action in ["opened", "reopened"]:
444-
self.auto_assign_issues_from_pr(pr_number, pr_author, pr_body)
445-
# We consider the event handled even if no issues were linked
446-
return True
447-
elif action == "closed":
439+
if action == "closed":
448440
if pr.get("merged", False):
449441
print(f"PR #{pr_number} was merged, keeping issue assignments")
450442
else:
451443
self.unassign_issues_from_pr(pr_body, pr_author)
452444
return True
445+
446+
if action in ["opened", "reopened", "edited", "ready_for_review"]:
447+
self.auto_assign_issues_from_pr(pr_number, pr_author, pr_body)
448+
pr_obj = self.repo.get_pull(pr_number)
449+
is_valid = self.validate_pr_issues(pr_obj)
450+
labels_lower = set()
451+
try:
452+
labels_lower = {label.name.lower() for label in pr_obj.labels}
453+
except (TypeError, AttributeError):
454+
pass
455+
456+
if is_valid:
457+
if "invalid" in labels_lower:
458+
pr_obj.remove_from_labels("invalid")
459+
print(f"Removed 'invalid' label from PR #{pr_number}")
460+
else:
461+
if "invalid" not in labels_lower:
462+
pr_obj.add_to_labels("invalid")
463+
print(f"Added 'invalid' label to PR #{pr_number}")
464+
465+
if not self.has_bot_comment(pr_obj, "invalid_unvalidated_issue"):
466+
comment_body = self.get_invalid_unvalidated_issue_comment(
467+
pr_author
468+
)
469+
pr_obj.create_issue_comment(comment_body)
470+
print(
471+
f"Posted unvalidated issue warning comment on PR #{pr_number}"
472+
)
473+
return True
453474
print(f"PR action '{action}' not handled")
454475
return True
455476
except Exception as e:

0 commit comments

Comments
 (0)