Skip to content

Commit e719d10

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 e719d10

10 files changed

Lines changed: 853 additions & 46 deletions

File tree

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

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

.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)