Skip to content

Commit 3c1ead7

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 3c1ead7

8 files changed

Lines changed: 570 additions & 6 deletions

File tree

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

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,154 @@ def __init__(self):
2525

2626
def load_event_payload(self, event_payload):
2727
self.event_payload = event_payload
28+
29+
def get_issue_projects(self, owner, repo_name, issue_number):
30+
query = """
31+
query($owner: String!, $repo: String!, $issueNumber: Int!) {
32+
repository(owner: $owner, name: $repo) {
33+
issue(number: $issueNumber) {
34+
projectItems(first: 10) {
35+
nodes {
36+
project {
37+
title
38+
}
39+
}
40+
}
41+
}
42+
}
43+
}
44+
"""
45+
variables = {
46+
"owner": owner,
47+
"repo": repo_name,
48+
"issueNumber": issue_number
49+
}
50+
result = self.github.raw_graphql(query, variables)
51+
if "errors" in result:
52+
raise Exception(f"GraphQL errors: {result['errors']}")
53+
54+
repo_data = result.get("data", {}).get("repository", {}) or {}
55+
issue_data = repo_data.get("issue", {}) or {}
56+
project_items = issue_data.get("projectItems", {}) or {}
57+
nodes = project_items.get("nodes", []) or []
58+
59+
return [
60+
node["project"]["title"]
61+
for node in nodes
62+
if node and node.get("project") and node["project"].get("title")
63+
]
64+
65+
def validate_pr_issues(self, pr):
66+
"""Validate if a pull request is from an exempt user or references a validated issue."""
67+
if not self.github or not self.repository_name:
68+
print("GitHub client or repository name not initialized")
69+
return False
70+
71+
pr_author = pr.user.login if pr.user and isinstance(getattr(pr.user, "login", None), str) else ""
72+
73+
# Check if the author is in the exclude list
74+
exclude_authors_env = os.environ.get("EXCLUDE_PR_AUTHORS", "dependabot[bot]")
75+
excluded_authors = [auth.strip() for auth in exclude_authors_env.split(",") if auth.strip()]
76+
if pr_author in excluded_authors:
77+
print(f"Author {pr_author} is in the exclude list. Proceeding.")
78+
return True
79+
80+
# Check if the author is a maintainer/collaborator
81+
author_association = getattr(pr, "author_association", None)
82+
if isinstance(author_association, str) and author_association in ["OWNER", "MEMBER", "COLLABORATOR"]:
83+
print(f"Author {pr_author} is exempt due to association: {author_association}. Proceeding.")
84+
return True
85+
86+
# For external contributors, extract linked issues
87+
from utils import extract_all_linked_issues
88+
pr_body = pr.body if isinstance(pr.body, str) else ""
89+
linked_issues = extract_all_linked_issues(pr_body, self.repository_name)
90+
if not linked_issues:
91+
print("No linked issues found in PR body for external contributor.")
92+
return False
93+
94+
current_org = self.repository_name.split("/")[0].lower()
95+
required_projects = [
96+
"OpenWISP Contributor's Board",
97+
"OpenWISP Priorities for next releases"
98+
]
99+
100+
for owner, repo_name, issue_number in linked_issues:
101+
# 1. Check if the issue belongs to the same organization
102+
if owner.lower() != current_org:
103+
print(f"Issue {owner}/{repo_name}#{issue_number} does not belong to organization {current_org}, skipping validation.")
104+
continue
105+
106+
try:
107+
from github import GithubException
108+
target_repo = self.github.get_repo(f"{owner}/{repo_name}")
109+
issue = target_repo.get_issue(issue_number)
110+
except Exception as e:
111+
if isinstance(e, GithubException) and e.status == 404:
112+
print(f"Issue {owner}/{repo_name}#{issue_number} not found, skipping validation.")
113+
continue
114+
else:
115+
print(f"Error fetching issue {owner}/{repo_name}#{issue_number}: {e}")
116+
raise
117+
118+
# 2. Check if the issue is a PR
119+
if issue.pull_request:
120+
print(f"Reference {owner}/{repo_name}#{issue_number} is a pull request, skipping validation.")
121+
continue
122+
123+
# 3. Check if the issue is open
124+
if issue.state != "open":
125+
print(f"Issue {owner}/{repo_name}#{issue_number} is not open, skipping validation.")
126+
continue
127+
128+
# 4. Check if the issue has at least one label, and none are invalid/wontfix
129+
issue_labels = [label.name.lower() for label in issue.labels]
130+
valid_labels = [l for l in issue_labels if l not in ["invalid", "wontfix"]]
131+
if not valid_labels:
132+
print(f"Issue {owner}/{repo_name}#{issue_number} has no valid labels, skipping validation.")
133+
continue
134+
if any(l in ["invalid", "wontfix"] for l in issue_labels):
135+
print(f"Issue {owner}/{repo_name}#{issue_number} contains invalid/wontfix label, skipping validation.")
136+
continue
137+
138+
# 5. Check if the issue is assigned to one of the required projects
139+
try:
140+
projects = self.get_issue_projects(owner, repo_name, issue_number)
141+
except Exception as e:
142+
print(f"Error fetching projects for issue {owner}/{repo_name}#{issue_number}: {e}")
143+
raise
144+
145+
has_valid_project = any(project in required_projects for project in projects)
146+
if has_valid_project:
147+
print(f"Issue {owner}/{repo_name}#{issue_number} is validated. PR is valid.")
148+
return True
149+
else:
150+
print(f"Issue {owner}/{repo_name}#{issue_number} is not assigned to any required project, skipping validation.")
151+
152+
return False
153+
154+
def has_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
155+
"""Check if PR already has a specific type of bot comment.
156+
157+
Uses HTML markers. If ``after_date`` is provided,
158+
only considers comments posted after that date.
159+
"""
160+
try:
161+
if issue_comments is None:
162+
issue_comments = list(pr.get_issue_comments())
163+
marker = f"<!-- bot:{comment_type} -->"
164+
for comment in issue_comments:
165+
if (
166+
comment.user
167+
and comment.user.type == "Bot"
168+
and marker in comment.body
169+
):
170+
if after_date and comment.created_at <= after_date:
171+
continue
172+
return True
173+
return False
174+
except Exception as e:
175+
print("Error checking bot comments" f" for PR #{pr.number}: {e}")
176+
return False
177+
178+

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

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,14 +442,55 @@ def handle_pull_request(self):
442442
return False
443443
if action in ["opened", "reopened"]:
444444
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
447445
elif action == "closed":
448446
if pr.get("merged", False):
449447
print(f"PR #{pr_number} was merged, keeping issue assignments")
450448
else:
451449
self.unassign_issues_from_pr(pr_body, pr_author)
452450
return True
451+
452+
if action in ["opened", "reopened", "edited", "ready_for_review"]:
453+
pr_obj = self.repo.get_pull(pr_number)
454+
is_valid = self.validate_pr_issues(pr_obj)
455+
labels = []
456+
if hasattr(pr_obj, "labels") and not hasattr(pr_obj.labels, "assert_called"):
457+
try:
458+
labels = [label.name for label in pr_obj.labels]
459+
except TypeError:
460+
pass
461+
462+
if is_valid:
463+
if "invalid" in labels:
464+
pr_obj.remove_from_labels("invalid")
465+
print(f"Removed 'invalid' label from PR #{pr_number}")
466+
else:
467+
if "invalid" not in labels:
468+
pr_obj.add_to_labels("invalid")
469+
print(f"Added 'invalid' label to PR #{pr_number}")
470+
471+
if not self.has_bot_comment(pr_obj, "invalid_unvalidated_issue"):
472+
comment_body = (
473+
"<!-- bot:invalid_unvalidated_issue -->\n\n"
474+
f"Hi @{pr_author},\n\n"
475+
"Thank you for your interest in contributing to OpenWISP.\n\n"
476+
"This pull request has been flagged because external contributors must target an "
477+
"issue validated by maintainers before requesting review.\n\n"
478+
"Please link this pull request to a validated issue by adding "
479+
"`Fixes #ISSUE_NUMBER`, `Closes #ISSUE_NUMBER`, or `Related to #ISSUE_NUMBER` to the pull request description. "
480+
"The issue may be in this repository or another OpenWISP repository.\n\n"
481+
"If there is no validated issue yet, please open one first and wait for "
482+
"maintainer validation before continuing with this pull request.\n\n"
483+
"An issue is considered validated when it is open, has an appropriate label other than "
484+
"`invalid` or `wontfix`, and is assigned to one of the OpenWISP contributor project boards "
485+
"mentioned in the contributing guidelines.\n\n"
486+
"Please see the OpenWISP policy on unsolicited and AI-assisted contributions:\n"
487+
"https://openwisp.io/docs/dev/general/code-of-conduct.html\n\n"
488+
"If this is not resolved within 24 hours, this pull request will be closed automatically. "
489+
"Thank you for your understanding."
490+
)
491+
pr_obj.create_issue_comment(comment_body)
492+
print(f"Posted unvalidated issue warning comment on PR #{pr_number}")
493+
return True
453494
print(f"PR action '{action}' not handled")
454495
return True
455496
except Exception as e:

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,64 @@ def process_stale_prs(self):
395395
for pr in open_prs:
396396
pr_count += 1
397397
try:
398+
pr_labels = [label.name for label in pr.labels]
399+
if "invalid" in pr_labels:
400+
if self.validate_pr_issues(pr):
401+
pr.remove_from_labels("invalid")
402+
print(f"PR #{pr.number} is now valid. Removed 'invalid' label.")
403+
else:
404+
warning_comment = None
405+
comments = list(pr.get_issue_comments())
406+
for comment in comments:
407+
if (
408+
comment.user
409+
and comment.user.type == "Bot"
410+
and "<!-- bot:invalid_unvalidated_issue -->" in comment.body
411+
):
412+
warning_comment = comment
413+
break
414+
if warning_comment:
415+
from datetime import datetime, timezone
416+
now = datetime.now(timezone.utc)
417+
if (now - warning_comment.created_at).total_seconds() >= 24 * 3600:
418+
close_message = (
419+
"<!-- bot:invalid_unvalidated_issue_closed -->\n\n"
420+
"This pull request has been automatically closed because it has been flagged as "
421+
"invalid (not referencing a validated issue) for more than 24 hours."
422+
)
423+
try:
424+
pr.create_issue_comment(close_message)
425+
except Exception as comment_error:
426+
print(f"Warning: Could not post close comment on PR #{pr.number}: {comment_error}")
427+
pr.edit(state="closed")
428+
print(f"Closed PR #{pr.number} automatically after 24 hours.")
429+
processed_count += 1
430+
else:
431+
pr_author = pr.user.login if pr.user else ""
432+
comment_body = (
433+
"<!-- bot:invalid_unvalidated_issue -->\n\n"
434+
f"Hi @{pr_author},\n\n"
435+
"Thank you for your interest in contributing to OpenWISP.\n\n"
436+
"This pull request has been flagged because external contributors must target an "
437+
"issue validated by maintainers before requesting review.\n\n"
438+
"Please link this pull request to a validated issue by adding "
439+
"`Fixes #ISSUE_NUMBER`, `Closes #ISSUE_NUMBER`, or `Related to #ISSUE_NUMBER` to the pull request description. "
440+
"The issue may be in this repository or another OpenWISP repository.\n\n"
441+
"If there is no validated issue yet, please open one first and wait for "
442+
"maintainer validation before continuing with this pull request.\n\n"
443+
"An issue is considered validated when it is open, has an appropriate label other than "
444+
"`invalid` or `wontfix`, and is assigned to one of the OpenWISP contributor project boards "
445+
"mentioned in the contributing guidelines.\n\n"
446+
"Please see the OpenWISP policy on unsolicited and AI-assisted contributions:\n"
447+
"https://openwisp.io/docs/dev/general/code-of-conduct.html\n\n"
448+
"If this is not resolved within 24 hours, this pull request will be closed automatically. "
449+
"Thank you for your understanding."
450+
)
451+
pr.create_issue_comment(comment_body)
452+
print(f"Posted missing warning comment on PR #{pr.number}")
453+
processed_count += 1
454+
continue
455+
398456
all_reviews = list(pr.get_reviews())
399457
last_changes_requested = self.get_last_changes_requested(
400458
pr, all_reviews

0 commit comments

Comments
 (0)