Skip to content

Commit 48ea848

Browse files
committed
feat: check each pull request commit as itself
Each PR commit's message was piped to the CLI, so the CLI never saw the commit's author: ignore_authors compared the runner's git identity or the merge commit's author, and an author-based rule had nothing to read. A commit the clone holds is now checked by its hash (--message --rev), so the CLI reads that commit's message and its own author. A commit listed only by the API (the default shallow checkout, or no checkout at all) is still piped in, with the author the API recorded passed as GIT_AUTHOR_NAME / GIT_AUTHOR_EMAIL, which is where the CLI reads the author of a message that has no commit.
1 parent 6a63e33 commit 48ea848

2 files changed

Lines changed: 116 additions & 9 deletions

File tree

‎main.py‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,10 @@ def get_messages_from_head_ref(base_ref: str) -> list[Commit]:
489489
#: The most commits GitHub's pull request commits endpoint returns.
490490
API_COMMIT_LIMIT = 250
491491

492+
#: Author (name, email) of each commit listed by the API, by hash. Those
493+
#: commits are not in the clone, so this is the only record of who wrote them.
494+
API_AUTHORS: dict[str, tuple[str, str]] = {}
495+
492496

493497
def get_messages_from_api() -> list[Commit]:
494498
"""The pull request's commits from the REST API, for a clone without them.
@@ -509,7 +513,12 @@ def get_messages_from_api() -> list[Commit]:
509513
pull = (
510514
Github(auth=Auth.Token(token), base_url=api_url).get_repo(repo).get_pull(number)
511515
)
512-
commits = [(c.sha, c.commit.message) for c in pull.get_commits()]
516+
listed = list(pull.get_commits())
517+
commits = [(c.sha, c.commit.message) for c in listed]
518+
for c in listed:
519+
author = c.commit.author
520+
if author is not None:
521+
API_AUTHORS[c.sha] = (author.name or "", author.email or "")
513522
# A force push after the event can leave the same count but other commits.
514523
head = get_pr_head_sha()
515524
if len(commits) != total or (head and commits[-1][0] != head):
@@ -578,7 +587,9 @@ def _relay_cli_notices(text: str) -> None:
578587

579588

580589
def run_check_json(
581-
args: list[str], input_text: str | None = None
590+
args: list[str],
591+
input_text: str | None = None,
592+
env: dict[str, str] | None = None,
582593
) -> tuple[int, dict[str, Any] | None, str]:
583594
"""Run ``commit-check --format json`` and return (exit code, parsed JSON, raw output).
584595
@@ -602,6 +613,7 @@ def run_check_json(
602613
text=True,
603614
encoding="utf-8",
604615
check=False,
616+
env={**os.environ, **env} if env else None,
605617
)
606618
out = result.stdout or ""
607619
err = result.stderr or ""
@@ -621,28 +633,45 @@ def run_check_json(
621633

622634

623635
def check_scope(
624-
label: str, args: list[str], input_text: str | None = None, sha: str = ""
636+
label: str,
637+
args: list[str],
638+
input_text: str | None = None,
639+
sha: str = "",
640+
env: dict[str, str] | None = None,
625641
) -> ScopeResult:
626642
"""Run commit-check for one scope and wrap the outcome in a ScopeResult.
627643
628644
``sha`` names the commit a message scope checked; it rides along on both
629645
outcomes so an unparsable CLI response still says which commit it was.
630646
"""
631-
_rc, data, raw = run_check_json(args, input_text=input_text)
647+
_rc, data, raw = run_check_json(args, input_text=input_text, env=env)
632648
if isinstance(data, dict):
633649
return ScopeResult(label=label, checks=data.get("checks", []), sha=sha)
634650
return ScopeResult(label=label, raw_text=raw, sha=sha)
635651

636652

637653
def run_pr_message_checks(pr_commits: list[Commit]) -> list[ScopeResult]:
638-
"""Check each PR commit message individually via commit-check --message."""
654+
"""Check each PR commit individually via commit-check --message.
655+
656+
A commit the clone holds is checked by its hash (``--rev``), so the CLI
657+
reads its message and its own author: the AI author rule (CC017) and
658+
``ignore_authors`` then judge that commit's author, not whoever HEAD or
659+
the runner's git config names. A commit listed only by the API is piped
660+
in, with the author the API recorded passed as git's author variables.
661+
"""
639662
results: list[ScopeResult] = []
640663
total = len(pr_commits)
641664
for index, (sha, msg) in enumerate(pr_commits, start=1):
665+
label = f"Commit {index}/{total}"
666+
if sha and _rev_resolves(sha):
667+
results.append(check_scope(label, ["--message", "--rev", sha], sha=sha))
668+
continue
669+
env = None
670+
if sha in API_AUTHORS:
671+
name, email = API_AUTHORS[sha]
672+
env = {"GIT_AUTHOR_NAME": name, "GIT_AUTHOR_EMAIL": email}
642673
results.append(
643-
check_scope(
644-
f"Commit {index}/{total}", ["--message"], input_text=msg, sha=sha
645-
)
674+
check_scope(label, ["--message"], input_text=msg, sha=sha, env=env)
646675
)
647676
return results
648677

‎main_test.py‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,45 @@ def test_invalid_json_falls_back_to_raw_text(self):
390390

391391

392392
class TestRunPrMessageChecks(unittest.TestCase):
393+
"""Commits the clone lacks are piped in; the rest are checked by hash."""
394+
395+
def setUp(self):
396+
resolves = patch("main._rev_resolves", return_value=False)
397+
resolves.start()
398+
self.addCleanup(resolves.stop)
399+
main.API_AUTHORS.clear()
400+
self.addCleanup(main.API_AUTHORS.clear)
401+
402+
def test_a_commit_in_the_clone_is_checked_by_its_hash(self):
403+
"""The CLI then reads that commit's own author (CC017, ignore_authors)."""
404+
mock_result = MagicMock(returncode=0, stdout=json_output(make_check("message")))
405+
with (
406+
patch("main._rev_resolves", return_value=True),
407+
patch("main.subprocess.run", return_value=mock_result) as mock_run,
408+
):
409+
scopes = main.run_pr_message_checks([(SHA_A, "fix: something")])
410+
self.assertEqual(scopes[0].status, "pass")
411+
self.assertEqual(scopes[0].sha, SHA_A)
412+
self.assertEqual(
413+
mock_run.call_args[0][0],
414+
["commit-check", "--format", "json", "--message", "--rev", SHA_A],
415+
)
416+
self.assertIsNone(mock_run.call_args[1]["input"])
417+
self.assertIsNone(mock_run.call_args[1]["env"])
418+
419+
def test_an_api_listed_commit_carries_its_author(self):
420+
"""No commit to read, so the API's author goes in git's variables."""
421+
main.API_AUTHORS[SHA_A] = ("google-labs-jules[bot]", "jules@example.com")
422+
mock_result = MagicMock(returncode=0, stdout=json_output(make_check("message")))
423+
with patch("main.subprocess.run", return_value=mock_result) as mock_run:
424+
main.run_pr_message_checks([(SHA_A, "fix: something")])
425+
self.assertEqual(mock_run.call_args[1]["input"], "fix: something")
426+
env = mock_run.call_args[1]["env"]
427+
self.assertEqual(env["GIT_AUTHOR_NAME"], "google-labs-jules[bot]")
428+
self.assertEqual(env["GIT_AUTHOR_EMAIL"], "jules@example.com")
429+
# The rest of the environment is kept: PATH finds the CLI.
430+
self.assertEqual(env.get("PATH"), os.environ.get("PATH"))
431+
393432
def test_single_message_pass(self):
394433
mock_result = MagicMock(returncode=0, stdout=json_output(make_check("message")))
395434
with patch("main.subprocess.run", return_value=mock_result) as mock_run:
@@ -1158,9 +1197,17 @@ class TestGetMessagesFromApi(unittest.TestCase):
11581197
def _run(self, total, api_commits, head="b2"):
11591198
pull = MagicMock()
11601199
pull.get_commits.return_value = [
1161-
MagicMock(sha=sha, commit=MagicMock(message=msg))
1200+
MagicMock(
1201+
sha=sha,
1202+
commit=MagicMock(
1203+
message=msg,
1204+
author=MagicMock(email=f"{sha}@example.com"),
1205+
),
1206+
)
11621207
for sha, msg in api_commits
11631208
]
1209+
for listed in pull.get_commits.return_value:
1210+
listed.commit.author.name = f"author {listed.sha}"
11641211
github_module = MagicMock()
11651212
github_module.Github.return_value.get_repo.return_value.get_pull.return_value = pull
11661213
env = {
@@ -1185,6 +1232,37 @@ def test_lists_the_commits_oldest_first(self):
11851232
"https://ghe.example.com/api/v3",
11861233
)
11871234

1235+
def test_records_each_commit_author(self):
1236+
main.API_AUTHORS.clear()
1237+
self.addCleanup(main.API_AUTHORS.clear)
1238+
self._run(2, [("a1", "feat: one"), ("b2", "fix: two")])
1239+
self.assertEqual(
1240+
main.API_AUTHORS,
1241+
{
1242+
"a1": ("author a1", "a1@example.com"),
1243+
"b2": ("author b2", "b2@example.com"),
1244+
},
1245+
)
1246+
1247+
def test_a_commit_without_an_author_records_nothing(self):
1248+
main.API_AUTHORS.clear()
1249+
self.addCleanup(main.API_AUTHORS.clear)
1250+
pull = MagicMock()
1251+
pull.get_commits.return_value = [
1252+
MagicMock(sha="b2", commit=MagicMock(message="fix: two", author=None))
1253+
]
1254+
github_module = MagicMock()
1255+
github_module.Github.return_value.get_repo.return_value.get_pull.return_value = pull
1256+
env = {"GITHUB_TOKEN": "token", "GITHUB_REPOSITORY": "owner/repo"}
1257+
with (
1258+
patch.dict(os.environ, env),
1259+
patch("main.get_pr_event", return_value={"number": 7, "commits": 1}),
1260+
patch("main.get_pr_head_sha", return_value="b2"),
1261+
patch.dict(sys.modules, {"github": github_module}),
1262+
):
1263+
self.assertEqual(main.get_messages_from_api(), [("b2", "fix: two")])
1264+
self.assertEqual(main.API_AUTHORS, {})
1265+
11881266
def test_a_list_that_ends_elsewhere_is_not_used(self):
11891267
# A force push after the event: same count, other commits.
11901268
result, _ = self._run(2, [("a1", "feat: one"), ("c3", "fix: other")])

0 commit comments

Comments
 (0)