-
Notifications
You must be signed in to change notification settings - Fork 1.2k
ci: reject @mentions in pull request descriptions #7496
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| #!/usr/bin/env python3 | ||
| # Copyright (c) 2026 The Dash Core developers | ||
| # Distributed under the MIT software license, see the accompanying | ||
| # file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
||
| """ | ||
| Reject GitHub @username mentions in pull request descriptions. | ||
|
|
||
| Mentions are copied into merge commits and re-notify people on merge, | ||
| rebase, or backport. Email addresses are allowed; empty descriptions pass. | ||
|
|
||
| Usage: | ||
| PR_BODY='...' python3 .github/workflows/check_pr_description_mentions.py | ||
| python3 .github/workflows/check_pr_description_mentions.py --body-file path | ||
| printf '%s' '...' | python3 .github/workflows/check_pr_description_mentions.py --stdin | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import os | ||
| import re | ||
| import sys | ||
| from typing import List, Optional, Sequence, Tuple | ||
|
|
||
|
|
||
| # Match complete, conventional dot-atom email addresses with a dotted domain. | ||
| # Email spans are excluded from the independent GitHub @username scan below. | ||
| EMAIL_LOCAL_ATOM = r"[A-Za-z0-9!#$%&'*+/=?^_`{|}~-]+" | ||
| EMAIL_DOMAIN_LABEL = r"[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?" | ||
| EMAIL_RE = re.compile( | ||
| rf"{EMAIL_LOCAL_ATOM}(?:\.{EMAIL_LOCAL_ATOM})*" | ||
| rf"@{EMAIL_DOMAIN_LABEL}(?:\.{EMAIL_DOMAIN_LABEL})+" | ||
| ) | ||
| EMAIL_LOCAL_SPECIALS = frozenset("!#$%&'*+/=?^_`{|}~-") | ||
|
|
||
| # GitHub @username: @ + 1-39 characters (alphanumeric or internal hyphens). | ||
| MENTION_RE = re.compile(r"@[A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?\b") | ||
|
|
||
| ERROR_MESSAGE = """\ | ||
| ::error::PR description contains GitHub @mentions. | ||
| Do not put @username mentions in PR descriptions. | ||
| They are copied into merge commits and notify people again | ||
| whenever the PR is merged, rebased, or backported. | ||
| Refer to people by name or GitHub URL without the leading @.\ | ||
| """ | ||
|
|
||
|
|
||
| def find_email_spans(line: str) -> List[Tuple[int, int]]: | ||
| """Return spans for complete email addresses in a line.""" | ||
| spans: List[Tuple[int, int]] = [] | ||
| for match in EMAIL_RE.finditer(line): | ||
| start, end = match.span() | ||
| if start > 0: | ||
| previous = line[start - 1] | ||
| if previous.isalnum() or previous == "." or previous in EMAIL_LOCAL_SPECIALS: | ||
| continue | ||
| if end < len(line) and ( | ||
| line[end].isalnum() or line[end] in "-_" | ||
| ): | ||
| continue | ||
| spans.append((start, end)) | ||
| return spans | ||
|
|
||
|
|
||
| def find_mentions(body: str) -> List[Tuple[int, str, str]]: | ||
| """Return (1-based line number, line text, match text) for each @mention.""" | ||
| matches: List[Tuple[int, str, str]] = [] | ||
| for line_no, line in enumerate(body.splitlines(), start=1): | ||
| email_spans = find_email_spans(line) | ||
| for match in MENTION_RE.finditer(line): | ||
| if any(start <= match.start() < end for start, end in email_spans): | ||
| continue | ||
| matches.append((line_no, line, match.group(0))) | ||
| return matches | ||
|
|
||
|
|
||
| def check_body(body: Optional[str]) -> int: | ||
| """Validate PR body. Return 0 on pass, 1 when @mentions are present.""" | ||
| if body is None or body == "": | ||
| print("PR description is empty; no @mentions to check.") | ||
| return 0 | ||
|
|
||
| matches = find_mentions(body) | ||
| if not matches: | ||
| print("No @mentions found in PR description.") | ||
| return 0 | ||
|
|
||
| for line_no, line, mention in matches: | ||
| print(f"{line_no}:{mention}: {line}") | ||
|
|
||
| print("", file=sys.stderr) | ||
| print(ERROR_MESSAGE, file=sys.stderr) | ||
| return 1 | ||
|
|
||
|
|
||
| def parse_args(argv: Sequence[str]) -> argparse.Namespace: | ||
| parser = argparse.ArgumentParser( | ||
| description="Reject GitHub @username mentions in pull request descriptions." | ||
| ) | ||
| source = parser.add_mutually_exclusive_group() | ||
| source.add_argument( | ||
| "--body-file", | ||
| metavar="PATH", | ||
| help="Read the PR body from PATH instead of the PR_BODY environment variable", | ||
| ) | ||
| source.add_argument( | ||
| "--stdin", | ||
| action="store_true", | ||
| help="Read the PR body from stdin instead of the PR_BODY environment variable", | ||
| ) | ||
| return parser.parse_args(argv) | ||
|
|
||
|
|
||
| def read_body(args: argparse.Namespace) -> Optional[str]: | ||
| if args.body_file is not None: | ||
| with open(args.body_file, encoding="utf-8") as handle: | ||
| return handle.read() | ||
| if args.stdin: | ||
| return sys.stdin.read() | ||
| # PR_BODY may be unset (treated as empty) or set to "" / multi-line text. | ||
| # Never shell-interpolate untrusted body content; workflows pass it via env. | ||
| if "PR_BODY" not in os.environ: | ||
| return None | ||
| return os.environ["PR_BODY"] | ||
|
|
||
|
|
||
| def main(argv: Sequence[str] | None = None) -> int: | ||
| args = parse_args(argv if argv is not None else sys.argv[1:]) | ||
| try: | ||
| body = read_body(args) | ||
| except OSError as exc: | ||
| print(f"error: failed to read PR body: {exc}", file=sys.stderr) | ||
| return 1 | ||
| return check_body(body) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| raise SystemExit(main()) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| #!/usr/bin/env python3 | ||
| # Copyright (c) 2026 The Dash Core developers | ||
| # Distributed under the MIT software license, see the accompanying | ||
| # file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
||
| import importlib.util | ||
| import io | ||
| import os | ||
| import tempfile | ||
| import unittest | ||
| from contextlib import redirect_stderr, redirect_stdout | ||
| from pathlib import Path | ||
| from unittest import mock | ||
|
|
||
|
|
||
| SCRIPT_PATH = Path(__file__).with_name("check_pr_description_mentions.py") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we have now tests for CI jobs? that's something new.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol; that's funny. @thepastaclaw please simplify. We don't need tests for this.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed in b49d9db — dropped
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good callout. I dropped the dedicated unit-test file in b49d9db so this stays a simple helper + workflow step only. |
||
| SPEC = importlib.util.spec_from_file_location("check_pr_description_mentions", SCRIPT_PATH) | ||
| assert SPEC is not None and SPEC.loader is not None | ||
| check_pr_description_mentions = importlib.util.module_from_spec(SPEC) | ||
| SPEC.loader.exec_module(check_pr_description_mentions) | ||
|
|
||
|
|
||
| class TestFindMentions(unittest.TestCase): | ||
| def test_empty_body_has_no_mentions(self): | ||
| self.assertEqual([], check_pr_description_mentions.find_mentions("")) | ||
|
|
||
| def test_plain_text_without_at_has_no_mentions(self): | ||
| body = "Explain the change and why.\n\n## Checklist\n- [x] tests" | ||
| self.assertEqual([], check_pr_description_mentions.find_mentions(body)) | ||
|
|
||
| def test_email_addresses_are_allowed(self): | ||
| body = ( | ||
| "Contact alice@example.com, bob.smith+ci@dash.org, " | ||
| "or dev!@example.com for details." | ||
| ) | ||
| self.assertEqual([], check_pr_description_mentions.find_mentions(body)) | ||
|
|
||
| def test_email_with_punctuation_boundaries_is_allowed(self): | ||
| body = "Contact <dev!@example.com>; backup: (first.last@example.co.uk)." | ||
| self.assertEqual([], check_pr_description_mentions.find_mentions(body)) | ||
|
|
||
| def test_single_username_mention_is_found(self): | ||
| matches = check_pr_description_mentions.find_mentions("Thanks @knst for the review.") | ||
| self.assertEqual([(1, "Thanks @knst for the review.", "@knst")], matches) | ||
|
|
||
| def test_mention_after_period_is_found(self): | ||
| matches = check_pr_description_mentions.find_mentions("Thanks.@knst") | ||
| self.assertEqual([(1, "Thanks.@knst", "@knst")], matches) | ||
|
|
||
| def test_mention_after_plus_is_found(self): | ||
| matches = check_pr_description_mentions.find_mentions("cc +@knst") | ||
| self.assertEqual([(1, "cc +@knst", "@knst")], matches) | ||
|
|
||
| def test_mention_at_start_of_line(self): | ||
| matches = check_pr_description_mentions.find_mentions("@PastaClaw requested this.") | ||
| self.assertEqual([(1, "@PastaClaw requested this.", "@PastaClaw")], matches) | ||
|
|
||
| def test_multiple_mentions_across_lines(self): | ||
| body = "Ping @alice\nand also @bob-user later." | ||
| matches = check_pr_description_mentions.find_mentions(body) | ||
| self.assertEqual( | ||
| [ | ||
| (1, "Ping @alice", "@alice"), | ||
| (2, "and also @bob-user later.", "@bob-user"), | ||
| ], | ||
| matches, | ||
| ) | ||
|
|
||
| def test_single_character_username(self): | ||
| matches = check_pr_description_mentions.find_mentions("ask @a please") | ||
| self.assertEqual([(1, "ask @a please", "@a")], matches) | ||
|
|
||
| def test_username_with_internal_hyphen(self): | ||
| matches = check_pr_description_mentions.find_mentions("cc @some-user-name") | ||
| self.assertEqual([(1, "cc @some-user-name", "@some-user-name")], matches) | ||
|
|
||
| def test_trailing_punctuation_does_not_break_match(self): | ||
| matches = check_pr_description_mentions.find_mentions("See @reviewer.") | ||
| self.assertEqual([(1, "See @reviewer.", "@reviewer")], matches) | ||
|
|
||
| def test_github_url_without_at_is_allowed(self): | ||
| body = "Discussed with https://github.com/knst in review." | ||
| self.assertEqual([], check_pr_description_mentions.find_mentions(body)) | ||
|
|
||
| def test_email_and_mention_mixed(self): | ||
| body = "Email dev@example.com and ping @maintainer" | ||
| matches = check_pr_description_mentions.find_mentions(body) | ||
| self.assertEqual( | ||
| [(1, "Email dev@example.com and ping @maintainer", "@maintainer")], | ||
| matches, | ||
| ) | ||
|
|
||
| def test_mention_adjacent_to_email_is_found(self): | ||
| body = "Email dev@example.com.@maintainer" | ||
| matches = check_pr_description_mentions.find_mentions(body) | ||
| self.assertEqual( | ||
| [(1, "Email dev@example.com.@maintainer", "@maintainer")], | ||
| matches, | ||
| ) | ||
|
|
||
| def test_incomplete_email_is_not_exempted(self): | ||
| body = "This is not a complete email: dev!@example" | ||
| matches = check_pr_description_mentions.find_mentions(body) | ||
| self.assertEqual( | ||
| [(1, "This is not a complete email: dev!@example", "@example")], | ||
| matches, | ||
| ) | ||
|
|
||
|
|
||
| class TestCheckBody(unittest.TestCase): | ||
| def test_none_and_empty_pass(self): | ||
| for body in (None, ""): | ||
| with self.subTest(body=body): | ||
| stdout = io.StringIO() | ||
| with redirect_stdout(stdout): | ||
| code = check_pr_description_mentions.check_body(body) | ||
| self.assertEqual(0, code) | ||
| self.assertIn("empty", stdout.getvalue().lower()) | ||
|
|
||
| def test_clean_body_passes(self): | ||
| stdout = io.StringIO() | ||
| with redirect_stdout(stdout): | ||
| code = check_pr_description_mentions.check_body( | ||
| "No people tagged.\nContact team@example.com if needed." | ||
| ) | ||
| self.assertEqual(0, code) | ||
| self.assertIn("No @mentions found", stdout.getvalue()) | ||
|
|
||
| def test_body_with_mention_fails_and_prints_guidance(self): | ||
| stdout = io.StringIO() | ||
| stderr = io.StringIO() | ||
| with redirect_stdout(stdout), redirect_stderr(stderr): | ||
| code = check_pr_description_mentions.check_body("cc @someone") | ||
| self.assertEqual(1, code) | ||
| self.assertIn("@someone", stdout.getvalue()) | ||
| self.assertIn("::error::PR description contains GitHub @mentions.", stderr.getvalue()) | ||
| self.assertIn("merge commits", stderr.getvalue()) | ||
|
|
||
|
|
||
| class TestMain(unittest.TestCase): | ||
| def test_main_reads_pr_body_env(self): | ||
| with mock.patch.dict(os.environ, {"PR_BODY": "hello @user"}, clear=False): | ||
| stderr = io.StringIO() | ||
| with redirect_stdout(io.StringIO()), redirect_stderr(stderr): | ||
| code = check_pr_description_mentions.main([]) | ||
| self.assertEqual(1, code) | ||
| self.assertIn("@mentions", stderr.getvalue()) | ||
|
|
||
| def test_main_missing_pr_body_env_treated_as_empty(self): | ||
| env = {k: v for k, v in os.environ.items() if k != "PR_BODY"} | ||
| with mock.patch.dict(os.environ, env, clear=True): | ||
| stdout = io.StringIO() | ||
| with redirect_stdout(stdout): | ||
| code = check_pr_description_mentions.main([]) | ||
| self.assertEqual(0, code) | ||
| self.assertIn("empty", stdout.getvalue().lower()) | ||
|
|
||
| def test_main_body_file(self): | ||
| with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False) as handle: | ||
| handle.write("safe body with email only: a@b.co\n") | ||
| path = handle.name | ||
| try: | ||
| stdout = io.StringIO() | ||
| with redirect_stdout(stdout): | ||
| code = check_pr_description_mentions.main(["--body-file", path]) | ||
| self.assertEqual(0, code) | ||
| self.assertIn("No @mentions found", stdout.getvalue()) | ||
| finally: | ||
| os.unlink(path) | ||
|
|
||
| def test_main_stdin(self): | ||
| stdin = io.StringIO("@bad\n") | ||
| with mock.patch.object(check_pr_description_mentions.sys, "stdin", stdin): | ||
| with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()): | ||
| code = check_pr_description_mentions.main(["--stdin"]) | ||
| self.assertEqual(1, code) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() | ||
Uh oh!
There was an error while loading. Please reload this page.