Skip to content

Commit c843e8d

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 c843e8d

10 files changed

Lines changed: 689 additions & 38 deletions

File tree

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

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ def __init__(self):
99
self.repository_name = os.environ.get("REPOSITORY")
1010
self.event_name = os.environ.get("GITHUB_EVENT_NAME")
1111
self.event_payload = None
12+
bot_username = os.environ.get("BOT_USERNAME", "openwisp-companion")
13+
self.bot_username = bot_username
14+
self.bot_login = (
15+
bot_username if bot_username.endswith("[bot]") else f"{bot_username}[bot]"
16+
)
1217

1318
if self.github_token and self.repository_name:
1419
try:
@@ -25,3 +30,236 @@ def __init__(self):
2530

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

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

Lines changed: 28 additions & 6 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:
@@ -442,14 +438,40 @@ def handle_pull_request(self):
442438
return False
443439
if action in ["opened", "reopened"]:
444440
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
447441
elif action == "closed":
448442
if pr.get("merged", False):
449443
print(f"PR #{pr_number} was merged, keeping issue assignments")
450444
else:
451445
self.unassign_issues_from_pr(pr_body, pr_author)
452446
return True
447+
448+
if action in ["opened", "reopened", "edited", "ready_for_review"]:
449+
pr_obj = self.repo.get_pull(pr_number)
450+
is_valid = self.validate_pr_issues(pr_obj)
451+
labels = []
452+
try:
453+
labels = [label.name for label in pr_obj.labels]
454+
except (TypeError, AttributeError):
455+
pass
456+
457+
if is_valid:
458+
if "invalid" in labels:
459+
pr_obj.remove_from_labels("invalid")
460+
print(f"Removed 'invalid' label from PR #{pr_number}")
461+
else:
462+
if "invalid" not in labels:
463+
pr_obj.add_to_labels("invalid")
464+
print(f"Added 'invalid' label to PR #{pr_number}")
465+
466+
if not self.has_bot_comment(pr_obj, "invalid_unvalidated_issue"):
467+
comment_body = self.get_invalid_unvalidated_issue_comment(
468+
pr_author
469+
)
470+
pr_obj.create_issue_comment(comment_body)
471+
print(
472+
f"Posted unvalidated issue warning comment on PR #{pr_number}"
473+
)
474+
return True
453475
print(f"PR action '{action}' not handled")
454476
return True
455477
except Exception as e:

0 commit comments

Comments
 (0)