From c4f19f2f32ac3b718bffbe2a7860769d592b9bd7 Mon Sep 17 00:00:00 2001 From: Ben Du Date: Sat, 27 Jun 2026 17:01:20 -0700 Subject: [PATCH 1/8] feat(github): add automatic pull request merging support Introduce automatic pull request merging capability (`auto_merge_pull_requests`) to class `Repository` in `github_rest_api/github.py` along with support functions, validating author/reviewer lists, title Conventional-Commit types, head commit minimum age, and clean mergeable state. Add comprehensive test coverage in `tests/test_github.py` and bump package version to 0.44.0. --- github_rest_api/__init__.py | 10 +- github_rest_api/github.py | 352 ++++++++++++++++++++++++++++++++- tests/test_github.py | 379 ++++++++++++++++++++++++++++++++++++ uv.lock | 2 +- 4 files changed, 740 insertions(+), 3 deletions(-) diff --git a/github_rest_api/__init__.py b/github_rest_api/__init__.py index e622b3d..3e17fe4 100644 --- a/github_rest_api/__init__.py +++ b/github_rest_api/__init__.py @@ -1,6 +1,7 @@ """GitHub REST APIs.""" from .github import ( + MergeMethod, Organization, Repository, RepositoryType, @@ -8,4 +9,11 @@ User, ) -__all__ = ["Organization", "Repository", "RepositoryType", "SecretVisibility", "User"] +__all__ = [ + "MergeMethod", + "Organization", + "Repository", + "RepositoryType", + "SecretVisibility", + "User", +] diff --git a/github_rest_api/github.py b/github_rest_api/github.py index 852a4a6..7a78ddc 100644 --- a/github_rest_api/github.py +++ b/github_rest_api/github.py @@ -2,9 +2,11 @@ import logging import re +import time from abc import ABCMeta, abstractmethod from base64 import b64encode from collections.abc import Sequence +from datetime import datetime, timedelta, timezone from enum import StrEnum from pathlib import Path from typing import Any, Callable @@ -26,6 +28,18 @@ # Default LiteLLM 'provider/model' used to generate PR titles and descriptions. DEFAULT_PR_MODEL = "anthropic/claude-haiku-4-5-20251001" +# Defaults for automatic pull request merging (see Repository.auto_merge_pull_requests). +# A marker comment from an allowlisted approver is accepted as an approval signal +# for AI reviewers that only comment instead of submitting a formal review. +DEFAULT_AUTO_MERGE_MARKER = "" +# Conventional-commit title types eligible for auto-merge. +DEFAULT_AUTO_MERGE_TYPES = ("chore", "docs", "deps") +# A PR is only auto-merged once its head commit is at least this old, an anti-race +# guard so a PR that momentarily reads `clean` before CI registers is not merged. +# A just-created PR is simply deferred to the next run, never dropped, so a small +# margin over the CI-registration latency suffices; set to 0 to disable the guard. +DEFAULT_MIN_AGE_MINUTES = 2 + def _validate_secret_name(name: str) -> None: """Validate a secret name against GitHub's naming rules. @@ -83,6 +97,125 @@ def _is_rust(file: str) -> bool: return False +def _login(obj: dict[str, Any]) -> str | None: + """Return the author login of a GitHub object (PR, review, comment), or None. + + The ``user`` field can be absent or null (e.g. a ghost/deleted account), so + it is guarded before reading ``login``. + + :param obj: A GitHub object carrying an optional ``user`` sub-object. + """ + return (obj.get("user") or {}).get("login") + + +# Matches the leading Conventional-Commits type token of a title, e.g. the +# `chore` in "chore(deps): bump x" or "feat!: ...". +_CONVENTIONAL_TYPE = re.compile(r"^(\w+)(\([^)]*\))?!?:") + + +def _conventional_type(title: str) -> str | None: + """Extract the lower-cased Conventional-Commits type token from a PR title. + + :param title: The pull request title. + :return: The type token (e.g. ``chore``) lower-cased, or None when the title + does not start with a Conventional-Commits type prefix. + """ + match = _CONVENTIONAL_TYPE.match(title) + return match.group(1).lower() if match else None + + +def _title_type_allowed(title: str, allowed_types: Sequence[str]) -> bool: + """Check whether a PR title's Conventional-Commits type is auto-merge eligible. + + :param title: The pull request title. + :param allowed_types: The Conventional-Commits types eligible for auto-merge. + """ + type_ = _conventional_type(title) + return type_ is not None and type_ in allowed_types + + +def _field_gate_failure( + pr: dict[str, Any], authors: Sequence[str], allowed_types: Sequence[str] +) -> str | None: + """Return why a PR fails the field-only auto-merge gates, or None if it passes. + + These gates (not a draft, trusted author, eligible title type) read only + fields carried by both the list and single-PR payloads, so they can filter + list items before the more expensive single-PR detail is fetched. + + :param pr: A PR payload (a list item or full detail). + :param authors: Logins whose PRs are eligible for auto-merge. + :param allowed_types: Conventional-Commits title types eligible for auto-merge. + """ + if pr.get("draft"): + return "it is a draft" + if _login(pr) not in set(authors): + return "its author is not in the allowlist" + if not _title_type_allowed(pr.get("title") or "", allowed_types): + return "its title type is not auto-merge eligible" + return None + + +def _approver_review_states( + reviews: Sequence[dict[str, Any]], approvers: Sequence[str] +) -> set[str]: + """Return the set of latest decisive review states among allowlisted reviewers. + + Only the latest decisive review per reviewer is kept. ``APPROVED``, + ``CHANGES_REQUESTED`` and ``DISMISSED`` are decisive (so a later + ``DISMISSED`` clears an earlier approval), while ``COMMENTED`` and + ``PENDING`` are ignored (a comment does not revoke an approval). + + Returning the set (rather than a single approved/not-approved boolean) lets + the caller treat ``CHANGES_REQUESTED`` as a veto independently of how + approval is granted, so a marker comment cannot override a standing change + request. + + :param reviews: Reviews as returned by ``get_pull_request_reviews``. + :param approvers: Logins whose reviews are trusted for auto-merge. + """ + approver_set = set(approvers) + # Reviews are returned in chronological order, so a later decisive entry + # overrides an earlier one for the same reviewer. + latest: dict[str, str] = {} + for review in reviews: + login = _login(review) + state = review.get("state") + if login in approver_set and state in ( + "APPROVED", + "CHANGES_REQUESTED", + "DISMISSED", + ): + latest[login] = state + return set(latest.values()) + + +def _marker_approved( + comments: Sequence[dict[str, Any]], approvers: Sequence[str], marker: str +) -> bool: + """Check whether an allowlisted approver left the auto-merge marker comment. + + :param comments: Issue comments as returned by ``get_issue_comments``. + :param approvers: Logins whose marker comments are trusted for auto-merge. + :param marker: The marker substring signalling approval. + """ + approver_set = set(approvers) + return any( + _login(comment) in approver_set and marker in (comment.get("body") or "") + for comment in comments + ) + + +def _old_enough(commit_iso: str, min_age_minutes: int) -> bool: + """Check whether a commit timestamp is at least ``min_age_minutes`` old. + + :param commit_iso: An ISO-8601 UTC timestamp (e.g. ``2026-06-27T12:00:00Z``). + :param min_age_minutes: The minimum age in minutes. + """ + committed = datetime.fromisoformat(commit_iso.replace("Z", "+00:00")) + return datetime.now(timezone.utc) - committed >= timedelta(minutes=min_age_minutes) + + class GitHub: def __init__(self, token: str): self._token = token @@ -209,6 +342,12 @@ class IssueState(StrEnum): ALL = "all" +class MergeMethod(StrEnum): + MERGE = "merge" + SQUASH = "squash" + REBASE = "rebase" + + class Repository(GitHub): """Abstraction of a GitHub repository.""" @@ -288,6 +427,28 @@ def get_pull_requests(self, n: int = 0) -> list[dict[str, Any]]: """List pull requests in this repository.""" return self._extract_all(url=self._url_pull, n=n) + def get_pull_request(self, pr_number: int) -> dict[str, Any]: + """Get the full detail of a single pull request. + + Unlike ``get_pull_requests``, this returns fields computed per-PR such as + ``mergeable`` and ``mergeable_state``. These are computed asynchronously + by GitHub, so ``mergeable_state`` may be ``unknown`` immediately after a + push; callers that need it should re-fetch until it settles. + + :param pr_number: The number of the pull request. + """ + return self._get(url=f"{self._url_pull}/{pr_number}").json() + + def get_pull_request_reviews( + self, pr_number: int, n: int = 0 + ) -> list[dict[str, Any]]: + """List reviews on a pull request. + + :param pr_number: The number of the pull request. + :param n: The maximum number of reviews to return (0 means all). + """ + return self._extract_all(url=f"{self._url_pull}/{pr_number}/reviews", n=n) + def get_issues( self, state: IssueState = IssueState.OPEN, n: int = 0 ) -> list[dict[str, Any]]: @@ -378,12 +539,26 @@ def create_pull_request( resp.raise_for_status() return resp.json() - def merge_pull_request(self, pr_number: int) -> dict[str, Any]: + def merge_pull_request( + self, + pr_number: int, + merge_method: MergeMethod | str = MergeMethod.MERGE, + sha: str = "", + ) -> dict[str, Any]: """Merge a pull request in this repository. :param pr_number: The number of the pull quest to be merged. + :param merge_method: The merge method to use (``merge``, ``squash`` or + ``rebase``). Defaults to ``merge``. + :param sha: When non-empty, the SHA that the PR head must still match for + the merge to proceed (GitHub rejects the merge with 409 if the head + has moved). Use it to pin a merge to a previously validated commit. """ + body: dict[str, Any] = {"merge_method": str(merge_method)} + if sha: + body["sha"] = sha return self._put( url=f"{self._url_pull}/{pr_number}/merge", + json=body, ).json() def update_branch(self, update: str, upstream: str) -> dict[str, Any] | None: @@ -405,6 +580,165 @@ def update_branch(self, update: str, upstream: str) -> dict[str, Any] | None: return return self.merge_pull_request(pr["number"]) + def _settle_mergeable_state( + self, pr: dict[str, Any], attempts: int = 3, delay: float = 2.0 + ) -> dict[str, Any]: + """Re-fetch a PR until GitHub finishes computing ``mergeable_state``. + + GitHub computes ``mergeable_state`` asynchronously, returning ``unknown`` + until it settles. The PR is re-fetched (up to ``attempts`` times, waiting + ``delay`` seconds between tries) until the state is no longer ``unknown``. + + :param pr: A PR detail dict as returned by ``get_pull_request``. + :param attempts: The maximum number of fetches before giving up. + :param delay: The delay in seconds between fetches. + """ + for _ in range(attempts): + if pr.get("mergeable_state") not in (None, "unknown"): + break + time.sleep(delay) + pr = self.get_pull_request(pr["number"]) + return pr + + def should_auto_merge( + self, + pr_number: int, + *, + authors: Sequence[str], + approvers: Sequence[str], + allowed_types: Sequence[str] = DEFAULT_AUTO_MERGE_TYPES, + min_age_minutes: int = DEFAULT_MIN_AGE_MINUTES, + marker: str = DEFAULT_AUTO_MERGE_MARKER, + ) -> str | None: + """Decide whether a pull request is eligible for automatic merging. + + All of the following must hold: the PR is not a draft; its author is in + ``authors``; its Conventional-Commits title type is in ``allowed_types``; + its ``mergeable_state`` is ``clean`` (no merge conflict and no failing or + pending status checks); its head commit is at least ``min_age_minutes`` + old; and it is approved either by an ``APPROVED`` review or by a + ``marker`` comment from an approver in ``approvers`` (with no outstanding + ``CHANGES_REQUESTED``). + + The age check guards against the race where a brand-new PR momentarily + reads ``clean`` before its checks register; it relies on the head + commit's timestamp, which is trustworthy given that ``authors`` is an + allowlist of trusted automation. + + Checks run cheapest-first, returning ``None`` (with a logged reason) on + the first failure so extra requests are avoided. + + :param pr_number: The number of the pull request. + :param authors: Logins whose PRs are eligible for auto-merge. + :param approvers: Logins whose reviews/marker comments grant approval. + :param allowed_types: Conventional-Commits title types eligible for auto-merge. + :param min_age_minutes: The minimum head-commit age in minutes. + :param marker: The marker substring an approver may comment to approve. + :return: The validated head SHA when the PR is eligible, else ``None``. + Passing the SHA to ``merge_pull_request`` pins the merge to the exact + commit checked here, so a push landing between this gate and the merge + is rejected rather than silently merged. + """ + pr = self.get_pull_request(pr_number) + + def skip(reason: str) -> None: + logger.info("Skipping auto-merge of PR #%s: %s.", pr_number, reason) + return None + + # Field-only gates first; these need no extra requests. + field_failure = _field_gate_failure(pr, authors, allowed_types) + if field_failure: + return skip(field_failure) + # Check age before mergeable_state: one commit-date fetch is cheaper than + # settling mergeable_state, and mergeable_state is only trustworthy once + # the PR is old enough for its checks to have registered. + committed = self._head_commit_date(pr["head"]["sha"]) + if not _old_enough(committed, min_age_minutes): + return skip(f"its head commit is newer than {min_age_minutes} minutes") + pr = self._settle_mergeable_state(pr) + if pr.get("mergeable_state") != "clean": + return skip(f"its mergeable_state is {pr.get('mergeable_state')!r}") + # The head SHA is read after settling so it matches the mergeable_state + # just validated, and is returned so the caller can pin the merge to it. + head_sha = pr["head"]["sha"] + reviews = self.get_pull_request_reviews(pr_number) + review_states = _approver_review_states(reviews, approvers) + # A standing change request vetoes the merge regardless of how approval + # would otherwise be granted, so it is checked before the marker path. + if "CHANGES_REQUESTED" in review_states: + return skip("an approver has requested changes") + if "APPROVED" in review_states: + return head_sha + comments = self.get_issue_comments(pr_number) + if _marker_approved(comments, approvers, marker): + return head_sha + return skip("it lacks an approving review or marker comment") + + def auto_merge_pull_requests( + self, + *, + authors: Sequence[str], + approvers: Sequence[str], + allowed_types: Sequence[str] = DEFAULT_AUTO_MERGE_TYPES, + min_age_minutes: int = DEFAULT_MIN_AGE_MINUTES, + marker: str = DEFAULT_AUTO_MERGE_MARKER, + merge_method: MergeMethod | str = MergeMethod.MERGE, + dry_run: bool = False, + ) -> list[int]: + """Auto-merge every open pull request that passes ``should_auto_merge``. + + :param authors: Logins whose PRs are eligible for auto-merge. An empty + allowlist makes nothing eligible (fail-safe). + :param approvers: Logins whose reviews/marker comments grant approval. + :param allowed_types: Conventional-Commits title types eligible for auto-merge. + :param min_age_minutes: The minimum head-commit age in minutes. + :param marker: The marker substring an approver may comment to approve. + :param merge_method: The merge method to use (``merge``, ``squash`` or + ``rebase``). + :param dry_run: When True, eligible PRs are logged and returned but not + merged. + :return: The numbers of the PRs that were merged (or, under ``dry_run``, + that would have been merged). + """ + eligible = [] + for pr in self.get_pull_requests(): + number = pr["number"] + # Pre-filter on the list payload so PRs failing a field-only gate are + # dropped without fetching their (more expensive) single-PR detail. + field_failure = _field_gate_failure(pr, authors, allowed_types) + if field_failure: + logger.info("Skipping auto-merge of PR #%s: %s.", number, field_failure) + continue + # Both the eligibility checks and the merge issue requests that can + # fail for a single PR (a force-pushed/deleted head, a PR that became + # unmergeable, or a transient error). Isolate each PR so one failure + # skips that PR without aborting the rest of the batch. + try: + sha = self.should_auto_merge( + number, + authors=authors, + approvers=approvers, + allowed_types=allowed_types, + min_age_minutes=min_age_minutes, + marker=marker, + ) + if sha is None: + continue + if dry_run: + logger.info("[dry-run] PR #%s is eligible for auto-merge.", number) + else: + logger.info("Auto-merging PR #%s.", number) + # Pin the merge to the SHA the gate validated so a push that + # landed in between is rejected (409) instead of merged. + self.merge_pull_request(number, merge_method, sha=sha) + except requests.HTTPError: + logger.warning( + "Skipping PR #%s after a request failure.", number, exc_info=True + ) + continue + eligible.append(number) + return eligible + def get_pull_request_files( self, pr_number: int, n: int = 0 ) -> list[dict[str, Any]]: @@ -429,6 +763,22 @@ def compare(self, base: str, head: str) -> dict[str, Any]: basehead = f"{quote(base, safe='')}...{quote(head, safe='')}" return self._get(url=f"{self._url_compare}/{basehead}").json() + def _head_commit_date(self, sha: str) -> str: + """Return the committer date of ``sha`` without fetching its file diff. + + The single-commit endpoint (``GET /commits/{sha}``) embeds the commit's + full file diff, which is wasteful when only the timestamp is needed. The + list-commits endpoint omits per-commit files, so requesting a single + commit starting at ``sha`` stays cheap even for a large diff. + + :param sha: The head commit SHA. + """ + commits = self._get( + url=f"{self._url_repo}/commits", + params={"sha": sha, "per_page": 1}, + ).json() + return commits[0]["commit"]["committer"]["date"] + def get_branches(self, n: int = 0) -> list[dict[str, Any]]: """List branches in this repository.""" return self._extract_all(url=self._url_branches, n=n) diff --git a/tests/test_github.py b/tests/test_github.py index a032bd2..ba3a4bc 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -1,15 +1,26 @@ import os from base64 import b64decode +from contextlib import ExitStack +from datetime import datetime, timedelta, timezone from unittest.mock import MagicMock, patch import pytest +import requests from nacl import encoding, public from github_rest_api.github import ( + MergeMethod, Organization, Repository, User, + _approver_review_states, + _conventional_type, _encrypt_secret, + _field_gate_failure, + _login, + _marker_approved, + _old_enough, + _title_type_allowed, _validate_secret_name, ) from github_rest_api.pr_content import deterministic_body, deterministic_title @@ -255,3 +266,371 @@ def test_generate_pull_request_content_empty_returns_none(): repo._generate_pull_request_content(base="main", head="dev", model="m") is None ) + + +@pytest.mark.parametrize( + ("title", "expected"), + [ + ("chore: bump deps", "chore"), + ("docs(readme): tweak", "docs"), + ("feat!: breaking change", "feat"), + ("Chore: capitalized", "chore"), + ("no conventional prefix", None), + ("", None), + ], +) +def test_conventional_type(title, expected): + assert _conventional_type(title) == expected + + +@pytest.mark.parametrize( + ("title", "allowed", "expected"), + [ + ("chore: x", ("chore", "docs"), True), + ("docs(scope): x", ("chore", "docs"), True), + ("feat: x", ("chore", "docs"), False), + ("no prefix", ("chore",), False), + ], +) +def test_title_type_allowed(title, allowed, expected): + assert _title_type_allowed(title, allowed) is expected + + +@pytest.mark.parametrize( + ("obj", "expected"), + [ + ({"user": {"login": "bot"}}, "bot"), + ({"user": None}, None), + ({}, None), + ], +) +def test_login(obj, expected): + assert _login(obj) == expected + + +def test_field_gate_failure_passes_eligible_pr(): + pr = {"draft": False, "user": {"login": "bot"}, "title": "chore: bump"} + assert _field_gate_failure(pr, ["bot"], ("chore",)) is None + + +@pytest.mark.parametrize( + "pr", + [ + {"draft": True, "user": {"login": "bot"}, "title": "chore: x"}, + {"draft": False, "user": {"login": "stranger"}, "title": "chore: x"}, + {"draft": False, "user": {"login": "bot"}, "title": "feat: x"}, + ], +) +def test_field_gate_failure_rejects(pr): + assert _field_gate_failure(pr, ["bot"], ("chore",)) is not None + + +def _review(login, state): + return {"user": {"login": login}, "state": state} + + +def test_approver_review_states_approved(): + assert _approver_review_states([_review("bot", "APPROVED")], ["bot"]) == { + "APPROVED" + } + + +def test_approver_review_states_ignores_non_approvers(): + assert _approver_review_states([_review("stranger", "APPROVED")], ["bot"]) == set() + + +def test_approver_review_states_uses_latest_state_per_login(): + # A later APPROVED overrides an earlier CHANGES_REQUESTED for the same login. + reviews = [_review("bot", "CHANGES_REQUESTED"), _review("bot", "APPROVED")] + assert _approver_review_states(reviews, ["bot"]) == {"APPROVED"} + + +def test_approver_review_states_latest_changes_requested(): + reviews = [_review("bot", "APPROVED"), _review("bot", "CHANGES_REQUESTED")] + assert _approver_review_states(reviews, ["bot"]) == {"CHANGES_REQUESTED"} + + +def test_approver_review_states_combines_multiple_approvers(): + reviews = [_review("bot", "APPROVED"), _review("human", "CHANGES_REQUESTED")] + assert _approver_review_states(reviews, ["bot", "human"]) == { + "APPROVED", + "CHANGES_REQUESTED", + } + + +def test_approver_review_states_ignores_commented_reviews(): + assert _approver_review_states([_review("bot", "COMMENTED")], ["bot"]) == set() + + +def test_approver_review_states_dismissed_clears_earlier_approval(): + reviews = [_review("bot", "APPROVED"), _review("bot", "DISMISSED")] + assert _approver_review_states(reviews, ["bot"]) == {"DISMISSED"} + + +def test_approver_review_states_comment_does_not_clear_approval(): + reviews = [_review("bot", "APPROVED"), _review("bot", "COMMENTED")] + assert _approver_review_states(reviews, ["bot"]) == {"APPROVED"} + + +def test_marker_approved_true(): + comments = [{"user": {"login": "bot"}, "body": "looks good "}] + assert _marker_approved(comments, ["bot"], "") is True + + +def test_marker_approved_requires_approver_author(): + comments = [{"user": {"login": "stranger"}, "body": ""}] + assert _marker_approved(comments, ["bot"], "") is False + + +def test_marker_approved_requires_marker(): + comments = [{"user": {"login": "bot"}, "body": "nice"}] + assert _marker_approved(comments, ["bot"], "") is False + + +def test_old_enough_true_for_old_commit(): + old = (datetime.now(timezone.utc) - timedelta(minutes=30)).isoformat() + assert _old_enough(old, 10) is True + + +def test_old_enough_false_for_recent_commit(): + recent = (datetime.now(timezone.utc) - timedelta(minutes=2)).isoformat() + assert _old_enough(recent, 10) is False + + +def test_old_enough_parses_zulu_suffix(): + old = "2000-01-01T00:00:00Z" + assert _old_enough(old, 10) is True + + +def test_merge_pull_request_forwards_merge_method(): + repo = Repository("token", "owner/name") + response = MagicMock() + response.json.return_value = {"merged": True} + with patch.object(repo, "_put", return_value=response) as mock_put: + repo.merge_pull_request(7) + # No sha given: the body omits it so GitHub does not pin the merge. + assert mock_put.call_args.kwargs["json"] == {"merge_method": "merge"} + repo.merge_pull_request(7, MergeMethod.SQUASH) + assert mock_put.call_args.kwargs["json"] == {"merge_method": "squash"} + repo.merge_pull_request(7, MergeMethod.SQUASH, sha="abc123") + assert mock_put.call_args.kwargs["json"] == { + "merge_method": "squash", + "sha": "abc123", + } + + +def test_head_commit_date_uses_list_endpoint_without_files(): + repo = Repository("token", "owner/name") + response = MagicMock() + response.json.return_value = [ + {"commit": {"committer": {"date": "2026-06-27T00:00:00Z"}}} + ] + with patch.object(repo, "_get", return_value=response) as mock_get: + date = repo._head_commit_date("abc123") + assert date == "2026-06-27T00:00:00Z" + # The list-commits endpoint (not /commits/{sha}) is used so no file diff is + # downloaded, limited to the single head commit. + assert mock_get.call_args.kwargs["url"] == ( + "https://api.github.com/repos/owner/name/commits" + ) + assert mock_get.call_args.kwargs["params"] == {"sha": "abc123", "per_page": 1} + + +def _auto_merge_repo(pr_overrides=None, *, reviews=None, comments=None, commit_age=30): + """Build a Repository with all should_auto_merge dependencies patched to pass. + + Individual tests override one field to exercise a single failing gate. + """ + repo = Repository("token", "owner/name") + pr = { + "number": 1, + "draft": False, + "user": {"login": "bot"}, + "title": "chore: bump", + "mergeable_state": "clean", + "head": {"sha": "abc123"}, + } + pr.update(pr_overrides or {}) + committed = (datetime.now(timezone.utc) - timedelta(minutes=commit_age)).isoformat() + patches = [ + patch.object(repo, "get_pull_request", return_value=pr), + patch.object(repo, "_head_commit_date", return_value=committed), + patch.object( + repo, + "get_pull_request_reviews", + return_value=[_review("bot", "APPROVED")] if reviews is None else reviews, + ), + patch.object( + repo, "get_issue_comments", return_value=comments if comments else [] + ), + ] + return repo, patches + + +def _run_should_auto_merge(repo, patches, **kwargs): + params = {"authors": ["bot"], "approvers": ["bot"]} + params.update(kwargs) + with ExitStack() as stack: + for p in patches: + stack.enter_context(p) + return repo.should_auto_merge(1, **params) + + +def test_should_auto_merge_all_pass(): + repo, patches = _auto_merge_repo() + # On eligibility, the validated head SHA is returned (head.sha in _auto_merge_repo). + assert _run_should_auto_merge(repo, patches) == "abc123" + + +def test_should_auto_merge_marker_comment_path(): + repo, patches = _auto_merge_repo( + reviews=[], + comments=[{"user": {"login": "bot"}, "body": ""}], + ) + # On eligibility, the validated head SHA is returned (head.sha in _auto_merge_repo). + assert _run_should_auto_merge(repo, patches) == "abc123" + + +def test_should_auto_merge_skips_draft(): + repo, patches = _auto_merge_repo({"draft": True}) + assert _run_should_auto_merge(repo, patches) is None + + +def test_should_auto_merge_skips_untrusted_author(): + repo, patches = _auto_merge_repo({"user": {"login": "stranger"}}) + assert _run_should_auto_merge(repo, patches) is None + + +def test_should_auto_merge_skips_disallowed_title_type(): + repo, patches = _auto_merge_repo({"title": "feat: shiny"}) + assert _run_should_auto_merge(repo, patches) is None + + +def test_should_auto_merge_skips_unclean_state(): + repo, patches = _auto_merge_repo({"mergeable_state": "blocked"}) + assert _run_should_auto_merge(repo, patches) is None + + +def test_should_auto_merge_skips_too_new(): + repo, patches = _auto_merge_repo(commit_age=0) + assert _run_should_auto_merge(repo, patches) is None + + +def test_should_auto_merge_skips_without_approval(): + repo, patches = _auto_merge_repo(reviews=[], comments=[]) + assert _run_should_auto_merge(repo, patches) is None + + +def test_should_auto_merge_skips_when_changes_requested(): + repo, patches = _auto_merge_repo(reviews=[_review("bot", "CHANGES_REQUESTED")]) + assert _run_should_auto_merge(repo, patches) is None + + +def test_should_auto_merge_changes_requested_vetoes_marker_comment(): + # A standing change request must block the merge even when an approver also + # left an approving marker comment. + repo, patches = _auto_merge_repo( + reviews=[_review("bot", "CHANGES_REQUESTED")], + comments=[{"user": {"login": "bot"}, "body": ""}], + ) + assert _run_should_auto_merge(repo, patches) is None + + +def _pr_item(number): + """A list-endpoint PR payload that passes the field-only pre-filter gate.""" + return { + "number": number, + "draft": False, + "user": {"login": "bot"}, + "title": "chore: bump", + } + + +def test_auto_merge_pull_requests_merges_only_eligible(): + repo = Repository("token", "owner/name") + with ( + patch.object( + repo, "get_pull_requests", return_value=[_pr_item(1), _pr_item(2)] + ), + # should_auto_merge returns the validated SHA for #1, None for #2. + patch.object(repo, "should_auto_merge", side_effect=["sha1", None]), + patch.object(repo, "merge_pull_request") as mock_merge, + ): + merged = repo.auto_merge_pull_requests( + authors=["bot"], approvers=["bot"], merge_method=MergeMethod.SQUASH + ) + assert merged == [1] + # The merge is pinned to the SHA the gate validated. + mock_merge.assert_called_once_with(1, MergeMethod.SQUASH, sha="sha1") + + +def test_auto_merge_pull_requests_prefilters_without_fetching_detail(): + repo = Repository("token", "owner/name") + # PR #1 fails the field gate (untrusted author); PR #2 passes it. + item_1 = {"number": 1, "draft": False, "user": {"login": "x"}, "title": "chore: a"} + with ( + patch.object(repo, "get_pull_requests", return_value=[item_1, _pr_item(2)]), + patch.object(repo, "should_auto_merge", return_value="sha2") as mock_eval, + patch.object(repo, "merge_pull_request") as mock_merge, + ): + merged = repo.auto_merge_pull_requests(authors=["bot"], approvers=["bot"]) + # PR #1 is dropped by the pre-filter, so should_auto_merge is only called for #2. + assert merged == [2] + mock_eval.assert_called_once() + assert mock_eval.call_args.args == (2,) + mock_merge.assert_called_once_with(2, MergeMethod.MERGE, sha="sha2") + + +def test_auto_merge_pull_requests_dry_run_merges_nothing(): + repo = Repository("token", "owner/name") + with ( + patch.object( + repo, "get_pull_requests", return_value=[_pr_item(1), _pr_item(2)] + ), + patch.object(repo, "should_auto_merge", return_value="sha"), + patch.object(repo, "merge_pull_request") as mock_merge, + ): + merged = repo.auto_merge_pull_requests( + authors=["bot"], approvers=["bot"], dry_run=True + ) + assert merged == [1, 2] + mock_merge.assert_not_called() + + +def test_auto_merge_pull_requests_continues_after_merge_failure(): + repo = Repository("token", "owner/name") + with ( + patch.object( + repo, "get_pull_requests", return_value=[_pr_item(1), _pr_item(2)] + ), + patch.object(repo, "should_auto_merge", return_value="sha"), + patch.object( + repo, + "merge_pull_request", + side_effect=[requests.HTTPError("409 conflict"), {"merged": True}], + ) as mock_merge, + ): + merged = repo.auto_merge_pull_requests(authors=["bot"], approvers=["bot"]) + # PR #1 fails to merge but the batch continues; only PR #2 is reported merged. + assert merged == [2] + assert mock_merge.call_count == 2 + + +def test_auto_merge_pull_requests_continues_after_evaluation_failure(): + repo = Repository("token", "owner/name") + with ( + patch.object( + repo, "get_pull_requests", return_value=[_pr_item(1), _pr_item(2)] + ), + patch.object( + repo, + "should_auto_merge", + side_effect=[requests.HTTPError("404 head gone"), "sha2"], + ), + patch.object(repo, "merge_pull_request") as mock_merge, + ): + merged = repo.auto_merge_pull_requests(authors=["bot"], approvers=["bot"]) + # Evaluating PR #1 raises (e.g. its head was force-pushed); the batch still + # evaluates and merges PR #2. + assert merged == [2] + mock_merge.assert_called_once_with(2, MergeMethod.MERGE, sha="sha2") diff --git a/uv.lock b/uv.lock index 5ee520e..1a078ec 100644 --- a/uv.lock +++ b/uv.lock @@ -542,7 +542,7 @@ wheels = [ [[package]] name = "github-rest-api" -version = "0.43.0" +version = "0.44.0" source = { editable = "." } dependencies = [ { name = "dulwich" }, From c8f734bf8db3e832696e1d7fe49ca9f3e7b5622e Mon Sep 17 00:00:00 2001 From: Ben Chuanlong Du Date: Sun, 28 Jun 2026 10:03:45 -0700 Subject: [PATCH 2/8] Update github_rest_api/github.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- github_rest_api/github.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/github_rest_api/github.py b/github_rest_api/github.py index 7a78ddc..fdd4ea5 100644 --- a/github_rest_api/github.py +++ b/github_rest_api/github.py @@ -777,6 +777,8 @@ def _head_commit_date(self, sha: str) -> str: url=f"{self._url_repo}/commits", params={"sha": sha, "per_page": 1}, ).json() + if not isinstance(commits, list) or not commits: + raise ValueError(f"No commits found for SHA {sha}") return commits[0]["commit"]["committer"]["date"] def get_branches(self, n: int = 0) -> list[dict[str, Any]]: From 17fbd4ad3cd163aa6984571865d3c8c644542758 Mon Sep 17 00:00:00 2001 From: Ben Chuanlong Du Date: Sun, 28 Jun 2026 10:04:43 -0700 Subject: [PATCH 3/8] Update github_rest_api/github.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- github_rest_api/github.py | 47 ++++++++++++++++++++++++++++++-------- tests/test_github.py | 48 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/github_rest_api/github.py b/github_rest_api/github.py index fdd4ea5..5a36c14 100644 --- a/github_rest_api/github.py +++ b/github_rest_api/github.py @@ -9,7 +9,7 @@ from datetime import datetime, timedelta, timezone from enum import StrEnum from pathlib import Path -from typing import Any, Callable +from typing import Any, Callable, TypeGuard from urllib.parse import quote import requests @@ -108,6 +108,32 @@ def _login(obj: dict[str, Any]) -> str | None: return (obj.get("user") or {}).get("login") +def _normalized_logins(logins: Sequence[str]) -> set[str]: + """Lower-case GitHub logins into a set for case-insensitive membership tests. + + GitHub logins are case-insensitive, so normalizing the allowlist once (rather + than per candidate) lets repeated membership tests compare against a stable + lower-cased set. + + :param logins: The logins eligible for a gated action. + """ + return {login.lower() for login in logins} + + +def _in_allowlist(login: str | None, allowlist: set[str]) -> TypeGuard[str]: + """Check whether a GitHub login is in a normalized allowlist. + + ``allowlist`` must already be lower-cased (see :func:`_normalized_logins`). + The login is lower-cased here before comparison, so casing differences + between the configured allowlist and the API response do not cause spurious + mismatches. A ``None`` login (e.g. a ghost/deleted account) is never a member. + + :param login: The login to test, or None. + :param allowlist: A lower-cased set of logins eligible for the gated action. + """ + return login is not None and login.lower() in allowlist + + # Matches the leading Conventional-Commits type token of a title, e.g. the # `chore` in "chore(deps): bump x" or "feat!: ...". _CONVENTIONAL_TYPE = re.compile(r"^(\w+)(\([^)]*\))?!?:") @@ -149,7 +175,7 @@ def _field_gate_failure( """ if pr.get("draft"): return "it is a draft" - if _login(pr) not in set(authors): + if not _in_allowlist(_login(pr), _normalized_logins(authors)): return "its author is not in the allowlist" if not _title_type_allowed(pr.get("title") or "", allowed_types): return "its title type is not auto-merge eligible" @@ -174,14 +200,14 @@ def _approver_review_states( :param reviews: Reviews as returned by ``get_pull_request_reviews``. :param approvers: Logins whose reviews are trusted for auto-merge. """ - approver_set = set(approvers) + approver_set = _normalized_logins(approvers) # Reviews are returned in chronological order, so a later decisive entry # overrides an earlier one for the same reviewer. latest: dict[str, str] = {} for review in reviews: login = _login(review) state = review.get("state") - if login in approver_set and state in ( + if _in_allowlist(login, approver_set) and state in ( "APPROVED", "CHANGES_REQUESTED", "DISMISSED", @@ -199,9 +225,10 @@ def _marker_approved( :param approvers: Logins whose marker comments are trusted for auto-merge. :param marker: The marker substring signalling approval. """ - approver_set = set(approvers) + approver_set = _normalized_logins(approvers) return any( - _login(comment) in approver_set and marker in (comment.get("body") or "") + _in_allowlist(_login(comment), approver_set) + and marker in (comment.get("body") or "") for comment in comments ) @@ -212,7 +239,7 @@ def _old_enough(commit_iso: str, min_age_minutes: int) -> bool: :param commit_iso: An ISO-8601 UTC timestamp (e.g. ``2026-06-27T12:00:00Z``). :param min_age_minutes: The minimum age in minutes. """ - committed = datetime.fromisoformat(commit_iso.replace("Z", "+00:00")) + committed = datetime.fromisoformat(commit_iso) return datetime.now(timezone.utc) - committed >= timedelta(minutes=min_age_minutes) @@ -731,9 +758,11 @@ def auto_merge_pull_requests( # Pin the merge to the SHA the gate validated so a push that # landed in between is rejected (409) instead of merged. self.merge_pull_request(number, merge_method, sha=sha) - except requests.HTTPError: + except Exception: logger.warning( - "Skipping PR #%s after a request failure.", number, exc_info=True + "Skipping PR #%s after an unexpected failure.", + number, + exc_info=True, ) continue eligible.append(number) diff --git a/tests/test_github.py b/tests/test_github.py index ba3a4bc..5b8e708 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -536,6 +536,54 @@ def test_should_auto_merge_changes_requested_vetoes_marker_comment(): assert _run_should_auto_merge(repo, patches) is None +def test_settle_mergeable_state_retries_until_settled(): + # GitHub returns "unknown" until it finishes computing mergeable_state; the + # PR is re-fetched (sleeping between tries) until the state settles. + repo = Repository("token", "owner/name") + pr = {"number": 1, "mergeable_state": "unknown"} + fetched = [ + {"number": 1, "mergeable_state": "unknown"}, + {"number": 1, "mergeable_state": "clean"}, + ] + with ( + patch("github_rest_api.github.time.sleep") as mock_sleep, + patch.object(repo, "get_pull_request", side_effect=fetched) as mock_get, + ): + settled = repo._settle_mergeable_state(pr) + assert settled["mergeable_state"] == "clean" + assert mock_get.call_count == 2 + assert mock_sleep.call_count == 2 + + +def test_settle_mergeable_state_returns_immediately_when_settled(): + # An already-settled state needs no re-fetch and no sleep. + repo = Repository("token", "owner/name") + pr = {"number": 1, "mergeable_state": "clean"} + with ( + patch("github_rest_api.github.time.sleep") as mock_sleep, + patch.object(repo, "get_pull_request") as mock_get, + ): + settled = repo._settle_mergeable_state(pr) + assert settled is pr + mock_get.assert_not_called() + mock_sleep.assert_not_called() + + +def test_settle_mergeable_state_gives_up_after_attempts(): + # When the state never settles, it gives up after ``attempts`` fetches and + # returns the last (still-unknown) PR rather than looping forever. + repo = Repository("token", "owner/name") + pr = {"number": 1, "mergeable_state": "unknown"} + with ( + patch("github_rest_api.github.time.sleep") as mock_sleep, + patch.object(repo, "get_pull_request", return_value=dict(pr)) as mock_get, + ): + settled = repo._settle_mergeable_state(pr, attempts=3) + assert settled["mergeable_state"] == "unknown" + assert mock_get.call_count == 3 + assert mock_sleep.call_count == 3 + + def _pr_item(number): """A list-endpoint PR payload that passes the field-only pre-filter gate.""" return { From e5e9b8caa5bfb0d9a5b71187b428b34a641956f2 Mon Sep 17 00:00:00 2001 From: Ben Chuanlong Du Date: Sun, 28 Jun 2026 20:31:55 -0700 Subject: [PATCH 4/8] Update github_rest_api/github.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- github_rest_api/github.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/github_rest_api/github.py b/github_rest_api/github.py index 5a36c14..809711f 100644 --- a/github_rest_api/github.py +++ b/github_rest_api/github.py @@ -240,6 +240,8 @@ def _old_enough(commit_iso: str, min_age_minutes: int) -> bool: :param min_age_minutes: The minimum age in minutes. """ committed = datetime.fromisoformat(commit_iso) + if committed.tzinfo is None: + committed = committed.replace(tzinfo=timezone.utc) return datetime.now(timezone.utc) - committed >= timedelta(minutes=min_age_minutes) From 1250afcf06a6a9176616480017f8d6843f727c01 Mon Sep 17 00:00:00 2001 From: Ben Chuanlong Du Date: Sun, 28 Jun 2026 20:33:21 -0700 Subject: [PATCH 5/8] Update github_rest_api/github.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- github_rest_api/github.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/github_rest_api/github.py b/github_rest_api/github.py index 809711f..e2916ba 100644 --- a/github_rest_api/github.py +++ b/github_rest_api/github.py @@ -810,7 +810,12 @@ def _head_commit_date(self, sha: str) -> str: ).json() if not isinstance(commits, list) or not commits: raise ValueError(f"No commits found for SHA {sha}") - return commits[0]["commit"]["committer"]["date"] + try: + return commits[0]["commit"]["committer"]["date"] + except (KeyError, TypeError) as e: + raise ValueError( + f"Unexpected commit payload structure for SHA {sha}" + ) from e def get_branches(self, n: int = 0) -> list[dict[str, Any]]: """List branches in this repository.""" From 2c8065ac0b2457cd33ab06ab69a3825228f75699 Mon Sep 17 00:00:00 2001 From: Ben Chuanlong Du Date: Sun, 28 Jun 2026 21:28:59 -0700 Subject: [PATCH 6/8] Update github_rest_api/github.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- github_rest_api/github.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/github_rest_api/github.py b/github_rest_api/github.py index e2916ba..92b7d23 100644 --- a/github_rest_api/github.py +++ b/github_rest_api/github.py @@ -585,10 +585,12 @@ def merge_pull_request( body: dict[str, Any] = {"merge_method": str(merge_method)} if sha: body["sha"] = sha - return self._put( + resp = self._put( url=f"{self._url_pull}/{pr_number}/merge", json=body, - ).json() + ) + resp.raise_for_status() + return resp.json() def update_branch(self, update: str, upstream: str) -> dict[str, Any] | None: """Update a branch by creating a PR from upstream and then merge it. From 2d3c760a90feb1cabd5fc6fa5de16c7fc21a0cec Mon Sep 17 00:00:00 2001 From: Ben Chuanlong Du Date: Sun, 28 Jun 2026 21:34:03 -0700 Subject: [PATCH 7/8] Update github_rest_api/github.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- github_rest_api/github.py | 38 +++++++++++++++----- tests/test_github.py | 76 +++++++++++++++++++++++++++++++++++---- 2 files changed, 98 insertions(+), 16 deletions(-) diff --git a/github_rest_api/github.py b/github_rest_api/github.py index 92b7d23..43f78ae 100644 --- a/github_rest_api/github.py +++ b/github_rest_api/github.py @@ -212,36 +212,56 @@ def _approver_review_states( "CHANGES_REQUESTED", "DISMISSED", ): - latest[login] = state + latest[login.lower()] = state return set(latest.values()) def _marker_approved( - comments: Sequence[dict[str, Any]], approvers: Sequence[str], marker: str + comments: Sequence[dict[str, Any]], + approvers: Sequence[str], + marker: str, + since: datetime, ) -> bool: - """Check whether an allowlisted approver left the auto-merge marker comment. + """Check whether an allowlisted approver marked the current head for merge. + + Only comments created after ``since`` (the head commit's timestamp) count: a + marker left before the current head commit approved an older revision, so + honouring it would auto-merge newer, unreviewed changes. :param comments: Issue comments as returned by ``get_issue_comments``. :param approvers: Logins whose marker comments are trusted for auto-merge. :param marker: The marker substring signalling approval. + :param since: The head commit's timestamp; earlier comments are ignored. """ approver_set = _normalized_logins(approvers) return any( _in_allowlist(_login(comment), approver_set) and marker in (comment.get("body") or "") + and _parse_iso(comment["created_at"]) > since for comment in comments ) +def _parse_iso(timestamp: str) -> datetime: + """Parse an ISO-8601 timestamp into a timezone-aware ``datetime``. + + A naive timestamp (no offset) is assumed to be UTC. + + :param timestamp: An ISO-8601 timestamp (e.g. ``2026-06-27T12:00:00Z``). + """ + parsed = datetime.fromisoformat(timestamp) + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed + + def _old_enough(commit_iso: str, min_age_minutes: int) -> bool: """Check whether a commit timestamp is at least ``min_age_minutes`` old. :param commit_iso: An ISO-8601 UTC timestamp (e.g. ``2026-06-27T12:00:00Z``). :param min_age_minutes: The minimum age in minutes. """ - committed = datetime.fromisoformat(commit_iso) - if committed.tzinfo is None: - committed = committed.replace(tzinfo=timezone.utc) + committed = _parse_iso(commit_iso) return datetime.now(timezone.utc) - committed >= timedelta(minutes=min_age_minutes) @@ -648,8 +668,8 @@ def should_auto_merge( its ``mergeable_state`` is ``clean`` (no merge conflict and no failing or pending status checks); its head commit is at least ``min_age_minutes`` old; and it is approved either by an ``APPROVED`` review or by a - ``marker`` comment from an approver in ``approvers`` (with no outstanding - ``CHANGES_REQUESTED``). + ``marker`` comment posted after the head commit by an approver in + ``approvers`` (with no outstanding ``CHANGES_REQUESTED``). The age check guards against the race where a brand-new PR momentarily reads ``clean`` before its checks register; it relies on the head @@ -701,7 +721,7 @@ def skip(reason: str) -> None: if "APPROVED" in review_states: return head_sha comments = self.get_issue_comments(pr_number) - if _marker_approved(comments, approvers, marker): + if _marker_approved(comments, approvers, marker, _parse_iso(committed)): return head_sha return skip("it lacks an approving review or marker comment") diff --git a/tests/test_github.py b/tests/test_github.py index 5b8e708..439f94d 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -372,19 +372,56 @@ def test_approver_review_states_comment_does_not_clear_approval(): assert _approver_review_states(reviews, ["bot"]) == {"APPROVED"} +_HEAD_COMMITTED = datetime(2026, 6, 28, 12, 0, tzinfo=timezone.utc) +_AFTER_HEAD = "2026-06-28T12:05:00Z" +_BEFORE_HEAD = "2026-06-28T11:55:00Z" + + def test_marker_approved_true(): - comments = [{"user": {"login": "bot"}, "body": "looks good "}] - assert _marker_approved(comments, ["bot"], "") is True + comments = [ + { + "user": {"login": "bot"}, + "body": "looks good ", + "created_at": _AFTER_HEAD, + } + ] + assert _marker_approved(comments, ["bot"], "", _HEAD_COMMITTED) is True def test_marker_approved_requires_approver_author(): - comments = [{"user": {"login": "stranger"}, "body": ""}] - assert _marker_approved(comments, ["bot"], "") is False + comments = [ + { + "user": {"login": "stranger"}, + "body": "", + "created_at": _AFTER_HEAD, + } + ] + assert _marker_approved(comments, ["bot"], "", _HEAD_COMMITTED) is False def test_marker_approved_requires_marker(): - comments = [{"user": {"login": "bot"}, "body": "nice"}] - assert _marker_approved(comments, ["bot"], "") is False + comments = [{"user": {"login": "bot"}, "body": "nice", "created_at": _AFTER_HEAD}] + assert _marker_approved(comments, ["bot"], "", _HEAD_COMMITTED) is False + + +def test_marker_approved_ignores_comment_before_head_commit(): + comments = [ + {"user": {"login": "bot"}, "body": "", "created_at": _BEFORE_HEAD} + ] + assert _marker_approved(comments, ["bot"], "", _HEAD_COMMITTED) is False + + +def test_marker_approved_ignores_comment_at_head_commit(): + # A marker posted in the same second as the head commit is rejected: the gate + # is strict ("after"), erring toward not merging unreviewed changes. + comments = [ + { + "user": {"login": "bot"}, + "body": "", + "created_at": _HEAD_COMMITTED.isoformat(), + } + ] + assert _marker_approved(comments, ["bot"], "", _HEAD_COMMITTED) is False def test_old_enough_true_for_old_commit(): @@ -485,12 +522,37 @@ def test_should_auto_merge_all_pass(): def test_should_auto_merge_marker_comment_path(): repo, patches = _auto_merge_repo( reviews=[], - comments=[{"user": {"login": "bot"}, "body": ""}], + comments=[ + { + "user": {"login": "bot"}, + "body": "", + # Posted after the head commit (which _auto_merge_repo dates 30 min ago). + "created_at": datetime.now(timezone.utc).isoformat(), + } + ], ) # On eligibility, the validated head SHA is returned (head.sha in _auto_merge_repo). assert _run_should_auto_merge(repo, patches) == "abc123" +def test_should_auto_merge_skips_marker_comment_before_head_commit(): + # A marker left before the current head commit approved an older revision and + # must not auto-merge the newer, unreviewed changes. + repo, patches = _auto_merge_repo( + reviews=[], + comments=[ + { + "user": {"login": "bot"}, + "body": "", + "created_at": ( + datetime.now(timezone.utc) - timedelta(hours=1) + ).isoformat(), + } + ], + ) + assert _run_should_auto_merge(repo, patches) is None + + def test_should_auto_merge_skips_draft(): repo, patches = _auto_merge_repo({"draft": True}) assert _run_should_auto_merge(repo, patches) is None From 0b65c4c5be4c7a9d9682a532b6394643f5624ddd Mon Sep 17 00:00:00 2001 From: Ben Chuanlong Du Date: Sun, 28 Jun 2026 22:54:27 -0700 Subject: [PATCH 8/8] Update github_rest_api/github.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- github_rest_api/github.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/github_rest_api/github.py b/github_rest_api/github.py index 43f78ae..e0778bd 100644 --- a/github_rest_api/github.py +++ b/github_rest_api/github.py @@ -826,10 +826,12 @@ def _head_commit_date(self, sha: str) -> str: :param sha: The head commit SHA. """ - commits = self._get( + resp = self._get( url=f"{self._url_repo}/commits", params={"sha": sha, "per_page": 1}, - ).json() + ) + resp.raise_for_status() + commits = resp.json() if not isinstance(commits, list) or not commits: raise ValueError(f"No commits found for SHA {sha}") try: