Skip to content

[feature] Flag invalid pull requests that do not reference validate #709 #716

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
prathmeshkulkarni-coder wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[feature] Flag invalid pull requests that do not reference validate #709 #716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e719d10
[feature] Flag invalid pull requests that do not reference validated …
prathmeshkulkarni-coder Jul 7, 2026
ceeff76
Merge branch 'master' into feature/709-flag-invalid-prs
prathmeshkulkarni-coder Jul 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
238 changes: 237 additions & 1 deletion .github/actions/bot-autoassign/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,26 @@

from github import Github

MAINTAINER_ROLES = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
DEFAULT_EXCLUDE_PR_AUTHORS = "dependabot[bot]"
REQUIRED_CONTRIBUTOR_PROJECTS = (
"OpenWISP Contributor's Board",
"OpenWISP Priorities for next releases",
)
INVALID_ISSUE_LABELS = frozenset({"invalid", "wontfix"})


class GitHubBot:
def __init__(self):
self.github_token = os.environ.get("GITHUB_TOKEN")
self.repository_name = os.environ.get("REPOSITORY")
self.event_name = os.environ.get("GITHUB_EVENT_NAME")
self.event_payload = None

bot_username = os.environ.get("BOT_USERNAME", "openwisp-companion")
self.bot_username = bot_username
self.bot_login = (
bot_username if bot_username.endswith("[bot]") else f"{bot_username}[bot]"
)
if self.github_token and self.repository_name:
try:
self.github = Github(self.github_token)
Expand All @@ -25,3 +37,227 @@ def __init__(self):

def load_event_payload(self, event_payload):
self.event_payload = event_payload

@staticmethod
def _normalize_project_title(title: str) -> str:
"""Normalize project titles for stable comparisons."""
return " ".join(title.replace("\u2019", "'").split()).strip()

@classmethod
def _project_title_key(cls, title: str) -> str:
"""Loose match key: casefold + ignore apostrophes."""
normalized = cls._normalize_project_title(title).casefold()
return normalized.replace("'", "")

def get_issue_projects(self, owner, repo_name, issue_number):
query = """
query($owner: String!, $repo: String!, $issueNumber: Int!) {
repository(owner: $owner, name: $repo) {
issue(number: $issueNumber) {
projectItems(first: 10) {
nodes {
project {
title
}
}
}
}
}
}
"""
variables = {"owner": owner, "repo": repo_name, "issueNumber": issue_number}
headers, result = self.github.requester.graphql_query(query, variables)
if "errors" in result:
raise ValueError(f"GraphQL API Permission Error: {result['errors']}")
repo_data = result.get("data", {}).get("repository")
if not repo_data:
raise ValueError(
f"GraphQL could not access repository {owner}/{repo_name}; "
"possible GitHub API or permission error"
)
issue_node = repo_data.get("issue")
if issue_node is None:
raise ValueError(
f"GraphQL could not access issue {owner}/{repo_name}#{issue_number}; "
"possible GitHub API or permission error"
)
project_items = issue_node.get("projectItems")
if project_items is None:
raise ValueError(
f"GraphQL could not read project assignments for "
f"{owner}/{repo_name}#{issue_number}; "
"possible GitHub API or permission error"
)
Comment on lines +72 to +90

@coderabbitai coderabbitai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path('.github/actions/bot-autoassign/base.py')
lines = p.read_text().splitlines()
for i in range(1, min(len(lines), 260)+1):
    if 55 <= i <= 95:
        print(f"{i:4d}: {lines[i-1]}")
PY

printf '\n--- search usage/tests ---\n'
rg -n "get_issue_projects|GraphQL could not access repository|possible GitHub API or permission error|result.get\\(\"data\"" .github/actions/bot-autoassign -S

Repository: openwisp/openwisp-utils

Length of output: 4099

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- tests around get_issue_projects ---'
sed -n '920,990p' .github/actions/bot-autoassign/tests/test_issue_assignment_bot.py

printf '\n%s\n' '--- graphql_query definitions/usages ---'
rg -n "def graphql_query|graphql_query\\(" .github/actions/bot-autoassign -S

Repository: openwisp/openwisp-utils

Length of output: 3078

🌐 Web query:

GitHub GraphQL API error response data null errors response shape documentation

💡 Result:

When the GitHub GraphQL API encounters errors, the response structure follows the standard GraphQL specification [1][2]. A response containing errors will include a JSON object with a top-level errors key, and the data key may be present or set to null depending on the nature of the error [1][3]. The standard error response structure is as follows: { "data": null, "errors": [ { "message": "Error description message", "locations": [ { "line": 1, "column": 1 } ] } ] } Key aspects of the error response shape: 1. Data Key: If the error is severe enough to prevent execution of the query (such as a validation or syntax error), the data key will typically be null [1][3]. However, in GraphQL, partial successes are possible; if some parts of a query succeed while others fail, the data key may contain the successfully retrieved information alongside an errors array containing the specific issues [1]. 2. Errors Array: This is a list of error objects [1]. Each object typically contains a message field describing the error [4][1]. 3. Additional Fields: Depending on the type of error, the error objects may include a locations field (indicating where in the query the error occurred) or other debugging information such as reference codes for unexpected system bugs [1][3]. GitHub recommends checking for the presence of the errors key in the response before attempting to use the returned data in production environments [1][5].

Citations:

Handle GraphQL responses with data: null result.get("data", {}) still yields None for {"data": null, "errors": [...]}, so .get("repository") raises AttributeError before the intended ValueError. Use (result.get("data") or {}) and add a test for the null-data error shape; the current GraphQL error test only raises GithubException.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/actions/bot-autoassign/base.py around lines 66 - 84, The GraphQL
response handling in the repository lookup path can still crash when
result["data"] is null because repo_data is read via result.get("data",
{}).get("repository"); update the parsing in the same flow that checks
repo_data, issue_node, and project_items to use a safe fallback like
result.get("data") or {} before accessing nested fields. Also add a test
covering the {"data": null, "errors": [...]} shape in the GraphQL error path,
since the existing error test only exercises GithubException.

nodes = project_items.get("nodes") or []
projects = []
for node in nodes:
if not node:
continue
project = node.get("project") or {}
title = project.get("title")
if title:
projects.append(self._normalize_project_title(title))
return projects

def validate_pr_issues(self, pr):
"""Validate if a pull request is from an exempt user or references a validated issue."""
if not self.github or not self.repository_name:
print("GitHub client or repository name not initialized")
return False
pr_author = (
pr.user.login
if pr.user and isinstance(getattr(pr.user, "login", None), str)
else ""
)
exclude_authors_env = os.environ.get(
"EXCLUDE_PR_AUTHORS", DEFAULT_EXCLUDE_PR_AUTHORS
)
excluded_authors = [
auth.strip() for auth in exclude_authors_env.split(",") if auth.strip()
]
if pr_author in excluded_authors:
print(f"Author {pr_author} is in the exclude list. Proceeding.")
return True
author_association = str(getattr(pr, "author_association", "") or "")
if author_association in MAINTAINER_ROLES:
print(
f"Author {pr_author} is exempt due to association: "
f"{author_association}. Proceeding."
)
return True
from utils import extract_all_linked_issues

pr_body = pr.body if isinstance(pr.body, str) else ""
linked_issues = extract_all_linked_issues(pr_body, self.repository_name)
if not linked_issues:
print("No linked issues found in PR body for external contributor.")
return False
current_org = self.repository_name.split("/")[0].lower()
required_projects = {
self._project_title_key(p) for p in REQUIRED_CONTRIBUTOR_PROJECTS
}
for owner, repo_name, issue_number in linked_issues:
if owner.lower() != current_org:
print(
f"Issue {owner}/{repo_name}#{issue_number} does not belong "
f"to organization {current_org}, skipping validation."
)
continue
try:
from github import GithubException

target_repo = self.github.get_repo(f"{owner}/{repo_name}")
issue = target_repo.get_issue(issue_number)
except Exception as e:
if isinstance(e, GithubException) and e.status == 404:
print(
f"Issue {owner}/{repo_name}#{issue_number} not found, skipping validation."
)
continue
print(f"Error fetching issue {owner}/{repo_name}#{issue_number}: {e}")
raise
if issue.pull_request:
print(
f"Reference {owner}/{repo_name}#{issue_number} is a pull request, skipping validation."
)
continue
if issue.state != "open":
print(
f"Issue {owner}/{repo_name}#{issue_number} is not open, skipping validation."
)
continue
issue_labels = [label.name.lower() for label in issue.labels]
valid_labels = [
lbl for lbl in issue_labels if lbl not in INVALID_ISSUE_LABELS
]
if not valid_labels:
print(
f"Issue {owner}/{repo_name}#{issue_number} has no valid labels, skipping validation."
)
continue
if any(lbl in INVALID_ISSUE_LABELS for lbl in issue_labels):
print(
f"Issue {owner}/{repo_name}#{issue_number} contains "
"invalid/wontfix label, skipping validation."
)
continue
try:
projects = self.get_issue_projects(owner, repo_name, issue_number)
except Exception as e:
print(
f"Error fetching projects for issue {owner}/{repo_name}#{issue_number}: {e}"
)
raise
project_keys = [self._project_title_key(p) for p in projects]
has_valid_project = any(key in required_projects for key in project_keys)
if has_valid_project:
print(
f"Issue {owner}/{repo_name}#{issue_number} is validated. PR is valid."
)
return True
else:
print(
f"Issue {owner}/{repo_name}#{issue_number} is not assigned "
f"to any required project (found: {projects or 'none'}), "
"skipping validation."
)
return False

def get_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
"""Get the comment of this bot with the given marker if it exists.
If ``after_date`` is provided, only considers comments posted after that date.
"""
try:
if issue_comments is None:
issue_comments = list(pr.get_issue_comments())
marker = f"<!-- bot:{comment_type} -->"
for comment in issue_comments:
if (
comment.user
and comment.user.login == self.bot_login
and marker in comment.body
):
if after_date and comment.created_at <= after_date:
continue
return comment
return None
except Exception as e:
print(f"Error getting bot comment for PR #{pr.number}: {e}")
return None

def has_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
"""Check if PR already has a specific type of bot comment.
Uses HTML markers. If ``after_date`` is provided,
only considers comments posted after that date.
"""
return bool(self.get_bot_comment(pr, comment_type, after_date, issue_comments))

def get_invalid_unvalidated_issue_comment(self, pr_author):
"""Returns the comment body warning that the PR is invalid/unvalidated."""
greeting = f"Hi @{pr_author},\n\n" if pr_author else "Hi,\n\n"
return (
"<!-- bot:invalid_unvalidated_issue -->\n\n"
f"{greeting}"
"Thank you for your interest in contributing to OpenWISP.\n\n"
"This pull request has been flagged because external contributors "
"must target an issue validated by maintainers before requesting "
"review.\n\n"
"Please link this pull request to a validated issue by adding "
"`Fixes #ISSUE_NUMBER`, `Closes #ISSUE_NUMBER`, or "
"`Related to #ISSUE_NUMBER` to the pull request description. "
"The issue may be in this repository or another OpenWISP "
"repository.\n\n"
"If there is no validated issue yet, please open one first and wait "
"for maintainer validation before continuing with this pull "
"request.\n\n"
"An issue is considered validated when it is open, has an appropriate "
"label other than `invalid` or `wontfix`, and is assigned to one of "
"the OpenWISP contributor project boards mentioned in the "
"contributing guidelines.\n\n"
"Please see the OpenWISP policy on unsolicited and AI-assisted "
"contributions:\n"
"https://openwisp.io/docs/dev/general/code-of-conduct.html\n\n"
"If this is not resolved within 24 hours, this pull request "
"will be closed automatically. "
"Thank you for your understanding."
)
40 changes: 28 additions & 12 deletions .github/actions/bot-autoassign/issue_assignment_bot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import os
import re

from base import GitHubBot
Expand All @@ -14,9 +13,6 @@


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

def is_bot_assign_command(self, comment_body):
if not comment_body:
Expand Down Expand Up @@ -44,7 +40,6 @@ def get_contributing_guidelines_url(self):

def detect_issue_type(self, issue):
"""Analyzes labels, title and body.

Returns 'bug', 'feature', or None.
"""
bug_keywords = [
Expand Down Expand Up @@ -369,11 +364,9 @@ def auto_assign_issues_from_pr(self, pr_number, pr_author, pr_body, max_issues=1

def unassign_issues_from_pr(self, pr_body, pr_author):
"""Unassign linked issues from PR author"""

if not self.repo:
print("GitHub client not initialized")
return []

try:
return unassign_linked_issues_helper(
self.repo, self.repository_name, pr_body, pr_author
Expand Down Expand Up @@ -440,16 +433,39 @@ def handle_pull_request(self):
if not all([pr_number, pr_author]):
print("Missing required PR data")
return False
if action in ["opened", "reopened"]:
self.auto_assign_issues_from_pr(pr_number, pr_author, pr_body)
# We consider the event handled even if no issues were linked
return True
elif action == "closed":
if action == "closed":
if pr.get("merged", False):
print(f"PR #{pr_number} was merged, keeping issue assignments")
else:
self.unassign_issues_from_pr(pr_body, pr_author)
return True
if action in ["opened", "reopened", "edited", "ready_for_review"]:
pr_obj = self.repo.get_pull(pr_number)
is_valid = self.validate_pr_issues(pr_obj)
if is_valid:
self.auto_assign_issues_from_pr(pr_number, pr_author, pr_body)
labels_lower = set()
try:
labels_lower = {label.name.lower() for label in pr_obj.labels}
except (TypeError, AttributeError):
pass
if is_valid:
if "invalid" in labels_lower:
pr_obj.remove_from_labels("invalid")
print(f"Removed 'invalid' label from PR #{pr_number}")
else:
if "invalid" not in labels_lower:
pr_obj.add_to_labels("invalid")
print(f"Added 'invalid' label to PR #{pr_number}")
if not self.has_bot_comment(pr_obj, "invalid_unvalidated_issue"):
comment_body = self.get_invalid_unvalidated_issue_comment(
pr_author
)
pr_obj.create_issue_comment(comment_body)
print(
f"Posted unvalidated issue warning comment on PR #{pr_number}"
)
return True
print(f"PR action '{action}' not handled")
return True
except Exception as e:
Expand Down
Loading
Loading